diff --git a/cmds/statsd/src/condition/SimpleConditionTracker.cpp b/cmds/statsd/src/condition/SimpleConditionTracker.cpp index a694dbf754ef5..60060fe60fbde 100644 --- a/cmds/statsd/src/condition/SimpleConditionTracker.cpp +++ b/cmds/statsd/src/condition/SimpleConditionTracker.cpp @@ -206,7 +206,7 @@ void SimpleConditionTracker::evaluateCondition(const LogEvent& event, vector& 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; } diff --git a/cmds/statsd/src/matchers/matcher_util.cpp b/cmds/statsd/src/matchers/matcher_util.cpp index cccc9b3317e73..1c699e822420a 100644 --- a/cmds/statsd/src/matchers/matcher_util.cpp +++ b/cmds/statsd/src/matchers/matcher_util.cpp @@ -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; } } } diff --git a/cmds/statsd/src/metrics/DurationMetricProducer.cpp b/cmds/statsd/src/metrics/DurationMetricProducer.cpp index aaf3ec2c298b4..de6f3653f7ada 100644 --- a/cmds/statsd/src/metrics/DurationMetricProducer.cpp +++ b/cmds/statsd/src/metrics/DurationMetricProducer.cpp @@ -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; } } diff --git a/cmds/statsd/tests/LogEntryMatcher_test.cpp b/cmds/statsd/tests/LogEntryMatcher_test.cpp index d0898b0fa8656..bb4930a9a342d 100644 --- a/cmds/statsd/tests/LogEntryMatcher_test.cpp +++ b/cmds/statsd/tests/LogEntryMatcher_test.cpp @@ -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; diff --git a/cmds/statsd/tests/condition/SimpleConditionTracker_test.cpp b/cmds/statsd/tests/condition/SimpleConditionTracker_test.cpp index 05aad290e3feb..855e66615f8e3 100644 --- a/cmds/statsd/tests/condition/SimpleConditionTracker_test.cpp +++ b/cmds/statsd/tests/condition/SimpleConditionTracker_test.cpp @@ -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);