From 1203bcf2647cfb8d10a217a0a04d43c6295bd12a Mon Sep 17 00:00:00 2001 From: Lalit Kumar Bhasin Date: Wed, 14 Aug 2024 10:47:59 -0700 Subject: [PATCH] [SDK] Support empty histogram buckets (#3027) * support empty buckets * Update histogram_test.cc * Update histogram_test.cc * test for negative values * fix count --- .../aggregation/histogram_aggregation.cc | 4 +- sdk/test/metrics/histogram_test.cc | 120 ++++++++++++++++++ 2 files changed, 122 insertions(+), 2 deletions(-) diff --git a/sdk/src/metrics/aggregation/histogram_aggregation.cc b/sdk/src/metrics/aggregation/histogram_aggregation.cc index da725cf09c..d6b4c90a0b 100644 --- a/sdk/src/metrics/aggregation/histogram_aggregation.cc +++ b/sdk/src/metrics/aggregation/histogram_aggregation.cc @@ -28,7 +28,7 @@ namespace metrics LongHistogramAggregation::LongHistogramAggregation(const AggregationConfig *aggregation_config) { auto ac = static_cast(aggregation_config); - if (ac && ac->boundaries_.size()) + if (ac) { point_data_.boundaries_ = ac->boundaries_; } @@ -109,7 +109,7 @@ PointType LongHistogramAggregation::ToPoint() const noexcept DoubleHistogramAggregation::DoubleHistogramAggregation(const AggregationConfig *aggregation_config) { auto ac = static_cast(aggregation_config); - if (ac && ac->boundaries_.size()) + if (ac) { point_data_.boundaries_ = ac->boundaries_; } diff --git a/sdk/test/metrics/histogram_test.cc b/sdk/test/metrics/histogram_test.cc index 4daceb685b..57f7790ba1 100644 --- a/sdk/test/metrics/histogram_test.cc +++ b/sdk/test/metrics/histogram_test.cc @@ -134,6 +134,66 @@ TEST(Histogram, DoubleCustomBuckets) ASSERT_EQ(std::vector({10, 20, 30, 40}), actual.boundaries_); ASSERT_EQ(std::vector({2, 2, 2, 2, 2}), actual.counts_); } + +TEST(Histogram, DoubleEmptyBuckets) +{ + MeterProvider mp; + auto m = mp.GetMeter("meter1", "version1", "schema1"); + std::string instrument_unit = "ms"; + std::string instrument_name = "historgram1"; + std::string instrument_desc = "histogram metrics"; + + std::unique_ptr exporter(new MockMetricExporter()); + std::shared_ptr reader{new MockMetricReader(std::move(exporter))}; + mp.AddMetricReader(reader); + + std::shared_ptr config(new HistogramAggregationConfig()); + config->boundaries_ = {}; + std::unique_ptr view{ + new View("view1", "view1_description", instrument_unit, AggregationType::kHistogram, config)}; + std::unique_ptr instrument_selector{ + new InstrumentSelector(InstrumentType::kHistogram, instrument_name, instrument_unit)}; + std::unique_ptr meter_selector{new MeterSelector("meter1", "version1", "schema1")}; + mp.AddView(std::move(instrument_selector), std::move(meter_selector), std::move(view)); + + auto h = m->CreateDoubleHistogram(instrument_name, instrument_desc, instrument_unit); + + h->Record(-5, {}); + h->Record(10, {}); + h->Record(15, {}); + h->Record(20, {}); + h->Record(25, {}); + h->Record(30, {}); + h->Record(35, {}); + h->Record(40, {}); + h->Record(45, {}); + h->Record(50, {}); + + std::vector actuals; + reader->Collect([&](ResourceMetrics &rm) { + for (const ScopeMetrics &smd : rm.scope_metric_data_) + { + for (const MetricData &md : smd.metric_data_) + { + for (const PointDataAttributes &dp : md.point_data_attr_) + { + actuals.push_back(opentelemetry::nostd::get(dp.point_data)); + } + } + } + return true; + }); + + ASSERT_EQ(1, actuals.size()); + + const auto &actual = actuals.at(0); + ASSERT_EQ(270.0, opentelemetry::nostd::get(actual.sum_)); + ASSERT_EQ(9, actual.count_); + ASSERT_EQ(10.0, opentelemetry::nostd::get(actual.min_)); + ASSERT_EQ(50.0, opentelemetry::nostd::get(actual.max_)); + ASSERT_EQ(std::vector({}), actual.boundaries_); + ASSERT_EQ(std::vector({9}), actual.counts_); +} #endif TEST(Histogram, UInt64) @@ -249,4 +309,64 @@ TEST(Histogram, UInt64CustomBuckets) ASSERT_EQ(std::vector({10, 20, 30, 40}), actual.boundaries_); ASSERT_EQ(std::vector({2, 2, 2, 2, 2}), actual.counts_); } + +TEST(Histogram, UInt64EmptyBuckets) +{ + MeterProvider mp; + auto m = mp.GetMeter("meter1", "version1", "schema1"); + std::string instrument_name = "historgram1"; + std::string instrument_desc = "histogram metrics"; + std::string instrument_unit = "ms"; + + std::unique_ptr exporter(new MockMetricExporter()); + std::shared_ptr reader{new MockMetricReader(std::move(exporter))}; + mp.AddMetricReader(reader); + + std::shared_ptr config(new HistogramAggregationConfig()); + config->boundaries_ = {}; + std::unique_ptr view{ + new View("view1", "view1_description", "ms", AggregationType::kHistogram, config)}; + std::unique_ptr instrument_selector{ + new InstrumentSelector(InstrumentType::kHistogram, instrument_name, instrument_unit)}; + std::unique_ptr meter_selector{new MeterSelector("meter1", "version1", "schema1")}; + mp.AddView(std::move(instrument_selector), std::move(meter_selector), std::move(view)); + + auto h = m->CreateUInt64Histogram(instrument_name, instrument_desc, instrument_unit); + + h->Record(5, {}); + h->Record(10, {}); + h->Record(15, {}); + h->Record(20, {}); + h->Record(25, {}); + h->Record(30, {}); + h->Record(35, {}); + h->Record(40, {}); + h->Record(45, {}); + h->Record(50, {}); + + std::vector actuals; + reader->Collect([&](ResourceMetrics &rm) { + for (const ScopeMetrics &smd : rm.scope_metric_data_) + { + for (const MetricData &md : smd.metric_data_) + { + for (const PointDataAttributes &dp : md.point_data_attr_) + { + actuals.push_back(opentelemetry::nostd::get(dp.point_data)); + } + } + } + return true; + }); + + ASSERT_EQ(1, actuals.size()); + + const auto &actual = actuals.at(0); + ASSERT_EQ(275, opentelemetry::nostd::get(actual.sum_)); + ASSERT_EQ(10, actual.count_); + ASSERT_EQ(5, opentelemetry::nostd::get(actual.min_)); + ASSERT_EQ(50, opentelemetry::nostd::get(actual.max_)); + ASSERT_EQ(std::vector({}), actual.boundaries_); + ASSERT_EQ(std::vector({10}), actual.counts_); +} #endif