Remove MeasureData::Add(), only used in tests. (#142)

Author: Ian Sturdy

opencensus/stats/internal/measure_data.cc

@@ -77,8 +77,14 @@ void MeasureData::AddToDistribution(const BucketBoundaries& boundaries,
   *count = new_count;
   *mean = new_mean;
 
-  *min = std::min(*min, min_);
-  *max = std::max(*max, max_);
+  if (*count == count_) {
+    // Overwrite in case the destination was zero-initialized.
+    *min = min_;
+    *max = max_;
+  } else {
+    *min = std::min(*min, min_);
+    *max = std::max(*max, max_);
+  }
 
   int histogram_index =
       std::find(boundaries_.begin(), boundaries_.end(), boundaries) -

opencensus/stats/internal/measure_data.h

@@ -51,7 +51,7 @@ class MeasureData final {
                          absl::Span<T> histogram_buckets) const;
 
  private:
-  absl::Span<const BucketBoundaries> boundaries_;
+  const absl::Span<const BucketBoundaries> boundaries_;
 
   uint64_t count_ = 0;
   double mean_ = 0;

opencensus/stats/internal/view_data_impl.cc

@@ -167,56 +167,6 @@ ViewDataImpl::ViewDataImpl(const ViewDataImpl& other)
   }
 }
 
-void ViewDataImpl::Add(double value, const std::vector<std::string>& tag_values,
-                       absl::Time now) {
-  end_time_ = std::max(end_time_, now);
-  switch (type_) {
-    case Type::kDouble: {
-      double_data_[tag_values] += value;
-      break;
-    }
-    case Type::kInt64: {
-      int_data_[tag_values] +=
-          (aggregation_.type() == Aggregation::Type::kSum ? value : 1);
-      break;
-    }
-    case Type::kDistribution: {
-      DataMap<Distribution>::iterator it = distribution_data_.find(tag_values);
-      if (it == distribution_data_.end()) {
-        it = distribution_data_.emplace_hint(
-            it, tag_values, Distribution(&aggregation_.bucket_boundaries()));
-      }
-      it->second.Add(value);
-      break;
-    }
-    case Type::kStatsObject: {
-      DataMap<IntervalStatsObject>::iterator it =
-          interval_data_.find(tag_values);
-      if (aggregation_.type() == Aggregation::Type::kDistribution) {
-        const auto& buckets = aggregation_.bucket_boundaries();
-        if (it == interval_data_.end()) {
-          it = interval_data_.emplace_hint(
-              it, std::piecewise_construct, std::make_tuple(tag_values),
-              std::make_tuple(buckets.num_buckets() + 5,
-                              aggregation_window_.duration(), now));
-        }
-        it->second.AddToDistribution(value, buckets.BucketForValue(value), now);
-      } else {
-        if (it == interval_data_.end()) {
-          it = interval_data_.emplace_hint(
-              it, std::piecewise_construct, std::make_tuple(tag_values),
-              std::make_tuple(1, aggregation_window_.duration(), now));
-        }
-        if (aggregation_ == Aggregation::Count()) {
-          it->second.MutableCurrentBucket(now)[0] += 1.0;
-        } else {
-          it->second.MutableCurrentBucket(now)[0] += value;
-        }
-      }
-    }
-  }
-}
-
 void ViewDataImpl::Merge(const std::vector<std::string>& tag_values,
                          const MeasureData& data, absl::Time now) {
   end_time_ = std::max(end_time_, now);

opencensus/stats/internal/view_data_impl.h

@@ -106,13 +106,6 @@ class ViewDataImpl {
   absl::Time start_time() const { return start_time_; }
   absl::Time end_time() const { return end_time_; }
 
-  // Adds data for the given tag values at 'now'. tag_values must be ordered
-  // according to the order of keys in the ViewDescriptor.
-  // TODO: Change to take Span<string_view> when heterogenous lookup is
-  // supported.
-  void Add(double value, const std::vector<std::string>& tag_values,
-           absl::Time now);
-
   // Merges bulk data for the given tag values at 'now'. tag_values must be
   // ordered according to the order of keys in the ViewDescriptor.
   // TODO: Change to take Span<string_view> when heterogenous lookup is

opencensus/stats/internal/view_data_impl_test.cc

@@ -30,6 +30,15 @@ namespace opencensus {
 namespace stats {
 namespace {
 
+void AddToViewDataImpl(double value, const std::vector<std::string>& tags,
+                       absl::Time time,
+                       const std::vector<BucketBoundaries>& boundaries,
+                       ViewDataImpl* data) {
+  MeasureData measure_data = MeasureData(boundaries);
+  measure_data.Add(value);
+  data->Merge(tags, measure_data, time);
+}
+
 TEST(ViewDataImplTest, Sum) {
   const absl::Time start_time = absl::UnixEpoch();
   const absl::Time end_time = absl::UnixEpoch() + absl::Seconds(1);
@@ -38,9 +47,9 @@ TEST(ViewDataImplTest, Sum) {
   const std::vector<std::string> tags1({"value1", "value2a"});
   const std::vector<std::string> tags2({"value1", "value2b"});
 
-  data.Add(1, tags1, start_time);
-  data.Add(2, tags1, start_time);
-  data.Add(5, tags2, end_time);
+  AddToViewDataImpl(1, tags1, start_time, {}, &data);
+  AddToViewDataImpl(2, tags1, start_time, {}, &data);
+  AddToViewDataImpl(5, tags2, end_time, {}, &data);
 
   EXPECT_EQ(Aggregation::Sum(), data.aggregation());
   EXPECT_EQ(AggregationWindow::Cumulative(), data.aggregation_window());
@@ -60,9 +69,9 @@ TEST(ViewDataImplTest, Count) {
   const std::vector<std::string> tags1({"value1", "value2a"});
   const std::vector<std::string> tags2({"value1", "value2b"});
 
-  data.Add(1, tags1, start_time);
-  data.Add(2, tags1, start_time);
-  data.Add(5, tags2, end_time);
+  AddToViewDataImpl(1, tags1, start_time, {}, &data);
+  AddToViewDataImpl(2, tags1, start_time, {}, &data);
+  AddToViewDataImpl(5, tags2, end_time, {}, &data);
 
   EXPECT_EQ(Aggregation::Count(), data.aggregation());
   EXPECT_EQ(AggregationWindow::Cumulative(), data.aggregation_window());
@@ -83,9 +92,9 @@ TEST(ViewDataImplTest, Distribution) {
   const std::vector<std::string> tags1({"value1", "value2a"});
   const std::vector<std::string> tags2({"value1", "value2b"});
 
-  data.Add(1, tags1, start_time);
-  data.Add(5, tags1, end_time);
-  data.Add(15, tags2, end_time);
+  AddToViewDataImpl(1, tags1, start_time, {buckets}, &data);
+  AddToViewDataImpl(5, tags1, end_time, {buckets}, &data);
+  AddToViewDataImpl(15, tags2, end_time, {buckets}, &data);
 
   EXPECT_EQ(Aggregation::Distribution(buckets), data.aggregation());
   EXPECT_EQ(AggregationWindow::Cumulative(), data.aggregation_window());
@@ -108,11 +117,11 @@ TEST(ViewDataImplTest, StatsObjectToCount) {
   const std::vector<std::string> tags1({"value1", "value2a"});
   const std::vector<std::string> tags2({"value1", "value2b"});
 
-  data.Add(1, tags1, time);
-  data.Add(2, tags1, time);
-  data.Add(2, tags2, time);
+  AddToViewDataImpl(1, tags1, time, {}, &data);
+  AddToViewDataImpl(2, tags1, time, {}, &data);
+  AddToViewDataImpl(2, tags2, time, {}, &data);
   time += interval / 2;
-  data.Add(1, tags1, time);
+  AddToViewDataImpl(1, tags1, time, {}, &data);
 
   const ViewDataImpl export_data1(data, time);
   EXPECT_EQ(Aggregation::Count(), export_data1.aggregation());
@@ -143,11 +152,11 @@ TEST(ViewDataImplTest, StatsObjectToSum) {
   const std::vector<std::string> tags1({"value1", "value2a"});
   const std::vector<std::string> tags2({"value1", "value2b"});
 
-  data.Add(1, tags1, time);
-  data.Add(3, tags1, time);
-  data.Add(2, tags2, time);
+  AddToViewDataImpl(1, tags1, time, {}, &data);
+  AddToViewDataImpl(3, tags1, time, {}, &data);
+  AddToViewDataImpl(2, tags2, time, {}, &data);
   time += interval / 2;
-  data.Add(2, tags1, time);
+  AddToViewDataImpl(2, tags1, time, {}, &data);
 
   const ViewDataImpl export_data1(data, time);
   EXPECT_EQ(Aggregation::Sum(), export_data1.aggregation());
@@ -180,11 +189,11 @@ TEST(ViewDataImplTest, StatsObjectToDistribution) {
   const std::vector<std::string> tags1({"value1", "value2a"});
   const std::vector<std::string> tags2({"value1", "value2b"});
 
-  data.Add(5, tags1, time);
-  data.Add(15, tags1, time);
-  data.Add(0, tags2, time);
+  AddToViewDataImpl(5, tags1, time, {buckets}, &data);
+  AddToViewDataImpl(15, tags1, time, {buckets}, &data);
+  AddToViewDataImpl(0, tags2, time, {buckets}, &data);
   time += interval / 2;
-  data.Add(10, tags1, time);
+  AddToViewDataImpl(10, tags1, time, {buckets}, &data);
 
   const ViewDataImpl export_data1(data, time);
   EXPECT_EQ(Aggregation::Distribution(buckets), export_data1.aggregation());

opencensus/stats/testing/test_utils.cc

@@ -14,11 +14,16 @@
 
 #include "opencensus/stats/testing/test_utils.h"
 
+#include <initializer_list>
 #include <memory>
+#include <string>
+#include <utility>
 
 #include "absl/memory/memory.h"
 #include "absl/time/time.h"
+#include "opencensus/stats/bucket_boundaries.h"
 #include "opencensus/stats/internal/delta_producer.h"
+#include "opencensus/stats/internal/measure_data.h"
 
 namespace opencensus {
 namespace stats {
@@ -29,8 +34,12 @@ ViewData TestUtils::MakeViewData(
     const ViewDescriptor& descriptor,
     std::initializer_list<std::pair<std::vector<std::string>, double>> values) {
   auto impl = absl::make_unique<ViewDataImpl>(absl::UnixEpoch(), descriptor);
+  std::vector<BucketBoundaries> boundaries = {
+      descriptor.aggregation().bucket_boundaries()};
   for (const auto& value : values) {
-    impl->Add(value.second, value.first, absl::UnixEpoch());
+    MeasureData measure_data = MeasureData(boundaries);
+    measure_data.Add(value.second);
+    impl->Merge(value.first, measure_data, absl::UnixEpoch());
   }
   if (impl->type() == ViewDataImpl::Type::kStatsObject) {
     return ViewData(absl::make_unique<ViewDataImpl>(*impl, absl::UnixEpoch()));