Merge "Fix a problem with ValueMetric when used with conditions and no diffs." into qt-dev

This commit is contained in:
Olivier Gaillard
2019-04-11 23:20:49 +00:00
committed by Android (Google) Code Review
3 changed files with 156 additions and 4 deletions

View File

@@ -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;
}

View File

@@ -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);

View File

@@ -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