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

Commit 5a4727d7 authored by Tej Singh's avatar Tej Singh
Browse files

Remove invalid configs from memory 

Right now if we receive an invalid config update, we keep the existing
config in memory. This makes us remove the existing config so that we
don't have any config for the invalid config key.

Test: atest statsd_test
Bug: 158617758
Change-Id: I9daecb1c96e3a63fea3a45b07d1295f3b4ba452a
parent 4c65d2cb

cmds/statsd/src/StatsLogProcessor.cpp

+2 −0
Original line number Diff line number Diff line
@@ -529,7 +529,9 @@ void StatsLogProcessor::OnConfigUpdatedLocked( 
        VLOG("StatsdConfig valid");

    } else {

        // If there is any error in the config, don't use it.

        // Remove any existing config with the same key.

        ALOGE("StatsdConfig NOT valid");

        mMetricsManagers.erase(key);

    }

}

cmds/statsd/src/StatsLogProcessor.h

+1 −0
Original line number Diff line number Diff line
@@ -284,6 +284,7 @@ private: 
    FRIEND_TEST(StatsLogProcessorTest, TestRateLimitByteSize);

    FRIEND_TEST(StatsLogProcessorTest, TestRateLimitBroadcast);

    FRIEND_TEST(StatsLogProcessorTest, TestDropWhenByteSizeTooLarge);

    FRIEND_TEST(StatsLogProcessorTest, InvalidConfigRemoved);

    FRIEND_TEST(StatsLogProcessorTest, TestActiveConfigMetricDiskWriteRead);

    FRIEND_TEST(StatsLogProcessorTest, TestActivationOnBoot);

    FRIEND_TEST(StatsLogProcessorTest, TestActivationOnBootMultipleActivations);

cmds/statsd/src/guardrail/StatsdStats.h

+2 −0
Original line number Diff line number Diff line
@@ -660,6 +660,8 @@ private: 
    FRIEND_TEST(StatsdStatsTest, TestAtomMetricsStats);

    FRIEND_TEST(StatsdStatsTest, TestActivationBroadcastGuardrailHit);

    FRIEND_TEST(StatsdStatsTest, TestAtomErrorStats);



    FRIEND_TEST(StatsLogProcessorTest, InvalidConfigRemoved);

};



}  // namespace statsd

cmds/statsd/tests/StatsLogProcessor_test.cpp

+35 −0
Original line number Diff line number Diff line
@@ -324,6 +324,41 @@ TEST(StatsLogProcessorTest, TestPullUidProviderSetOnConfigUpdate) { 
    EXPECT_EQ(pullerManager->mPullUidProviders.find(key), pullerManager->mPullUidProviders.end());

}



TEST(StatsLogProcessorTest, InvalidConfigRemoved) {

    // Setup simple config key corresponding to empty config.

    StatsdStats::getInstance().reset();

    sp<UidMap> m = new UidMap();

    sp<StatsPullerManager> pullerManager = new StatsPullerManager();

    m->updateMap(1, {1, 2}, {1, 2}, {String16("v1"), String16("v2")},

                 {String16("p1"), String16("p2")}, {String16(""), String16("")});

    sp<AlarmMonitor> anomalyAlarmMonitor;

    sp<AlarmMonitor> subscriberAlarmMonitor;

    StatsLogProcessor p(m, pullerManager, anomalyAlarmMonitor, subscriberAlarmMonitor, 0,

                        [](const ConfigKey& key) { return true; },

                        [](const int&, const vector<int64_t>&) {return true;});

    ConfigKey key(3, 4);

    StatsdConfig config = MakeConfig(true);

    p.OnConfigUpdated(0, key, config);

    EXPECT_EQ(1, p.mMetricsManagers.size());

    EXPECT_NE(p.mMetricsManagers.find(key), p.mMetricsManagers.end());

    // Cannot assert the size of mConfigStats since it is static and does not get cleared on reset.

    EXPECT_NE(StatsdStats::getInstance().mConfigStats.end(),

              StatsdStats::getInstance().mConfigStats.find(key));

    EXPECT_EQ(0, StatsdStats::getInstance().mIceBox.size());



    StatsdConfig invalidConfig = MakeConfig(true);

    invalidConfig.clear_allowed_log_source();

    p.OnConfigUpdated(0, key, invalidConfig);

    EXPECT_EQ(0, p.mMetricsManagers.size());

    // The current configs should not contain the invalid config.

    EXPECT_EQ(StatsdStats::getInstance().mConfigStats.end(),

              StatsdStats::getInstance().mConfigStats.find(key));

    // Both "config" and "invalidConfig" should be in the icebox.

    EXPECT_EQ(2, StatsdStats::getInstance().mIceBox.size());



}





TEST(StatsLogProcessorTest, TestActiveConfigMetricDiskWriteRead) {

    int uid = 1111;