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

Commit 3be5671c authored by Jeffrey Huang's avatar Jeffrey Huang
Browse files

Fix ConditionUnknown after valid buckets 

Because non-zero enums are evaluated to true, it is possible for pulls
to occur on bucket split even if the condition is unknown. Explicitly
check that the condition is true instead.

Bug: 155489707
Test: atest statsd_test
Change-Id: I0bc3de6e885c2200885caf01d07c793b83eca17a
parent a2d82aeb

cmds/statsd/src/metrics/ValueMetricProducer.h

+2 −2
Original line number Diff line number Diff line
@@ -75,7 +75,7 @@ public: 
        if (!mSplitBucketForAppUpgrade) {

            return;

        }

        if (mIsPulled && mCondition) {

        if (mIsPulled && mCondition == ConditionState::kTrue) {

            pullAndMatchEventsLocked(eventTimeNs);

        }

        flushCurrentBucketLocked(eventTimeNs, eventTimeNs);
@@ -84,7 +84,7 @@ public: 
    // ValueMetric needs special logic if it's a pulled atom.

    void onStatsdInitCompleted(const int64_t& eventTimeNs) override {

        std::lock_guard<std::mutex> lock(mMutex);

        if (mIsPulled && mCondition) {

        if (mIsPulled && mCondition == ConditionState::kTrue) {

            pullAndMatchEventsLocked(eventTimeNs);

        }

        flushCurrentBucketLocked(eventTimeNs, eventTimeNs);

cmds/statsd/tests/metrics/ValueMetricProducer_test.cpp

+40 −0
Original line number Diff line number Diff line
@@ -4722,6 +4722,46 @@ TEST(ValueMetricProducerTest, TestSlicedStateWithPrimaryField_WithDimensions) { 
    EXPECT_EQ(5, report.value_metrics().data(4).bucket_info(1).values(0).value_long());

}



/*

 * Test bucket splits when condition is unknown.

 */

TEST(ValueMetricProducerTest, TestForcedBucketSplitWhenConditionUnknownSkipsBucket) {

    ValueMetric metric = ValueMetricProducerTestHelper::createMetricWithCondition();



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



    sp<ValueMetricProducer> valueProducer =

            ValueMetricProducerTestHelper::createValueProducerWithCondition(

                    pullerManager, metric,

                    ConditionState::kUnknown);



    // App update event.

    int64_t appUpdateTimeNs = bucketStartTimeNs + 1000;

    valueProducer->notifyAppUpgrade(appUpdateTimeNs);



    // Check dump report.

    ProtoOutputStream output;

    std::set<string> strSet;

    int64_t dumpReportTimeNs = bucketStartTimeNs + 10000000000; // 10 seconds

    valueProducer->onDumpReport(dumpReportTimeNs, false /* include current buckets */, true,

                                NO_TIME_CONSTRAINTS /* dumpLatency */, &strSet, &output);



    StatsLogReport report = outputStreamToProto(&output);

    EXPECT_TRUE(report.has_value_metrics());

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

    ASSERT_EQ(1, report.value_metrics().skipped_size());



    EXPECT_EQ(NanoToMillis(bucketStartTimeNs),

              report.value_metrics().skipped(0).start_bucket_elapsed_millis());

    EXPECT_EQ(NanoToMillis(appUpdateTimeNs),

              report.value_metrics().skipped(0).end_bucket_elapsed_millis());

    ASSERT_EQ(1, report.value_metrics().skipped(0).drop_event_size());



    auto dropEvent = report.value_metrics().skipped(0).drop_event(0);

    EXPECT_EQ(BucketDropReason::NO_DATA, dropEvent.drop_reason());

    EXPECT_EQ(NanoToMillis(appUpdateTimeNs), dropEvent.drop_time_millis());

}



}  // namespace statsd

}  // namespace os

}  // namespace android