From 0dbc7a434345fa318ee129eaa5cf83681de4936b Mon Sep 17 00:00:00 2001 From: Bookatz Date: Fri, 30 Mar 2018 16:21:17 -0700 Subject: [PATCH] Statsd MAX now obeys refractory periods too The logic of where refractory period enforcement was moved out of the anomaly tracker and into the metric's predictAnomalyTimestamp. It was done for ORING, but not for MAX. This fixes MAX. Bug: 74446029 Test: adb sync data && adb shell data/nativetest64/statsd_test/statsd_test Change-Id: I51e43c7c132f424af8fe20a37f2ad10cc55b5989 --- .../duration_helper/MaxDurationTracker.cpp | 5 +-- .../tests/metrics/MaxDurationTracker_test.cpp | 31 ++++++++++++++++++- 2 files changed, 33 insertions(+), 3 deletions(-) diff --git a/cmds/statsd/src/metrics/duration_helper/MaxDurationTracker.cpp b/cmds/statsd/src/metrics/duration_helper/MaxDurationTracker.cpp index 15d9619f02203..5c1e9c0cd65e8 100644 --- a/cmds/statsd/src/metrics/duration_helper/MaxDurationTracker.cpp +++ b/cmds/statsd/src/metrics/duration_helper/MaxDurationTracker.cpp @@ -327,8 +327,9 @@ int64_t MaxDurationTracker::predictAnomalyTimestampNs(const DurationAnomalyTrack } } } - int64_t threshold = anomalyTracker.getAnomalyThreshold(); - return currentTimestamp + threshold - maxElapsed; + int64_t anomalyTimeNs = currentTimestamp + anomalyTracker.getAnomalyThreshold() - maxElapsed; + int64_t refractoryEndNs = anomalyTracker.getRefractoryPeriodEndsSec(mEventKey) * NS_PER_SEC; + return std::max(anomalyTimeNs, refractoryEndNs); } void MaxDurationTracker::dumpStates(FILE* out, bool verbose) const { diff --git a/cmds/statsd/tests/metrics/MaxDurationTracker_test.cpp b/cmds/statsd/tests/metrics/MaxDurationTracker_test.cpp index 57a8925a122e3..a0f1c00b7b486 100644 --- a/cmds/statsd/tests/metrics/MaxDurationTracker_test.cpp +++ b/cmds/statsd/tests/metrics/MaxDurationTracker_test.cpp @@ -344,8 +344,37 @@ TEST(MaxDurationTrackerTest, TestAnomalyPredictedTimestamp) { tracker.noteConditionChanged(key1, true, conditionStarts2); EXPECT_EQ(1U, anomalyTracker->mAlarms.size()); auto alarm = anomalyTracker->mAlarms.begin()->second; + uint64_t anomalyFireTimeSec = alarm->timestampSec; EXPECT_EQ(conditionStarts2 + 36 * NS_PER_SEC, - (unsigned long long)(alarm->timestampSec * NS_PER_SEC)); + (unsigned long long)anomalyFireTimeSec * NS_PER_SEC); + + // Now we test the calculation now that there's a refractory period. + // At the correct time, declare the anomaly. This will set a refractory period. Make sure it + // gets correctly taken into account in future predictAnomalyTimestampNs calculations. + std::unordered_set, SpHash> firedAlarms({alarm}); + anomalyTracker->informAlarmsFired(anomalyFireTimeSec * NS_PER_SEC, firedAlarms); + EXPECT_EQ(0u, anomalyTracker->mAlarms.size()); + uint64_t refractoryPeriodEndsSec = anomalyFireTimeSec + refPeriodSec; + EXPECT_EQ(anomalyTracker->getRefractoryPeriodEndsSec(eventKey), refractoryPeriodEndsSec); + + // Now stop and start again. Make sure the new predictAnomalyTimestampNs takes into account + // the refractory period correctly. + uint64_t eventStopTimeNs = anomalyFireTimeSec * NS_PER_SEC + 10; + tracker.noteStop(key1, eventStopTimeNs, false); + tracker.noteStop(key2, eventStopTimeNs, false); + tracker.noteStart(key1, true, eventStopTimeNs + 1000000, conditionKey1); + // Anomaly is ongoing, but we're still in the refractory period. + EXPECT_EQ(1U, anomalyTracker->mAlarms.size()); + alarm = anomalyTracker->mAlarms.begin()->second; + EXPECT_EQ(refractoryPeriodEndsSec, (unsigned long long)(alarm->timestampSec)); + + // Makes sure it is correct after the refractory period is over. + tracker.noteStop(key1, eventStopTimeNs + 2000000, false); + uint64_t justBeforeRefPeriodNs = (refractoryPeriodEndsSec - 2) * NS_PER_SEC; + tracker.noteStart(key1, true, justBeforeRefPeriodNs, conditionKey1); + alarm = anomalyTracker->mAlarms.begin()->second; + EXPECT_EQ(justBeforeRefPeriodNs + 40 * NS_PER_SEC, + (unsigned long long)(alarm->timestampSec * NS_PER_SEC)); } // Suppose that within one tracker there are two dimensions A and B.