Merge "Add a guardrail to limit minimum bucket duration to be 5 minutes except when configured through adb command"

This commit is contained in:
Howard Ro
2018-02-15 00:18:22 +00:00
committed by Android (Google) Code Review
16 changed files with 70 additions and 20 deletions

View File

@@ -63,7 +63,8 @@ CountMetricProducer::CountMetricProducer(const ConfigKey& key, const CountMetric
: MetricProducer(metric.id(), key, startTimeNs, conditionIndex, wizard) {
// TODO: evaluate initial conditions. and set mConditionMet.
if (metric.has_bucket()) {
mBucketSizeNs = TimeUnitToBucketSizeInMillis(metric.bucket()) * 1000000;
mBucketSizeNs =
TimeUnitToBucketSizeInMillisGuardrailed(key.GetUid(), metric.bucket()) * 1000000;
} else {
mBucketSizeNs = LLONG_MAX;
}

View File

@@ -72,7 +72,8 @@ DurationMetricProducer::DurationMetricProducer(const ConfigKey& key, const Durat
// them in the base class, because the proto generated CountMetric, and DurationMetric are
// not related. Maybe we should add a template in the future??
if (metric.has_bucket()) {
mBucketSizeNs = TimeUnitToBucketSizeInMillis(metric.bucket()) * 1000000;
mBucketSizeNs =
TimeUnitToBucketSizeInMillisGuardrailed(key.GetUid(), metric.bucket()) * 1000000;
} else {
mBucketSizeNs = LLONG_MAX;
}

View File

@@ -69,7 +69,7 @@ GaugeMetricProducer::GaugeMetricProducer(const ConfigKey& key, const GaugeMetric
mCurrentSlicedBucketForAnomaly = std::make_shared<DimToValMap>();
int64_t bucketSizeMills = 0;
if (metric.has_bucket()) {
bucketSizeMills = TimeUnitToBucketSizeInMillis(metric.bucket());
bucketSizeMills = TimeUnitToBucketSizeInMillisGuardrailed(key.GetUid(), metric.bucket());
} else {
bucketSizeMills = TimeUnitToBucketSizeInMillis(ONE_HOUR);
}

View File

@@ -137,6 +137,11 @@ public:
return mBucketSizeNs;
}
// Only needed for unit-testing to override guardrail.
void setBucketSize(int64_t bucketSize) {
mBucketSizeNs = bucketSize;
}
inline const int64_t& getMetricId() {
return mMetricId;
}

View File

@@ -72,7 +72,7 @@ ValueMetricProducer::ValueMetricProducer(const ConfigKey& key, const ValueMetric
// TODO: valuemetric for pushed events may need unlimited bucket length
int64_t bucketSizeMills = 0;
if (metric.has_bucket()) {
bucketSizeMills = TimeUnitToBucketSizeInMillis(metric.bucket());
bucketSizeMills = TimeUnitToBucketSizeInMillisGuardrailed(key.GetUid(), metric.bucket());
} else {
bucketSizeMills = TimeUnitToBucketSizeInMillis(ONE_HOUR);
}

View File

@@ -17,6 +17,7 @@
#include "stats_log_util.h"
#include <logd/LogEvent.h>
#include <private/android_filesystem_config.h>
#include <utils/Log.h>
#include <set>
#include <stack>
@@ -216,6 +217,14 @@ void writeFieldValueTreeToStream(int tagId, const std::vector<FieldValue>& value
protoOutput->end(atomToken);
}
int64_t TimeUnitToBucketSizeInMillisGuardrailed(int uid, TimeUnit unit) {
int64_t bucketSizeMillis = TimeUnitToBucketSizeInMillis(unit);
if (bucketSizeMillis > 1000 && bucketSizeMillis < 5 * 60 * 1000LL && uid != AID_SHELL) {
bucketSizeMillis = 5 * 60 * 1000LL;
}
return bucketSizeMillis;
}
int64_t TimeUnitToBucketSizeInMillis(TimeUnit unit) {
switch (unit) {
case ONE_MINUTE:
@@ -283,4 +292,4 @@ int64_t getWallClockMillis() {
} // namespace statsd
} // namespace os
} // namespace android
} // namespace android

View File

@@ -32,6 +32,10 @@ void writeFieldValueTreeToStream(int tagId, const std::vector<FieldValue>& value
void writeDimensionToProto(const HashableDimensionKey& dimension,
util::ProtoOutputStream* protoOutput);
// Convert the TimeUnit enum to the bucket size in millis with a guardrail on
// bucket size.
int64_t TimeUnitToBucketSizeInMillisGuardrailed(int uid, TimeUnit unit);
// Convert the TimeUnit enum to the bucket size in millis.
int64_t TimeUnitToBucketSizeInMillis(TimeUnit unit);
@@ -71,4 +75,4 @@ bool parseProtoOutputStream(util::ProtoOutputStream& protoOutput, T* message) {
} // namespace statsd
} // namespace os
} // namespace android
} // namespace android

View File

@@ -47,7 +47,7 @@ StatsdConfig CreateStatsdConfig() {
*countMetric->mutable_dimensions_in_what() =
CreateAttributionUidAndTagDimensions(
android::util::WAKELOCK_STATE_CHANGED, {Position::FIRST});
countMetric->set_bucket(ONE_MINUTE);
countMetric->set_bucket(FIVE_MINUTES);
return config;
}
@@ -206,4 +206,4 @@ GTEST_LOG_(INFO) << "This test does nothing.\n";
} // namespace statsd
} // namespace os
} // namespace android
} // namespace android

View File

@@ -61,7 +61,7 @@ StatsdConfig CreateCountMetricWithNoLinkConfig() {
CreateDimensions(android::util::SCREEN_BRIGHTNESS_CHANGED, {1 /* level */});
*metric->mutable_dimensions_in_condition() = CreateAttributionUidDimensions(
android::util::WAKELOCK_STATE_CHANGED, {Position::FIRST});
metric->set_bucket(ONE_MINUTE);
metric->set_bucket(FIVE_MINUTES);
return config;
}
@@ -252,7 +252,7 @@ StatsdConfig CreateCountMetricWithLinkConfig() {
addPredicateToPredicateCombination(isSyncingPredicate, combinationPredicate);
auto metric = config.add_count_metric();
metric->set_bucket(ONE_MINUTE);
metric->set_bucket(FIVE_MINUTES);
metric->set_id(StringToId("AppCrashMetric"));
metric->set_what(appCrashMatcher.id());
metric->set_condition(combinationPredicate->id());
@@ -441,7 +441,7 @@ StatsdConfig CreateDurationMetricConfigNoLink(DurationMetric::AggregationType ag
addPredicateToPredicateCombination(isSyncingPredicate, combinationPredicate);
auto metric = config.add_duration_metric();
metric->set_bucket(ONE_MINUTE);
metric->set_bucket(FIVE_MINUTES);
metric->set_id(StringToId("BatterySaverModeDurationMetric"));
metric->set_what(inBatterySaverModePredicate.id());
metric->set_condition(combinationPredicate->id());
@@ -595,7 +595,7 @@ StatsdConfig CreateDurationMetricConfigWithLink(DurationMetric::AggregationType
addPredicateToPredicateCombination(isSyncingPredicate, combinationPredicate);
auto metric = config.add_duration_metric();
metric->set_bucket(ONE_MINUTE);
metric->set_bucket(FIVE_MINUTES);
metric->set_id(StringToId("AppInBackgroundMetric"));
metric->set_what(isInBackgroundPredicate.id());
metric->set_condition(combinationPredicate->id());
@@ -730,4 +730,4 @@ GTEST_LOG_(INFO) << "This test does nothing.\n";
} // namespace statsd
} // namespace os
} // namespace android
} // namespace android

View File

@@ -53,7 +53,7 @@ StatsdConfig CreateStatsdConfigForPushedEvent() {
fieldMatcher->add_child()->set_field(7); // activity_start_msec(int64)
*gaugeMetric->mutable_dimensions_in_what() =
CreateDimensions(android::util::APP_START_CHANGED, {1 /* uid field */ });
gaugeMetric->set_bucket(ONE_MINUTE);
gaugeMetric->set_bucket(FIVE_MINUTES);
auto links = gaugeMetric->add_links();
links->set_condition(isInBackgroundPredicate.id());
@@ -198,4 +198,4 @@ GTEST_LOG_(INFO) << "This test does nothing.\n";
} // namespace statsd
} // namespace os
} // namespace android
} // namespace android

View File

@@ -71,7 +71,7 @@ StatsdConfig CreateStatsdConfig() {
// The metric is dimensioning by uid only.
*countMetric->mutable_dimensions_in_what() =
CreateDimensions(android::util::PROCESS_LIFE_CYCLE_STATE_CHANGED, {1});
countMetric->set_bucket(ONE_MINUTE);
countMetric->set_bucket(FIVE_MINUTES);
// Links between crash atom and condition of app is in syncing.
auto links = countMetric->add_links();
@@ -337,4 +337,4 @@ GTEST_LOG_(INFO) << "This test does nothing.\n";
} // namespace statsd
} // namespace os
} // namespace android
} // namespace android

View File

@@ -56,7 +56,7 @@ StatsdConfig CreateStatsdConfig(DurationMetric::AggregationType aggregationType)
*durationMetric->mutable_dimensions_in_what() =
CreateAttributionUidDimensions(
android::util::WAKELOCK_STATE_CHANGED, {Position::FIRST});
durationMetric->set_bucket(ONE_MINUTE);
durationMetric->set_bucket(FIVE_MINUTES);
return config;
}
@@ -327,4 +327,4 @@ GTEST_LOG_(INFO) << "This test does nothing.\n";
} // namespace statsd
} // namespace os
} // namespace android
} // namespace android

View File

@@ -56,6 +56,7 @@ TEST(CountMetricProducerTest, TestNonDimensionalEvents) {
CountMetricProducer countProducer(kConfigKey, metric, -1 /*-1 meaning no condition*/, wizard,
bucketStartTimeNs);
countProducer.setBucketSize(60 * NS_PER_SEC);
// 2 events in bucket 1.
countProducer.onMatchedLogEvent(1 /*log matcher index*/, event1);
@@ -118,6 +119,7 @@ TEST(CountMetricProducerTest, TestEventsWithNonSlicedCondition) {
sp<MockConditionWizard> wizard = new NaggyMock<MockConditionWizard>();
CountMetricProducer countProducer(kConfigKey, metric, 1, wizard, bucketStartTimeNs);
countProducer.setBucketSize(60 * NS_PER_SEC);
countProducer.onConditionChanged(true, bucketStartTimeNs);
countProducer.onMatchedLogEvent(1 /*matcher index*/, event1);
@@ -179,6 +181,7 @@ TEST(CountMetricProducerTest, TestEventsWithSlicedCondition) {
CountMetricProducer countProducer(kConfigKey, metric, 1 /*condition tracker index*/, wizard,
bucketStartTimeNs);
countProducer.setBucketSize(60 * NS_PER_SEC);
countProducer.onMatchedLogEvent(1 /*log matcher index*/, event1);
countProducer.flushIfNeededLocked(bucketStartTimeNs + 1);
@@ -217,6 +220,8 @@ TEST(CountMetricProducerTest, TestEventWithAppUpgrade) {
sp<MockConditionWizard> wizard = new NaggyMock<MockConditionWizard>();
CountMetricProducer countProducer(kConfigKey, metric, -1 /* no condition */, wizard,
bucketStartTimeNs);
countProducer.setBucketSize(60 * NS_PER_SEC);
sp<AnomalyTracker> anomalyTracker = countProducer.addAnomalyTracker(alert);
EXPECT_TRUE(anomalyTracker != nullptr);
@@ -274,6 +279,7 @@ TEST(CountMetricProducerTest, TestEventWithAppUpgradeInNextBucket) {
sp<MockConditionWizard> wizard = new NaggyMock<MockConditionWizard>();
CountMetricProducer countProducer(kConfigKey, metric, -1 /* no condition */, wizard,
bucketStartTimeNs);
countProducer.setBucketSize(60 * NS_PER_SEC);
// Bucket is flushed yet.
countProducer.onMatchedLogEvent(1 /*log matcher index*/, event1);
@@ -329,6 +335,8 @@ TEST(CountMetricProducerTest, TestAnomalyDetectionUnSliced) {
sp<MockConditionWizard> wizard = new NaggyMock<MockConditionWizard>();
CountMetricProducer countProducer(kConfigKey, metric, -1 /*-1 meaning no condition*/, wizard,
bucketStartTimeNs);
countProducer.setBucketSize(60 * NS_PER_SEC);
sp<AnomalyTracker> anomalyTracker = countProducer.addAnomalyTracker(alert);
int tagId = 1;

View File

@@ -59,6 +59,7 @@ TEST(DurationMetricTrackerTest, TestNoCondition) {
DurationMetricProducer durationProducer(
kConfigKey, metric, -1 /*no condition*/, 1 /* start index */, 2 /* stop index */,
3 /* stop_all index */, false /*nesting*/, wizard, dimensions, bucketStartTimeNs);
durationProducer.setBucketSize(60 * NS_PER_SEC);
durationProducer.onMatchedLogEvent(1 /* start index*/, event1);
durationProducer.onMatchedLogEvent(2 /* stop index*/, event2);
@@ -100,6 +101,8 @@ TEST(DurationMetricTrackerTest, TestNonSlicedCondition) {
DurationMetricProducer durationProducer(
kConfigKey, metric, 0 /* condition index */, 1 /* start index */, 2 /* stop index */,
3 /* stop_all index */, false /*nesting*/, wizard, dimensions, bucketStartTimeNs);
durationProducer.setBucketSize(60 * NS_PER_SEC);
EXPECT_FALSE(durationProducer.mCondition);
EXPECT_FALSE(durationProducer.isConditionSliced());
@@ -149,6 +152,7 @@ TEST(DurationMetricTrackerTest, TestSumDurationWithUpgrade) {
DurationMetricProducer durationProducer(
kConfigKey, metric, -1 /* no condition */, 1 /* start index */, 2 /* stop index */,
3 /* stop_all index */, false /*nesting*/, wizard, dimensions, bucketStartTimeNs);
durationProducer.setBucketSize(60 * NS_PER_SEC);
LogEvent start_event(tagId, startTimeNs);
start_event.init();
@@ -203,6 +207,7 @@ TEST(DurationMetricTrackerTest, TestSumDurationWithUpgradeInFollowingBucket) {
DurationMetricProducer durationProducer(
kConfigKey, metric, -1 /* no condition */, 1 /* start index */, 2 /* stop index */,
3 /* stop_all index */, false /*nesting*/, wizard, dimensions, bucketStartTimeNs);
durationProducer.setBucketSize(60 * NS_PER_SEC);
LogEvent start_event(tagId, startTimeNs);
start_event.init();
@@ -256,6 +261,8 @@ TEST(DurationMetricTrackerTest, TestSumDurationAnomalyWithUpgrade) {
DurationMetricProducer durationProducer(
kConfigKey, metric, -1 /* no condition */, 1 /* start index */, 2 /* stop index */,
3 /* stop_all index */, false /*nesting*/, wizard, dimensions, bucketStartTimeNs);
durationProducer.setBucketSize(60 * NS_PER_SEC);
sp<AnomalyTracker> anomalyTracker = durationProducer.addAnomalyTracker(alert);
EXPECT_TRUE(anomalyTracker != nullptr);
@@ -293,6 +300,7 @@ TEST(DurationMetricTrackerTest, TestMaxDurationWithUpgrade) {
DurationMetricProducer durationProducer(
kConfigKey, metric, -1 /* no condition */, 1 /* start index */, 2 /* stop index */,
3 /* stop_all index */, false /*nesting*/, wizard, dimensions, bucketStartTimeNs);
durationProducer.setBucketSize(60 * NS_PER_SEC);
LogEvent start_event(tagId, startTimeNs);
start_event.init();
@@ -340,6 +348,7 @@ TEST(DurationMetricTrackerTest, TestMaxDurationWithUpgradeInNextBucket) {
DurationMetricProducer durationProducer(
kConfigKey, metric, -1 /* no condition */, 1 /* start index */, 2 /* stop index */,
3 /* stop_all index */, false /*nesting*/, wizard, dimensions, bucketStartTimeNs);
durationProducer.setBucketSize(60 * NS_PER_SEC);
LogEvent start_event(tagId, startTimeNs);
start_event.init();

View File

@@ -67,6 +67,7 @@ TEST(GaugeMetricProducerTest, TestNoCondition) {
GaugeMetricProducer gaugeProducer(kConfigKey, metric, -1 /*-1 meaning no condition*/, wizard,
tagId, bucketStartTimeNs, pullerManager);
gaugeProducer.setBucketSize(60 * NS_PER_SEC);
vector<shared_ptr<LogEvent>> allData;
allData.clear();
@@ -144,6 +145,7 @@ TEST(GaugeMetricProducerTest, TestPushedEventsWithUpgrade) {
GaugeMetricProducer gaugeProducer(kConfigKey, metric, -1 /*-1 meaning no condition*/, wizard,
-1 /* -1 means no pulling */, bucketStartTimeNs,
pullerManager);
gaugeProducer.setBucketSize(60 * NS_PER_SEC);
sp<AnomalyTracker> anomalyTracker = gaugeProducer.addAnomalyTracker(alert);
EXPECT_TRUE(anomalyTracker != nullptr);
@@ -225,6 +227,7 @@ TEST(GaugeMetricProducerTest, TestPulledWithUpgrade) {
GaugeMetricProducer gaugeProducer(kConfigKey, metric, -1 /*-1 meaning no condition*/, wizard,
tagId, bucketStartTimeNs, pullerManager);
gaugeProducer.setBucketSize(60 * NS_PER_SEC);
vector<shared_ptr<LogEvent>> allData;
shared_ptr<LogEvent> event = make_shared<LogEvent>(tagId, bucketStartTimeNs + 1);
@@ -292,6 +295,7 @@ TEST(GaugeMetricProducerTest, TestWithCondition) {
GaugeMetricProducer gaugeProducer(kConfigKey, metric, 1, wizard, tagId,
bucketStartTimeNs, pullerManager);
gaugeProducer.setBucketSize(60 * NS_PER_SEC);
gaugeProducer.onConditionChanged(true, bucketStartTimeNs + 8);
EXPECT_EQ(1UL, gaugeProducer.mCurrentSlicedBucket->size());
@@ -350,6 +354,7 @@ TEST(GaugeMetricProducerTest, TestAnomalyDetection) {
gaugeFieldMatcher->add_child()->set_field(2);
GaugeMetricProducer gaugeProducer(kConfigKey, metric, -1 /*-1 meaning no condition*/, wizard,
tagId, bucketStartTimeNs, pullerManager);
gaugeProducer.setBucketSize(60 * NS_PER_SEC);
Alert alert;
alert.set_id(101);

View File

@@ -66,6 +66,7 @@ TEST(ValueMetricProducerTest, TestNonDimensionalEvents) {
ValueMetricProducer valueProducer(kConfigKey, metric, -1 /*-1 meaning no condition*/, wizard,
tagId, bucketStartTimeNs, pullerManager);
valueProducer.setBucketSize(60 * NS_PER_SEC);
vector<shared_ptr<LogEvent>> allData;
allData.clear();
@@ -79,6 +80,8 @@ TEST(ValueMetricProducerTest, TestNonDimensionalEvents) {
// has one slice
EXPECT_EQ(1UL, valueProducer.mCurrentSlicedBucket.size());
ValueMetricProducer::Interval curInterval = valueProducer.mCurrentSlicedBucket.begin()->second;
valueProducer.setBucketSize(60 * NS_PER_SEC);
// startUpdated:true tainted:0 sum:0 start:11
EXPECT_EQ(true, curInterval.startUpdated);
EXPECT_EQ(0, curInterval.tainted);
@@ -162,7 +165,7 @@ TEST(ValueMetricProducerTest, TestEventsWithNonSlicedCondition) {
ValueMetricProducer valueProducer(kConfigKey, metric, 1, wizard, tagId, bucketStartTimeNs,
pullerManager);
valueProducer.setBucketSize(60 * NS_PER_SEC);
valueProducer.onConditionChanged(true, bucketStartTimeNs + 8);
// has one slice
@@ -215,6 +218,7 @@ TEST(ValueMetricProducerTest, TestPushedEventsWithUpgrade) {
make_shared<StrictMock<MockStatsPullerManager>>();
ValueMetricProducer valueProducer(kConfigKey, metric, -1, wizard, -1, bucketStartTimeNs,
pullerManager);
valueProducer.setBucketSize(60 * NS_PER_SEC);
shared_ptr<LogEvent> event1 = make_shared<LogEvent>(tagId, bucketStartTimeNs + 10);
event1->write(1);
@@ -269,6 +273,7 @@ TEST(ValueMetricProducerTest, TestPulledValueWithUpgrade) {
}));
ValueMetricProducer valueProducer(kConfigKey, metric, -1, wizard, tagId, bucketStartTimeNs,
pullerManager);
valueProducer.setBucketSize(60 * NS_PER_SEC);
vector<shared_ptr<LogEvent>> allData;
allData.clear();
@@ -311,6 +316,7 @@ TEST(ValueMetricProducerTest, TestPushedEventsWithoutCondition) {
ValueMetricProducer valueProducer(kConfigKey, metric, -1, wizard, -1, bucketStartTimeNs,
pullerManager);
valueProducer.setBucketSize(60 * NS_PER_SEC);
shared_ptr<LogEvent> event1 = make_shared<LogEvent>(tagId, bucketStartTimeNs + 10);
event1->write(1);
@@ -357,6 +363,8 @@ TEST(ValueMetricProducerTest, TestAnomalyDetection) {
sp<MockConditionWizard> wizard = new NaggyMock<MockConditionWizard>();
ValueMetricProducer valueProducer(kConfigKey, metric, -1 /*-1 meaning no condition*/, wizard,
-1 /*not pulled*/, bucketStartTimeNs);
valueProducer.setBucketSize(60 * NS_PER_SEC);
sp<AnomalyTracker> anomalyTracker = valueProducer.addAnomalyTracker(alert);