From faa1af535bf6ede5fcab2e0bad5f6c16908916a9 Mon Sep 17 00:00:00 2001 From: David Chen Date: Fri, 30 Mar 2018 15:14:04 -0700 Subject: [PATCH] Includes annotations with statsd reports. It's tricky to determine the source of the metrics on a device currently since we can take the union of multiple configs and send only one giant statsd_config into statsd. We will use the int64 field to track the sub config id's and the int32 field to track the version for each sub config, but the fields are named more generically as annotations. The annotations are available in both the reports and metadata. Test: Check that all unit-tests pass on marlin-eng Bug: 77327261 Change-Id: Ic37c549c8b2991676f69948c515156765c9f5108 --- cmds/statsd/src/guardrail/StatsdStats.cpp | 27 ++++++++++++++++++++-- cmds/statsd/src/guardrail/StatsdStats.h | 7 +++++- cmds/statsd/src/metrics/MetricsManager.cpp | 26 +++++++++++++++++---- cmds/statsd/src/metrics/MetricsManager.h | 3 +++ cmds/statsd/src/stats_log.proto | 11 +++++++++ cmds/statsd/src/statsd_config.proto | 9 ++++++++ cmds/statsd/tests/StatsLogProcessor_test.cpp | 29 ++++++++++++++++++++++++ cmds/statsd/tests/guardrail/StatsdStats_test.cpp | 14 ++++++++---- 8 files changed, 113 insertions(+), 13 deletions(-) diff --git a/cmds/statsd/src/guardrail/StatsdStats.cpp b/cmds/statsd/src/guardrail/StatsdStats.cpp index 22ff9428c83a..77773fe4e493 100644 --- a/cmds/statsd/src/guardrail/StatsdStats.cpp +++ b/cmds/statsd/src/guardrail/StatsdStats.cpp @@ -77,6 +77,9 @@ const int FIELD_ID_CONFIG_STATS_CONDITION_STATS = 14; const int FIELD_ID_CONFIG_STATS_METRIC_STATS = 15; const int FIELD_ID_CONFIG_STATS_ALERT_STATS = 16; const int FIELD_ID_CONFIG_STATS_METRIC_DIMENSION_IN_CONDITION_STATS = 17; +const int FIELD_ID_CONFIG_STATS_ANNOTATION = 18; +const int FIELD_ID_CONFIG_STATS_ANNOTATION_INT64 = 1; +const int FIELD_ID_CONFIG_STATS_ANNOTATION_INT32 = 2; const int FIELD_ID_MATCHER_STATS_ID = 1; const int FIELD_ID_MATCHER_STATS_COUNT = 2; @@ -116,8 +119,10 @@ void StatsdStats::addToIceBoxLocked(shared_ptr& stats) { mIceBox.push_back(stats); } -void StatsdStats::noteConfigReceived(const ConfigKey& key, int metricsCount, int conditionsCount, - int matchersCount, int alertsCount, bool isValid) { +void StatsdStats::noteConfigReceived( + const ConfigKey& key, int metricsCount, int conditionsCount, int matchersCount, + int alertsCount, const std::list>& annotations, + bool isValid) { lock_guard lock(mLock); int32_t nowTimeSec = getWallClockSec(); @@ -133,6 +138,9 @@ void StatsdStats::noteConfigReceived(const ConfigKey& key, int metricsCount, int configStats->matcher_count = matchersCount; configStats->alert_count = alertsCount; configStats->is_valid = isValid; + for (auto& v : annotations) { + configStats->annotations.emplace_back(v); + } if (isValid) { mConfigStats[key] = configStats; @@ -351,6 +359,7 @@ void StatsdStats::resetInternalLocked() { config.second->broadcast_sent_time_sec.clear(); config.second->data_drop_time_sec.clear(); config.second->dump_report_time_sec.clear(); + config.second->annotations.clear(); config.second->matcher_stats.clear(); config.second->condition_stats.clear(); config.second->metric_stats.clear(); @@ -394,6 +403,11 @@ void StatsdStats::dumpStats(FILE* out) const { configStats->deletion_time_sec, configStats->metric_count, configStats->condition_count, configStats->matcher_count, configStats->alert_count, configStats->is_valid); + for (const auto& annotation : configStats->annotations) { + fprintf(out, "\tannotation: %lld, %d\n", (long long)annotation.first, + annotation.second); + } + for (const auto& broadcastTime : configStats->broadcast_sent_time_sec) { fprintf(out, "\tbroadcast time: %d\n", broadcastTime); } @@ -497,6 +511,15 @@ void addConfigStatsToProto(const ConfigStats& configStats, ProtoOutputStream* pr dump); } + for (const auto& annotation : configStats.annotations) { + uint64_t token = proto->start(FIELD_TYPE_MESSAGE | FIELD_COUNT_REPEATED | + FIELD_ID_CONFIG_STATS_ANNOTATION); + proto->write(FIELD_TYPE_INT64 | FIELD_ID_CONFIG_STATS_ANNOTATION_INT64, + (long long)annotation.first); + proto->write(FIELD_TYPE_INT32 | FIELD_ID_CONFIG_STATS_ANNOTATION_INT32, annotation.second); + proto->end(token); + } + for (const auto& pair : configStats.matcher_stats) { uint64_t tmpToken = proto->start(FIELD_TYPE_MESSAGE | FIELD_COUNT_REPEATED | FIELD_ID_CONFIG_STATS_MATCHER_STATS); diff --git a/cmds/statsd/src/guardrail/StatsdStats.h b/cmds/statsd/src/guardrail/StatsdStats.h index bd395c4c232b..a1b0ee2309f2 100644 --- a/cmds/statsd/src/guardrail/StatsdStats.h +++ b/cmds/statsd/src/guardrail/StatsdStats.h @@ -66,6 +66,9 @@ struct ConfigStats { // Stores the number of times an anomaly detection alert has been declared. // The map size is capped by kMaxConfigCount. std::map alert_stats; + + // Stores the config ID for each sub-config used. + std::list> annotations; }; struct UidMapStats { @@ -142,7 +145,9 @@ public: * If the config is not valid, this config stats will be put into icebox immediately. */ void noteConfigReceived(const ConfigKey& key, int metricsCount, int conditionsCount, - int matchersCount, int alertCount, bool isValid); + int matchersCount, int alertCount, + const std::list>& annotations, + bool isValid); /** * Report a config has been removed. */ diff --git a/cmds/statsd/src/metrics/MetricsManager.cpp b/cmds/statsd/src/metrics/MetricsManager.cpp index c61fa7663d45..772f017b695a 100644 --- a/cmds/statsd/src/metrics/MetricsManager.cpp +++ b/cmds/statsd/src/metrics/MetricsManager.cpp @@ -33,6 +33,8 @@ #include using android::util::FIELD_COUNT_REPEATED; +using android::util::FIELD_TYPE_INT32; +using android::util::FIELD_TYPE_INT64; using android::util::FIELD_TYPE_MESSAGE; using android::util::ProtoOutputStream; @@ -47,6 +49,9 @@ namespace os { namespace statsd { const int FIELD_ID_METRICS = 1; +const int FIELD_ID_ANNOTATIONS = 7; +const int FIELD_ID_ANNOTATIONS_INT64 = 1; +const int FIELD_ID_ANNOTATIONS_INT32 = 2; MetricsManager::MetricsManager(const ConfigKey& key, const StatsdConfig& config, const long timeBaseSec, const long currentTimeSec, @@ -85,6 +90,11 @@ MetricsManager::MetricsManager(const ConfigKey& key, const StatsdConfig& config, } } + // Store the sub-configs used. + for (const auto& annotation : config.annotation()) { + mAnnotations.emplace_back(annotation.field_int64(), annotation.field_int32()); + } + // Guardrail. Reject the config if it's too big. if (mAllMetricProducers.size() > StatsdStats::kMaxMetricCountPerConfig || mAllConditionTrackers.size() > StatsdStats::kMaxConditionCountPerConfig || @@ -97,11 +107,9 @@ MetricsManager::MetricsManager(const ConfigKey& key, const StatsdConfig& config, mConfigValid = false; } // no matter whether this config is valid, log it in the stats. - StatsdStats::getInstance().noteConfigReceived(key, mAllMetricProducers.size(), - mAllConditionTrackers.size(), - mAllAtomMatchers.size(), - mAllAnomalyTrackers.size(), - mConfigValid); + StatsdStats::getInstance().noteConfigReceived( + key, mAllMetricProducers.size(), mAllConditionTrackers.size(), mAllAtomMatchers.size(), + mAllAnomalyTrackers.size(), mAnnotations, mConfigValid); } MetricsManager::~MetricsManager() { @@ -188,6 +196,14 @@ void MetricsManager::onDumpReport(const uint64_t dumpTimeStampNs, ProtoOutputStr protoOutput->end(token); } } + for (const auto& annotation : mAnnotations) { + uint64_t token = protoOutput->start(FIELD_TYPE_MESSAGE | FIELD_COUNT_REPEATED | + FIELD_ID_ANNOTATIONS); + protoOutput->write(FIELD_TYPE_INT64 | FIELD_ID_ANNOTATIONS_INT64, + (long long)annotation.first); + protoOutput->write(FIELD_TYPE_INT32 | FIELD_ID_ANNOTATIONS_INT32, annotation.second); + protoOutput->end(token); + } mLastReportTimeNs = dumpTimeStampNs; mLastReportWallClockNs = getWallClockNs(); VLOG("=========================Metric Reports End=========================="); diff --git a/cmds/statsd/src/metrics/MetricsManager.h b/cmds/statsd/src/metrics/MetricsManager.h index 4fd64b7baf8f..be4644c277e7 100644 --- a/cmds/statsd/src/metrics/MetricsManager.h +++ b/cmds/statsd/src/metrics/MetricsManager.h @@ -107,6 +107,9 @@ private: // Logs from uids that are not in the list will be ignored to avoid spamming. std::set mAllowedLogSources; + // Contains the annotations passed in with StatsdConfig. + std::list> mAnnotations; + // To guard access to mAllowedLogSources mutable std::mutex mAllowedLogSourcesMutex; diff --git a/cmds/statsd/src/stats_log.proto b/cmds/statsd/src/stats_log.proto index a25df3fc4c09..9fd17b614316 100644 --- a/cmds/statsd/src/stats_log.proto +++ b/cmds/statsd/src/stats_log.proto @@ -186,6 +186,12 @@ message ConfigMetricsReport { optional int64 last_report_wall_clock_nanos = 5; optional int64 current_report_wall_clock_nanos = 6; + + message Annotation { + optional int64 field_int64 = 1; + optional int32 field_int32 = 2; + } + repeated Annotation annotation = 7; } message ConfigMetricsReportList { @@ -242,6 +248,11 @@ message StatsdStatsReport { repeated MetricStats metric_stats = 15; repeated AlertStats alert_stats = 16; repeated MetricStats metric_dimension_in_condition_stats = 17; + message Annotation { + optional int64 field_int64 = 1; + optional int32 field_int32 = 2; + } + repeated Annotation annotation = 18; } repeated ConfigStats config_stats = 3; diff --git a/cmds/statsd/src/statsd_config.proto b/cmds/statsd/src/statsd_config.proto index 93df9b824ac3..687f5ea39136 100644 --- a/cmds/statsd/src/statsd_config.proto +++ b/cmds/statsd/src/statsd_config.proto @@ -347,4 +347,13 @@ message StatsdConfig { repeated string allowed_log_source = 12; repeated int64 no_report_metric = 13; + + message Annotation { + optional int64 field_int64 = 1; + optional int32 field_int32 = 2; + } + repeated Annotation annotation = 14; + + // Field number 1000 is reserved for later use. + reserved 1000; } diff --git a/cmds/statsd/tests/StatsLogProcessor_test.cpp b/cmds/statsd/tests/StatsLogProcessor_test.cpp index 5a8ba7915a00..98f1a5931903 100644 --- a/cmds/statsd/tests/StatsLogProcessor_test.cpp +++ b/cmds/statsd/tests/StatsLogProcessor_test.cpp @@ -149,6 +149,35 @@ TEST(StatsLogProcessorTest, TestUidMapHasSnapshot) { EXPECT_EQ(2, uidmap.snapshots(0).package_info_size()); } +TEST(StatsLogProcessorTest, TestReportIncludesSubConfig) { + // Setup simple config key corresponding to empty config. + sp m = new UidMap(); + sp anomalyAlarmMonitor; + sp subscriberAlarmMonitor; + int broadcastCount = 0; + StatsLogProcessor p(m, anomalyAlarmMonitor, subscriberAlarmMonitor, 0, + [&broadcastCount](const ConfigKey& key) { broadcastCount++; }); + ConfigKey key(3, 4); + StatsdConfig config; + auto annotation = config.add_annotation(); + annotation->set_field_int64(1); + annotation->set_field_int32(2); + config.add_allowed_log_source("AID_ROOT"); + p.OnConfigUpdated(1, key, config); + + // Expect to get no metrics, but snapshot specified above in uidmap. + vector bytes; + p.onDumpReport(key, 1, &bytes); + + ConfigMetricsReportList output; + output.ParseFromArray(bytes.data(), bytes.size()); + EXPECT_TRUE(output.reports_size() > 0); + auto report = output.reports(0); + EXPECT_EQ(1, report.annotation_size()); + EXPECT_EQ(1, report.annotation(0).field_int64()); + EXPECT_EQ(2, report.annotation(0).field_int32()); +} + #else GTEST_LOG_(INFO) << "This test does nothing.\n"; #endif diff --git a/cmds/statsd/tests/guardrail/StatsdStats_test.cpp b/cmds/statsd/tests/guardrail/StatsdStats_test.cpp index 04ce73a76d3b..5c35d96a5ce8 100644 --- a/cmds/statsd/tests/guardrail/StatsdStats_test.cpp +++ b/cmds/statsd/tests/guardrail/StatsdStats_test.cpp @@ -34,7 +34,7 @@ TEST(StatsdStatsTest, TestValidConfigAdd) { const int conditionsCount = 20; const int matchersCount = 30; const int alertsCount = 10; - stats.noteConfigReceived(key, metricsCount, conditionsCount, matchersCount, alertsCount, + stats.noteConfigReceived(key, metricsCount, conditionsCount, matchersCount, alertsCount, {}, true /*valid config*/); vector output; stats.dumpStats(&output, false /*reset stats*/); @@ -61,7 +61,7 @@ TEST(StatsdStatsTest, TestInvalidConfigAdd) { const int conditionsCount = 20; const int matchersCount = 30; const int alertsCount = 10; - stats.noteConfigReceived(key, metricsCount, conditionsCount, matchersCount, alertsCount, + stats.noteConfigReceived(key, metricsCount, conditionsCount, matchersCount, alertsCount, {}, false /*bad config*/); vector output; stats.dumpStats(&output, false); @@ -82,7 +82,8 @@ TEST(StatsdStatsTest, TestConfigRemove) { const int conditionsCount = 20; const int matchersCount = 30; const int alertsCount = 10; - stats.noteConfigReceived(key, metricsCount, conditionsCount, matchersCount, alertsCount, true); + stats.noteConfigReceived(key, metricsCount, conditionsCount, matchersCount, alertsCount, {}, + true); vector output; stats.dumpStats(&output, false); StatsdStatsReport report; @@ -104,7 +105,7 @@ TEST(StatsdStatsTest, TestConfigRemove) { TEST(StatsdStatsTest, TestSubStats) { StatsdStats stats; ConfigKey key(0, 12345); - stats.noteConfigReceived(key, 2, 3, 4, 5, true); + stats.noteConfigReceived(key, 2, 3, 4, 5, {std::make_pair(123, 456)}, true); stats.noteMatcherMatched(key, StringToId("matcher1")); stats.noteMatcherMatched(key, StringToId("matcher1")); @@ -142,6 +143,9 @@ TEST(StatsdStatsTest, TestSubStats) { EXPECT_EQ(2, configReport.broadcast_sent_time_sec_size()); EXPECT_EQ(1, configReport.data_drop_time_sec_size()); EXPECT_EQ(3, configReport.dump_report_time_sec_size()); + EXPECT_EQ(1, configReport.annotation_size()); + EXPECT_EQ(123, configReport.annotation(0).field_int64()); + EXPECT_EQ(456, configReport.annotation(0).field_int32()); EXPECT_EQ(2, configReport.matcher_stats_size()); // matcher1 is the first in the list @@ -259,7 +263,7 @@ TEST(StatsdStatsTest, TestTimestampThreshold) { timestamps.push_back(i); } ConfigKey key(0, 12345); - stats.noteConfigReceived(key, 2, 3, 4, 5, true); + stats.noteConfigReceived(key, 2, 3, 4, 5, {}, true); for (int i = 0; i < StatsdStats::kMaxTimestampCount; i++) { stats.noteDataDropped(key, timestamps[i]); -- 2.11.0