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

Commit 0e6be91f authored by Christine Tsai's avatar Christine Tsai Committed by Android (Google) Code Review
Browse files

Merge "Set current state key earlier and do not reset hasCurrentState" into rvc-dev

parents 96a8a298 0893db53

cmds/statsd/src/metrics/MetricProducer.cpp

+1 −1
Original line number Diff line number Diff line
@@ -98,7 +98,7 @@ void MetricProducer::onMatchedLogEventLocked(const size_t matcherIndex, const Lo 


    // Stores atom id to primary key pairs for each state atom that the metric is

    // sliced by.

    std::map<int, HashableDimensionKey> statePrimaryKeys;

    std::map<int32_t, HashableDimensionKey> statePrimaryKeys;



    // For states with primary fields, use MetricStateLinks to get the primary

    // field values from the log event. These values will form a primary key

cmds/statsd/src/metrics/ValueMetricProducer.cpp

+8 −8
Original line number Diff line number Diff line
@@ -189,11 +189,6 @@ void ValueMetricProducer::onStateChanged(int64_t eventTimeNs, int32_t atomId, 
    VLOG("ValueMetric %lld onStateChanged time %lld, State %d, key %s, %d -> %d",

         (long long)mMetricId, (long long)eventTimeNs, atomId, primaryKey.toString().c_str(),

         oldState.mValue.int_value, newState.mValue.int_value);

    // If condition is not true or metric is not active, we do not need to pull

    // for this state change.

    if (mCondition != ConditionState::kTrue || !mIsActive) {

        return;

    }



    // If old and new states are in the same StateGroup, then we do not need to

    // pull for this state change.
@@ -205,6 +200,12 @@ void ValueMetricProducer::onStateChanged(int64_t eventTimeNs, int32_t atomId, 
        return;

    }



    // If condition is not true or metric is not active, we do not need to pull

    // for this state change.

    if (mCondition != ConditionState::kTrue || !mIsActive) {

        return;

    }



    bool isEventLate = eventTimeNs < mCurrentBucketStartTimeNs;

    if (isEventLate) {

        VLOG("Skip event due to late arrival: %lld vs %lld", (long long)eventTimeNs,
@@ -412,7 +413,6 @@ void ValueMetricProducer::resetBase() { 
    for (auto& slice : mCurrentBaseInfo) {

        for (auto& baseInfo : slice.second) {

            baseInfo.hasBase = false;

            baseInfo.hasCurrentState = false;

        }

    }

    mHasGlobalBase = false;
@@ -625,7 +625,6 @@ void ValueMetricProducer::accumulateEvents(const std::vector<std::shared_ptr<Log 
            auto it = mCurrentBaseInfo.find(whatKey);

            for (auto& baseInfo : it->second) {

                baseInfo.hasBase = false;

                baseInfo.hasCurrentState = false;

            }

        }

    }
@@ -820,6 +819,8 @@ void ValueMetricProducer::onMatchedLogEventInternalLocked( 
        Interval& interval = intervals[i];

        interval.valueIndex = i;

        Value value;

        baseInfo.hasCurrentState = true;

        baseInfo.currentState = stateKey;

        if (!getDoubleOrLong(event, matcher, value)) {

            VLOG("Failed to get value %d from event %s", i, event.ToString().c_str());

            StatsdStats::getInstance().noteBadValueType(mMetricId);
@@ -907,7 +908,6 @@ void ValueMetricProducer::onMatchedLogEventInternalLocked( 
            interval.hasValue = true;

        }

        interval.sampleSize += 1;

        baseInfo.currentState = stateKey;

    }



    // Only trigger the tracker if all intervals are correct

cmds/statsd/src/metrics/ValueMetricProducer.h

+1 −0
Original line number Diff line number Diff line
@@ -313,6 +313,7 @@ private: 
    FRIEND_TEST(ValueMetricProducerTest, TestSlicedState);

    FRIEND_TEST(ValueMetricProducerTest, TestSlicedStateWithMap);

    FRIEND_TEST(ValueMetricProducerTest, TestSlicedStateWithPrimaryField_WithDimensions);

    FRIEND_TEST(ValueMetricProducerTest, TestSlicedStateWithCondition);

    FRIEND_TEST(ValueMetricProducerTest, TestTrimUnusedDimensionKey);

    FRIEND_TEST(ValueMetricProducerTest, TestUseZeroDefaultBase);

    FRIEND_TEST(ValueMetricProducerTest, TestUseZeroDefaultBaseWithPullFailures);

cmds/statsd/tests/metrics/ValueMetricProducer_test.cpp

+202 −3
Original line number Diff line number Diff line
@@ -167,6 +167,32 @@ public: 
        return valueProducer;

    }



    static sp<ValueMetricProducer> createValueProducerWithConditionAndState(

            sp<MockStatsPullerManager>& pullerManager, ValueMetric& metric,

            vector<int32_t> slicedStateAtoms,

            unordered_map<int, unordered_map<int, int64_t>> stateGroupMap,

            ConditionState conditionAfterFirstBucketPrepared) {

        UidMap uidMap;

        SimpleAtomMatcher atomMatcher;

        atomMatcher.set_atom_id(tagId);

        sp<EventMatcherWizard> eventMatcherWizard =

                new EventMatcherWizard({new SimpleLogMatchingTracker(

                        atomMatcherId, logEventMatcherIndex, atomMatcher, uidMap)});

        sp<MockConditionWizard> wizard = new NaggyMock<MockConditionWizard>();

        EXPECT_CALL(*pullerManager, RegisterReceiver(tagId, kConfigKey, _, _, _))

                .WillOnce(Return());

        EXPECT_CALL(*pullerManager, UnRegisterReceiver(tagId, kConfigKey, _))

                .WillRepeatedly(Return());



        sp<ValueMetricProducer> valueProducer = new ValueMetricProducer(

                kConfigKey, metric, 0 /* condition tracker index */, {ConditionState::kUnknown},

                wizard, logEventMatcherIndex, eventMatcherWizard, tagId, bucketStartTimeNs,

                bucketStartTimeNs, pullerManager, {}, {}, slicedStateAtoms, stateGroupMap);

        valueProducer->prepareFirstBucket();

        valueProducer->mCondition = conditionAfterFirstBucketPrepared;

        return valueProducer;

    }



    static ValueMetric createMetric() {

        ValueMetric metric;

        metric.set_id(metricId);
@@ -188,6 +214,13 @@ public: 
        metric.add_slice_by_state(StringToId(state));

        return metric;

    }



    static ValueMetric createMetricWithConditionAndState(string state) {

        ValueMetric metric = ValueMetricProducerTestHelper::createMetric();

        metric.set_condition(StringToId("SCREEN_ON"));

        metric.add_slice_by_state(StringToId(state));

        return metric;

    }

};



// Setup for parameterized tests.
@@ -3893,13 +3926,13 @@ TEST(ValueMetricProducerTest, TestSlicedState) { 
                return true;

            }));



    StateManager::getInstance().clear();

    sp<ValueMetricProducer> valueProducer =

            ValueMetricProducerTestHelper::createValueProducerWithState(

                    pullerManager, metric, {util::SCREEN_STATE_CHANGED}, {});

    EXPECT_EQ(1, valueProducer->mSlicedStateAtoms.size());



    // Set up StateManager and check that StateTrackers are initialized.

    StateManager::getInstance().clear();

    StateManager::getInstance().registerListener(SCREEN_STATE_ATOM_ID, valueProducer);

    EXPECT_EQ(1, StateManager::getInstance().getStateTrackersCount());

    EXPECT_EQ(1, StateManager::getInstance().getListenersCount(SCREEN_STATE_ATOM_ID));
@@ -4105,12 +4138,12 @@ TEST(ValueMetricProducerTest, TestSlicedStateWithMap) { 
        }

    }



    StateManager::getInstance().clear();

    sp<ValueMetricProducer> valueProducer =

            ValueMetricProducerTestHelper::createValueProducerWithState(

                    pullerManager, metric, {util::SCREEN_STATE_CHANGED}, stateGroupMap);



    // Set up StateManager and check that StateTrackers are initialized.

    StateManager::getInstance().clear();

    StateManager::getInstance().registerListener(SCREEN_STATE_ATOM_ID, valueProducer);

    EXPECT_EQ(1, StateManager::getInstance().getStateTrackersCount());

    EXPECT_EQ(1, StateManager::getInstance().getListenersCount(SCREEN_STATE_ATOM_ID));
@@ -4357,12 +4390,12 @@ TEST(ValueMetricProducerTest, TestSlicedStateWithPrimaryField_WithDimensions) { 
                return true;

            }));



    StateManager::getInstance().clear();

    sp<ValueMetricProducer> valueProducer =

            ValueMetricProducerTestHelper::createValueProducerWithState(

                    pullerManager, metric, {UID_PROCESS_STATE_ATOM_ID}, {});



    // Set up StateManager and check that StateTrackers are initialized.

    StateManager::getInstance().clear();

    StateManager::getInstance().registerListener(UID_PROCESS_STATE_ATOM_ID, valueProducer);

    EXPECT_EQ(1, StateManager::getInstance().getStateTrackersCount());

    EXPECT_EQ(1, StateManager::getInstance().getListenersCount(UID_PROCESS_STATE_ATOM_ID));
@@ -4722,6 +4755,172 @@ TEST(ValueMetricProducerTest, TestSlicedStateWithPrimaryField_WithDimensions) { 
    EXPECT_EQ(5, report.value_metrics().data(4).bucket_info(1).values(0).value_long());

}



TEST(ValueMetricProducerTest, TestSlicedStateWithCondition) {

    // Set up ValueMetricProducer.

    ValueMetric metric = ValueMetricProducerTestHelper::createMetricWithConditionAndState(

            "BATTERY_SAVER_MODE_STATE");

    sp<MockStatsPullerManager> pullerManager = new StrictMock<MockStatsPullerManager>();

    EXPECT_CALL(*pullerManager, Pull(tagId, kConfigKey, _, _, _))

            // Condition changed to true.

            .WillOnce(Invoke([](int tagId, const ConfigKey&, const int64_t eventTimeNs,

                                vector<std::shared_ptr<LogEvent>>* data, bool) {

                EXPECT_EQ(eventTimeNs, bucketStartTimeNs + 20 * NS_PER_SEC);

                data->clear();

                data->push_back(

                        CreateRepeatedValueLogEvent(tagId, bucketStartTimeNs + 20 * NS_PER_SEC, 3));

                return true;

            }))

            // Battery saver mode state changed to OFF.

            .WillOnce(Invoke([](int tagId, const ConfigKey&, const int64_t eventTimeNs,

                                vector<std::shared_ptr<LogEvent>>* data, bool) {

                EXPECT_EQ(eventTimeNs, bucketStartTimeNs + 30 * NS_PER_SEC);

                data->clear();

                data->push_back(

                        CreateRepeatedValueLogEvent(tagId, bucketStartTimeNs + 30 * NS_PER_SEC, 5));

                return true;

            }))

            // Condition changed to false.

            .WillOnce(Invoke([](int tagId, const ConfigKey&, const int64_t eventTimeNs,

                                vector<std::shared_ptr<LogEvent>>* data, bool) {

                EXPECT_EQ(eventTimeNs, bucket2StartTimeNs + 10 * NS_PER_SEC);

                data->clear();

                data->push_back(CreateRepeatedValueLogEvent(

                        tagId, bucket2StartTimeNs + 10 * NS_PER_SEC, 15));

                return true;

            }));



    StateManager::getInstance().clear();

    sp<ValueMetricProducer> valueProducer =

            ValueMetricProducerTestHelper::createValueProducerWithConditionAndState(

                    pullerManager, metric, {util::BATTERY_SAVER_MODE_STATE_CHANGED}, {},

                    ConditionState::kFalse);

    EXPECT_EQ(1, valueProducer->mSlicedStateAtoms.size());



    // Set up StateManager and check that StateTrackers are initialized.

    StateManager::getInstance().registerListener(util::BATTERY_SAVER_MODE_STATE_CHANGED,

                                                 valueProducer);

    EXPECT_EQ(1, StateManager::getInstance().getStateTrackersCount());

    EXPECT_EQ(1, StateManager::getInstance().getListenersCount(

                         util::BATTERY_SAVER_MODE_STATE_CHANGED));



    // Bucket status after battery saver mode ON event.

    // Condition is false so we do nothing.

    unique_ptr<LogEvent> batterySaverOnEvent =

            CreateBatterySaverOnEvent(/*timestamp=*/bucketStartTimeNs + 10 * NS_PER_SEC);

    StateManager::getInstance().onLogEvent(*batterySaverOnEvent);

    EXPECT_EQ(0UL, valueProducer->mCurrentSlicedBucket.size());

    EXPECT_EQ(0UL, valueProducer->mCurrentBaseInfo.size());



    // Bucket status after condition change to true.

    valueProducer->onConditionChanged(true, bucketStartTimeNs + 20 * NS_PER_SEC);

    // Base for dimension key {}

    ASSERT_EQ(1UL, valueProducer->mCurrentBaseInfo.size());

    std::unordered_map<HashableDimensionKey, std::vector<ValueMetricProducer::BaseInfo>>::iterator

            itBase = valueProducer->mCurrentBaseInfo.find(DEFAULT_DIMENSION_KEY);

    EXPECT_TRUE(itBase->second[0].hasBase);

    EXPECT_EQ(3, itBase->second[0].base.long_value);

    EXPECT_TRUE(itBase->second[0].hasCurrentState);

    ASSERT_EQ(1, itBase->second[0].currentState.getValues().size());

    EXPECT_EQ(BatterySaverModeStateChanged::ON,

              itBase->second[0].currentState.getValues()[0].mValue.int_value);

    // Value for key {{}, -1}

    ASSERT_EQ(1UL, valueProducer->mCurrentSlicedBucket.size());

    std::unordered_map<MetricDimensionKey, std::vector<ValueMetricProducer::Interval>>::iterator

            it = valueProducer->mCurrentSlicedBucket.begin();

    EXPECT_EQ(0, it->first.getDimensionKeyInWhat().getValues().size());

    ASSERT_EQ(1, it->first.getStateValuesKey().getValues().size());

    EXPECT_EQ(-1 /*StateTracker::kUnknown*/,

              it->first.getStateValuesKey().getValues()[0].mValue.int_value);

    EXPECT_FALSE(it->second[0].hasValue);



    // Bucket status after battery saver mode OFF event.

    unique_ptr<LogEvent> batterySaverOffEvent =

            CreateBatterySaverOffEvent(/*timestamp=*/bucketStartTimeNs + 30 * NS_PER_SEC);

    StateManager::getInstance().onLogEvent(*batterySaverOffEvent);

    // Base for dimension key {}

    ASSERT_EQ(1UL, valueProducer->mCurrentBaseInfo.size());

    itBase = valueProducer->mCurrentBaseInfo.find(DEFAULT_DIMENSION_KEY);

    EXPECT_TRUE(itBase->second[0].hasBase);

    EXPECT_EQ(5, itBase->second[0].base.long_value);

    EXPECT_TRUE(itBase->second[0].hasCurrentState);

    ASSERT_EQ(1, itBase->second[0].currentState.getValues().size());

    EXPECT_EQ(BatterySaverModeStateChanged::OFF,

              itBase->second[0].currentState.getValues()[0].mValue.int_value);

    // Value for key {{}, ON}

    ASSERT_EQ(2UL, valueProducer->mCurrentSlicedBucket.size());

    it = valueProducer->mCurrentSlicedBucket.begin();

    EXPECT_EQ(0, it->first.getDimensionKeyInWhat().getValues().size());

    ASSERT_EQ(1, it->first.getStateValuesKey().getValues().size());

    EXPECT_EQ(BatterySaverModeStateChanged::ON,

              it->first.getStateValuesKey().getValues()[0].mValue.int_value);

    EXPECT_TRUE(it->second[0].hasValue);

    EXPECT_EQ(2, it->second[0].value.long_value);



    // Pull at end of first bucket.

    vector<shared_ptr<LogEvent>> allData;

    allData.clear();

    allData.push_back(CreateRepeatedValueLogEvent(tagId, bucket2StartTimeNs, 11));

    valueProducer->onDataPulled(allData, /** succeed */ true, bucket2StartTimeNs);



    EXPECT_EQ(2UL, valueProducer->mPastBuckets.size());

    EXPECT_EQ(3UL, valueProducer->mCurrentSlicedBucket.size());

    // Base for dimension key {}

    ASSERT_EQ(1UL, valueProducer->mCurrentBaseInfo.size());

    itBase = valueProducer->mCurrentBaseInfo.find(DEFAULT_DIMENSION_KEY);

    EXPECT_TRUE(itBase->second[0].hasBase);

    EXPECT_EQ(11, itBase->second[0].base.long_value);

    EXPECT_TRUE(itBase->second[0].hasCurrentState);

    ASSERT_EQ(1, itBase->second[0].currentState.getValues().size());

    EXPECT_EQ(BatterySaverModeStateChanged::OFF,

              itBase->second[0].currentState.getValues()[0].mValue.int_value);



    // Bucket 2 status after condition change to false.

    valueProducer->onConditionChanged(false, bucket2StartTimeNs + 10 * NS_PER_SEC);

    // Base for dimension key {}

    ASSERT_EQ(1UL, valueProducer->mCurrentBaseInfo.size());

    itBase = valueProducer->mCurrentBaseInfo.find(DEFAULT_DIMENSION_KEY);

    EXPECT_FALSE(itBase->second[0].hasBase);

    EXPECT_TRUE(itBase->second[0].hasCurrentState);

    ASSERT_EQ(1, itBase->second[0].currentState.getValues().size());

    EXPECT_EQ(BatterySaverModeStateChanged::OFF,

              itBase->second[0].currentState.getValues()[0].mValue.int_value);

    // Value for key {{}, OFF}

    ASSERT_EQ(3UL, valueProducer->mCurrentSlicedBucket.size());

    it = valueProducer->mCurrentSlicedBucket.begin();

    EXPECT_EQ(0, it->first.getDimensionKeyInWhat().getValues().size());

    ASSERT_EQ(1, it->first.getStateValuesKey().getValues().size());

    EXPECT_EQ(BatterySaverModeStateChanged::OFF,

              it->first.getStateValuesKey().getValues()[0].mValue.int_value);

    EXPECT_TRUE(it->second[0].hasValue);

    EXPECT_EQ(4, it->second[0].value.long_value);



    // Start dump report and check output.

    ProtoOutputStream output;

    std::set<string> strSet;

    valueProducer->onDumpReport(bucket2StartTimeNs + 50 * NS_PER_SEC,

                                true /* include recent buckets */, true, NO_TIME_CONSTRAINTS,

                                &strSet, &output);



    StatsLogReport report = outputStreamToProto(&output);

    EXPECT_TRUE(report.has_value_metrics());

    ASSERT_EQ(2, report.value_metrics().data_size());



    ValueMetricData data = report.value_metrics().data(0);

    EXPECT_EQ(util::BATTERY_SAVER_MODE_STATE_CHANGED, data.slice_by_state(0).atom_id());

    EXPECT_TRUE(data.slice_by_state(0).has_value());

    EXPECT_EQ(BatterySaverModeStateChanged::ON, data.slice_by_state(0).value());

    ASSERT_EQ(1, data.bucket_info_size());

    EXPECT_EQ(2, data.bucket_info(0).values(0).value_long());



    data = report.value_metrics().data(1);

    EXPECT_EQ(util::BATTERY_SAVER_MODE_STATE_CHANGED, data.slice_by_state(0).atom_id());

    EXPECT_TRUE(data.slice_by_state(0).has_value());

    EXPECT_EQ(BatterySaverModeStateChanged::OFF, data.slice_by_state(0).value());

    ASSERT_EQ(2, data.bucket_info_size());

    EXPECT_EQ(6, data.bucket_info(0).values(0).value_long());

    EXPECT_EQ(4, data.bucket_info(1).values(0).value_long());

}



/*

 * Test bucket splits when condition is unknown.

 */

cmds/statsd/tests/statsd_test_util.cpp

+4 −0
Original line number Diff line number Diff line
@@ -663,6 +663,8 @@ std::unique_ptr<LogEvent> CreateBatterySaverOnEvent(uint64_t timestampNs) { 
    AStatsEvent_setAtomId(statsEvent, util::BATTERY_SAVER_MODE_STATE_CHANGED);

    AStatsEvent_overwriteTimestamp(statsEvent, timestampNs);

    AStatsEvent_writeInt32(statsEvent, BatterySaverModeStateChanged::ON);

    AStatsEvent_addBoolAnnotation(statsEvent, util::ANNOTATION_ID_EXCLUSIVE_STATE, true);

    AStatsEvent_addBoolAnnotation(statsEvent, util::ANNOTATION_ID_STATE_NESTED, false);



    std::unique_ptr<LogEvent> logEvent = std::make_unique<LogEvent>(/*uid=*/0, /*pid=*/0);

    parseStatsEventToLogEvent(statsEvent, logEvent.get());
@@ -674,6 +676,8 @@ std::unique_ptr<LogEvent> CreateBatterySaverOffEvent(uint64_t timestampNs) { 
    AStatsEvent_setAtomId(statsEvent, util::BATTERY_SAVER_MODE_STATE_CHANGED);

    AStatsEvent_overwriteTimestamp(statsEvent, timestampNs);

    AStatsEvent_writeInt32(statsEvent, BatterySaverModeStateChanged::OFF);

    AStatsEvent_addBoolAnnotation(statsEvent, util::ANNOTATION_ID_EXCLUSIVE_STATE, true);

    AStatsEvent_addBoolAnnotation(statsEvent, util::ANNOTATION_ID_STATE_NESTED, false);



    std::unique_ptr<LogEvent> logEvent = std::make_unique<LogEvent>(/*uid=*/0, /*pid=*/0);

    parseStatsEventToLogEvent(statsEvent, logEvent.get());