Android/frameworks_base
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Prevent incorrect alert triggering

Browse Source 
Alerts were being incorrectly triggered in pulled value metrics because
a diffed value is always assumed to be over one bucket interval, which
may not be true when multiple buckets are skipped.

Bug: 159390273
Test: atest statsd_test
Change-Id: Iada3df54ebb30353572a14e0db86bed49c5515b0
This commit is contained in:
Jeffrey Huang
2020-06-29 15:17:48 -07:00
parent 6a9be8e9c5
commit ec72e87563
3 changed files with 54 additions and 5 deletions
Download Patch File Download Diff File Expand all files Collapse all files

14
cmds/statsd/src/metrics/ValueMetricProducer.cpp
View File

@@ -733,6 +733,11 @@ bool getDoubleOrLong(const LogEvent& event, const Matcher& matcher, Value& ret)
return false;
}
bool ValueMetricProducer::multipleBucketsSkipped(const int64_t numBucketsForward) {
// Skip buckets if this is a pulled metric or a pushed metric that is diffed.
return numBucketsForward > 1 && (mIsPulled || mUseDiff);
}
void ValueMetricProducer::onMatchedLogEventInternalLocked(
const size_t matcherIndex, const MetricDimensionKey& eventKey,
const ConditionKey& conditionKey, bool condition, const LogEvent& event,
@@ -910,8 +915,9 @@ void ValueMetricProducer::onMatchedLogEventInternalLocked(
interval.sampleSize += 1;
}
// Only trigger the tracker if all intervals are correct
if (useAnomalyDetection) {
// Only trigger the tracker if all intervals are correct and we have not skipped the bucket due
// to MULTIPLE_BUCKETS_SKIPPED.
if (useAnomalyDetection && !multipleBucketsSkipped(calcBucketsForwardCount(eventTimeNs))) {
// TODO: propgate proper values down stream when anomaly support doubles
long wholeBucketVal = intervals[0].value.long_value;
auto prev = mCurrentFullBucket.find(eventKey);
@@ -961,9 +967,7 @@ void ValueMetricProducer::flushCurrentBucketLocked(const int64_t& eventTimeNs,
int64_t bucketEndTime = fullBucketEndTimeNs;
int64_t numBucketsForward = calcBucketsForwardCount(eventTimeNs);
// Skip buckets if this is a pulled metric or a pushed metric that is diffed.
if (numBucketsForward > 1 && (mIsPulled || mUseDiff)) {
if (multipleBucketsSkipped(numBucketsForward)) {
VLOG("Skipping forward %lld buckets", (long long)numBucketsForward);
StatsdStats::getInstance().noteSkippedForwardBuckets(mMetricId);
// Something went wrong. Maybe the device was sleeping for a long time. It is better

2
cmds/statsd/src/metrics/ValueMetricProducer.h
View File

@@ -219,6 +219,8 @@ private:
void pullAndMatchEventsLocked(const int64_t timestampNs);
bool multipleBucketsSkipped(const int64_t numBucketsForward);
void accumulateEvents(const std::vector<std::shared_ptr<LogEvent>>& allData,
int64_t originalPullTimeNs, int64_t eventElapsedTimeNs);

43
cmds/statsd/tests/metrics/ValueMetricProducer_test.cpp
View File

@@ -1081,6 +1081,49 @@ TEST(ValueMetricProducerTest, TestAnomalyDetection) {
std::ceil(1.0 * event6.GetElapsedTimestampNs() / NS_PER_SEC + refPeriodSec));
}
TEST(ValueMetricProducerTest, TestAnomalyDetectionMultipleBucketsSkipped) {
sp<AlarmMonitor> alarmMonitor;
Alert alert;
alert.set_id(101);
alert.set_metric_id(metricId);
alert.set_trigger_if_sum_gt(100);
alert.set_num_buckets(1);
const int32_t refPeriodSec = 3;
alert.set_refractory_period_secs(refPeriodSec);
ValueMetric metric = ValueMetricProducerTestHelper::createMetricWithCondition();
sp<MockStatsPullerManager> pullerManager = new StrictMock<MockStatsPullerManager>();
EXPECT_CALL(*pullerManager, Pull(tagId, kConfigKey, _, _))
.WillOnce(Invoke([](int tagId, const ConfigKey&, const int64_t eventTimeNs,
vector<std::shared_ptr<LogEvent>>* data) {
EXPECT_EQ(eventTimeNs, bucketStartTimeNs + 1); // Condition change to true time.
data->clear();
data->push_back(CreateRepeatedValueLogEvent(tagId, bucketStartTimeNs + 1, 0));
return true;
}))
.WillOnce(Invoke([](int tagId, const ConfigKey&, const int64_t eventTimeNs,
vector<std::shared_ptr<LogEvent>>* data) {
EXPECT_EQ(eventTimeNs,
bucket3StartTimeNs + 100); // Condition changed to false time.
data->clear();
data->push_back(CreateRepeatedValueLogEvent(tagId, bucket3StartTimeNs + 100, 120));
return true;
}));
sp<ValueMetricProducer> valueProducer =
ValueMetricProducerTestHelper::createValueProducerWithCondition(pullerManager, metric,
ConditionState::kFalse);
sp<AnomalyTracker> anomalyTracker = valueProducer->addAnomalyTracker(alert, alarmMonitor);
valueProducer->onConditionChanged(true, bucketStartTimeNs + 1);
// multiple buckets should be skipped here.
valueProducer->onConditionChanged(false, bucket3StartTimeNs + 100);
// No alert is fired when multiple buckets are skipped.
EXPECT_EQ(anomalyTracker->getRefractoryPeriodEndsSec(DEFAULT_METRIC_DIMENSION_KEY), 0U);
}
// Test value metric no condition, the pull on bucket boundary come in time and too late
TEST(ValueMetricProducerTest, TestBucketBoundaryNoCondition) {
ValueMetric metric = ValueMetricProducerTestHelper::createMetric();