Change data type for Sum/int64 to int64. (#141)

Author: Ian Sturdy

opencensus/exporters/stats/prometheus/internal/prometheus_utils.cc

@@ -53,13 +53,20 @@ io::prometheus::client::MetricType MetricType(
   }
 }
 
-void SetValue(double value, io::prometheus::client::Metric* metric) {
+void SetValue(double value, io::prometheus::client::MetricType type,
+              io::prometheus::client::Metric* metric) {
   metric->mutable_untyped()->set_value(value);
 }
-void SetValue(int64_t value, io::prometheus::client::Metric* metric) {
-  metric->mutable_counter()->set_value(value);
+void SetValue(int64_t value, io::prometheus::client::MetricType type,
+              io::prometheus::client::Metric* metric) {
+  if (type == io::prometheus::client::MetricType::COUNTER) {
+    metric->mutable_counter()->set_value(value);
+  } else {
+    metric->mutable_untyped()->set_value(value);
+  }
 }
 void SetValue(const opencensus::stats::Distribution& value,
+              io::prometheus::client::MetricType type,
               io::prometheus::client::Metric* metric) {
   auto* histogram = metric->mutable_histogram();
   histogram->set_sample_count(value.count());
@@ -82,6 +89,7 @@ void SetValue(const opencensus::stats::Distribution& value,
 template <typename T>
 void SetData(const opencensus::stats::ViewDescriptor& descriptor,
              const opencensus::stats::ViewData::DataMap<T>& data, int64_t time,
+             io::prometheus::client::MetricType type,
              io::prometheus::client::MetricFamily* metric_family) {
   for (const auto& row : data) {
     auto* metric = metric_family->add_metric();
@@ -91,7 +99,7 @@ void SetData(const opencensus::stats::ViewDescriptor& descriptor,
       label->set_name(SanitizeName(descriptor.columns()[i].name()));
       label->set_value(row.first[i]);
     }
-    SetValue(row.second, metric);
+    SetValue(row.second, type, metric);
   }
 }
 
@@ -100,24 +108,26 @@ void SetData(const opencensus::stats::ViewDescriptor& descriptor,
 void SetMetricFamily(const opencensus::stats::ViewDescriptor& descriptor,
                      const opencensus::stats::ViewData& data,
                      io::prometheus::client::MetricFamily* metric_family) {
+  const io::prometheus::client::MetricType type =
+      MetricType(descriptor.aggregation().type());
   // TODO(sturdy): convert common units into base units (e.g. ms->s).
   metric_family->set_name(SanitizeName(absl::StrCat(
       descriptor.name(), "_", descriptor.measure_descriptor().units())));
   metric_family->set_help(descriptor.description());
-  metric_family->set_type(MetricType(descriptor.aggregation().type()));
+  metric_family->set_type(type);
 
   const int64_t time = absl::ToUnixMillis(data.end_time());
   switch (data.type()) {
     case opencensus::stats::ViewData::Type::kDouble: {
-      SetData(descriptor, data.double_data(), time, metric_family);
+      SetData(descriptor, data.double_data(), time, type, metric_family);
       break;
     }
     case opencensus::stats::ViewData::Type::kInt64: {
-      SetData(descriptor, data.int_data(), time, metric_family);
+      SetData(descriptor, data.int_data(), time, type, metric_family);
       break;
     }
     case opencensus::stats::ViewData::Type::kDistribution: {
-      SetData(descriptor, data.distribution_data(), time, metric_family);
+      SetData(descriptor, data.distribution_data(), time, type, metric_family);
       break;
     }
   }

opencensus/stats/internal/stats_manager_test.cc

@@ -84,10 +84,10 @@ TEST_F(StatsManagerTest, Count) {
           ::testing::Pair(::testing::ElementsAre("value1", "value2"), 1.0)));
 }
 
-TEST_F(StatsManagerTest, Sum) {
+TEST_F(StatsManagerTest, SumDouble) {
   ViewDescriptor view_descriptor = ViewDescriptor()
-                                       .set_measure(kSecondMeasureId)
-                                       .set_name("sum")
+                                       .set_measure(kFirstMeasureId)
+                                       .set_name("sum_double")
                                        .set_aggregation(Aggregation::Sum())
                                        .add_column(key1_)
                                        .add_column(key2_);
@@ -96,12 +96,12 @@ TEST_F(StatsManagerTest, Sum) {
   EXPECT_TRUE(view.GetData().double_data().empty());
 
   // Stats under a different measure should be ignored.
-  Record({{FirstMeasure(), 1.0}});
+  Record({{SecondMeasure(), 1}});
   testing::TestUtils::Flush();
   EXPECT_TRUE(view.GetData().double_data().empty());
 
-  Record({{SecondMeasure(), 2}, {SecondMeasure(), 3}});
-  Record({{SecondMeasure(), 4}},
+  Record({{FirstMeasure(), 2.0}, {FirstMeasure(), 3.0}});
+  Record({{FirstMeasure(), 4.0}},
          {{key1_, "value1"}, {key2_, "value2"}, {key3_, "value3"}});
   testing::TestUtils::Flush();
   const opencensus::stats::ViewData data = view.GetData();
@@ -112,6 +112,34 @@ TEST_F(StatsManagerTest, Sum) {
           ::testing::Pair(::testing::ElementsAre("value1", "value2"), 4.0)));
 }
 
+TEST_F(StatsManagerTest, SumInt) {
+  ViewDescriptor view_descriptor = ViewDescriptor()
+                                       .set_measure(kSecondMeasureId)
+                                       .set_name("sum_int")
+                                       .set_aggregation(Aggregation::Sum())
+                                       .add_column(key1_)
+                                       .add_column(key2_);
+  View view(view_descriptor);
+  ASSERT_EQ(ViewData::Type::kInt64, view.GetData().type());
+  EXPECT_TRUE(view.GetData().int_data().empty());
+
+  // Stats under a different measure should be ignored.
+  Record({{FirstMeasure(), 1.0}});
+  testing::TestUtils::Flush();
+  EXPECT_TRUE(view.GetData().int_data().empty());
+
+  Record({{SecondMeasure(), 2}, {SecondMeasure(), 3}});
+  Record({{SecondMeasure(), 4}},
+         {{key1_, "value1"}, {key2_, "value2"}, {key3_, "value3"}});
+  testing::TestUtils::Flush();
+  const opencensus::stats::ViewData data = view.GetData();
+  EXPECT_THAT(
+      data.int_data(),
+      ::testing::UnorderedElementsAre(
+          ::testing::Pair(::testing::ElementsAre("", ""), 5),
+          ::testing::Pair(::testing::ElementsAre("value1", "value2"), 4)));
+}
+
 TEST_F(StatsManagerTest, Distribution) {
   ViewDescriptor view_descriptor =
       ViewDescriptor()

opencensus/stats/internal/view_data_impl.cc

@@ -20,6 +20,8 @@
 
 #include "absl/memory/memory.h"
 #include "opencensus/stats/distribution.h"
+#include "opencensus/stats/measure_descriptor.h"
+#include "opencensus/stats/view_descriptor.h"
 
 namespace opencensus {
 namespace stats {
@@ -31,7 +33,12 @@ ViewDataImpl::Type ViewDataImpl::TypeForDescriptor(
     case AggregationWindow::Type::kDelta:
       switch (descriptor.aggregation().type()) {
         case Aggregation::Type::kSum:
-          return ViewDataImpl::Type::kDouble;
+          switch (descriptor.measure_descriptor().type()) {
+            case MeasureDescriptor::Type::kDouble:
+              return ViewDataImpl::Type::kDouble;
+            case MeasureDescriptor::Type::kInt64:
+              return ViewDataImpl::Type::kInt64;
+          }
         case Aggregation::Type::kCount:
           return ViewDataImpl::Type::kInt64;
         case Aggregation::Type::kDistribution:
@@ -169,7 +176,8 @@ void ViewDataImpl::Add(double value, const std::vector<std::string>& tag_values,
       break;
     }
     case Type::kInt64: {
-      ++int_data_[tag_values];
+      int_data_[tag_values] +=
+          (aggregation_.type() == Aggregation::Type::kSum ? value : 1);
       break;
     }
     case Type::kDistribution: {
@@ -218,7 +226,9 @@ void ViewDataImpl::Merge(const std::vector<std::string>& tag_values,
       break;
     }
     case Type::kInt64: {
-      int_data_[tag_values] += data.count();
+      int_data_[tag_values] +=
+          (aggregation_.type() == Aggregation::Type::kSum ? data.sum()
+                                                          : data.count());
       break;
     }
     case Type::kDistribution: {