Snap for 8902501 from abcad7b838eec4a525da9adcd0884552fbf8ea21 to mainline-go-networking-release
Change-Id: Ie7b77bb324516e6b2825d074365c5072d7eb57b5
diff --git a/service/Android.bp b/service/Android.bp
index bad74ce..5e96b1e 100644
--- a/service/Android.bp
+++ b/service/Android.bp
@@ -25,6 +25,7 @@
java_library {
name: "service-statsd",
+ defaults: ["standalone-system-server-module-optimize-defaults"],
srcs: [ ":service-statsd-sources" ],
sdk_version: "system_server_current",
libs: [
diff --git a/statsd/src/metrics/DurationMetricProducer.cpp b/statsd/src/metrics/DurationMetricProducer.cpp
index 5b7a1c1..b32a170 100644
--- a/statsd/src/metrics/DurationMetricProducer.cpp
+++ b/statsd/src/metrics/DurationMetricProducer.cpp
@@ -434,27 +434,28 @@
onSlicedConditionMayChangeInternalLocked(overallCondition, eventTime);
}
-void DurationMetricProducer::onActiveStateChangedLocked(const int64_t eventTimeNs) {
- MetricProducer::onActiveStateChangedLocked(eventTimeNs);
+void DurationMetricProducer::onActiveStateChangedLocked(const int64_t eventTimeNs,
+ const bool isActive) {
+ MetricProducer::onActiveStateChangedLocked(eventTimeNs, isActive);
if (!mConditionSliced) {
if (ConditionState::kTrue != mCondition) {
return;
}
- if (mIsActive) {
+ if (isActive) {
flushIfNeededLocked(eventTimeNs);
}
for (auto& whatIt : mCurrentSlicedDurationTrackerMap) {
- whatIt.second->onConditionChanged(mIsActive, eventTimeNs);
+ whatIt.second->onConditionChanged(isActive, eventTimeNs);
}
- } else if (mIsActive) {
+ } else if (isActive) {
flushIfNeededLocked(eventTimeNs);
- onSlicedConditionMayChangeInternalLocked(mIsActive, eventTimeNs);
- } else { // mConditionSliced == true && !mIsActive
+ onSlicedConditionMayChangeInternalLocked(isActive, eventTimeNs);
+ } else { // mConditionSliced == true && !isActive
for (auto& whatIt : mCurrentSlicedDurationTrackerMap) {
- whatIt.second->onConditionChanged(mIsActive, eventTimeNs);
+ whatIt.second->onConditionChanged(isActive, eventTimeNs);
}
}
}
diff --git a/statsd/src/metrics/DurationMetricProducer.h b/statsd/src/metrics/DurationMetricProducer.h
index 206882b..ba5fe95 100644
--- a/statsd/src/metrics/DurationMetricProducer.h
+++ b/statsd/src/metrics/DurationMetricProducer.h
@@ -99,7 +99,7 @@
void onConditionChangedLocked(const bool conditionMet, const int64_t eventTime) override;
// Internal interface to handle active state change.
- void onActiveStateChangedLocked(const int64_t eventTimeNs) override;
+ void onActiveStateChangedLocked(const int64_t eventTimeNs, const bool isActive) override;
// Internal interface to handle sliced condition change.
void onSlicedConditionMayChangeLocked(bool overallCondition, const int64_t eventTime) override;
diff --git a/statsd/src/metrics/GaugeMetricProducer.cpp b/statsd/src/metrics/GaugeMetricProducer.cpp
index 1627742..faa284c 100644
--- a/statsd/src/metrics/GaugeMetricProducer.cpp
+++ b/statsd/src/metrics/GaugeMetricProducer.cpp
@@ -406,12 +406,15 @@
}
}
-void GaugeMetricProducer::onActiveStateChangedLocked(const int64_t eventTimeNs) {
- MetricProducer::onActiveStateChangedLocked(eventTimeNs);
- if (ConditionState::kTrue != mCondition || !mIsPulled) {
+void GaugeMetricProducer::onActiveStateChangedLocked(const int64_t eventTimeNs,
+ const bool isActive) {
+ MetricProducer::onActiveStateChangedLocked(eventTimeNs, isActive);
+
+ if (ConditionState::kTrue != mCondition) {
return;
}
- if (mTriggerAtomId == -1 || (mIsActive && mSamplingType == GaugeMetric::RANDOM_ONE_SAMPLE)) {
+
+ if (isActive && mIsPulled && mSamplingType == GaugeMetric::RANDOM_ONE_SAMPLE) {
pullAndMatchEventsLocked(eventTimeNs);
}
}
@@ -426,7 +429,9 @@
}
flushIfNeededLocked(eventTimeNs);
- if (conditionMet && mIsPulled && mTriggerAtomId == -1) {
+ if (conditionMet && mIsPulled &&
+ (mSamplingType == GaugeMetric::RANDOM_ONE_SAMPLE ||
+ mSamplingType == GaugeMetric::CONDITION_CHANGE_TO_TRUE)) {
pullAndMatchEventsLocked(eventTimeNs);
} // else: Push mode. No need to proactively pull the gauge data.
}
@@ -641,7 +646,7 @@
VLOG("Gauge gauge metric %lld, dump key value: %s", (long long)mMetricId,
slice.first.toString().c_str());
}
- } else {
+ } else if (mIsActive) {
mCurrentSkippedBucket.bucketStartTimeNs = mCurrentBucketStartTimeNs;
mCurrentSkippedBucket.bucketEndTimeNs = bucketEndTime;
if (!maxDropEventsReached()) {
diff --git a/statsd/src/metrics/GaugeMetricProducer.h b/statsd/src/metrics/GaugeMetricProducer.h
index 8bd4a8c..af99535 100644
--- a/statsd/src/metrics/GaugeMetricProducer.h
+++ b/statsd/src/metrics/GaugeMetricProducer.h
@@ -120,7 +120,7 @@
void onConditionChangedLocked(const bool conditionMet, const int64_t eventTime) override;
// Internal interface to handle active state change.
- void onActiveStateChangedLocked(const int64_t eventTimeNs) override;
+ void onActiveStateChangedLocked(const int64_t eventTimeNs, const bool isActive) override;
// Internal interface to handle sliced condition change.
void onSlicedConditionMayChangeLocked(bool overallCondition, const int64_t eventTime) override;
diff --git a/statsd/src/metrics/MetricProducer.cpp b/statsd/src/metrics/MetricProducer.cpp
index 17aa312..75c2476 100644
--- a/statsd/src/metrics/MetricProducer.cpp
+++ b/statsd/src/metrics/MetricProducer.cpp
@@ -194,9 +194,13 @@
if (!mIsActive) {
return;
}
- mIsActive = evaluateActiveStateLocked(elapsedTimestampNs);
- if (!mIsActive) {
- onActiveStateChangedLocked(elapsedTimestampNs);
+ const bool isActive = evaluateActiveStateLocked(elapsedTimestampNs);
+ if (!isActive) { // Metric went from active to not active.
+ onActiveStateChangedLocked(elapsedTimestampNs, false);
+
+ // Set mIsActive to false after onActiveStateChangedLocked to ensure any pulls that occur
+ // through onActiveStateChangedLocked are processed.
+ mIsActive = false;
}
}
@@ -215,10 +219,12 @@
}
activation->start_ns = elapsedTimestampNs;
activation->state = ActivationState::kActive;
- bool oldActiveState = mIsActive;
- mIsActive = true;
- if (!oldActiveState) { // Metric went from not active to active.
- onActiveStateChangedLocked(elapsedTimestampNs);
+ if (!mIsActive) { // Metric was previously inactive and now is active.
+ // Set mIsActive to true before onActiveStateChangedLocked to ensure any pulls that occur
+ // through onActiveStateChangedLocked are processed.
+ mIsActive = true;
+
+ onActiveStateChangedLocked(elapsedTimestampNs, true);
}
}
diff --git a/statsd/src/metrics/MetricProducer.h b/statsd/src/metrics/MetricProducer.h
index 95f8dd4..ab74951 100644
--- a/statsd/src/metrics/MetricProducer.h
+++ b/statsd/src/metrics/MetricProducer.h
@@ -394,7 +394,7 @@
/**
* Flushes all the data including the current partial bucket.
*/
- virtual void flushLocked(const int64_t& eventTimeNs) {
+ void flushLocked(const int64_t& eventTimeNs) {
flushIfNeededLocked(eventTimeNs);
flushCurrentBucketLocked(eventTimeNs, eventTimeNs);
};
@@ -445,8 +445,8 @@
bool evaluateActiveStateLocked(int64_t elapsedTimestampNs);
- virtual void onActiveStateChangedLocked(const int64_t eventTimeNs) {
- if (!mIsActive) {
+ virtual void onActiveStateChangedLocked(const int64_t eventTimeNs, const bool isActive) {
+ if (!isActive) {
flushLocked(eventTimeNs);
}
}
diff --git a/statsd/src/metrics/NumericValueMetricProducer.cpp b/statsd/src/metrics/NumericValueMetricProducer.cpp
index 703b07b..4f8ae23 100644
--- a/statsd/src/metrics/NumericValueMetricProducer.cpp
+++ b/statsd/src/metrics/NumericValueMetricProducer.cpp
@@ -128,10 +128,11 @@
protoOutput->end(valueToken);
}
-void NumericValueMetricProducer::onActiveStateChangedInternalLocked(const int64_t eventTimeNs) {
+void NumericValueMetricProducer::onActiveStateChangedInternalLocked(const int64_t eventTimeNs,
+ const bool isActive) {
// When active state changes from true to false for pulled metric, clear diff base but don't
// reset other counters as we may accumulate more value in the bucket.
- if (mUseDiff && !mIsActive) {
+ if (mUseDiff && !isActive) {
resetBase();
}
}
diff --git a/statsd/src/metrics/NumericValueMetricProducer.h b/statsd/src/metrics/NumericValueMetricProducer.h
index 8eca3f8..1246815 100644
--- a/statsd/src/metrics/NumericValueMetricProducer.h
+++ b/statsd/src/metrics/NumericValueMetricProducer.h
@@ -66,7 +66,8 @@
return config.value_metric(configIndex).links();
}
- void onActiveStateChangedInternalLocked(const int64_t eventTimeNs) override;
+ void onActiveStateChangedInternalLocked(const int64_t eventTimeNs,
+ const bool isActive) override;
// Only called when mIsActive and the event is NOT too late.
void onConditionChangedInternalLocked(const ConditionState oldCondition,
diff --git a/statsd/src/metrics/ValueMetricProducer.cpp b/statsd/src/metrics/ValueMetricProducer.cpp
index f829cb4..0500284 100644
--- a/statsd/src/metrics/ValueMetricProducer.cpp
+++ b/statsd/src/metrics/ValueMetricProducer.cpp
@@ -447,19 +447,24 @@
template <typename AggregatedValue, typename DimExtras>
void ValueMetricProducer<AggregatedValue, DimExtras>::skipCurrentBucket(
const int64_t dropTimeNs, const BucketDropReason reason) {
+ if (!mIsActive) {
+ // Don't keep track of skipped buckets if metric is not active.
+ return;
+ }
+
if (!maxDropEventsReached()) {
mCurrentSkippedBucket.dropEvents.push_back(buildDropEvent(dropTimeNs, reason));
}
mCurrentBucketIsSkipped = true;
}
-// Handle active state change. Active state change is treated like a condition change:
+// Handle active state change. Active state change is *mostly* treated like a condition change:
// - drop bucket if active state change event arrives too late
// - if condition is true, pull data on active state changes
// - ConditionTimer tracks changes based on AND of condition and active state.
template <typename AggregatedValue, typename DimExtras>
void ValueMetricProducer<AggregatedValue, DimExtras>::onActiveStateChangedLocked(
- const int64_t eventTimeNs) {
+ const int64_t eventTimeNs, const bool isActive) {
const bool eventLate = isEventLateLocked(eventTimeNs);
if (eventLate) {
// Drop bucket because event arrived too late, ie. we are missing data for this bucket.
@@ -467,10 +472,9 @@
invalidateCurrentBucket(eventTimeNs, BucketDropReason::EVENT_IN_WRONG_BUCKET);
}
- // Call parent method once we've verified the validity of current bucket.
- MetricProducer::onActiveStateChangedLocked(eventTimeNs);
-
if (ConditionState::kTrue != mCondition) {
+ // Call parent method before early return.
+ MetricProducer::onActiveStateChangedLocked(eventTimeNs, isActive);
return;
}
@@ -480,15 +484,17 @@
pullAndMatchEventsLocked(eventTimeNs);
}
- onActiveStateChangedInternalLocked(eventTimeNs);
+ onActiveStateChangedInternalLocked(eventTimeNs, isActive);
}
- flushIfNeededLocked(eventTimeNs);
+ // Once any pulls are processed, call through to parent method which might flush the current
+ // bucket.
+ MetricProducer::onActiveStateChangedLocked(eventTimeNs, isActive);
// Let condition timer know of new active state.
- mConditionTimer.onConditionChanged(mIsActive, eventTimeNs);
+ mConditionTimer.onConditionChanged(isActive, eventTimeNs);
- updateCurrentSlicedBucketConditionTimers(mIsActive, eventTimeNs);
+ updateCurrentSlicedBucketConditionTimers(isActive, eventTimeNs);
}
template <typename AggregatedValue, typename DimExtras>
diff --git a/statsd/src/metrics/ValueMetricProducer.h b/statsd/src/metrics/ValueMetricProducer.h
index 956580a..80d0bb6 100644
--- a/statsd/src/metrics/ValueMetricProducer.h
+++ b/statsd/src/metrics/ValueMetricProducer.h
@@ -167,9 +167,10 @@
void clearPastBucketsLocked(const int64_t dumpTimeNs) override;
// ValueMetricProducer internal interface to handle active state change.
- void onActiveStateChangedLocked(const int64_t eventTimeNs) override;
+ void onActiveStateChangedLocked(const int64_t eventTimeNs, const bool isActive) override;
- virtual void onActiveStateChangedInternalLocked(const int64_t eventTimeNs) {
+ virtual void onActiveStateChangedInternalLocked(const int64_t eventTimeNs,
+ const bool isActive) {
}
// ValueMetricProducer internal interface to handle condition change.
diff --git a/statsd/tests/e2e/GaugeMetric_e2e_pull_test.cpp b/statsd/tests/e2e/GaugeMetric_e2e_pull_test.cpp
index 1f79f87..984b72e 100644
--- a/statsd/tests/e2e/GaugeMetric_e2e_pull_test.cpp
+++ b/statsd/tests/e2e/GaugeMetric_e2e_pull_test.cpp
@@ -61,6 +61,7 @@
gaugeMetric->set_max_pull_delay_sec(INT_MAX);
config.set_hash_strings_in_metric_report(false);
gaugeMetric->set_split_bucket_for_app_upgrade(true);
+ gaugeMetric->set_min_bucket_size_nanos(1000);
return config;
}
@@ -441,10 +442,10 @@
EXPECT_TRUE(processor->mMetricsManagers.begin()->second->isConfigValid());
processor->mPullerManager->ForceClearPullerCache();
- int startBucketNum = processor->mMetricsManagers.begin()
- ->second->mAllMetricProducers[0]
- ->getCurrentBucketNum();
- EXPECT_GT(startBucketNum, (int64_t)0);
+ const int startBucketNum = processor->mMetricsManagers.begin()
+ ->second->mAllMetricProducers[0]
+ ->getCurrentBucketNum();
+ EXPECT_EQ(startBucketNum, 2);
EXPECT_FALSE(processor->mMetricsManagers.begin()->second->mAllMetricProducers[0]->isActive());
// When creating the config, the gauge metric producer should register the alarm at the
@@ -470,13 +471,28 @@
EXPECT_EQ(pulledAtomStats.atom_id(), ATOM_TAG);
EXPECT_EQ(pulledAtomStats.total_pull(), 0);
+ // Check skipped bucket is not added when metric is not active.
+ int64_t dumpReportTimeNs = appUpgradeTimeNs + 1; // 10 mins + 3 ns.
+ vector<uint8_t> buffer;
+ processor->onDumpReport(cfgKey, dumpReportTimeNs, true /* include_current_partial_bucket */,
+ true /* erase_data */, ADB_DUMP, NO_TIME_CONSTRAINTS, &buffer);
+ ConfigMetricsReportList reports;
+ EXPECT_TRUE(buffer.size() > 0);
+ EXPECT_TRUE(reports.ParseFromArray(&buffer[0], buffer.size()));
+ ASSERT_EQ(1, reports.reports_size());
+ ASSERT_EQ(1, reports.reports(0).metrics_size());
+ StatsLogReport::GaugeMetricDataWrapper gaugeMetrics =
+ reports.reports(0).metrics(0).gauge_metrics();
+ EXPECT_EQ(gaugeMetrics.skipped_size(), 0);
+
// Pulling alarm arrives on time and reset the sequential pulling alarm.
// Event should not be kept.
processor->informPullAlarmFired(nextPullTimeNs + 1); // 15 mins + 1 ns.
EXPECT_EQ(baseTimeNs + startBucketNum * bucketSizeNs + 2 * bucketSizeNs, nextPullTimeNs);
EXPECT_FALSE(processor->mMetricsManagers.begin()->second->mAllMetricProducers[0]->isActive());
- // Activate the metric. A pull occurs upon activation.
+ // Activate the metric. A pull occurs upon activation. The event is kept. 1 total
+ // 15 mins + 2 ms
const int64_t activationNs = configAddedTimeNs + bucketSizeNs + (2 * 1000 * 1000); // 2 millis.
auto batterySaverOnEvent = CreateBatterySaverOnEvent(activationNs);
processor->OnLogEvent(batterySaverOnEvent.get()); // 15 mins + 2 ms.
@@ -491,19 +507,27 @@
EXPECT_EQ(baseTimeNs + startBucketNum * bucketSizeNs + 4 * bucketSizeNs, nextPullTimeNs);
// Create random event to deactivate metric.
- auto deactivationEvent = CreateScreenBrightnessChangedEvent(activationNs + ttlNs + 1, 50);
+ // A pull should not occur here. 3 total.
+ // 25 mins + 2 ms + 1 ns.
+ const int64_t deactivationNs = activationNs + ttlNs + 1;
+ auto deactivationEvent = CreateScreenBrightnessChangedEvent(deactivationNs, 50);
processor->OnLogEvent(deactivationEvent.get());
EXPECT_FALSE(processor->mMetricsManagers.begin()->second->mAllMetricProducers[0]->isActive());
// Event should not be kept. 3 total.
+ // 30 mins + 3 ns.
processor->informPullAlarmFired(nextPullTimeNs + 3);
EXPECT_EQ(baseTimeNs + startBucketNum * bucketSizeNs + 5 * bucketSizeNs, nextPullTimeNs);
+ // Event should not be kept. 3 total.
+ // 35 mins + 2 ns.
processor->informPullAlarmFired(nextPullTimeNs + 2);
+ EXPECT_EQ(baseTimeNs + startBucketNum * bucketSizeNs + 6 * bucketSizeNs, nextPullTimeNs);
- ConfigMetricsReportList reports;
- vector<uint8_t> buffer;
- processor->onDumpReport(cfgKey, configAddedTimeNs + 7 * bucketSizeNs + 10, false, true,
+ buffer.clear();
+ // 40 mins + 10 ns.
+ processor->onDumpReport(cfgKey, configAddedTimeNs + 6 * bucketSizeNs + 10,
+ false /* include_current_partial_bucket */, true /* erase_data */,
ADB_DUMP, FAST, &buffer);
EXPECT_TRUE(buffer.size() > 0);
EXPECT_TRUE(reports.ParseFromArray(&buffer[0], buffer.size()));
@@ -513,7 +537,7 @@
backfillAggregatedAtoms(&reports);
ASSERT_EQ(1, reports.reports_size());
ASSERT_EQ(1, reports.reports(0).metrics_size());
- StatsLogReport::GaugeMetricDataWrapper gaugeMetrics;
+ gaugeMetrics = StatsLogReport::GaugeMetricDataWrapper();
sortMetricDataByDimensionsValue(reports.reports(0).metrics(0).gauge_metrics(), &gaugeMetrics);
ASSERT_GT((int)gaugeMetrics.data_size(), 0);
@@ -552,10 +576,21 @@
ASSERT_EQ(0, bucketInfo.wall_clock_timestamp_nanos_size());
EXPECT_EQ(MillisToNano(NanoToMillis(baseTimeNs + 5 * bucketSizeNs)),
bucketInfo.start_bucket_elapsed_nanos());
- EXPECT_EQ(MillisToNano(NanoToMillis(activationNs + ttlNs + 1)),
- bucketInfo.end_bucket_elapsed_nanos());
+ EXPECT_EQ(MillisToNano(NanoToMillis(deactivationNs)), bucketInfo.end_bucket_elapsed_nanos());
EXPECT_TRUE(bucketInfo.atom(0).subsystem_sleep_state().subsystem_name().empty());
EXPECT_GT(bucketInfo.atom(0).subsystem_sleep_state().time_millis(), 0);
+
+ // Check skipped bucket is not added after deactivation.
+ dumpReportTimeNs = configAddedTimeNs + 8 * bucketSizeNs + 10;
+ buffer.clear();
+ processor->onDumpReport(cfgKey, dumpReportTimeNs, true /* include_current_partial_bucket */,
+ true /* erase_data */, ADB_DUMP, NO_TIME_CONSTRAINTS, &buffer);
+ EXPECT_TRUE(buffer.size() > 0);
+ EXPECT_TRUE(reports.ParseFromArray(&buffer[0], buffer.size()));
+ ASSERT_EQ(1, reports.reports_size());
+ ASSERT_EQ(1, reports.reports(0).metrics_size());
+ gaugeMetrics = reports.reports(0).metrics(0).gauge_metrics();
+ EXPECT_EQ(gaugeMetrics.skipped_size(), 0);
}
TEST_F(GaugeMetricE2ePulledTest, TestRandomSamplePulledEventsNoCondition) {
diff --git a/statsd/tests/e2e/ValueMetric_pull_e2e_test.cpp b/statsd/tests/e2e/ValueMetric_pull_e2e_test.cpp
index 996c2fb..385a473 100644
--- a/statsd/tests/e2e/ValueMetric_pull_e2e_test.cpp
+++ b/statsd/tests/e2e/ValueMetric_pull_e2e_test.cpp
@@ -61,6 +61,7 @@
valueMetric->set_skip_zero_diff_output(false);
valueMetric->set_max_pull_delay_sec(INT_MAX);
valueMetric->set_split_bucket_for_app_upgrade(true);
+ valueMetric->set_min_bucket_size_nanos(1000);
return config;
}
@@ -433,10 +434,10 @@
EXPECT_TRUE(processor->mMetricsManagers.begin()->second->isConfigValid());
processor->mPullerManager->ForceClearPullerCache();
- int startBucketNum = processor->mMetricsManagers.begin()
- ->second->mAllMetricProducers[0]
- ->getCurrentBucketNum();
- EXPECT_GT(startBucketNum, (int64_t)0);
+ const int startBucketNum = processor->mMetricsManagers.begin()
+ ->second->mAllMetricProducers[0]
+ ->getCurrentBucketNum();
+ EXPECT_EQ(startBucketNum, 2);
EXPECT_FALSE(processor->mMetricsManagers.begin()->second->mAllMetricProducers[0]->isActive());
// When creating the config, the value metric producer should register the alarm at the
@@ -448,61 +449,109 @@
processor->mPullerManager->mReceivers.begin()->second.front().nextPullTimeNs;
EXPECT_EQ(baseTimeNs + startBucketNum * bucketSizeNs + bucketSizeNs, expectedPullTimeNs);
- // Check no pull occurred on metric initialization when it's not active.
+ // Initialize metric.
const int64_t metricInitTimeNs = configAddedTimeNs + 1; // 10 mins + 1 ns.
processor->onStatsdInitCompleted(metricInitTimeNs);
+
+ // Check no pull occurred since metric not active.
StatsdStatsReport_PulledAtomStats pulledAtomStats = getPulledAtomStats();
EXPECT_EQ(pulledAtomStats.atom_id(), util::SUBSYSTEM_SLEEP_STATE);
EXPECT_EQ(pulledAtomStats.total_pull(), 0);
- // Check no pull occurred on app upgrade when metric is not active.
- const int64_t appUpgradeTimeNs = metricInitTimeNs + 1; // 10 mins + 2 ns.
- processor->notifyAppUpgrade(appUpgradeTimeNs, "appName", 1000 /* uid */, 2 /* version */);
- pulledAtomStats = getPulledAtomStats();
- EXPECT_EQ(pulledAtomStats.atom_id(), util::SUBSYSTEM_SLEEP_STATE);
- EXPECT_EQ(pulledAtomStats.total_pull(), 0);
-
- // Check no pull occurred on dump report when metric is not active.
- int64_t dumpReportTimeNs = appUpgradeTimeNs + 1; // 10 mins + 3 ns.
+ // Check skip bucket is not added when metric is not active.
+ int64_t dumpReportTimeNs = metricInitTimeNs + 1; // 10 mins + 2 ns.
vector<uint8_t> buffer;
- processor->onDumpReport(cfgKey, dumpReportTimeNs, true, true, ADB_DUMP, NO_TIME_CONSTRAINTS,
- &buffer);
+ processor->onDumpReport(cfgKey, dumpReportTimeNs, true /* include_current_partial_bucket */,
+ true /* erase_data */, ADB_DUMP, FAST, &buffer);
+ ConfigMetricsReportList reports;
+ EXPECT_TRUE(buffer.size() > 0);
+ EXPECT_TRUE(reports.ParseFromArray(&buffer[0], buffer.size()));
+ ASSERT_EQ(1, reports.reports_size());
+ ASSERT_EQ(1, reports.reports(0).metrics_size());
+ StatsLogReport::ValueMetricDataWrapper valueMetrics =
+ reports.reports(0).metrics(0).value_metrics();
+ EXPECT_EQ(valueMetrics.skipped_size(), 0);
+
+ // App upgrade.
+ const int64_t appUpgradeTimeNs = dumpReportTimeNs + 1; // 10 mins + 3 ns.
+ processor->notifyAppUpgrade(appUpgradeTimeNs, "appName", 1000 /* uid */, 2 /* version */);
+
+ // Check no pull occurred since metric not active.
pulledAtomStats = getPulledAtomStats();
EXPECT_EQ(pulledAtomStats.atom_id(), util::SUBSYSTEM_SLEEP_STATE);
EXPECT_EQ(pulledAtomStats.total_pull(), 0);
- // Pulling alarm arrives on time and reset the sequential pulling alarm.
+ // Check skip bucket is not added when metric is not active.
+ dumpReportTimeNs = appUpgradeTimeNs + 1; // 10 mins + 4 ns.
+ buffer.clear();
+ processor->onDumpReport(cfgKey, dumpReportTimeNs, true /* include_current_partial_bucket */,
+ true /* erase_data */, ADB_DUMP, FAST, &buffer);
+ EXPECT_TRUE(buffer.size() > 0);
+ EXPECT_TRUE(reports.ParseFromArray(&buffer[0], buffer.size()));
+ ASSERT_EQ(1, reports.reports_size());
+ ASSERT_EQ(1, reports.reports(0).metrics_size());
+ valueMetrics = reports.reports(0).metrics(0).value_metrics();
+ EXPECT_EQ(valueMetrics.skipped_size(), 0);
+
+ // Dump report with a pull. The pull should not happen because metric is inactive.
+ dumpReportTimeNs = dumpReportTimeNs + 1; // 10 mins + 6 ns.
+ buffer.clear();
+ processor->onDumpReport(cfgKey, dumpReportTimeNs, true /* include_current_partial_bucket */,
+ true /* erase_data */, ADB_DUMP, NO_TIME_CONSTRAINTS, &buffer);
+ pulledAtomStats = getPulledAtomStats();
+ EXPECT_EQ(pulledAtomStats.atom_id(), util::SUBSYSTEM_SLEEP_STATE);
+ EXPECT_EQ(pulledAtomStats.total_pull(), 0);
+
+ // Check skipped bucket is not added from the dump operation when metric is not active.
+ EXPECT_TRUE(buffer.size() > 0);
+ EXPECT_TRUE(reports.ParseFromArray(&buffer[0], buffer.size()));
+ ASSERT_EQ(1, reports.reports_size());
+ ASSERT_EQ(1, reports.reports(0).metrics_size());
+ valueMetrics = reports.reports(0).metrics(0).value_metrics();
+ EXPECT_EQ(valueMetrics.skipped_size(), 0);
+
+ // Pulling alarm arrives on time and reset the sequential pulling alarm. This bucket is skipped.
processor->informPullAlarmFired(expectedPullTimeNs + 1); // 15 mins + 1 ns.
EXPECT_EQ(baseTimeNs + startBucketNum * bucketSizeNs + 2 * bucketSizeNs, expectedPullTimeNs);
EXPECT_FALSE(processor->mMetricsManagers.begin()->second->mAllMetricProducers[0]->isActive());
- // Activate the metric. A pull occurs here
+ // Activate the metric. A pull occurs here that sets the base.
+ // 15 mins + 2 ms
const int64_t activationNs = configAddedTimeNs + bucketSizeNs + (2 * 1000 * 1000); // 2 millis.
auto batterySaverOnEvent = CreateBatterySaverOnEvent(activationNs);
processor->OnLogEvent(batterySaverOnEvent.get()); // 15 mins + 2 ms.
EXPECT_TRUE(processor->mMetricsManagers.begin()->second->mAllMetricProducers[0]->isActive());
+ // This bucket should be kept. 1 total
processor->informPullAlarmFired(expectedPullTimeNs + 1); // 20 mins + 1 ns.
EXPECT_EQ(baseTimeNs + startBucketNum * bucketSizeNs + 3 * bucketSizeNs, expectedPullTimeNs);
+ // 25 mins + 2 ns.
+ // This bucket should be kept. 2 total
processor->informPullAlarmFired(expectedPullTimeNs + 2); // 25 mins + 2 ns.
EXPECT_EQ(baseTimeNs + startBucketNum * bucketSizeNs + 4 * bucketSizeNs, expectedPullTimeNs);
// Create random event to deactivate metric.
- auto deactivationEvent = CreateScreenBrightnessChangedEvent(activationNs + ttlNs + 1, 50);
+ // A pull occurs here and a partial bucket is created. The bucket ending here is kept. 3 total.
+ // 25 mins + 2 ms + 1 ns.
+ const int64_t deactivationNs = activationNs + ttlNs + 1;
+ auto deactivationEvent = CreateScreenBrightnessChangedEvent(deactivationNs, 50);
processor->OnLogEvent(deactivationEvent.get());
EXPECT_FALSE(processor->mMetricsManagers.begin()->second->mAllMetricProducers[0]->isActive());
+ // 30 mins + 3 ns. This bucket is skipped.
processor->informPullAlarmFired(expectedPullTimeNs + 3);
EXPECT_EQ(baseTimeNs + startBucketNum * bucketSizeNs + 5 * bucketSizeNs, expectedPullTimeNs);
+ // 35 mins + 4 ns. This bucket is skipped
processor->informPullAlarmFired(expectedPullTimeNs + 4);
EXPECT_EQ(baseTimeNs + startBucketNum * bucketSizeNs + 6 * bucketSizeNs, expectedPullTimeNs);
- ConfigMetricsReportList reports;
+ dumpReportTimeNs = configAddedTimeNs + 6 * bucketSizeNs + 10;
buffer.clear();
- processor->onDumpReport(cfgKey, configAddedTimeNs + 7 * bucketSizeNs + 10, false, true,
- ADB_DUMP, FAST, &buffer);
+ // 40 mins + 10 ns.
+ processor->onDumpReport(cfgKey, dumpReportTimeNs, false /* include_current_partial_bucket */,
+ true /* erase_data */, ADB_DUMP, FAST, &buffer);
EXPECT_TRUE(buffer.size() > 0);
EXPECT_TRUE(reports.ParseFromArray(&buffer[0], buffer.size()));
backfillDimensionPath(&reports);
@@ -510,7 +559,7 @@
backfillStartEndTimestamp(&reports);
ASSERT_EQ(1, reports.reports_size());
ASSERT_EQ(1, reports.reports(0).metrics_size());
- StatsLogReport::ValueMetricDataWrapper valueMetrics;
+ valueMetrics = StatsLogReport::ValueMetricDataWrapper();
sortMetricDataByDimensionsValue(reports.reports(0).metrics(0).value_metrics(), &valueMetrics);
ASSERT_GT((int)valueMetrics.data_size(), 0);
@@ -520,8 +569,8 @@
EXPECT_EQ(1 /* subsystem name field */,
data.dimensions_in_what().value_tuple().dimensions_value(0).field());
EXPECT_FALSE(data.dimensions_in_what().value_tuple().dimensions_value(0).value_str().empty());
- // We have 2 full buckets, the two surrounding the activation are dropped.
- ASSERT_EQ(2, data.bucket_info_size());
+ // We have 3 full buckets, the two surrounding the activation are dropped.
+ ASSERT_EQ(3, data.bucket_info_size());
auto bucketInfo = data.bucket_info(0);
EXPECT_EQ(baseTimeNs + 3 * bucketSizeNs, bucketInfo.start_bucket_elapsed_nanos());
@@ -532,6 +581,25 @@
EXPECT_EQ(baseTimeNs + 4 * bucketSizeNs, bucketInfo.start_bucket_elapsed_nanos());
EXPECT_EQ(baseTimeNs + 5 * bucketSizeNs, bucketInfo.end_bucket_elapsed_nanos());
ASSERT_EQ(1, bucketInfo.values_size());
+
+ bucketInfo = data.bucket_info(2);
+ EXPECT_EQ(MillisToNano(NanoToMillis(baseTimeNs + 5 * bucketSizeNs)),
+ bucketInfo.start_bucket_elapsed_nanos());
+ EXPECT_EQ(MillisToNano(NanoToMillis(deactivationNs)), bucketInfo.end_bucket_elapsed_nanos());
+ ASSERT_EQ(1, bucketInfo.values_size());
+
+ // Check skipped bucket is not added after deactivation.
+ dumpReportTimeNs = configAddedTimeNs + 7 * bucketSizeNs + 10;
+ buffer.clear();
+ // 45 mins + 10 ns.
+ processor->onDumpReport(cfgKey, dumpReportTimeNs, true /* include_current_partial_bucket */,
+ true /* erase_data */, ADB_DUMP, FAST, &buffer);
+ EXPECT_TRUE(buffer.size() > 0);
+ EXPECT_TRUE(reports.ParseFromArray(&buffer[0], buffer.size()));
+ ASSERT_EQ(1, reports.reports_size());
+ ASSERT_EQ(1, reports.reports(0).metrics_size());
+ valueMetrics = reports.reports(0).metrics(0).value_metrics();
+ EXPECT_EQ(valueMetrics.skipped_size(), 0);
}
/**
diff --git a/tests/src/android/cts/statsd/metric/ValueMetricsTests.java b/tests/src/android/cts/statsd/metric/ValueMetricsTests.java
index 0cf5bbb..adfc565 100644
--- a/tests/src/android/cts/statsd/metric/ValueMetricsTests.java
+++ b/tests/src/android/cts/statsd/metric/ValueMetricsTests.java
@@ -319,11 +319,8 @@
LogUtil.CLog.d("Got the following value metric data: " + metricReport.toString());
assertThat(metricReport.getMetricId()).isEqualTo(MetricsUtils.VALUE_METRIC_ID);
assertThat(metricReport.getValueMetrics().getDataList()).isEmpty();
- // Bucket is skipped because metric is not activated.
- assertThat(metricReport.getValueMetrics().getSkippedList()).isNotEmpty();
- assertThat(metricReport.getValueMetrics().getSkipped(0).getDropEventList()).isNotEmpty();
- assertThat(metricReport.getValueMetrics().getSkipped(0).getDropEvent(0).getDropReason())
- .isEqualTo(BucketDropReason.NO_DATA);
+ // Skipped buckets are not added when metric is empty.
+ assertThat(metricReport.getValueMetrics().getSkippedList()).isEmpty();
}
public void testValueMetricWithConditionAndActivation() throws Exception {
