[Metrics SDK] Histogram min/max support (open-telemetry#1540)

bazel/repository.bzl

@@ -87,10 +87,10 @@ def opentelemetry_cpp_deps():
  http_archive,
  name = "com_github_opentelemetry_proto",
  build_file = "@io_opentelemetry_cpp//bazel:opentelemetry_proto.BUILD",
- sha256 = "f269fbcb30e17b03caa1decd231ce826e59d7651c0f71c3b28eb5140b4bb5412",
- strip_prefix = "opentelemetry-proto-0.17.0",
+ sha256 = "134ce87f0a623daac19b9507b92da0d9b82929e3db796bba631e422f6ea8d3b3",
+ strip_prefix = "opentelemetry-proto-0.18.0",
  urls = [
- "https://github.com/open-telemetry/opentelemetry-proto/archive/v0.17.0.tar.gz",
+ "https://github.com/open-telemetry/opentelemetry-proto/archive/v0.18.0.tar.gz",
  ],
  )
 

exporters/ostream/src/metric_exporter.cc

@@ -174,6 +174,26 @@ void OStreamMetricExporter::printPointData(const opentelemetry::sdk::metrics::Po
  sout_ << nostd::get<long>(histogram_point_data.sum_);
  }
 
+ if (histogram_point_data.record_min_max_)
+ {
+ if (nostd::holds_alternative<long>(histogram_point_data.min_))
+ {
+ sout_ << "\n min : " << nostd::get<long>(histogram_point_data.min_);
+ }
+ else if (nostd::holds_alternative<double>(histogram_point_data.min_))
+ {
+ sout_ << "\n min : " << nostd::get<double>(histogram_point_data.min_);
+ }
+ if (nostd::holds_alternative<long>(histogram_point_data.max_))
+ {
+ sout_ << "\n max : " << nostd::get<long>(histogram_point_data.max_);
+ }
+ if (nostd::holds_alternative<double>(histogram_point_data.max_))
+ {
+ sout_ << "\n max : " << nostd::get<double>(histogram_point_data.max_);
+ }
+ }
+
  sout_ << "\n buckets : ";
  if (nostd::holds_alternative<std::list<double>>(histogram_point_data.boundaries_))
  {

exporters/ostream/test/ostream_metric_test.cc

@@ -102,6 +102,8 @@ TEST(OStreamMetricsExporter, ExportHistogramPointData)
  histogram_point_data.count_ = 3;
  histogram_point_data.counts_ = {200, 300, 400, 500};
  histogram_point_data.sum_ = 900.5;
+ histogram_point_data.min_ = 1.8;
+ histogram_point_data.max_ = 12.0;
  metric_sdk::HistogramPointData histogram_point_data2{};
  histogram_point_data2.boundaries_ = std::list<long>{10, 20, 30};
  histogram_point_data2.count_ = 3;
@@ -146,6 +148,8 @@ TEST(OStreamMetricsExporter, ExportHistogramPointData)
  "\n type : HistogramPointData"
  "\n count : 3"
  "\n sum : 900.5"
+ "\n min : 1.8"
+ "\n max : 12"
  "\n buckets : [10.1, 20.2, 30.2, ]"
  "\n counts : [200, 300, 400, 500, ]"
  "\n attributes\t\t: "
@@ -154,6 +158,8 @@ TEST(OStreamMetricsExporter, ExportHistogramPointData)
  "\n type : HistogramPointData"
  "\n count : 3"
  "\n sum : 900"
+ "\n min : 0"
+ "\n max : 0"
  "\n buckets : [10, 20, 30, ]"
  "\n counts : [200, 300, 400, 500, ]"
  "\n attributes\t\t: "

exporters/otlp/src/otlp_metric_utils.cc

@@ -106,6 +106,25 @@ void OtlpMetricUtils::ConvertHistogramMetric(
  }
  // count
  proto_histogram_point_data->set_count(histogram_data.count_);
+ if (histogram_data.record_min_max_)
+ {
+ if (nostd::holds_alternative<long>(histogram_data.min_))
+ {
+ proto_histogram_point_data->set_min(nostd::get<long>(histogram_data.min_));
+ }
+ else
+ {
+ proto_histogram_point_data->set_min(nostd::get<double>(histogram_data.min_));
+ }
+ if (nostd::holds_alternative<long>(histogram_data.max_))
+ {
+ proto_histogram_point_data->set_min(nostd::get<long>(histogram_data.max_));
+ }
+ else
+ {
+ proto_histogram_point_data->set_max(nostd::get<double>(histogram_data.max_));
+ }
+ }
  // buckets
  if ((nostd::holds_alternative<std::list<double>>(histogram_data.boundaries_)))
  {

exporters/otlp/test/otlp_http_metric_exporter_test.cc

@@ -502,6 +502,8 @@ class OtlpHttpMetricExporterTestPeer : public ::testing::Test
  histogram_point_data.count_ = 3;
  histogram_point_data.counts_ = {200, 300, 400, 500};
  histogram_point_data.sum_ = 900.5;
+ histogram_point_data.min_ = 1.8;
+ histogram_point_data.max_ = 19.0;
  opentelemetry::sdk::metrics::HistogramPointData histogram_point_data2{};
  histogram_point_data2.boundaries_ = std::list<long>{10, 20, 30};
  histogram_point_data2.count_ = 3;
@@ -551,6 +553,8 @@ class OtlpHttpMetricExporterTestPeer : public ::testing::Test
  auto data_points = metric["histogram"]["data_points"];
  EXPECT_EQ(3, JsonToInteger<int64_t>(data_points[0]["count"]));
  EXPECT_EQ(900.5, data_points[0]["sum"].get<double>());
+ EXPECT_EQ(1.8, data_points[0]["min"].get<double>());
+ EXPECT_EQ(19, data_points[0]["max"].get<double>());
  EXPECT_EQ(4, data_points[0]["bucket_counts"].size());
  if (4 == data_points[0]["bucket_counts"].size())
  {

sdk/include/opentelemetry/sdk/metrics/aggregation/aggregation_config.h

@@ -21,6 +21,7 @@ class HistogramAggregationConfig : public AggregationConfig
 {
 public:
  std::list<T> boundaries_;
+ bool record_min_max_ = true;
 };
 } // namespace metrics
 } // namespace sdk

sdk/include/opentelemetry/sdk/metrics/aggregation/histogram_aggregation.h

@@ -42,6 +42,7 @@ class LongHistogramAggregation : public Aggregation
 private:
  opentelemetry::common::SpinLockMutex lock_;
  HistogramPointData point_data_;
+ bool record_min_max_ = true;
 };
 
 class DoubleHistogramAggregation : public Aggregation
@@ -72,6 +73,7 @@ class DoubleHistogramAggregation : public Aggregation
 private:
  mutable opentelemetry::common::SpinLockMutex lock_;
  mutable HistogramPointData point_data_;
+ bool record_min_max_ = true;
 };
 
 template <class T>
@@ -83,9 +85,15 @@ void HistogramMerge(HistogramPointData &current,
  {
  merge.counts_[i] = current.counts_[i] + delta.counts_[i];
  }
- merge.boundaries_ = current.boundaries_;
- merge.sum_ = nostd::get<T>(current.sum_) + nostd::get<T>(delta.sum_);
- merge.count_ = current.count_ + delta.count_;
+ merge.boundaries_ = current.boundaries_;
+ merge.sum_ = nostd::get<T>(current.sum_) + nostd::get<T>(delta.sum_);
+ merge.count_ = current.count_ + delta.count_;
+ merge.record_min_max_ = current.record_min_max_ && delta.record_min_max_;
+ if (merge.record_min_max_)
+ {
+ merge.min_ = std::min(nostd::get<T>(current.min_), nostd::get<T>(delta.min_));
+ merge.max_ = std::max(nostd::get<T>(current.max_), nostd::get<T>(delta.max_));
+ }
 }
 
 template <class T>
@@ -95,8 +103,9 @@ void HistogramDiff(HistogramPointData &current, HistogramPointData &next, Histog
  {
  diff.counts_[i] = next.counts_[i] - current.counts_[i];
  }
- diff.boundaries_ = current.boundaries_;
- diff.count_ = next.count_ - current.count_;
+ diff.boundaries_ = current.boundaries_;
+ diff.count_ = next.count_ - current.count_;
+ diff.record_min_max_ = false;
 }
 
 } // namespace metrics

sdk/include/opentelemetry/sdk/metrics/data/point_data.h

@@ -58,8 +58,11 @@ class HistogramPointData
 
  ListType boundaries_ = {};
  ValueType sum_ = {};
+ ValueType min_ = {};
+ ValueType max_ = {};
  std::vector<uint64_t> counts_ = {};
  uint64_t count_ = {};
+ bool record_min_max_ = true;
 };
 
 class DropPointData

sdk/src/metrics/aggregation/histogram_aggregation.cc

@@ -3,6 +3,9 @@
 
 #ifndef ENABLE_METRICS_PREVIEW
 # include "opentelemetry/sdk/metrics/aggregation/histogram_aggregation.h"
+# include <algorithm>
+# include <iomanip>
+# include <limits>
 # include "opentelemetry/version.h"
 
 # include <mutex>
@@ -23,26 +26,38 @@ LongHistogramAggregation::LongHistogramAggregation(
  {
  point_data_.boundaries_ = std::list<long>{0l, 5l, 10l, 25l, 50l, 75l, 100l, 250l, 500l, 1000l};
  }
+ if (aggregation_config)
+ {
+ record_min_max_ = aggregation_config->record_min_max_;
+ }
  point_data_.counts_ =
  std::vector<uint64_t>(nostd::get<std::list<long>>(point_data_.boundaries_).size() + 1, 0);
- point_data_.sum_ = 0l;
- point_data_.count_ = 0;
+ point_data_.sum_ = 0l;
+ point_data_.count_ = 0;
+ point_data_.record_min_max_ = record_min_max_;
+ point_data_.min_ = std::numeric_limits<long>::max();
+ point_data_.max_ = std::numeric_limits<long>::min();
 }
 
 LongHistogramAggregation::LongHistogramAggregation(HistogramPointData &&data)
- : point_data_{std::move(data)}
+ : point_data_{std::move(data)}, record_min_max_{point_data_.record_min_max_}
 {}
 
 LongHistogramAggregation::LongHistogramAggregation(const HistogramPointData &data)
- : point_data_{data}
+ : point_data_{data}, record_min_max_{point_data_.record_min_max_}
 {}
 
 void LongHistogramAggregation::Aggregate(long value, const PointAttributes &attributes) noexcept
 {
  const std::lock_guard<opentelemetry::common::SpinLockMutex> locked(lock_);
  point_data_.count_ += 1;
  point_data_.sum_ = nostd::get<long>(point_data_.sum_) + value;
- size_t index = 0;
+ if (record_min_max_)
+ {
+ point_data_.min_ = std::min(nostd::get<long>(point_data_.min_), value);
+ point_data_.max_ = std::max(nostd::get<long>(point_data_.max_), value);
+ }
+ size_t index = 0;
  for (auto it = nostd::get<std::list<long>>(point_data_.boundaries_).begin();
  it != nostd::get<std::list<long>>(point_data_.boundaries_).end(); ++it)
  {
@@ -93,10 +108,17 @@ DoubleHistogramAggregation::DoubleHistogramAggregation(
  point_data_.boundaries_ =
  std::list<double>{0.0, 5.0, 10.0, 25.0, 50.0, 75.0, 100.0, 250.0, 500.0, 1000.0};
  }
+ if (aggregation_config)
+ {
+ record_min_max_ = aggregation_config->record_min_max_;
+ }
  point_data_.counts_ =
  std::vector<uint64_t>(nostd::get<std::list<double>>(point_data_.boundaries_).size() + 1, 0);
- point_data_.sum_ = 0.0;
- point_data_.count_ = 0;
+ point_data_.sum_ = 0.0;
+ point_data_.count_ = 0;
+ point_data_.record_min_max_ = record_min_max_;
+ point_data_.min_ = std::numeric_limits<double>::max();
+ point_data_.max_ = std::numeric_limits<double>::min();
 }
 
 DoubleHistogramAggregation::DoubleHistogramAggregation(HistogramPointData &&data)
@@ -112,7 +134,12 @@ void DoubleHistogramAggregation::Aggregate(double value, const PointAttributes &
  const std::lock_guard<opentelemetry::common::SpinLockMutex> locked(lock_);
  point_data_.count_ += 1;
  point_data_.sum_ = nostd::get<double>(point_data_.sum_) + value;
- size_t index = 0;
+ if (record_min_max_)
+ {
+ point_data_.min_ = std::min(nostd::get<double>(point_data_.min_), value);
+ point_data_.max_ = std::max(nostd::get<double>(point_data_.max_), value);
+ }
+ size_t index = 0;
  for (auto it = nostd::get<std::list<double>>(point_data_.boundaries_).begin();
  it != nostd::get<std::list<double>>(point_data_.boundaries_).end(); ++it)
  {

sdk/test/metrics/aggregation_test.cc

@@ -81,6 +81,8 @@ TEST(Aggregation, LongHistogramAggregation)
  EXPECT_NO_THROW(aggr.Aggregate(12l, {})); // lies in fourth bucket
  EXPECT_NO_THROW(aggr.Aggregate(100l, {})); // lies in eight bucket
  histogram_data = nostd::get<HistogramPointData>(aggr.ToPoint());
+ EXPECT_EQ(nostd::get<long>(histogram_data.min_), 12);
+ EXPECT_EQ(nostd::get<long>(histogram_data.max_), 100);
  EXPECT_EQ(nostd::get<long>(histogram_data.sum_), 112);
  EXPECT_EQ(histogram_data.count_, 2);
  EXPECT_EQ(histogram_data.counts_[3], 1);
@@ -91,6 +93,8 @@ TEST(Aggregation, LongHistogramAggregation)
  EXPECT_EQ(histogram_data.count_, 4);
  EXPECT_EQ(histogram_data.counts_[3], 2);
  EXPECT_EQ(histogram_data.counts_[8], 1);
+ EXPECT_EQ(nostd::get<long>(histogram_data.min_), 12);
+ EXPECT_EQ(nostd::get<long>(histogram_data.max_), 252);
 
  // Merge
  LongHistogramAggregation aggr1;
@@ -113,6 +117,8 @@ TEST(Aggregation, LongHistogramAggregation)
  EXPECT_EQ(histogram_data.counts_[3], 2); // 11, 13
  EXPECT_EQ(histogram_data.counts_[4], 2); // 25, 28
  EXPECT_EQ(histogram_data.counts_[7], 1); // 105
+ EXPECT_EQ(nostd::get<long>(histogram_data.min_), 1);
+ EXPECT_EQ(nostd::get<long>(histogram_data.max_), 105);
 
  // Diff
  auto aggr4 = aggr1.Diff(aggr2);
@@ -170,13 +176,17 @@ TEST(Aggregation, DoubleHistogramAggregation)
  EXPECT_EQ(histogram_data.count_, 2);
  EXPECT_EQ(histogram_data.counts_[3], 1);
  EXPECT_EQ(histogram_data.counts_[7], 1);
+ EXPECT_EQ(nostd::get<double>(histogram_data.min_), 12);
+ EXPECT_EQ(nostd::get<double>(histogram_data.max_), 100);
  EXPECT_NO_THROW(aggr.Aggregate(13.0, {})); // lies in fourth bucket
  EXPECT_NO_THROW(aggr.Aggregate(252.0, {})); // lies in ninth bucket
  histogram_data = nostd::get<HistogramPointData>(aggr.ToPoint());
  EXPECT_EQ(histogram_data.count_, 4);
  EXPECT_EQ(histogram_data.counts_[3], 2);
  EXPECT_EQ(histogram_data.counts_[8], 1);
  EXPECT_EQ(nostd::get<double>(histogram_data.sum_), 377);
+ EXPECT_EQ(nostd::get<double>(histogram_data.min_), 12);
+ EXPECT_EQ(nostd::get<double>(histogram_data.max_), 252);
 
  // Merge
  DoubleHistogramAggregation aggr1;
@@ -199,6 +209,8 @@ TEST(Aggregation, DoubleHistogramAggregation)
  EXPECT_EQ(histogram_data.counts_[3], 2); // 11.0, 13.0
  EXPECT_EQ(histogram_data.counts_[4], 2); // 25.1, 28.1
  EXPECT_EQ(histogram_data.counts_[7], 1); // 105.0
+ EXPECT_EQ(nostd::get<double>(histogram_data.min_), 1);
+ EXPECT_EQ(nostd::get<double>(histogram_data.max_), 105);
 
  // Diff
  auto aggr4 = aggr1.Diff(aggr2);

third_party_release

@@ -18,6 +18,6 @@ benchmark=v1.5.3
 googletest=release-1.10.0-459-ga6dfd3ac
 ms-gsl=v3.1.0-67-g6f45293
 nlohmann-json=v3.10.5
-opentelemetry-proto=v0.17.0
+opentelemetry-proto=v0.18.0
 prometheus-cpp=v1.0.0
 vcpkg=2022.04.12