Fix some bugs found in statsd (d41c4220) · Commits · e / os / android_frameworks_base

cmds/statsd/src/condition/SimpleConditionTracker.cpp

+17 −2

Original line number	Diff line number	Diff line
		@@ -206,7 +206,7 @@ void SimpleConditionTracker::evaluateCondition(const LogEvent& event,
		vector<bool>& conditionChangedCache) {
		if (conditionCache[mIndex] != ConditionState::kNotEvaluated) {
		// it has been evaluated.
		VLOG("Yes, already evaluated, %s %d", mName.c_str(), mNonSlicedConditionState);
		VLOG("Yes, already evaluated, %s %d", mName.c_str(), conditionCache[mIndex]);
		return;
		}

		@@ -230,8 +230,23 @@ void SimpleConditionTracker::evaluateCondition(const LogEvent& event,
		}

		if (matchedState < 0) {
		// The event doesn't match this condition. So we just report existing condition values.
		conditionChangedCache[mIndex] = false;
		conditionCache[mIndex] = mNonSlicedConditionState;
		if (mSliced) {
		// if the condition result is sliced. metrics won't directly get value from the
		// cache, so just set any value other than kNotEvaluated.
		conditionCache[mIndex] = ConditionState::kUnknown;
		} else if (mSlicedConditionState.find(DEFAULT_DIMENSION_KEY) ==
		mSlicedConditionState.end()) {
		// condition not sliced, but we haven't seen the matched start or stop yet. so return
		// initial value.
		conditionCache[mIndex] = mInitialValue;
		} else {
		// return the cached condition.
		conditionCache[mIndex] = mSlicedConditionState[DEFAULT_DIMENSION_KEY] > 0
		? ConditionState::kTrue
		: ConditionState::kFalse;
		}
		return;
		}

cmds/statsd/src/matchers/matcher_util.cpp

+9 −1

Original line number	Diff line number	Diff line
		@@ -112,6 +112,7 @@ bool matchesSimple(const SimpleLogEntryMatcher& simpleMatcher, const LogEvent& e
		if (err == NO_ERROR && val != NULL) {
		if (!(cur.eq_string() == val)) {
		allMatched = false;
		break;
		}
		}
		} else if (matcherCase == KeyValueMatcher::ValueMatcherCase::kEqInt \|\|
		@@ -126,26 +127,30 @@ bool matchesSimple(const SimpleLogEntryMatcher& simpleMatcher, const LogEvent& e
		if (matcherCase == KeyValueMatcher::ValueMatcherCase::kEqInt) {
		if (!(val == cur.eq_int())) {
		allMatched = false;
		break;
		}
		} else if (matcherCase == KeyValueMatcher::ValueMatcherCase::kLtInt) {
		if (!(val < cur.lt_int())) {
		allMatched = false;
		break;
		}
		} else if (matcherCase == KeyValueMatcher::ValueMatcherCase::kGtInt) {
		if (!(val > cur.gt_int())) {
		allMatched = false;
		break;
		}
		} else if (matcherCase == KeyValueMatcher::ValueMatcherCase::kLteInt) {
		if (!(val <= cur.lte_int())) {
		allMatched = false;
		break;
		}
		} else if (matcherCase == KeyValueMatcher::ValueMatcherCase::kGteInt) {
		if (!(val >= cur.gte_int())) {
		allMatched = false;
		break;
		}
		}
		}
		break;
		} else if (matcherCase == KeyValueMatcher::ValueMatcherCase::kEqBool) {
		// Boolean fields
		status_t err = NO_ERROR;
		@@ -153,6 +158,7 @@ bool matchesSimple(const SimpleLogEntryMatcher& simpleMatcher, const LogEvent& e
		if (err == NO_ERROR) {
		if (!(cur.eq_bool() == val)) {
		allMatched = false;
		break;
		}
		}
		} else if (matcherCase == KeyValueMatcher::ValueMatcherCase::kLtFloat \|\|
		@@ -164,10 +170,12 @@ bool matchesSimple(const SimpleLogEntryMatcher& simpleMatcher, const LogEvent& e
		if (matcherCase == KeyValueMatcher::ValueMatcherCase::kLtFloat) {
		if (!(cur.lt_float() <= val)) {
		allMatched = false;
		break;
		}
		} else if (matcherCase == KeyValueMatcher::ValueMatcherCase::kGtFloat) {
		if (!(cur.gt_float() >= val)) {
		allMatched = false;
		break;
		}
		}
		}

cmds/statsd/src/metrics/DurationMetricProducer.cpp

+4 −2

Original line number	Diff line number	Diff line
		@@ -233,10 +233,12 @@ void DurationMetricProducer::flushIfNeeded(uint64_t eventTime) {
		}

		VLOG("flushing...........");
		for (auto it = mCurrentSlicedDuration.begin(); it != mCurrentSlicedDuration.end(); ++it) {
		for (auto it = mCurrentSlicedDuration.begin(); it != mCurrentSlicedDuration.end();) {
		if (it->second->flushIfNeeded(eventTime)) {
		VLOG("erase bucket for key %s", it->first.c_str());
		mCurrentSlicedDuration.erase(it);
		it = mCurrentSlicedDuration.erase(it);
		} else {
		++it;
		}
		}

cmds/statsd/tests/LogEntryMatcher_test.cpp

+33 −0

Original line number	Diff line number	Diff line
		@@ -109,6 +109,39 @@ TEST(LogEntryMatcherTest, TestStringMatcher) {
		EXPECT_TRUE(matchesSimple(*simpleMatcher, event));
		}

		TEST(LogEntryMatcherTest, TestMultiFieldsMatcher) {
		// Set up the matcher
		LogEntryMatcher matcher;
		auto simpleMatcher = matcher.mutable_simple_log_entry_matcher();
		simpleMatcher->set_tag(TAG_ID);
		auto keyValue1 = simpleMatcher->add_key_value_matcher();
		keyValue1->mutable_key_matcher()->set_key(FIELD_ID_1);
		auto keyValue2 = simpleMatcher->add_key_value_matcher();
		keyValue2->mutable_key_matcher()->set_key(FIELD_ID_2);

		// Set up the event
		LogEvent event(TAG_ID, 0);
		auto list = event.GetAndroidLogEventList();
		*list << 2;
		*list << 3;

		// Convert to a LogEvent
		event.init();

		// Test
		keyValue1->set_eq_int(2);
		keyValue2->set_eq_int(3);
		EXPECT_TRUE(matchesSimple(*simpleMatcher, event));

		keyValue1->set_eq_int(2);
		keyValue2->set_eq_int(4);
		EXPECT_FALSE(matchesSimple(*simpleMatcher, event));

		keyValue1->set_eq_int(4);
		keyValue2->set_eq_int(3);
		EXPECT_FALSE(matchesSimple(*simpleMatcher, event));
		}

		TEST(LogEntryMatcherTest, TestIntComparisonMatcher) {
		// Set up the matcher
		LogEntryMatcher matcher;

cmds/statsd/tests/condition/SimpleConditionTracker_test.cpp

+12 −0

Original line number	Diff line number	Diff line
		@@ -108,6 +108,18 @@ TEST(SimpleConditionTrackerTest, TestNonSlicedCondition) {
		EXPECT_EQ(ConditionState::kTrue, conditionCache[0]);
		EXPECT_TRUE(changedCache[0]);

		// match nothing.
		matcherState.clear();
		matcherState.push_back(MatchingState::kNotMatched);
		matcherState.push_back(MatchingState::kNotMatched);
		conditionCache[0] = ConditionState::kNotEvaluated;
		changedCache[0] = false;

		conditionTracker.evaluateCondition(event, matcherState, allConditions, conditionCache,
		changedCache);
		EXPECT_EQ(ConditionState::kTrue, conditionCache[0]);
		EXPECT_FALSE(changedCache[0]);

		// the case for match stop.
		matcherState.clear();
		matcherState.push_back(MatchingState::kNotMatched);