Merge "Drop value if the bucket is totally tainted" into pi-dev
am: 5bc03014c0

Change-Id: I92b47ad538eeec38fbf4b4d7211dc75ebf903880
diff --git a/cmds/statsd/src/metrics/ValueMetricProducer.cpp b/cmds/statsd/src/metrics/ValueMetricProducer.cpp
index 51fac8c..89efae3 100644
--- a/cmds/statsd/src/metrics/ValueMetricProducer.cpp
+++ b/cmds/statsd/src/metrics/ValueMetricProducer.cpp
@@ -334,6 +334,7 @@
                 } else {
                     interval.sum += value;
                 }
+                interval.hasValue = true;
                 interval.startUpdated = false;
             } else {
                 VLOG("No start for matching end %ld", value);
@@ -342,6 +343,7 @@
         }
     } else {    // for pushed events
         interval.sum += value;
+        interval.hasValue = true;
     }
 
     long wholeBucketVal = interval.sum;
@@ -393,10 +395,12 @@
     for (const auto& slice : mCurrentSlicedBucket) {
         tainted += slice.second.tainted;
         tainted += slice.second.startUpdated;
-        info.mValue = slice.second.sum;
-        // it will auto create new vector of ValuebucketInfo if the key is not found.
-        auto& bucketList = mPastBuckets[slice.first];
-        bucketList.push_back(info);
+        if (slice.second.hasValue) {
+            info.mValue = slice.second.sum;
+            // it will auto create new vector of ValuebucketInfo if the key is not found.
+            auto& bucketList = mPastBuckets[slice.first];
+            bucketList.push_back(info);
+        }
     }
     VLOG("%d tainted pairs in the bucket", tainted);
 
diff --git a/cmds/statsd/src/metrics/ValueMetricProducer.h b/cmds/statsd/src/metrics/ValueMetricProducer.h
index b5f6429..b19adbe 100644
--- a/cmds/statsd/src/metrics/ValueMetricProducer.h
+++ b/cmds/statsd/src/metrics/ValueMetricProducer.h
@@ -135,6 +135,9 @@
         int tainted;
         // Running sum of known pairs in this bucket
         long sum;
+        // If this dimension has any non-tainted value. If not, don't report the
+        // dimension.
+        bool hasValue;
     } Interval;
 
     std::unordered_map<MetricDimensionKey, Interval> mCurrentSlicedBucket;
diff --git a/cmds/statsd/tests/metrics/ValueMetricProducer_test.cpp b/cmds/statsd/tests/metrics/ValueMetricProducer_test.cpp
index a2bb734..7b52265 100644
--- a/cmds/statsd/tests/metrics/ValueMetricProducer_test.cpp
+++ b/cmds/statsd/tests/metrics/ValueMetricProducer_test.cpp
@@ -90,8 +90,7 @@
     EXPECT_EQ(0, curInterval.tainted);
     EXPECT_EQ(0, curInterval.sum);
     EXPECT_EQ(11, curInterval.start);
-    EXPECT_EQ(1UL, valueProducer.mPastBuckets.size());
-    EXPECT_EQ(0, valueProducer.mPastBuckets.begin()->second.back().mValue);
+    EXPECT_EQ(0UL, valueProducer.mPastBuckets.size());
 
     allData.clear();
     event = make_shared<LogEvent>(tagId, bucket3StartTimeNs + 1);
@@ -108,7 +107,7 @@
     EXPECT_EQ(0, curInterval.tainted);
     EXPECT_EQ(0, curInterval.sum);
     EXPECT_EQ(1UL, valueProducer.mPastBuckets.size());
-    EXPECT_EQ(2UL, valueProducer.mPastBuckets.begin()->second.size());
+    EXPECT_EQ(1UL, valueProducer.mPastBuckets.begin()->second.size());
     EXPECT_EQ(12, valueProducer.mPastBuckets.begin()->second.back().mValue);
 
     allData.clear();
@@ -125,7 +124,7 @@
     EXPECT_EQ(0, curInterval.tainted);
     EXPECT_EQ(0, curInterval.sum);
     EXPECT_EQ(1UL, valueProducer.mPastBuckets.size());
-    EXPECT_EQ(3UL, valueProducer.mPastBuckets.begin()->second.size());
+    EXPECT_EQ(2UL, valueProducer.mPastBuckets.begin()->second.size());
     EXPECT_EQ(13, valueProducer.mPastBuckets.begin()->second.back().mValue);
 }
 
@@ -464,15 +463,13 @@
     // has one slice
     EXPECT_EQ(1UL, valueProducer.mCurrentSlicedBucket.size());
     ValueMetricProducer::Interval curInterval = valueProducer.mCurrentSlicedBucket.begin()->second;
-    valueProducer.setBucketSize(60 * NS_PER_SEC);
 
     // startUpdated:true tainted:0 sum:0 start:11
     EXPECT_EQ(true, curInterval.startUpdated);
     EXPECT_EQ(0, curInterval.tainted);
     EXPECT_EQ(0, curInterval.sum);
     EXPECT_EQ(11, curInterval.start);
-    EXPECT_EQ(1UL, valueProducer.mPastBuckets.size());
-    EXPECT_EQ(0, valueProducer.mPastBuckets.begin()->second.back().mValue);
+    EXPECT_EQ(0UL, valueProducer.mPastBuckets.size());
 
     // pull 2 at correct time
     allData.clear();
@@ -490,7 +487,7 @@
     EXPECT_EQ(0, curInterval.tainted);
     EXPECT_EQ(0, curInterval.sum);
     EXPECT_EQ(1UL, valueProducer.mPastBuckets.size());
-    EXPECT_EQ(2UL, valueProducer.mPastBuckets.begin()->second.size());
+    EXPECT_EQ(1UL, valueProducer.mPastBuckets.begin()->second.size());
     EXPECT_EQ(12, valueProducer.mPastBuckets.begin()->second.back().mValue);
 
     // pull 3 come late.
@@ -512,11 +509,8 @@
     EXPECT_EQ(36, curInterval.start);
     EXPECT_EQ(0, curInterval.sum);
     EXPECT_EQ(1UL, valueProducer.mPastBuckets.size());
-    EXPECT_EQ(4UL, valueProducer.mPastBuckets.begin()->second.size());
-    EXPECT_EQ(0, valueProducer.mPastBuckets.begin()->second[0].mValue);
-    EXPECT_EQ(12, valueProducer.mPastBuckets.begin()->second[1].mValue);
-    EXPECT_EQ(0, valueProducer.mPastBuckets.begin()->second[2].mValue);
-    EXPECT_EQ(0, valueProducer.mPastBuckets.begin()->second[3].mValue);
+    EXPECT_EQ(1UL, valueProducer.mPastBuckets.begin()->second.size());
+    EXPECT_EQ(12, valueProducer.mPastBuckets.begin()->second.back().mValue);
 }
 
 /*
@@ -582,9 +576,7 @@
     EXPECT_EQ(false, curInterval.startUpdated);
     EXPECT_EQ(1, curInterval.tainted);
     EXPECT_EQ(0, curInterval.sum);
-    EXPECT_EQ(1UL, valueProducer.mPastBuckets.size());
-    EXPECT_EQ(1UL, valueProducer.mPastBuckets.begin()->second.size());
-    EXPECT_EQ(0, valueProducer.mPastBuckets.begin()->second[0].mValue);
+    EXPECT_EQ(0UL, valueProducer.mPastBuckets.size());
 
     // Now the alarm is delivered.
     // since the condition turned to off before this pull finish, it has no effect
@@ -601,9 +593,7 @@
     EXPECT_EQ(false, curInterval.startUpdated);
     EXPECT_EQ(1, curInterval.tainted);
     EXPECT_EQ(0, curInterval.sum);
-    EXPECT_EQ(1UL, valueProducer.mPastBuckets.size());
-    EXPECT_EQ(1UL, valueProducer.mPastBuckets.begin()->second.size());
-    EXPECT_EQ(0, valueProducer.mPastBuckets.begin()->second[0].mValue);
+    EXPECT_EQ(0UL, valueProducer.mPastBuckets.size());
 }
 
 /*
@@ -680,9 +670,7 @@
     EXPECT_EQ(false, curInterval.startUpdated);
     EXPECT_EQ(1, curInterval.tainted);
     EXPECT_EQ(0, curInterval.sum);
-    EXPECT_EQ(1UL, valueProducer.mPastBuckets.size());
-    EXPECT_EQ(1UL, valueProducer.mPastBuckets.begin()->second.size());
-    EXPECT_EQ(0, valueProducer.mPastBuckets.begin()->second[0].mValue);
+    EXPECT_EQ(0UL, valueProducer.mPastBuckets.size());
 
     // condition changed to true again, before the pull alarm is delivered
     valueProducer.onConditionChanged(true, bucket2StartTimeNs + 25);
@@ -691,9 +679,7 @@
     EXPECT_EQ(130, curInterval.start);
     EXPECT_EQ(1, curInterval.tainted);
     EXPECT_EQ(0, curInterval.sum);
-    EXPECT_EQ(1UL, valueProducer.mPastBuckets.size());
-    EXPECT_EQ(1UL, valueProducer.mPastBuckets.begin()->second.size());
-    EXPECT_EQ(0, valueProducer.mPastBuckets.begin()->second[0].mValue);
+    EXPECT_EQ(0UL, valueProducer.mPastBuckets.size());
 
     // Now the alarm is delivered, but it is considered late, it has no effect
     vector<shared_ptr<LogEvent>> allData;
@@ -710,9 +696,7 @@
     EXPECT_EQ(130, curInterval.start);
     EXPECT_EQ(1, curInterval.tainted);
     EXPECT_EQ(0, curInterval.sum);
-    EXPECT_EQ(1UL, valueProducer.mPastBuckets.size());
-    EXPECT_EQ(1UL, valueProducer.mPastBuckets.begin()->second.size());
-    EXPECT_EQ(0, valueProducer.mPastBuckets.begin()->second[0].mValue);
+    EXPECT_EQ(0UL, valueProducer.mPastBuckets.size());
 }
 
 /*
@@ -779,9 +763,7 @@
     EXPECT_EQ(false, curInterval.startUpdated);
     EXPECT_EQ(1, curInterval.tainted);
     EXPECT_EQ(0, curInterval.sum);
-    EXPECT_EQ(1UL, valueProducer.mPastBuckets.size());
-    EXPECT_EQ(1UL, valueProducer.mPastBuckets.begin()->second.size());
-    EXPECT_EQ(0, valueProducer.mPastBuckets.begin()->second[0].mValue);
+    EXPECT_EQ(0UL, valueProducer.mPastBuckets.size());
 
     // Alarm is delivered in time, but the pull is very slow, and pullers are called in order,
     // so this one comes even later
@@ -798,9 +780,7 @@
     EXPECT_EQ(false, curInterval.startUpdated);
     EXPECT_EQ(1, curInterval.tainted);
     EXPECT_EQ(0, curInterval.sum);
-    EXPECT_EQ(1UL, valueProducer.mPastBuckets.size());
-    EXPECT_EQ(1UL, valueProducer.mPastBuckets.begin()->second.size());
-    EXPECT_EQ(0, valueProducer.mPastBuckets.begin()->second[0].mValue);
+    EXPECT_EQ(0UL, valueProducer.mPastBuckets.size());
 }
 
 }  // namespace statsd