From 4c31f67965bd39cd9ec0579e6e732976e1e5bb36 Mon Sep 17 00:00:00 2001 From: Chenjie Yu Date: Tue, 21 Aug 2018 15:42:40 -0700 Subject: [PATCH] Remove dimension fields in GaugeMetric output GaugeMetric output is atom format and can contain all fields including those already included in dimensions. This is not necessary and duplicate. And, if we do set dimensions, we can use this to reduce data size similar to repeated fields, where same dimension value only appear once. Bug: 113061955 Test: unit test Change-Id: I299bd1cb1b9b90ea7426ef182df78d2ffc091910 --- .../src/metrics/GaugeMetricProducer.cpp | 19 ++- cmds/statsd/src/metrics/GaugeMetricProducer.h | 1 + .../tests/e2e/GaugeMetric_e2e_pull_test.cpp | 27 ++--- .../metrics/GaugeMetricProducer_test.cpp | 108 +++++++++++++++++- 4 files changed, 136 insertions(+), 19 deletions(-) diff --git a/cmds/statsd/src/metrics/GaugeMetricProducer.cpp b/cmds/statsd/src/metrics/GaugeMetricProducer.cpp index 284c4511a16f6..bcfcd7a4f06ee 100644 --- a/cmds/statsd/src/metrics/GaugeMetricProducer.cpp +++ b/cmds/statsd/src/metrics/GaugeMetricProducer.cpp @@ -367,13 +367,26 @@ void GaugeMetricProducer::onSlicedConditionMayChangeLocked(bool overallCondition } std::shared_ptr> GaugeMetricProducer::getGaugeFields(const LogEvent& event) { + std::shared_ptr> gaugeFields; if (mFieldMatchers.size() > 0) { - std::shared_ptr> gaugeFields = std::make_shared>(); + gaugeFields = std::make_shared>(); filterGaugeValues(mFieldMatchers, event.getValues(), gaugeFields.get()); - return gaugeFields; } else { - return std::make_shared>(event.getValues()); + gaugeFields = std::make_shared>(event.getValues()); } + // Trim all dimension fields from output. Dimensions will appear in output report and will + // benefit from dictionary encoding. For large pulled atoms, this can give the benefit of + // optional repeated field. + for (const auto& field : mDimensionsInWhat) { + for (auto it = gaugeFields->begin(); it != gaugeFields->end();) { + if (it->mField.matches(field)) { + it = gaugeFields->erase(it); + } else { + it++; + } + } + } + return gaugeFields; } void GaugeMetricProducer::onDataPulled(const std::vector>& allData) { diff --git a/cmds/statsd/src/metrics/GaugeMetricProducer.h b/cmds/statsd/src/metrics/GaugeMetricProducer.h index 15be1d7c470ba..e3da5db4b90ed 100644 --- a/cmds/statsd/src/metrics/GaugeMetricProducer.h +++ b/cmds/statsd/src/metrics/GaugeMetricProducer.h @@ -175,6 +175,7 @@ private: FRIEND_TEST(GaugeMetricProducerTest, TestPulledEventsAnomalyDetection); FRIEND_TEST(GaugeMetricProducerTest, TestFirstBucket); FRIEND_TEST(GaugeMetricProducerTest, TestPullOnTrigger); + FRIEND_TEST(GaugeMetricProducerTest, TestRemoveDimensionInOutput); }; } // namespace statsd diff --git a/cmds/statsd/tests/e2e/GaugeMetric_e2e_pull_test.cpp b/cmds/statsd/tests/e2e/GaugeMetric_e2e_pull_test.cpp index d98395e78467b..ea6eb3f3d7126 100644 --- a/cmds/statsd/tests/e2e/GaugeMetric_e2e_pull_test.cpp +++ b/cmds/statsd/tests/e2e/GaugeMetric_e2e_pull_test.cpp @@ -48,6 +48,7 @@ StatsdConfig CreateStatsdConfig(const GaugeMetric::SamplingType sampling_type) { *gaugeMetric->mutable_dimensions_in_what() = CreateDimensions(android::util::TEMPERATURE, {2/* sensor name field */ }); gaugeMetric->set_bucket(FIVE_MINUTES); + config.set_hash_strings_in_metric_report(false); return config; } @@ -150,7 +151,7 @@ TEST(GaugeMetricE2eTest, TestRandomSamplePulledEvents) { EXPECT_EQ(1, data.bucket_info(0).wall_clock_timestamp_nanos_size()); EXPECT_EQ(baseTimeNs + 2 * bucketSizeNs, data.bucket_info(0).start_bucket_elapsed_nanos()); EXPECT_EQ(baseTimeNs + 3 * bucketSizeNs, data.bucket_info(0).end_bucket_elapsed_nanos()); - EXPECT_FALSE(data.bucket_info(0).atom(0).temperature().sensor_name().empty()); + EXPECT_TRUE(data.bucket_info(0).atom(0).temperature().sensor_name().empty()); EXPECT_GT(data.bucket_info(0).atom(0).temperature().temperature_dc(), 0); EXPECT_EQ(1, data.bucket_info(1).atom_size()); @@ -159,7 +160,7 @@ TEST(GaugeMetricE2eTest, TestRandomSamplePulledEvents) { EXPECT_EQ(configAddedTimeNs + 55, data.bucket_info(0).elapsed_timestamp_nanos(0)); EXPECT_EQ(baseTimeNs + 3 * bucketSizeNs, data.bucket_info(1).start_bucket_elapsed_nanos()); EXPECT_EQ(baseTimeNs + 4 * bucketSizeNs, data.bucket_info(1).end_bucket_elapsed_nanos()); - EXPECT_FALSE(data.bucket_info(1).atom(0).temperature().sensor_name().empty()); + EXPECT_TRUE(data.bucket_info(1).atom(0).temperature().sensor_name().empty()); EXPECT_GT(data.bucket_info(1).atom(0).temperature().temperature_dc(), 0); EXPECT_EQ(1, data.bucket_info(2).atom_size()); @@ -168,7 +169,7 @@ TEST(GaugeMetricE2eTest, TestRandomSamplePulledEvents) { data.bucket_info(2).elapsed_timestamp_nanos(0)); EXPECT_EQ(baseTimeNs + 4 * bucketSizeNs, data.bucket_info(2).start_bucket_elapsed_nanos()); EXPECT_EQ(baseTimeNs + 5 * bucketSizeNs, data.bucket_info(2).end_bucket_elapsed_nanos()); - EXPECT_FALSE(data.bucket_info(2).atom(0).temperature().sensor_name().empty()); + EXPECT_TRUE(data.bucket_info(2).atom(0).temperature().sensor_name().empty()); EXPECT_GT(data.bucket_info(2).atom(0).temperature().temperature_dc(), 0); EXPECT_EQ(1, data.bucket_info(3).atom_size()); @@ -177,7 +178,7 @@ TEST(GaugeMetricE2eTest, TestRandomSamplePulledEvents) { data.bucket_info(3).elapsed_timestamp_nanos(0)); EXPECT_EQ(baseTimeNs + 5 * bucketSizeNs, data.bucket_info(3).start_bucket_elapsed_nanos()); EXPECT_EQ(baseTimeNs + 6 * bucketSizeNs, data.bucket_info(3).end_bucket_elapsed_nanos()); - EXPECT_FALSE(data.bucket_info(3).atom(0).temperature().sensor_name().empty()); + EXPECT_TRUE(data.bucket_info(3).atom(0).temperature().sensor_name().empty()); EXPECT_GT(data.bucket_info(3).atom(0).temperature().temperature_dc(), 0); EXPECT_EQ(1, data.bucket_info(4).atom_size()); @@ -186,7 +187,7 @@ TEST(GaugeMetricE2eTest, TestRandomSamplePulledEvents) { data.bucket_info(4).elapsed_timestamp_nanos(0)); EXPECT_EQ(baseTimeNs + 7 * bucketSizeNs, data.bucket_info(4).start_bucket_elapsed_nanos()); EXPECT_EQ(baseTimeNs + 8 * bucketSizeNs, data.bucket_info(4).end_bucket_elapsed_nanos()); - EXPECT_FALSE(data.bucket_info(4).atom(0).temperature().sensor_name().empty()); + EXPECT_TRUE(data.bucket_info(4).atom(0).temperature().sensor_name().empty()); EXPECT_GT(data.bucket_info(4).atom(0).temperature().temperature_dc(), 0); EXPECT_EQ(1, data.bucket_info(5).atom_size()); @@ -195,7 +196,7 @@ TEST(GaugeMetricE2eTest, TestRandomSamplePulledEvents) { data.bucket_info(5).elapsed_timestamp_nanos(0)); EXPECT_EQ(baseTimeNs + 8 * bucketSizeNs, data.bucket_info(5).start_bucket_elapsed_nanos()); EXPECT_EQ(baseTimeNs + 9 * bucketSizeNs, data.bucket_info(5).end_bucket_elapsed_nanos()); - EXPECT_FALSE(data.bucket_info(5).atom(0).temperature().sensor_name().empty()); + EXPECT_TRUE(data.bucket_info(5).atom(0).temperature().sensor_name().empty()); EXPECT_GT(data.bucket_info(5).atom(0).temperature().temperature_dc(), 0); } @@ -273,7 +274,7 @@ TEST(GaugeMetricE2eTest, TestAllConditionChangesSamplePulledEvents) { EXPECT_EQ(1, data.bucket_info(0).wall_clock_timestamp_nanos_size()); EXPECT_EQ(baseTimeNs + 2 * bucketSizeNs, data.bucket_info(0).start_bucket_elapsed_nanos()); EXPECT_EQ(baseTimeNs + 3 * bucketSizeNs, data.bucket_info(0).end_bucket_elapsed_nanos()); - EXPECT_FALSE(data.bucket_info(0).atom(0).temperature().sensor_name().empty()); + EXPECT_TRUE(data.bucket_info(0).atom(0).temperature().sensor_name().empty()); EXPECT_GT(data.bucket_info(0).atom(0).temperature().temperature_dc(), 0); EXPECT_EQ(1, data.bucket_info(1).atom_size()); @@ -282,7 +283,7 @@ TEST(GaugeMetricE2eTest, TestAllConditionChangesSamplePulledEvents) { EXPECT_EQ(configAddedTimeNs + 55, data.bucket_info(0).elapsed_timestamp_nanos(0)); EXPECT_EQ(baseTimeNs + 3 * bucketSizeNs, data.bucket_info(1).start_bucket_elapsed_nanos()); EXPECT_EQ(baseTimeNs + 4 * bucketSizeNs, data.bucket_info(1).end_bucket_elapsed_nanos()); - EXPECT_FALSE(data.bucket_info(1).atom(0).temperature().sensor_name().empty()); + EXPECT_TRUE(data.bucket_info(1).atom(0).temperature().sensor_name().empty()); EXPECT_GT(data.bucket_info(1).atom(0).temperature().temperature_dc(), 0); EXPECT_EQ(2, data.bucket_info(2).atom_size()); @@ -293,9 +294,9 @@ TEST(GaugeMetricE2eTest, TestAllConditionChangesSamplePulledEvents) { data.bucket_info(2).elapsed_timestamp_nanos(1)); EXPECT_EQ(baseTimeNs + 7 * bucketSizeNs, data.bucket_info(2).start_bucket_elapsed_nanos()); EXPECT_EQ(baseTimeNs + 8 * bucketSizeNs, data.bucket_info(2).end_bucket_elapsed_nanos()); - EXPECT_FALSE(data.bucket_info(2).atom(0).temperature().sensor_name().empty()); + EXPECT_TRUE(data.bucket_info(2).atom(0).temperature().sensor_name().empty()); EXPECT_GT(data.bucket_info(2).atom(0).temperature().temperature_dc(), 0); - EXPECT_FALSE(data.bucket_info(2).atom(1).temperature().sensor_name().empty()); + EXPECT_TRUE(data.bucket_info(2).atom(1).temperature().sensor_name().empty()); EXPECT_GT(data.bucket_info(2).atom(1).temperature().temperature_dc(), 0); } @@ -377,7 +378,7 @@ TEST(GaugeMetricE2eTest, TestRandomSamplePulledEvent_LateAlarm) { EXPECT_EQ(1, data.bucket_info(0).wall_clock_timestamp_nanos_size()); EXPECT_EQ(baseTimeNs + 2 * bucketSizeNs, data.bucket_info(0).start_bucket_elapsed_nanos()); EXPECT_EQ(baseTimeNs + 3 * bucketSizeNs, data.bucket_info(0).end_bucket_elapsed_nanos()); - EXPECT_FALSE(data.bucket_info(0).atom(0).temperature().sensor_name().empty()); + EXPECT_TRUE(data.bucket_info(0).atom(0).temperature().sensor_name().empty()); EXPECT_GT(data.bucket_info(0).atom(0).temperature().temperature_dc(), 0); EXPECT_EQ(1, data.bucket_info(1).atom_size()); @@ -386,7 +387,7 @@ TEST(GaugeMetricE2eTest, TestRandomSamplePulledEvent_LateAlarm) { EXPECT_EQ(configAddedTimeNs + 55, data.bucket_info(0).elapsed_timestamp_nanos(0)); EXPECT_EQ(baseTimeNs + 5 * bucketSizeNs, data.bucket_info(1).start_bucket_elapsed_nanos()); EXPECT_EQ(baseTimeNs + 6 * bucketSizeNs, data.bucket_info(1).end_bucket_elapsed_nanos()); - EXPECT_FALSE(data.bucket_info(1).atom(0).temperature().sensor_name().empty()); + EXPECT_TRUE(data.bucket_info(1).atom(0).temperature().sensor_name().empty()); EXPECT_GT(data.bucket_info(1).atom(0).temperature().temperature_dc(), 0); EXPECT_EQ(1, data.bucket_info(2).atom_size()); @@ -395,7 +396,7 @@ TEST(GaugeMetricE2eTest, TestRandomSamplePulledEvent_LateAlarm) { data.bucket_info(2).elapsed_timestamp_nanos(0)); EXPECT_EQ(baseTimeNs + 6 * bucketSizeNs, data.bucket_info(2).start_bucket_elapsed_nanos()); EXPECT_EQ(baseTimeNs + 7 * bucketSizeNs, data.bucket_info(2).end_bucket_elapsed_nanos()); - EXPECT_FALSE(data.bucket_info(2).atom(0).temperature().sensor_name().empty()); + EXPECT_TRUE(data.bucket_info(2).atom(0).temperature().sensor_name().empty()); EXPECT_GT(data.bucket_info(2).atom(0).temperature().temperature_dc(), 0); } diff --git a/cmds/statsd/tests/metrics/GaugeMetricProducer_test.cpp b/cmds/statsd/tests/metrics/GaugeMetricProducer_test.cpp index 9471faa89547d..bf58b9c7e4d46 100644 --- a/cmds/statsd/tests/metrics/GaugeMetricProducer_test.cpp +++ b/cmds/statsd/tests/metrics/GaugeMetricProducer_test.cpp @@ -95,6 +95,8 @@ TEST(GaugeMetricProducerTest, TestPulledEventsNoCondition) { data->clear(); shared_ptr event = make_shared(tagId, bucketStartTimeNs + 10); event->write(3); + event->write("some value"); + event->write(11); event->init(); data->push_back(event); return true; @@ -600,9 +602,109 @@ TEST(GaugeMetricProducerTest, TestPullOnTrigger) { EXPECT_EQ(10, it->mValue.int_value); EXPECT_EQ(1UL, gaugeProducer.mPastBuckets.size()); EXPECT_EQ(3UL, gaugeProducer.mPastBuckets.begin()->second.back().mGaugeAtoms.size()); - EXPECT_EQ(3, gaugeProducer.mPastBuckets.begin()->second.back().mGaugeAtoms[0].mFields->begin()->mValue.int_value); - EXPECT_EQ(4, gaugeProducer.mPastBuckets.begin()->second.back().mGaugeAtoms[1].mFields->begin()->mValue.int_value); - EXPECT_EQ(5, gaugeProducer.mPastBuckets.begin()->second.back().mGaugeAtoms[2].mFields->begin()->mValue.int_value); + EXPECT_EQ(3, gaugeProducer.mPastBuckets.begin() + ->second.back() + .mGaugeAtoms[0] + .mFields->begin() + ->mValue.int_value); + EXPECT_EQ(4, gaugeProducer.mPastBuckets.begin() + ->second.back() + .mGaugeAtoms[1] + .mFields->begin() + ->mValue.int_value); + EXPECT_EQ(5, gaugeProducer.mPastBuckets.begin() + ->second.back() + .mGaugeAtoms[2] + .mFields->begin() + ->mValue.int_value); +} + +TEST(GaugeMetricProducerTest, TestRemoveDimensionInOutput) { + GaugeMetric metric; + metric.set_id(metricId); + metric.set_bucket(ONE_MINUTE); + metric.set_sampling_type(GaugeMetric::ALL_CONDITION_CHANGES); + metric.mutable_gauge_fields_filter()->set_include_all(true); + auto dimensionMatcher = metric.mutable_dimensions_in_what(); + // use field 1 as dimension. + dimensionMatcher->set_field(tagId); + dimensionMatcher->add_child()->set_field(1); + + sp wizard = new NaggyMock(); + + sp pullerManager = new StrictMock(); + EXPECT_CALL(*pullerManager, Pull(tagId, _, _)) + .WillOnce(Invoke([](int tagId, int64_t timeNs, + vector>* data) { + data->clear(); + shared_ptr event = make_shared(tagId, bucketStartTimeNs + 3); + event->write(3); + event->write(4); + event->init(); + data->push_back(event); + return true; + })) + .WillOnce(Invoke([](int tagId, int64_t timeNs, + vector>* data) { + data->clear(); + shared_ptr event = make_shared(tagId, bucketStartTimeNs + 10); + event->write(4); + event->write(5); + event->init(); + data->push_back(event); + return true; + })) + .WillOnce(Invoke([](int tagId, int64_t timeNs, + vector>* data) { + data->clear(); + shared_ptr event = make_shared(tagId, bucketStartTimeNs + 20); + event->write(4); + event->write(6); + event->init(); + data->push_back(event); + return true; + })); + + int triggerId = 5; + GaugeMetricProducer gaugeProducer(kConfigKey, metric, -1 /*-1 meaning no condition*/, wizard, + tagId, triggerId, tagId, bucketStartTimeNs, bucketStartTimeNs, + pullerManager); + + vector> allData; + allData.clear(); + + EXPECT_EQ(1UL, gaugeProducer.mCurrentSlicedBucket->size()); + LogEvent trigger(triggerId, bucketStartTimeNs + 10); + trigger.init(); + gaugeProducer.onMatchedLogEvent(1 /*log matcher index*/, trigger); + EXPECT_EQ(2UL, gaugeProducer.mCurrentSlicedBucket->size()); + EXPECT_EQ(1UL, gaugeProducer.mCurrentSlicedBucket->begin()->second.size()); + trigger.setElapsedTimestampNs(bucketStartTimeNs + 20); + gaugeProducer.onMatchedLogEvent(1 /*log matcher index*/, trigger); + EXPECT_EQ(2UL, gaugeProducer.mCurrentSlicedBucket->begin()->second.size()); + + allData.clear(); + shared_ptr event = make_shared(tagId, bucket2StartTimeNs + 1); + event->write(4); + event->write(11); + event->init(); + allData.push_back(event); + + gaugeProducer.onDataPulled(allData); + EXPECT_EQ(1UL, gaugeProducer.mCurrentSlicedBucket->size()); + auto it = gaugeProducer.mCurrentSlicedBucket->begin()->second.front().mFields->begin(); + EXPECT_EQ(INT, it->mValue.getType()); + EXPECT_EQ(11, it->mValue.int_value); + EXPECT_EQ(2UL, gaugeProducer.mPastBuckets.size()); + auto bucketIt = gaugeProducer.mPastBuckets.begin(); + EXPECT_EQ(1UL, bucketIt->second.back().mGaugeAtoms.size()); + EXPECT_EQ(3, bucketIt->first.getDimensionKeyInWhat().getValues().begin()->mValue.int_value); + EXPECT_EQ(4, bucketIt->second.back().mGaugeAtoms[0].mFields->begin()->mValue.int_value); + bucketIt++; + EXPECT_EQ(2UL, bucketIt->second.back().mGaugeAtoms.size()); + EXPECT_EQ(4, bucketIt->first.getDimensionKeyInWhat().getValues().begin()->mValue.int_value); + EXPECT_EQ(5, bucketIt->second.back().mGaugeAtoms[0].mFields->begin()->mValue.int_value); + EXPECT_EQ(6, bucketIt->second.back().mGaugeAtoms[1].mFields->begin()->mValue.int_value); } } // namespace statsd