Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 9e6dbbda authored by David Chen's avatar David Chen
Browse files

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
parent efaec53a

cmds/statsd/src/StatsLogProcessor.cpp

+7 −4
Original line number Diff line number Diff line
@@ -382,10 +382,13 @@ void StatsLogProcessor::onConfigMetricsReportLocked(const ConfigKey& key, 
    it->second->onDumpReport(dumpTimeStampNs, include_current_partial_bucket,

                             &str_set, proto);



    // Fill in UidMap.

    // 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,

cmds/statsd/src/metrics/MetricsManager.h

+4 −0
Original line number Diff line number Diff line
@@ -89,6 +89,10 @@ public: 
        return mLastReportWallClockNs;

    };



    inline size_t getNumMetrics() const {

        return mAllMetricProducers.size();

    }



    virtual void dropData(const int64_t dropTimeNs);



    virtual void onDumpReport(const int64_t dumpTimeNs,

cmds/statsd/tests/StatsLogProcessor_test.cpp

+41 −2
Original line number Diff line number Diff line
@@ -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 @@ TEST(StatsLogProcessorTest, TestDropWhenByteSizeTooLarge) { 
    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 @@ TEST(StatsLogProcessorTest, TestUidMapHasSnapshot) { 
    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 @@ TEST(StatsLogProcessorTest, TestUidMapHasSnapshot) { 
    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();