Merge "Fix a problem with ValueMetric when used with conditions and no diffs." into qt-dev (da22b269) · Commits · e / os / android_frameworks_base

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