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

Commit 56ae0d9a authored by David Chen's avatar David Chen
Browse files

Fixes statsd reports missing strings and SCS. 

Reports written to disk don't contain the strings used, which will
make this report unusable if there are strings that don't show up
again. We should always include the strings, so this option is
removed entirely.

Also, we hard-coded the wrong number of fields when pulling
ModemActivityInfo. There are actually 10 fields, not 6.

Bug: 79601503
Test: Tested unit-tests pass on marlin-eng.
Change-Id: I6834b096ced77418a9cc2ddd79b08d1c9c447fae
parent 26111fc8

cmds/statsd/src/StatsLogProcessor.cpp

+5 −10
Original line number Diff line number Diff line
@@ -306,7 +306,6 @@ void StatsLogProcessor::dumpStates(FILE* out, bool verbose) { 
 */

void StatsLogProcessor::onDumpReport(const ConfigKey& key, const int64_t dumpTimeStampNs,

                                     const bool include_current_partial_bucket,

                                     const bool include_string,

                                     const DumpReportReason dumpReportReason,

                                     vector<uint8_t>* outData) {

    std::lock_guard<std::mutex> lock(mMetricsMutex);
@@ -334,7 +333,7 @@ void StatsLogProcessor::onDumpReport(const ConfigKey& key, const int64_t dumpTim 
        uint64_t reportsToken =

                proto.start(FIELD_TYPE_MESSAGE | FIELD_COUNT_REPEATED | FIELD_ID_REPORTS);

        onConfigMetricsReportLocked(key, dumpTimeStampNs, include_current_partial_bucket,

                                    include_string, dumpReportReason, &proto);

                                    dumpReportReason, &proto);

        proto.end(reportsToken);

        // End of ConfigMetricsReport (reports).

    } else {
@@ -363,7 +362,6 @@ void StatsLogProcessor::onDumpReport(const ConfigKey& key, const int64_t dumpTim 
void StatsLogProcessor::onConfigMetricsReportLocked(const ConfigKey& key,

                                                    const int64_t dumpTimeStampNs,

                                                    const bool include_current_partial_bucket,

                                                    const bool include_string,

                                                    const DumpReportReason dumpReportReason,

                                                    ProtoOutputStream* proto) {

    // We already checked whether key exists in mMetricsManagers in
@@ -402,12 +400,10 @@ void StatsLogProcessor::onConfigMetricsReportLocked(const ConfigKey& key, 
    // Dump report reason

    proto->write(FIELD_TYPE_INT32 | FIELD_ID_DUMP_REPORT_REASON, dumpReportReason);



    if (include_string) {

    for (const auto& str : str_set) {

        proto->write(FIELD_TYPE_STRING | FIELD_COUNT_REPEATED | FIELD_ID_STRINGS, str);

    }

}

}



void StatsLogProcessor::resetConfigsLocked(const int64_t timestampNs,

                                           const std::vector<ConfigKey>& configs) {
@@ -508,9 +504,8 @@ void StatsLogProcessor::WriteDataToDiskLocked(const ConfigKey& key, 
        return;

    }

    ProtoOutputStream proto;

    onConfigMetricsReportLocked(key, timestampNs,

                                true /* include_current_partial_bucket*/,

                                false /* include strings */, dumpReportReason, &proto);

    onConfigMetricsReportLocked(key, timestampNs, true /* include_current_partial_bucket*/,

                                dumpReportReason, &proto);

    string file_name = StringPrintf("%s/%ld_%d_%lld", STATS_DATA_DIR,

         (long)getWallClockSec(), key.GetUid(), (long long)key.GetId());

    android::base::unique_fd fd(open(file_name.c_str(),

cmds/statsd/src/StatsLogProcessor.h

+1 −2
Original line number Diff line number Diff line
@@ -63,7 +63,7 @@ public: 
    size_t GetMetricsSize(const ConfigKey& key) const;



    void onDumpReport(const ConfigKey& key, const int64_t dumpTimeNs,

                      const bool include_current_partial_bucket, const bool include_string,

                      const bool include_current_partial_bucket,

                      const DumpReportReason dumpReportReason, vector<uint8_t>* outData);



    /* Tells MetricsManager that the alarms in alarmSet have fired. Modifies anomaly alarmSet. */
@@ -143,7 +143,6 @@ private: 


    void onConfigMetricsReportLocked(const ConfigKey& key, const int64_t dumpTimeStampNs,

                                     const bool include_current_partial_bucket,

                                     const bool include_string,

                                     const DumpReportReason dumpReportReason,

                                     util::ProtoOutputStream* proto);

cmds/statsd/src/StatsService.cpp

+3 −5
Original line number Diff line number Diff line
@@ -602,8 +602,7 @@ status_t StatsService::cmd_dump_report(FILE* out, FILE* err, const Vector<String 
        if (good) {

            vector<uint8_t> data;

            mProcessor->onDumpReport(ConfigKey(uid, StrToInt64(name)), getElapsedRealtimeNs(),

                                     false /* include_current_bucket*/,

                                     true /* include strings */, ADB_DUMP, &data);

                                     false /* include_current_bucket*/, ADB_DUMP, &data);

            // TODO: print the returned StatsLogReport to file instead of printing to logcat.

            if (proto) {

                for (size_t i = 0; i < data.size(); i ++) {
@@ -892,8 +891,7 @@ Status StatsService::getData(int64_t key, const String16& packageName, vector<ui 
    IPCThreadState* ipc = IPCThreadState::self();

    VLOG("StatsService::getData with Pid %i, Uid %i", ipc->getCallingPid(), ipc->getCallingUid());

    ConfigKey configKey(ipc->getCallingUid(), key);

    mProcessor->onDumpReport(configKey, getElapsedRealtimeNs(),

                             false /* include_current_bucket*/, true /* include strings */,

    mProcessor->onDumpReport(configKey, getElapsedRealtimeNs(), false /* include_current_bucket*/,

                             GET_DATA_CALLED, output);

    return Status::ok();

}

cmds/statsd/tests/StatsLogProcessor_test.cpp

+3 −3
Original line number Diff line number Diff line
@@ -155,7 +155,7 @@ TEST(StatsLogProcessorTest, TestUidMapHasSnapshot) { 


    // Expect to get no metrics, but snapshot specified above in uidmap.

    vector<uint8_t> bytes;

    p.onDumpReport(key, 1, false, true, ADB_DUMP, &bytes);

    p.onDumpReport(key, 1, false, ADB_DUMP, &bytes);



    ConfigMetricsReportList output;

    output.ParseFromArray(bytes.data(), bytes.size());
@@ -180,7 +180,7 @@ TEST(StatsLogProcessorTest, TestEmptyConfigHasNoUidMap) { 


    // Expect to get no metrics, but snapshot specified above in uidmap.

    vector<uint8_t> bytes;

    p.onDumpReport(key, 1, false, true, ADB_DUMP, &bytes);

    p.onDumpReport(key, 1, false, ADB_DUMP, &bytes);



    ConfigMetricsReportList output;

    output.ParseFromArray(bytes.data(), bytes.size());
@@ -206,7 +206,7 @@ TEST(StatsLogProcessorTest, TestReportIncludesSubConfig) { 


    // Expect to get no metrics, but snapshot specified above in uidmap.

    vector<uint8_t> bytes;

    p.onDumpReport(key, 1, false, true, ADB_DUMP, &bytes);

    p.onDumpReport(key, 1, false, ADB_DUMP, &bytes);



    ConfigMetricsReportList output;

    output.ParseFromArray(bytes.data(), bytes.size());

cmds/statsd/tests/e2e/Attribution_e2e_test.cpp

+4 −4
Original line number Diff line number Diff line
@@ -144,8 +144,8 @@ TEST(AttributionE2eTest, TestAttributionMatchAndSliceByFirstUid) { 
    }

    ConfigMetricsReportList reports;

    vector<uint8_t> buffer;

    processor->onDumpReport(cfgKey, bucketStartTimeNs + 4 * bucketSizeNs + 1, false, true,

                            ADB_DUMP, &buffer);

    processor->onDumpReport(cfgKey, bucketStartTimeNs + 4 * bucketSizeNs + 1, false, ADB_DUMP,

                            &buffer);

    EXPECT_TRUE(buffer.size() > 0);

    EXPECT_TRUE(reports.ParseFromArray(&buffer[0], buffer.size()));

    backfillDimensionPath(&reports);
@@ -290,8 +290,8 @@ TEST(AttributionE2eTest, TestAttributionMatchAndSliceByChain) { 
    }

    ConfigMetricsReportList reports;

    vector<uint8_t> buffer;

    processor->onDumpReport(cfgKey, bucketStartTimeNs + 4 * bucketSizeNs + 1, false, true,

                            ADB_DUMP, &buffer);

    processor->onDumpReport(cfgKey, bucketStartTimeNs + 4 * bucketSizeNs + 1, false, ADB_DUMP,

                            &buffer);

    EXPECT_TRUE(buffer.size() > 0);

    EXPECT_TRUE(reports.ParseFromArray(&buffer[0], buffer.size()));

    backfillDimensionPath(&reports);