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

Commit 549a6718 authored by Olivier Gaillard's avatar Olivier Gaillard Committed by android-build-merger
Browse files

Merge "Fix a problem with ValueMetric when used with conditions and no diffs."... 

Merge "Fix a problem with ValueMetric when used with conditions and no diffs." into qt-dev am: 769fc614
am: da22b269

Change-Id: I24d8bd40bcad174f9e420fd31fbad76fb9f4cf37
parents 19846f9a da22b269

cmds/statsd/src/metrics/ValueMetricProducer.cpp

+11 −4
Original line number Diff line number Diff line
@@ -633,10 +633,17 @@ void ValueMetricProducer::onMatchedLogEventInternalLocked(const size_t matcherIn 
        flushIfNeededLocked(eventTimeNs);

    }



    // For pulled data, we already check condition when we decide to pull or

    // in onDataPulled. So take all of them.

    // For pushed data, just check condition.

    if (!(mIsPulled || condition)) {

    // We should not accumulate the data for pushed metrics when the condition is false.

    bool shouldSkipForPushMetric = !mIsPulled && !condition;

    // For pulled metrics, there are two cases:

    // - to compute diffs, we need to process all the state changes

    // - for non-diffs metrics, we should ignore the data if the condition wasn't true. If we have a

    // state change from

    //     + True -> True: we should process the data, it might be a bucket boundary

    //     + True -> False: we als need to process the data.

    bool shouldSkipForPulledMetric = mIsPulled && !mUseDiff

            && mCondition != ConditionState::kTrue;

    if (shouldSkipForPushMetric || shouldSkipForPulledMetric) {

        VLOG("ValueMetric skip event because condition is false");

        return;

    }

cmds/statsd/src/metrics/ValueMetricProducer.h

+5 −0
Original line number Diff line number Diff line
@@ -259,6 +259,11 @@ private: 
    FRIEND_TEST(ValueMetricProducerTest, TestLateOnDataPulledWithoutDiff);

    FRIEND_TEST(ValueMetricProducerTest, TestPartialBucketCreated);

    FRIEND_TEST(ValueMetricProducerTest, TestPartialResetOnBucketBoundaries);

    FRIEND_TEST(ValueMetricProducerTest, TestPulledData_noDiff_bucketBoundaryFalse);

    FRIEND_TEST(ValueMetricProducerTest, TestPulledData_noDiff_bucketBoundaryTrue);

    FRIEND_TEST(ValueMetricProducerTest, TestPulledData_noDiff_withFailure);

    FRIEND_TEST(ValueMetricProducerTest, TestPulledData_noDiff_withMultipleConditionChanges);

    FRIEND_TEST(ValueMetricProducerTest, TestPulledData_noDiff_withoutCondition);

    FRIEND_TEST(ValueMetricProducerTest, TestPulledEventsNoCondition);

    FRIEND_TEST(ValueMetricProducerTest, TestPulledEventsTakeAbsoluteValueOnReset);

    FRIEND_TEST(ValueMetricProducerTest, TestPulledEventsTakeZeroOnReset);

cmds/statsd/tests/metrics/ValueMetricProducer_test.cpp

+140 −0
Original line number Diff line number Diff line
@@ -2964,6 +2964,146 @@ TEST(ValueMetricProducerTest, TestPullNeededNoTimeConstraints) { 
    EXPECT_EQ(10, report.value_metrics().data(0).bucket_info(0).condition_true_nanos());

}



TEST(ValueMetricProducerTest, TestPulledData_noDiff_withoutCondition) {

    ValueMetric metric = ValueMetricProducerTestHelper::createMetric();

    metric.set_use_diff(false);



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

    sp<ValueMetricProducer> valueProducer =

            ValueMetricProducerTestHelper::createValueProducerNoConditions(pullerManager, metric);



    vector<shared_ptr<LogEvent>> allData;

    allData.push_back(ValueMetricProducerTestHelper::createEvent(bucket2StartTimeNs + 30, 10));

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



    // Bucket should have been completed.

    assertPastBucketValuesSingleKey(valueProducer->mPastBuckets, {10}, {bucketSizeNs});

}



TEST(ValueMetricProducerTest, TestPulledData_noDiff_withMultipleConditionChanges) {

    ValueMetric metric = ValueMetricProducerTestHelper::createMetricWithCondition();

    metric.set_use_diff(false);



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

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

            // condition becomes true

            .WillOnce(Invoke([](int tagId, vector<std::shared_ptr<LogEvent>>* data) {

                data->clear();

                data->push_back(ValueMetricProducerTestHelper::createEvent(

                        bucketStartTimeNs + 30, 10));

                return true;

            }))

            // condition becomes false

            .WillOnce(Invoke([](int tagId, vector<std::shared_ptr<LogEvent>>* data) {

                data->clear();

                data->push_back(ValueMetricProducerTestHelper::createEvent(

                        bucketStartTimeNs + 50, 20));

                return true;

            }));

    sp<ValueMetricProducer> valueProducer =

            ValueMetricProducerTestHelper::createValueProducerWithCondition(pullerManager, metric);

    valueProducer->mCondition = ConditionState::kFalse;



    valueProducer->onConditionChanged(true, bucketStartTimeNs + 8);

    valueProducer->onConditionChanged(false, bucketStartTimeNs + 50);

    // has one slice

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

    ValueMetricProducer::Interval curInterval =

            valueProducer->mCurrentSlicedBucket.begin()->second[0];

    EXPECT_EQ(false, curInterval.hasBase);

    EXPECT_EQ(true, curInterval.hasValue);

    EXPECT_EQ(20, curInterval.value.long_value);





    // Now the alarm is delivered. Condition is off though.

    vector<shared_ptr<LogEvent>> allData;

    allData.push_back(ValueMetricProducerTestHelper::createEvent(bucket2StartTimeNs + 30, 110));

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



    assertPastBucketValuesSingleKey(valueProducer->mPastBuckets, {20}, {50 - 8});

    curInterval = valueProducer->mCurrentSlicedBucket.begin()->second[0];

    EXPECT_EQ(false, curInterval.hasBase);

    EXPECT_EQ(false, curInterval.hasValue);

}



TEST(ValueMetricProducerTest, TestPulledData_noDiff_bucketBoundaryTrue) {

    ValueMetric metric = ValueMetricProducerTestHelper::createMetricWithCondition();

    metric.set_use_diff(false);



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

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

            // condition becomes true

            .WillOnce(Invoke([](int tagId, vector<std::shared_ptr<LogEvent>>* data) {

                data->clear();

                data->push_back(ValueMetricProducerTestHelper::createEvent(

                        bucketStartTimeNs + 30, 10));

                return true;

            }));

    sp<ValueMetricProducer> valueProducer =

            ValueMetricProducerTestHelper::createValueProducerWithCondition(pullerManager, metric);

    valueProducer->mCondition = ConditionState::kFalse;



    valueProducer->onConditionChanged(true, bucketStartTimeNs + 8);



    // Now the alarm is delivered. Condition is off though.

    vector<shared_ptr<LogEvent>> allData;

    allData.push_back(ValueMetricProducerTestHelper::createEvent(bucket2StartTimeNs + 30, 30));

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



    assertPastBucketValuesSingleKey(valueProducer->mPastBuckets, {30}, {bucketSizeNs - 8});

    ValueMetricProducer::Interval curInterval =

            valueProducer->mCurrentSlicedBucket.begin()->second[0];

    EXPECT_EQ(false, curInterval.hasBase);

    EXPECT_EQ(false, curInterval.hasValue);

}



TEST(ValueMetricProducerTest, TestPulledData_noDiff_bucketBoundaryFalse) {

    ValueMetric metric = ValueMetricProducerTestHelper::createMetricWithCondition();

    metric.set_use_diff(false);



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

    sp<ValueMetricProducer> valueProducer =

            ValueMetricProducerTestHelper::createValueProducerWithCondition(pullerManager, metric);

    valueProducer->mCondition = ConditionState::kFalse;



    // Now the alarm is delivered. Condition is off though.

    vector<shared_ptr<LogEvent>> allData;

    allData.push_back(ValueMetricProducerTestHelper::createEvent(bucket2StartTimeNs + 30, 30));

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



    // Condition was always false.

    assertPastBucketValuesSingleKey(valueProducer->mPastBuckets, {}, {});

}



TEST(ValueMetricProducerTest, TestPulledData_noDiff_withFailure) {

    ValueMetric metric = ValueMetricProducerTestHelper::createMetricWithCondition();

    metric.set_use_diff(false);



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

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

            // condition becomes true

            .WillOnce(Invoke([](int tagId, vector<std::shared_ptr<LogEvent>>* data) {

                data->clear();

                data->push_back(ValueMetricProducerTestHelper::createEvent(

                        bucketStartTimeNs + 30, 10));

                return true;

            }))

            .WillOnce(Return(false));

    sp<ValueMetricProducer> valueProducer =

            ValueMetricProducerTestHelper::createValueProducerWithCondition(pullerManager, metric);

    valueProducer->mCondition = ConditionState::kFalse;



    valueProducer->onConditionChanged(true, bucketStartTimeNs + 8);

    valueProducer->onConditionChanged(false, bucketStartTimeNs + 50);



    // Now the alarm is delivered. Condition is off though.

    vector<shared_ptr<LogEvent>> allData;

    allData.push_back(ValueMetricProducerTestHelper::createEvent(bucket2StartTimeNs + 30, 30));

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



    // No buckets, we had a failure.

    assertPastBucketValuesSingleKey(valueProducer->mPastBuckets, {}, {});

}



}  // namespace statsd

}  // namespace os