deprecate ALL_CONDITION_CHANGE in gauge
The feature set for GaugeMetric is the following:
1. Pull/Push once per bucket ----> RANDON_ONE_SAMPLE
Have 1 gauge per bucket, if condition is true
2. Pull when condition becomes true ----> CONDITION_CHANGE_TO_TRUE
Have 1 gauge per condition change to true, up to n
3. Pushed events multiple copies ----> FIRST_N_SAMPLE
Have 1 gauge per event, up to n
4. Pull on trigger ----> FIRST_N_SAMPLE
Have 1 gauge per trigger, up to n
For P, feature 1,2,3 work and same config can be used for P&Q.
(FIRST_N_SAMPLES is new but the code works, as would otherwise be
intended)
Feature 4 is new in Q.
Bug: 119783642
Bug: 118639742
Test: unit test
Change-Id: I671f7a5d3fdf9c2db4e8b6a137e3e45211874dcc
diff --git a/cmds/statsd/src/metrics/GaugeMetricProducer.cpp b/cmds/statsd/src/metrics/GaugeMetricProducer.cpp
index 461ad28..3a34743 100644
--- a/cmds/statsd/src/metrics/GaugeMetricProducer.cpp
+++ b/cmds/statsd/src/metrics/GaugeMetricProducer.cpp
@@ -320,11 +320,11 @@
triggerPuller = mCondition && mCurrentSlicedBucket->empty();
break;
}
- case GaugeMetric::ALL_CONDITION_CHANGES: {
- triggerPuller = true;
+ case GaugeMetric::CONDITION_CHANGE_TO_TRUE: {
+ triggerPuller = mCondition;
break;
}
- case GaugeMetric::CONDITION_CHANGE_TO_TRUE: {
+ case GaugeMetric::FIRST_N_SAMPLES: {
triggerPuller = mCondition;
break;
}
@@ -352,7 +352,7 @@
VLOG("GaugeMetric %lld onConditionChanged", (long long)mMetricId);
flushIfNeededLocked(eventTimeNs);
mCondition = conditionMet;
- if (mIsPulled) {
+ if (mIsPulled && mTriggerAtomId == -1) {
pullAndMatchEventsLocked(eventTimeNs);
} // else: Push mode. No need to proactively pull the gauge data.
}
@@ -365,7 +365,7 @@
// If the condition is sliced, mCondition is true if any of the dimensions is true. And we will
// pull for every dimension.
mCondition = overallCondition;
- if (mIsPulled) {
+ if (mIsPulled && mTriggerAtomId == -1) {
pullAndMatchEventsLocked(eventTimeNs);
} // else: Push mode. No need to proactively pull the gauge data.
}
diff --git a/cmds/statsd/src/metrics/metrics_manager_util.cpp b/cmds/statsd/src/metrics/metrics_manager_util.cpp
index 47b0376..4ac55b5 100644
--- a/cmds/statsd/src/metrics/metrics_manager_util.cpp
+++ b/cmds/statsd/src/metrics/metrics_manager_util.cpp
@@ -535,9 +535,13 @@
int triggerTrackerIndex;
int triggerAtomId = -1;
- if (pullTagId != -1 && metric.has_trigger_event()) {
- // event_trigger should be used with ALL_CONDITION_CHANGES
- if (metric.sampling_type() != GaugeMetric::ALL_CONDITION_CHANGES) {
+ if (metric.has_trigger_event()) {
+ if (pullTagId == -1) {
+ ALOGW("Pull atom not specified for trigger");
+ return false;
+ }
+ // event_trigger should be used with FIRST_N_SAMPLES
+ if (metric.sampling_type() != GaugeMetric::FIRST_N_SAMPLES) {
return false;
}
if (!handlePullMetricTriggerWithLogTrackers(metric.trigger_event(), metricIndex,
@@ -549,6 +553,12 @@
triggerAtomId = *(triggerAtomMatcher->getAtomIds().begin());
}
+ if (!metric.has_trigger_event() && pullTagId != -1 &&
+ metric.sampling_type() == GaugeMetric::FIRST_N_SAMPLES) {
+ ALOGW("FIRST_N_SAMPLES is only for pushed event or pull_on_trigger");
+ return false;
+ }
+
int conditionIndex = -1;
if (metric.has_condition()) {
bool good = handleMetricWithConditions(
diff --git a/cmds/statsd/src/statsd_config.proto b/cmds/statsd/src/statsd_config.proto
index f955df2..61854a4 100644
--- a/cmds/statsd/src/statsd_config.proto
+++ b/cmds/statsd/src/statsd_config.proto
@@ -233,8 +233,9 @@
enum SamplingType {
RANDOM_ONE_SAMPLE = 1;
- ALL_CONDITION_CHANGES = 2;
+ ALL_CONDITION_CHANGES = 2 [deprecated = true];
CONDITION_CHANGE_TO_TRUE = 3;
+ FIRST_N_SAMPLES = 4;
}
optional SamplingType sampling_type = 9 [default = RANDOM_ONE_SAMPLE] ;
diff --git a/cmds/statsd/tests/e2e/GaugeMetric_e2e_push_test.cpp b/cmds/statsd/tests/e2e/GaugeMetric_e2e_push_test.cpp
index 71afedf..3af8212 100644
--- a/cmds/statsd/tests/e2e/GaugeMetric_e2e_push_test.cpp
+++ b/cmds/statsd/tests/e2e/GaugeMetric_e2e_push_test.cpp
@@ -89,7 +89,7 @@
TEST(GaugeMetricE2eTest, TestMultipleFieldsForPushedEvent) {
for (const auto& sampling_type :
- { GaugeMetric::ALL_CONDITION_CHANGES, GaugeMetric:: RANDOM_ONE_SAMPLE }) {
+ { GaugeMetric::FIRST_N_SAMPLES, GaugeMetric:: RANDOM_ONE_SAMPLE }) {
auto config = CreateStatsdConfigForPushedEvent(sampling_type);
int64_t bucketStartTimeNs = 10000000000;
int64_t bucketSizeNs =
@@ -170,7 +170,7 @@
data.dimensions_in_what().value_tuple().dimensions_value(0).field());
EXPECT_EQ(appUid1, data.dimensions_in_what().value_tuple().dimensions_value(0).value_int());
EXPECT_EQ(3, data.bucket_info_size());
- if (sampling_type == GaugeMetric::ALL_CONDITION_CHANGES) {
+ if (sampling_type == GaugeMetric::FIRST_N_SAMPLES) {
EXPECT_EQ(2, data.bucket_info(0).atom_size());
EXPECT_EQ(2, data.bucket_info(0).elapsed_timestamp_nanos_size());
EXPECT_EQ(0, data.bucket_info(0).wall_clock_timestamp_nanos_size());
diff --git a/cmds/statsd/tests/metrics/GaugeMetricProducer_test.cpp b/cmds/statsd/tests/metrics/GaugeMetricProducer_test.cpp
index 737408d..67a9f7f 100644
--- a/cmds/statsd/tests/metrics/GaugeMetricProducer_test.cpp
+++ b/cmds/statsd/tests/metrics/GaugeMetricProducer_test.cpp
@@ -589,7 +589,7 @@
GaugeMetric metric;
metric.set_id(metricId);
metric.set_bucket(ONE_MINUTE);
- metric.set_sampling_type(GaugeMetric::ALL_CONDITION_CHANGES);
+ metric.set_sampling_type(GaugeMetric::FIRST_N_SAMPLES);
metric.mutable_gauge_fields_filter()->set_include_all(false);
auto gaugeFieldMatcher = metric.mutable_gauge_fields_filter()->mutable_fields();
gaugeFieldMatcher->set_field(tagId);
@@ -662,7 +662,7 @@
GaugeMetric metric;
metric.set_id(metricId);
metric.set_bucket(ONE_MINUTE);
- metric.set_sampling_type(GaugeMetric::ALL_CONDITION_CHANGES);
+ metric.set_sampling_type(GaugeMetric::FIRST_N_SAMPLES);
metric.mutable_gauge_fields_filter()->set_include_all(true);
auto dimensionMatcher = metric.mutable_dimensions_in_what();
// use field 1 as dimension.