Fix statsd returning uidmap with empty reports.
We notice devices uploading a bunch of bytes for the uidmap even if
the device is running an empty config, so there are no actual metrics
to report. This hardcodes some logic to skip the inclusion of the
uidmap if there are exactly 0 metrics.
Bug: 79381210
Test: Tested unit-tests on marlin-eng
Change-Id: I96348235341a7faf15ff57d4d1eccac635a3a999
diff --git a/cmds/statsd/src/StatsLogProcessor.cpp b/cmds/statsd/src/StatsLogProcessor.cpp
index 4d27948..daafe9c 100644
--- a/cmds/statsd/src/StatsLogProcessor.cpp
+++ b/cmds/statsd/src/StatsLogProcessor.cpp
@@ -382,10 +382,13 @@
it->second->onDumpReport(dumpTimeStampNs, include_current_partial_bucket,
&str_set, proto);
- // Fill in UidMap.
- uint64_t uidMapToken = proto->start(FIELD_TYPE_MESSAGE | FIELD_ID_UID_MAP);
- mUidMap->appendUidMap(dumpTimeStampNs, key, &str_set, proto);
- proto->end(uidMapToken);
+ // Fill in UidMap if there is at least one metric to report.
+ // This skips the uid map if it's an empty config.
+ if (it->second->getNumMetrics() > 0) {
+ uint64_t uidMapToken = proto->start(FIELD_TYPE_MESSAGE | FIELD_ID_UID_MAP);
+ mUidMap->appendUidMap(dumpTimeStampNs, key, &str_set, proto);
+ proto->end(uidMapToken);
+ }
// Fill in the timestamps.
proto->write(FIELD_TYPE_INT64 | FIELD_ID_LAST_REPORT_ELAPSED_NANOS,
diff --git a/cmds/statsd/src/metrics/MetricsManager.h b/cmds/statsd/src/metrics/MetricsManager.h
index 4f39df9e..456da98 100644
--- a/cmds/statsd/src/metrics/MetricsManager.h
+++ b/cmds/statsd/src/metrics/MetricsManager.h
@@ -89,6 +89,10 @@
return mLastReportWallClockNs;
};
+ inline size_t getNumMetrics() const {
+ return mAllMetricProducers.size();
+ }
+
virtual void dropData(const int64_t dropTimeNs);
virtual void onDumpReport(const int64_t dumpTimeNs,
diff --git a/cmds/statsd/tests/StatsLogProcessor_test.cpp b/cmds/statsd/tests/StatsLogProcessor_test.cpp
index de6e6e5..3395aa6 100644
--- a/cmds/statsd/tests/StatsLogProcessor_test.cpp
+++ b/cmds/statsd/tests/StatsLogProcessor_test.cpp
@@ -24,6 +24,8 @@
#include <gmock/gmock.h>
#include <gtest/gtest.h>
+#include "tests/statsd_test_util.h"
+
#include <stdio.h>
using namespace android;
@@ -123,6 +125,21 @@
EXPECT_EQ(0, broadcastCount);
}
+StatsdConfig MakeConfig(bool includeMetric) {
+ StatsdConfig config;
+ config.add_allowed_log_source("AID_ROOT"); // LogEvent defaults to UID of root.
+
+ if (includeMetric) {
+ auto appCrashMatcher = CreateProcessCrashAtomMatcher();
+ *config.add_atom_matcher() = appCrashMatcher;
+ auto countMetric = config.add_count_metric();
+ countMetric->set_id(StringToId("AppCrashes"));
+ countMetric->set_what(appCrashMatcher.id());
+ countMetric->set_bucket(FIVE_MINUTES);
+ }
+ return config;
+}
+
TEST(StatsLogProcessorTest, TestUidMapHasSnapshot) {
// Setup simple config key corresponding to empty config.
sp<UidMap> m = new UidMap();
@@ -133,8 +150,7 @@
StatsLogProcessor p(m, anomalyAlarmMonitor, subscriberAlarmMonitor, 0,
[&broadcastCount](const ConfigKey& key) { broadcastCount++; return true;});
ConfigKey key(3, 4);
- StatsdConfig config;
- config.add_allowed_log_source("AID_ROOT");
+ StatsdConfig config = MakeConfig(true);
p.OnConfigUpdated(0, key, config);
// Expect to get no metrics, but snapshot specified above in uidmap.
@@ -149,6 +165,29 @@
EXPECT_EQ(2, uidmap.snapshots(0).package_info_size());
}
+TEST(StatsLogProcessorTest, TestEmptyConfigHasNoUidMap) {
+ // Setup simple config key corresponding to empty config.
+ sp<UidMap> m = new UidMap();
+ m->updateMap(1, {1, 2}, {1, 2}, {String16("p1"), String16("p2")});
+ sp<AlarmMonitor> anomalyAlarmMonitor;
+ sp<AlarmMonitor> subscriberAlarmMonitor;
+ int broadcastCount = 0;
+ StatsLogProcessor p(m, anomalyAlarmMonitor, subscriberAlarmMonitor, 0,
+ [&broadcastCount](const ConfigKey& key) { broadcastCount++; return true;});
+ ConfigKey key(3, 4);
+ StatsdConfig config = MakeConfig(false);
+ p.OnConfigUpdated(0, key, config);
+
+ // Expect to get no metrics, but snapshot specified above in uidmap.
+ vector<uint8_t> bytes;
+ p.onDumpReport(key, 1, false, true, ADB_DUMP, &bytes);
+
+ ConfigMetricsReportList output;
+ output.ParseFromArray(bytes.data(), bytes.size());
+ EXPECT_TRUE(output.reports_size() > 0);
+ EXPECT_FALSE(output.reports(0).has_uid_map());
+}
+
TEST(StatsLogProcessorTest, TestReportIncludesSubConfig) {
// Setup simple config key corresponding to empty config.
sp<UidMap> m = new UidMap();