Flush the past buckets in anomaly tracker when time jumps forward
E2e test for count/duration anomaly trackers.
Test: new statsd tests.
BUG: b/74446029
Change-Id: Ia9be0240ba5021d44c1e1f096d67563e9138bb59
diff --git a/cmds/statsd/src/anomaly/AnomalyTracker.cpp b/cmds/statsd/src/anomaly/AnomalyTracker.cpp
index 49de1ac..f0960e3 100644
--- a/cmds/statsd/src/anomaly/AnomalyTracker.cpp
+++ b/cmds/statsd/src/anomaly/AnomalyTracker.cpp
@@ -66,6 +66,9 @@
void AnomalyTracker::advanceMostRecentBucketTo(const int64_t& bucketNum) {
VLOG("advanceMostRecentBucketTo() called.");
+ if (mNumOfPastBuckets <= 0) {
+ return;
+ }
if (bucketNum <= mMostRecentBucketNum) {
ALOGW("Cannot advance buckets backwards (bucketNum=%lld but mMostRecentBucketNum=%lld)",
(long long)bucketNum, (long long)mMostRecentBucketNum);
@@ -170,7 +173,8 @@
int64_t AnomalyTracker::getPastBucketValue(const MetricDimensionKey& key,
const int64_t& bucketNum) const {
- if (bucketNum < 0 || bucketNum <= mMostRecentBucketNum - mNumOfPastBuckets
+ if (bucketNum < 0 || mMostRecentBucketNum < 0
+ || bucketNum <= mMostRecentBucketNum - mNumOfPastBuckets
|| bucketNum > mMostRecentBucketNum) {
return 0;
}
@@ -241,14 +245,10 @@
}
bool AnomalyTracker::isInRefractoryPeriod(const uint64_t& timestampNs,
- const MetricDimensionKey& key) {
+ const MetricDimensionKey& key) const {
const auto& it = mRefractoryPeriodEndsSec.find(key);
if (it != mRefractoryPeriodEndsSec.end()) {
- if (timestampNs < it->second * NS_PER_SEC) {
- return true;
- } else {
- mRefractoryPeriodEndsSec.erase(key);
- }
+ return timestampNs < it->second * NS_PER_SEC;
}
return false;
}
diff --git a/cmds/statsd/src/anomaly/AnomalyTracker.h b/cmds/statsd/src/anomaly/AnomalyTracker.h
index d3da7dc..ae0af64 100644
--- a/cmds/statsd/src/anomaly/AnomalyTracker.h
+++ b/cmds/statsd/src/anomaly/AnomalyTracker.h
@@ -113,6 +113,13 @@
}
protected:
+ // For testing only.
+ // Returns the alarm timestamp in seconds for the query dimension if it exists. Otherwise
+ // returns 0.
+ virtual uint32_t getAlarmTimestampSec(const MetricDimensionKey& dimensionKey) const {
+ return 0; // The base AnomalyTracker class doesn't have alarms.
+ }
+
// statsd_config.proto Alert message that defines this tracker.
const Alert mAlert;
@@ -159,8 +166,7 @@
void subtractValueFromSum(const MetricDimensionKey& key, const int64_t& bucketValue);
// Returns true if in the refractory period, else false.
- // If there is a stored refractory period but it ended prior to timestampNs, it is removed.
- bool isInRefractoryPeriod(const uint64_t& timestampNs, const MetricDimensionKey& key);
+ bool isInRefractoryPeriod(const uint64_t& timestampNs, const MetricDimensionKey& key) const;
// Calculates the corresponding bucket index within the circular array.
// Requires bucketNum >= 0.
@@ -176,6 +182,9 @@
FRIEND_TEST(AnomalyTrackerTest, TestSparseBuckets);
FRIEND_TEST(GaugeMetricProducerTest, TestAnomalyDetection);
FRIEND_TEST(CountMetricProducerTest, TestAnomalyDetectionUnSliced);
+ FRIEND_TEST(AnomalyDetectionE2eTest, TestDurationMetric_SUM_single_bucket);
+ FRIEND_TEST(AnomalyDetectionE2eTest, TestDurationMetric_SUM_multiple_buckets);
+ FRIEND_TEST(AnomalyDetectionE2eTest, TestDurationMetric_SUM_long_refractory_period);
};
} // namespace statsd
diff --git a/cmds/statsd/src/anomaly/DurationAnomalyTracker.cpp b/cmds/statsd/src/anomaly/DurationAnomalyTracker.cpp
index 79067eb..cdc4251 100644
--- a/cmds/statsd/src/anomaly/DurationAnomalyTracker.cpp
+++ b/cmds/statsd/src/anomaly/DurationAnomalyTracker.cpp
@@ -38,11 +38,10 @@
void DurationAnomalyTracker::startAlarm(const MetricDimensionKey& dimensionKey,
const uint64_t& timestampNs) {
// Alarms are stored in secs. Must round up, since if it fires early, it is ignored completely.
- uint32_t timestampSec = static_cast<uint32_t>((timestampNs -1)/ NS_PER_SEC) + 1; // round up
+ uint32_t timestampSec = static_cast<uint32_t>((timestampNs -1) / NS_PER_SEC) + 1; // round up
if (isInRefractoryPeriod(timestampNs, dimensionKey)) {
- // TODO: Bug! By the refractory's end, the data might be erased and the alarm inapplicable.
- VLOG("Setting a delayed anomaly alarm lest it fall in the refractory period");
- timestampSec = getRefractoryPeriodEndsSec(dimensionKey) + 1;
+ VLOG("Not setting anomaly alarm since it would fall in the refractory period.");
+ return;
}
auto itr = mAlarms.find(dimensionKey);
diff --git a/cmds/statsd/src/anomaly/DurationAnomalyTracker.h b/cmds/statsd/src/anomaly/DurationAnomalyTracker.h
index 92bb2bc..53155d9 100644
--- a/cmds/statsd/src/anomaly/DurationAnomalyTracker.h
+++ b/cmds/statsd/src/anomaly/DurationAnomalyTracker.h
@@ -52,6 +52,13 @@
unordered_set<sp<const InternalAlarm>, SpHash<InternalAlarm>>& firedAlarms) override;
protected:
+ // Returns the alarm timestamp in seconds for the query dimension if it exists. Otherwise
+ // returns 0.
+ uint32_t getAlarmTimestampSec(const MetricDimensionKey& dimensionKey) const override {
+ auto it = mAlarms.find(dimensionKey);
+ return it == mAlarms.end() ? 0 : it->second->timestampSec;
+ }
+
// The alarms owned by this tracker. The alarm monitor also shares the alarm pointers when they
// are still active.
std::unordered_map<MetricDimensionKey, sp<const InternalAlarm>> mAlarms;