From b62565f9b5d8aa2b6008ad6b3d2faa812bfbf190 Mon Sep 17 00:00:00 2001 From: Ruslan Nigmatullin Date: Fri, 4 Apr 2025 17:18:46 +0000 Subject: [PATCH 01/48] add base2 expo histo aggregation --- .../metrics/aggregation/aggregation_config.h | 9 +++ .../base2_exponential_histogram_aggregation.h | 56 +++++++++++++++++++ .../metrics/aggregation/default_aggregation.h | 8 +++ .../sdk/metrics/data/metric_data.h | 7 ++- .../sdk/metrics/data/point_data.h | 26 ++++++++- .../opentelemetry/sdk/metrics/instruments.h | 3 +- sdk/src/metrics/CMakeLists.txt | 1 + 7 files changed, 106 insertions(+), 4 deletions(-) create mode 100644 sdk/include/opentelemetry/sdk/metrics/aggregation/base2_exponential_histogram_aggregation.h diff --git a/sdk/include/opentelemetry/sdk/metrics/aggregation/aggregation_config.h b/sdk/include/opentelemetry/sdk/metrics/aggregation/aggregation_config.h index f5a48d7ddb..0cae37f39e 100644 --- a/sdk/include/opentelemetry/sdk/metrics/aggregation/aggregation_config.h +++ b/sdk/include/opentelemetry/sdk/metrics/aggregation/aggregation_config.h @@ -24,6 +24,15 @@ class HistogramAggregationConfig : public AggregationConfig std::vector boundaries_; bool record_min_max_ = true; }; + +class Base2ExponentialHistogramAggregationConfig : public AggregationConfig +{ +public: + size_t max_buckets_ = 160; + int32_t max_scale_ = 20; + bool record_min_max_ = true; +}; + } // namespace metrics } // namespace sdk OPENTELEMETRY_END_NAMESPACE diff --git a/sdk/include/opentelemetry/sdk/metrics/aggregation/base2_exponential_histogram_aggregation.h b/sdk/include/opentelemetry/sdk/metrics/aggregation/base2_exponential_histogram_aggregation.h new file mode 100644 index 0000000000..8a2cbbd4cc --- /dev/null +++ b/sdk/include/opentelemetry/sdk/metrics/aggregation/base2_exponential_histogram_aggregation.h @@ -0,0 +1,56 @@ +// Copyright The OpenTelemetry Authors +// SPDX-License-Identifier: Apache-2.0 + +#pragma once + +#include "opentelemetry/common/spin_lock_mutex.h" +#include "opentelemetry/sdk/metrics/aggregation/aggregation.h" +#include "opentelemetry/sdk/metrics/aggregation/aggregation_config.h" +#include "opentelemetry/sdk/metrics/aggregation/base2_exponential_histogram_indexer.h" + +#include +#include + +OPENTELEMETRY_BEGIN_NAMESPACE +namespace sdk +{ +namespace metrics +{ + +class Base2ExponentialHistogramAggregation : public Aggregation +{ +public: + Base2ExponentialHistogramAggregation(const AggregationConfig *aggregation_config = nullptr); + Base2ExponentialHistogramAggregation(const Base2ExponentialHistogramPointData &point_data); + Base2ExponentialHistogramAggregation(Base2ExponentialHistogramPointData &&point_data); + + + void Aggregate(int64_t value, const PointAttributes &attributes = {}) noexcept override; + void Aggregate(double value, const PointAttributes &attributes = {}) noexcept override; + + /* Returns the result of merge of the existing aggregation with delta + * aggregation with same boundaries */ + std::unique_ptr Merge(const Aggregation &delta) const noexcept override; + + /* Returns the new delta aggregation by comparing existing aggregation with + * next aggregation with same boundaries. Data points for `next` aggregation + * (sum , bucket-counts) should be more than the current aggregation - which + * is the normal scenario as measurements values are monotonic increasing. + */ + std::unique_ptr Diff(const Aggregation &next) const noexcept override; + + PointType ToPoint() const noexcept override; + +private: + void AggregateIntoBuckets(AdaptingCircularBufferCounter *buckets, double value) noexcept; + void Downscale(uint32_t by) noexcept; + + mutable opentelemetry::common::SpinLockMutex lock_; + Base2ExponentialHistogramPointData point_data_; + Base2ExponentialHistogramIndexer indexer_; + bool record_min_max_ = true; +}; + +} // namespace metrics +} // namespace sdk +OPENTELEMETRY_END_NAMESPACE diff --git a/sdk/include/opentelemetry/sdk/metrics/aggregation/default_aggregation.h b/sdk/include/opentelemetry/sdk/metrics/aggregation/default_aggregation.h index 9b1b1c2d7d..70f6f8ee74 100644 --- a/sdk/include/opentelemetry/sdk/metrics/aggregation/default_aggregation.h +++ b/sdk/include/opentelemetry/sdk/metrics/aggregation/default_aggregation.h @@ -6,6 +6,7 @@ #include #include "opentelemetry/sdk/metrics/aggregation/aggregation.h" +#include "opentelemetry/sdk/metrics/aggregation/base2_exponential_histogram_aggregation.h" #include "opentelemetry/sdk/metrics/aggregation/drop_aggregation.h" #include "opentelemetry/sdk/metrics/aggregation/histogram_aggregation.h" #include "opentelemetry/sdk/metrics/aggregation/lastvalue_aggregation.h" @@ -80,6 +81,10 @@ class DefaultAggregation return std::unique_ptr(new DoubleHistogramAggregation(aggregation_config)); } break; + case AggregationType::kBase2ExponentialHistogram: + return std::unique_ptr( + new Base2ExponentialHistogramAggregation(aggregation_config)); + break; case AggregationType::kLastValue: if (instrument_descriptor.value_type_ == InstrumentValueType::kLong) { @@ -139,6 +144,9 @@ class DefaultAggregation return std::unique_ptr( new DoubleHistogramAggregation(nostd::get(point_data))); } + case AggregationType::kBase2ExponentialHistogram: + return std::unique_ptr(new Base2ExponentialHistogramAggregation( + nostd::get(point_data))); case AggregationType::kLastValue: if (instrument_descriptor.value_type_ == InstrumentValueType::kLong) { diff --git a/sdk/include/opentelemetry/sdk/metrics/data/metric_data.h b/sdk/include/opentelemetry/sdk/metrics/data/metric_data.h index ad1084f581..e1da0b7f95 100644 --- a/sdk/include/opentelemetry/sdk/metrics/data/metric_data.h +++ b/sdk/include/opentelemetry/sdk/metrics/data/metric_data.h @@ -18,8 +18,11 @@ namespace metrics { using PointAttributes = opentelemetry::sdk::common::OrderedAttributeMap; -using PointType = opentelemetry::nostd:: - variant; +using PointType = opentelemetry::nostd::variant; struct PointDataAttributes { diff --git a/sdk/include/opentelemetry/sdk/metrics/data/point_data.h b/sdk/include/opentelemetry/sdk/metrics/data/point_data.h index 32853316a5..cc84a3af8d 100644 --- a/sdk/include/opentelemetry/sdk/metrics/data/point_data.h +++ b/sdk/include/opentelemetry/sdk/metrics/data/point_data.h @@ -7,6 +7,7 @@ #include "opentelemetry/common/timestamp.h" #include "opentelemetry/nostd/variant.h" +#include "opentelemetry/sdk/metrics/data/circular_buffer.h" #include "opentelemetry/version.h" OPENTELEMETRY_BEGIN_NAMESPACE @@ -17,7 +18,8 @@ namespace metrics using ValueType = nostd::variant; -// TODO: remove ctors and initializers from below classes when GCC<5 stops shipping on Ubuntu +// TODO: remove ctors and initializers from below classes when GCC<5 stops +// shipping on Ubuntu class SumPointData { @@ -64,6 +66,28 @@ class HistogramPointData bool record_min_max_ = true; }; +class Base2ExponentialHistogramPointData +{ +public: + // TODO: remove ctors and initializers when GCC<5 stops shipping on Ubuntu + Base2ExponentialHistogramPointData(Base2ExponentialHistogramPointData &&) = default; + Base2ExponentialHistogramPointData &operator=(Base2ExponentialHistogramPointData &&) = default; + Base2ExponentialHistogramPointData(const Base2ExponentialHistogramPointData &) = default; + Base2ExponentialHistogramPointData() = default; + + uint64_t count_ = {}; + double sum_ = {}; + int32_t scale_ = {}; + uint64_t zero_count_ = {}; + AdaptingCircularBufferCounter positive_buckets_{0}; + AdaptingCircularBufferCounter negative_buckets_{0}; + double min_ = {}; + double max_ = {}; + double zero_threshold_ = {}; + bool record_min_max_ = true; + size_t max_buckets_ = {}; +}; + class DropPointData { public: diff --git a/sdk/include/opentelemetry/sdk/metrics/instruments.h b/sdk/include/opentelemetry/sdk/metrics/instruments.h index 6473a267f2..9e3c843f5e 100644 --- a/sdk/include/opentelemetry/sdk/metrics/instruments.h +++ b/sdk/include/opentelemetry/sdk/metrics/instruments.h @@ -45,7 +45,8 @@ enum class AggregationType kHistogram, kLastValue, kSum, - kDefault + kDefault, + kBase2ExponentialHistogram }; enum class AggregationTemporality diff --git a/sdk/src/metrics/CMakeLists.txt b/sdk/src/metrics/CMakeLists.txt index f1ba16a116..e1c8b6e0e9 100644 --- a/sdk/src/metrics/CMakeLists.txt +++ b/sdk/src/metrics/CMakeLists.txt @@ -20,6 +20,7 @@ add_library( state/observable_registry.cc state/sync_metric_storage.cc state/temporal_metric_storage.cc + aggregation/base2_exponential_histogram_aggregation.cc aggregation/base2_exponential_histogram_indexer.cc aggregation/histogram_aggregation.cc aggregation/lastvalue_aggregation.cc From 338cce44683f6b99b53ee69636eb1d66b2813e95 Mon Sep 17 00:00:00 2001 From: etmcdonald Date: Fri, 4 Apr 2025 17:19:12 +0000 Subject: [PATCH 02/48] add base2 expo histo, test, benchmark --- ...base2_exponential_histogram_aggregation.cc | 312 ++++++++++++++++++ sdk/test/metrics/aggregation_test.cc | 94 ++++++ .../histogram_aggregation_benchmark.cc | 73 +++- 3 files changed, 474 insertions(+), 5 deletions(-) create mode 100644 sdk/src/metrics/aggregation/base2_exponential_histogram_aggregation.cc diff --git a/sdk/src/metrics/aggregation/base2_exponential_histogram_aggregation.cc b/sdk/src/metrics/aggregation/base2_exponential_histogram_aggregation.cc new file mode 100644 index 0000000000..41b63f10ea --- /dev/null +++ b/sdk/src/metrics/aggregation/base2_exponential_histogram_aggregation.cc @@ -0,0 +1,312 @@ +// Copyright The OpenTelemetry Authors +// SPDX-License-Identifier: Apache-2.0 + +#include "opentelemetry/sdk/metrics/aggregation/base2_exponential_histogram_aggregation.h" +#include "opentelemetry/sdk/metrics/aggregation/aggregation.h" +#include "opentelemetry/sdk/metrics/data/circular_buffer.h" +#include "opentelemetry/sdk/metrics/data/point_data.h" +#include "opentelemetry/version.h" + +#include +#include +#include +#include +#include +#include + +#include + +OPENTELEMETRY_BEGIN_NAMESPACE +namespace sdk +{ +namespace metrics +{ + +namespace +{ + +uint32_t GetScaleReduction(int32_t start_index, int32_t end_index, size_t max_buckets) noexcept +{ + uint32_t scale_reduction = 0; + while (static_cast(end_index - start_index + 1) > max_buckets) + { + start_index >>= 1; + end_index >>= 1; + scale_reduction++; + } + return scale_reduction; +} + +uint32_t GetScaleReduction(const AdaptingCircularBufferCounter &first, + const AdaptingCircularBufferCounter &second, + size_t max_buckets) +{ + if (first.Empty() || second.Empty()) + { + return 0; + } + + const int32_t start_index = std::min(first.StartIndex(), second.StartIndex()); + const int32_t end_index = std::max(first.EndIndex(), second.EndIndex()); + return GetScaleReduction(start_index, end_index, max_buckets); +} + +void DownscaleBuckets(AdaptingCircularBufferCounter *buckets, uint32_t by) noexcept +{ + if (buckets->Empty()) + { + return; + } + + // We want to preserve other optimisations here as well, e.g. integer size. + // Instead of creating a new counter, we copy the existing one (for bucket size + // optimisations), and clear the values before writing the new ones. + // TODO(euroelessar): Do downscaling in-place. + AdaptingCircularBufferCounter new_buckets = *buckets; + new_buckets.Clear(); + + for (int i = buckets->StartIndex(); i <= buckets->EndIndex(); i++) + { + const uint64_t count = buckets->Get(i); + if (count > 0) + { + new_buckets.Increment(i >> by, count); + } + } + *buckets = std::move(new_buckets); +} + +} // namespace + +Base2ExponentialHistogramAggregation::Base2ExponentialHistogramAggregation( + const AggregationConfig *aggregation_config) +{ + const Base2ExponentialHistogramAggregationConfig default_config; + auto ac = static_cast(aggregation_config); + if (!ac) + { + ac = &default_config; + } + + point_data_.max_buckets_ = ac->max_buckets_; + point_data_.scale_ = ac->max_scale_; + point_data_.record_min_max_ = ac->record_min_max_; + point_data_.min_ = std::numeric_limits::max(); + point_data_.max_ = std::numeric_limits::min(); + + indexer_ = Base2ExponentialHistogramIndexer(point_data_.scale_); +} + +Base2ExponentialHistogramAggregation::Base2ExponentialHistogramAggregation( + const Base2ExponentialHistogramPointData &point_data) + : point_data_{point_data}, indexer_(point_data.scale_), record_min_max_{point_data.record_min_max_} +{} + +Base2ExponentialHistogramAggregation::Base2ExponentialHistogramAggregation(Base2ExponentialHistogramPointData &&point_data) + : point_data_{std::move(point_data)}, indexer_(point_data_.scale_), record_min_max_{point_data_.record_min_max_} +{} + +void Base2ExponentialHistogramAggregation::Aggregate( + int64_t value, + const PointAttributes & /* attributes */) noexcept +{ + Aggregate(double(value)); +} + +void Base2ExponentialHistogramAggregation::Aggregate( + double value, + const PointAttributes & /* attributes */) noexcept +{ + const std::lock_guard locked(lock_); + point_data_.sum_ += value; + point_data_.count_++; + + if (record_min_max_) + { + point_data_.min_ = std::min(point_data_.min_, value); + point_data_.max_ = std::max(point_data_.max_, value); + } + + if (value == 0) + { + point_data_.zero_count_++; + return; + } + else if (value > 0) + { + AggregateIntoBuckets(&point_data_.positive_buckets_, value); + } + else + { + AggregateIntoBuckets(&point_data_.negative_buckets_, -value); + } +} + +void Base2ExponentialHistogramAggregation::AggregateIntoBuckets( + AdaptingCircularBufferCounter *buckets, + double value) noexcept +{ + if (buckets->MaxSize() == 0) + { + *buckets = AdaptingCircularBufferCounter{point_data_.max_buckets_}; + } + + const int32_t index = indexer_.ComputeIndex(value); + if (!buckets->Increment(index, 1)) + { + const int32_t start_index = std::min(buckets->StartIndex(), index); + const int32_t end_index = std::max(buckets->EndIndex(), index); + const uint32_t scale_reduction = + GetScaleReduction(start_index, end_index, point_data_.max_buckets_); + Downscale(scale_reduction); + + buckets->Increment(index >> scale_reduction, 1); + } +} + +void Base2ExponentialHistogramAggregation::Downscale(uint32_t by) noexcept +{ + if (by == 0) + { + return; + } + + DownscaleBuckets(&point_data_.positive_buckets_, by); + DownscaleBuckets(&point_data_.negative_buckets_, by); + + point_data_.scale_ -= by; + indexer_ = Base2ExponentialHistogramIndexer(point_data_.scale_); +} + +std::unique_ptr Base2ExponentialHistogramAggregation::Merge( + const Aggregation &delta) const noexcept +{ + auto left = nostd::get(ToPoint()); + auto right = nostd::get( + (static_cast(delta).ToPoint())); + + auto low_res = left.scale_ < right.scale_ ? left : right; + auto high_res = left.scale_ < right.scale_ ? right : left; + auto scale_reduction = high_res.scale_ - low_res.scale_; + + if (scale_reduction > 0) + { + DownscaleBuckets(&high_res.positive_buckets_, scale_reduction); + DownscaleBuckets(&high_res.negative_buckets_, scale_reduction); + high_res.scale_ -= scale_reduction; + } + + Base2ExponentialHistogramPointData result_value; + result_value.count_ = low_res.count_ + high_res.count_; + result_value.sum_ = low_res.sum_ + high_res.sum_; + result_value.zero_count_ = low_res.zero_count_ + high_res.zero_count_; + result_value.scale_ = std::min(low_res.scale_, high_res.scale_); + result_value.max_buckets_ = low_res.max_buckets_; + result_value.record_min_max_ = low_res.record_min_max_ && high_res.record_min_max_; + if (result_value.record_min_max_) + { + result_value.min_ = std::min(low_res.min_, high_res.min_); + result_value.max_ = std::max(low_res.max_, high_res.max_); + } + if (!high_res.positive_buckets_.Empty()) + { + for (int i = high_res.positive_buckets_.StartIndex(); + i <= high_res.positive_buckets_.EndIndex(); i++) + { + low_res.positive_buckets_.Increment(i, high_res.positive_buckets_.Get(i)); + } + } + result_value.positive_buckets_ = std::move(low_res.positive_buckets_); + + if (!high_res.negative_buckets_.Empty()) + { + for (int i = high_res.negative_buckets_.StartIndex(); + i <= high_res.negative_buckets_.EndIndex(); i++) + { + low_res.negative_buckets_.Increment(i, high_res.negative_buckets_.Get(i)); + } + } + result_value.negative_buckets_ = std::move(low_res.negative_buckets_); + + return std::unique_ptr{ + new Base2ExponentialHistogramAggregation(std::move(result_value))}; +} + +std::unique_ptr Base2ExponentialHistogramAggregation::Diff( + const Aggregation &next) const noexcept +{ + auto left = nostd::get(ToPoint()); + auto right = nostd::get( + (static_cast(next).ToPoint())); + + auto low_res = left.scale_ < right.scale_ ? left : right; + auto high_res = left.scale_ < right.scale_ ? right : left; + auto scale_reduction = high_res.scale_ - low_res.scale_; + + if (scale_reduction > 0) + { + DownscaleBuckets(&high_res.positive_buckets_, scale_reduction); + DownscaleBuckets(&high_res.negative_buckets_, scale_reduction); + high_res.scale_ -= scale_reduction; + } + + Base2ExponentialHistogramPointData result_value; + result_value.scale_ = low_res.scale_; + result_value.max_buckets_ = low_res.max_buckets_; + result_value.record_min_max_ = false; + // caution for underflow + result_value.count_ = (left.count_ >= right.count_) ? (left.count_ - right.count_) : 0; + result_value.sum_ = (left.sum_ >= right.sum_) ? (left.sum_ - right.sum_) : 0.0; + result_value.zero_count_ = (left.zero_count_ >= right.zero_count_) ? (left.zero_count_ - right.zero_count_) : 0; + if (!high_res.positive_buckets_.Empty()) + { + for (int i = high_res.positive_buckets_.StartIndex(); + i <= high_res.positive_buckets_.EndIndex(); i++) + { + low_res.positive_buckets_.Increment(i, 0-high_res.positive_buckets_.Get(i)); + } + } + result_value.positive_buckets_ = std::move(low_res.positive_buckets_); + + if (!high_res.negative_buckets_.Empty()) + { + for (int i = high_res.negative_buckets_.StartIndex(); + i <= high_res.negative_buckets_.EndIndex(); i++) + { + low_res.negative_buckets_.Increment(i, 0-high_res.negative_buckets_.Get(i)); + } + } + result_value.negative_buckets_ = std::move(low_res.negative_buckets_); + + return std::unique_ptr{ + new Base2ExponentialHistogramAggregation(std::move(result_value))}; +} + +// std::unique_ptr Base2ExponentialHistogramAggregation::Diff( +// const Aggregation &next) const noexcept +// { +// auto curr_value = nostd::get(ToPoint()); +// auto next_value = nostd::get( +// (static_cast(next).ToPoint())); + +// Base2ExponentialHistogramPointData result_value; +// result_value.scale_ = curr_value.scale_; +// result_value.max_buckets_ = curr_value.max_buckets_; +// result_value.record_min_max_ = false; +// result_value.count_ = next_value.count_ - curr_value.count_; +// result_value.sum_ = next_value.sum_ - curr_value.sum_; +// result_value.zero_count_ = next_value.zero_count_ - curr_value.zero_count_; + +// return std::unique_ptr{ +// new Base2ExponentialHistogramAggregation(std::move(result_value))}; +// } + +PointType Base2ExponentialHistogramAggregation::ToPoint() const noexcept +{ + const std::lock_guard locked(lock_); + return point_data_; +} + +} // namespace metrics +} // namespace sdk +OPENTELEMETRY_END_NAMESPACE diff --git a/sdk/test/metrics/aggregation_test.cc b/sdk/test/metrics/aggregation_test.cc index 92855863a8..f2721a8199 100644 --- a/sdk/test/metrics/aggregation_test.cc +++ b/sdk/test/metrics/aggregation_test.cc @@ -11,6 +11,7 @@ #include "opentelemetry/sdk/metrics/aggregation/aggregation.h" #include "opentelemetry/sdk/metrics/aggregation/aggregation_config.h" #include "opentelemetry/sdk/metrics/aggregation/histogram_aggregation.h" +#include "opentelemetry/sdk/metrics/aggregation/base2_exponential_histogram_aggregation.h" #include "opentelemetry/sdk/metrics/aggregation/lastvalue_aggregation.h" #include "opentelemetry/sdk/metrics/aggregation/sum_aggregation.h" #include "opentelemetry/sdk/metrics/data/point_data.h" @@ -223,3 +224,96 @@ TEST(Aggregation, DoubleHistogramAggregation) EXPECT_EQ(histogram_data.counts_[4], 0); // aggr2(28.1) - aggr1(25.1) EXPECT_EQ(histogram_data.counts_[7], 1); // aggr2(105.0) - aggr1(0) } + +TEST(aggregation, Base2ExponentialHistogramAggregation) +{ + // Low res histo + auto SCALE0 = 0; + auto MAX_BUCKETS0 = 7; + Base2ExponentialHistogramAggregationConfig scale0_config; + scale0_config.max_scale_ = SCALE0; + scale0_config.max_buckets_ = MAX_BUCKETS0; + scale0_config.record_min_max_ = true; + Base2ExponentialHistogramAggregation scale0_aggr(&scale0_config); + auto point = scale0_aggr.ToPoint(); + ASSERT_TRUE(nostd::holds_alternative(point)); + auto histo_point = nostd::get(point); + EXPECT_EQ(histo_point.count_, 0); + EXPECT_EQ(histo_point.sum_, 0.0); + EXPECT_EQ(histo_point.zero_count_, 0); + EXPECT_EQ(histo_point.min_, std::numeric_limits::max()); + EXPECT_EQ(histo_point.max_, std::numeric_limits::min()); + EXPECT_EQ(histo_point.scale_, SCALE0); + EXPECT_EQ(histo_point.max_buckets_, MAX_BUCKETS0); + ASSERT_TRUE(histo_point.positive_buckets_.Empty()); + + // zero point + scale0_aggr.Aggregate(0.0, {}); + histo_point = nostd::get(scale0_aggr.ToPoint()); + EXPECT_EQ(histo_point.count_, 1); + EXPECT_EQ(histo_point.zero_count_, 1); + + // Two recordings in the same bucket (bucket 1 at scale 0) + scale0_aggr.Aggregate(3.0, {}); + scale0_aggr.Aggregate(3.5, {}); + histo_point = nostd::get(scale0_aggr.ToPoint()); + EXPECT_EQ(histo_point.count_, 3); + EXPECT_EQ(histo_point.sum_, 6.5); + EXPECT_EQ(histo_point.min_, 0.0); + EXPECT_EQ(histo_point.max_, 3.5); + ASSERT_FALSE(histo_point.positive_buckets_.Empty()); + auto start_index = histo_point.positive_buckets_.StartIndex(); + auto end_index = histo_point.positive_buckets_.EndIndex(); + EXPECT_EQ(start_index, 1); + EXPECT_EQ(end_index, 1); + EXPECT_EQ(histo_point.positive_buckets_.Get(start_index), 2); + + // Recording in a different bucket (bucket -2 at scale 0) + scale0_aggr.Aggregate(-0.3, {}); + histo_point = nostd::get(scale0_aggr.ToPoint()); + EXPECT_EQ(histo_point.count_, 4); + EXPECT_EQ(histo_point.sum_, 6.2); + EXPECT_EQ(histo_point.min_, -0.3); + EXPECT_EQ(histo_point.max_, 3.5); + EXPECT_EQ(histo_point.negative_buckets_.Get(-2), 1); + EXPECT_EQ(histo_point.positive_buckets_.Get(1), 2); + + Base2ExponentialHistogramAggregationConfig scale1_config; + scale1_config.max_scale_ = 1; + scale1_config.max_buckets_ = 14; + scale1_config.record_min_max_ = true; + Base2ExponentialHistogramAggregation scale1_aggr(&scale1_config); + + scale1_aggr.Aggregate(0.0, {}); + scale1_aggr.Aggregate(3.0, {}); + scale1_aggr.Aggregate(3.5, {}); + scale1_aggr.Aggregate(0.3, {}); + auto scale1_point = nostd::get(scale1_aggr.ToPoint()); + EXPECT_EQ(scale1_point.count_, 4); + EXPECT_EQ(scale1_point.sum_, 6.8); + EXPECT_EQ(scale1_point.zero_count_, 1); + EXPECT_EQ(scale1_point.min_, 0.0); + EXPECT_EQ(scale1_point.max_, 3.5); + + auto merged = scale0_aggr.Merge(scale1_aggr); + auto merged_point = nostd::get(merged->ToPoint()); + EXPECT_EQ(merged_point.count_, 8); + EXPECT_EQ(merged_point.sum_, 13.0); + EXPECT_EQ(merged_point.zero_count_, 2); + EXPECT_EQ(merged_point.min_, -0.3); + EXPECT_EQ(merged_point.max_, 3.5); + EXPECT_EQ(merged_point.scale_, 0); + EXPECT_EQ(merged_point.positive_buckets_.Get(1), 4); + EXPECT_EQ(merged_point.negative_buckets_.Get(-2), 1); + EXPECT_EQ(merged_point.positive_buckets_.Get(2), 0); + + auto diffd = merged->Diff(scale1_aggr); + auto diffd_point = nostd::get(diffd->ToPoint()); + EXPECT_EQ(diffd_point.count_, 4); + EXPECT_EQ(diffd_point.sum_, 6.2); + EXPECT_EQ(diffd_point.zero_count_, 1); + EXPECT_EQ(diffd_point.scale_, 0); + EXPECT_EQ(diffd_point.positive_buckets_.Get(1), 2); + EXPECT_EQ(diffd_point.negative_buckets_.Get(-2), 1); + EXPECT_EQ(diffd_point.positive_buckets_.Get(2), 0); +} \ No newline at end of file diff --git a/sdk/test/metrics/histogram_aggregation_benchmark.cc b/sdk/test/metrics/histogram_aggregation_benchmark.cc index b66538042c..262fb5a99e 100644 --- a/sdk/test/metrics/histogram_aggregation_benchmark.cc +++ b/sdk/test/metrics/histogram_aggregation_benchmark.cc @@ -25,6 +25,8 @@ #include "opentelemetry/sdk/metrics/meter_provider.h" #include "opentelemetry/sdk/metrics/metric_reader.h" #include "opentelemetry/sdk/metrics/push_metric_exporter.h" +#include "opentelemetry/sdk/metrics/view/instrument_selector.h" +#include "opentelemetry/sdk/metrics/view/view_registry.h" using namespace opentelemetry; using namespace opentelemetry::sdk::instrumentationscope; @@ -32,9 +34,11 @@ using namespace opentelemetry::sdk::metrics; namespace { -void BM_HistogramAggregation(benchmark::State &state) + +template +void HistogramAggregation(benchmark::State &state, std::unique_ptr views) { - MeterProvider mp; + MeterProvider mp(std::move(views)); auto m = mp.GetMeter("meter1", "version1", "schema1"); std::unique_ptr exporter(new MockMetricExporter()); @@ -50,7 +54,7 @@ void BM_HistogramAggregation(benchmark::State &state) { measurements[i] = static_cast(distribution(generator)); } - std::vector actuals; + std::vector actuals; std::vector collectionThreads; std::function collectMetrics = [&reader, &actuals]() { reader->Collect([&](ResourceMetrics &rm) { @@ -60,7 +64,7 @@ void BM_HistogramAggregation(benchmark::State &state) { for (const PointDataAttributes &dp : md.point_data_attr_) { - actuals.push_back(opentelemetry::nostd::get(dp.point_data)); + actuals.push_back(opentelemetry::nostd::get(dp.point_data)); } } } @@ -68,7 +72,7 @@ void BM_HistogramAggregation(benchmark::State &state) }); }; - while (state.KeepRunning()) + while (state.KeepRunningBatch(TOTAL_MEASUREMENTS)) { for (size_t i = 0; i < TOTAL_MEASUREMENTS; i++) { @@ -85,7 +89,66 @@ void BM_HistogramAggregation(benchmark::State &state) } } +void BM_HistogramAggregation(benchmark::State &state) +{ + std::unique_ptr views{new ViewRegistry()}; + HistogramAggregation(state, std::move(views)); +} + BENCHMARK(BM_HistogramAggregation); +// Add this helper function before your benchmark functions + +void RunBase2ExponentialHistogramAggregation(benchmark::State &state, int scale) { + std::string instrument_unit = "histogram1_unit"; + std::unique_ptr histogram_instrument_selector{ + new InstrumentSelector(InstrumentType::kHistogram, ".*", instrument_unit)}; + std::unique_ptr histogram_meter_selector{ + new MeterSelector("meter1", "version1", "schema1")}; + + Base2ExponentialHistogramAggregationConfig config; + config.max_scale_ = scale; + + std::unique_ptr histogram_view{ + new View("base2_expohisto", "description", instrument_unit, AggregationType::kBase2ExponentialHistogram, + std::make_shared(config))}; + + std::unique_ptr views{new ViewRegistry()}; + views->AddView(std::move(histogram_instrument_selector), std::move(histogram_meter_selector), + std::move(histogram_view)); + + HistogramAggregation(state, std::move(views)); +} + +void BM_Base2ExponentialHistogramAggregationZeroScale(benchmark::State &state) { + RunBase2ExponentialHistogramAggregation(state, 0); +} +BENCHMARK(BM_Base2ExponentialHistogramAggregationZeroScale); + +void BM_Base2ExponentialHistogramAggregationOneScale(benchmark::State &state) { + RunBase2ExponentialHistogramAggregation(state, 1); +} +BENCHMARK(BM_Base2ExponentialHistogramAggregationOneScale); + +void BM_Base2ExponentialHistogramAggregationTwoScale(benchmark::State &state) { + RunBase2ExponentialHistogramAggregation(state, 2); +} +BENCHMARK(BM_Base2ExponentialHistogramAggregationTwoScale); + +void BM_Base2ExponentialHistogramAggregationFourScale(benchmark::State &state) { + RunBase2ExponentialHistogramAggregation(state, 4); +} +BENCHMARK(BM_Base2ExponentialHistogramAggregationFourScale); + +void BM_Base2ExponentialHistogramAggregationEightScale(benchmark::State &state) { + RunBase2ExponentialHistogramAggregation(state, 8); +} +BENCHMARK(BM_Base2ExponentialHistogramAggregationEightScale); + +void BM_Base2ExponentialHistogramAggregationSixteenScale(benchmark::State &state) { + RunBase2ExponentialHistogramAggregation(state, 16); +} +BENCHMARK(BM_Base2ExponentialHistogramAggregationSixteenScale); + } // namespace BENCHMARK_MAIN(); From b03d915081f30e2e031af888aa4858a27f8d78ac Mon Sep 17 00:00:00 2001 From: "Felipe C. Dos Santos" Date: Fri, 4 Apr 2025 17:19:52 +0000 Subject: [PATCH 03/48] add prom, otlp exporter and example --- .../common/metrics_foo_library/foo_library.cc | 21 ++++- .../common/metrics_foo_library/foo_library.h | 1 + examples/otlp/grpc_metric_main.cc | 82 +++++++++++++++++-- .../exporters/otlp/otlp_metric_utils.h | 3 + exporters/otlp/src/otlp_metric_utils.cc | 72 ++++++++++++++++ exporters/prometheus/src/exporter_utils.cc | 4 + 6 files changed, 177 insertions(+), 6 deletions(-) diff --git a/examples/common/metrics_foo_library/foo_library.cc b/examples/common/metrics_foo_library/foo_library.cc index 81cb840718..00f1a398ff 100644 --- a/examples/common/metrics_foo_library/foo_library.cc +++ b/examples/common/metrics_foo_library/foo_library.cc @@ -7,6 +7,7 @@ #include #include #include +#include #include #include @@ -24,6 +25,7 @@ #include "opentelemetry/semconv/http_metrics.h" #include "opentelemetry/semconv/incubating/container_metrics.h" #include "opentelemetry/semconv/incubating/system_metrics.h" +#include "opentelemetry/sdk/metrics/view/view_factory.h" namespace metrics_api = opentelemetry::metrics; @@ -100,7 +102,24 @@ void foo_library::histogram_example(const std::string &name) std::string histogram_name = name + "_histogram"; auto provider = metrics_api::Provider::GetMeterProvider(); opentelemetry::nostd::shared_ptr meter = provider->GetMeter(name, "1.2.0"); - auto histogram_counter = meter->CreateDoubleHistogram(histogram_name, "des", "unit"); + auto histogram_counter = meter->CreateDoubleHistogram(histogram_name, "des", "histogram-unit"); + auto context = opentelemetry::context::Context{}; + for (uint32_t i = 0; i < 20; ++i) + { + double val = (rand() % 700) + 1.1; + std::map labels = get_random_attr(); + auto labelkv = opentelemetry::common::KeyValueIterableView{labels}; + histogram_counter->Record(val, labelkv, context); + std::this_thread::sleep_for(std::chrono::milliseconds(250)); + } +} + +void foo_library::histogram_exp_example(const std::string &name) +{ + std::string histogram_name = name + "_exponential_histogram"; + auto provider = metrics_api::Provider::GetMeterProvider(); + opentelemetry::nostd::shared_ptr meter = provider->GetMeter(name, "1.2.0"); + auto histogram_counter = meter->CreateDoubleHistogram(histogram_name, "des", "histogram-unit"); auto context = opentelemetry::context::Context{}; for (uint32_t i = 0; i < 20; ++i) { diff --git a/examples/common/metrics_foo_library/foo_library.h b/examples/common/metrics_foo_library/foo_library.h index 65997398a5..d157a78f28 100644 --- a/examples/common/metrics_foo_library/foo_library.h +++ b/examples/common/metrics_foo_library/foo_library.h @@ -10,6 +10,7 @@ class foo_library public: static void counter_example(const std::string &name); static void histogram_example(const std::string &name); + static void histogram_exp_example(const std::string &name); static void observable_counter_example(const std::string &name); #if OPENTELEMETRY_ABI_VERSION_NO >= 2 static void gauge_example(const std::string &name); diff --git a/examples/otlp/grpc_metric_main.cc b/examples/otlp/grpc_metric_main.cc index 7b6550edba..4ac40e53bb 100644 --- a/examples/otlp/grpc_metric_main.cc +++ b/examples/otlp/grpc_metric_main.cc @@ -1,6 +1,9 @@ // Copyright The OpenTelemetry Authors // SPDX-License-Identifier: Apache-2.0 +#include "grpcpp/grpcpp.h" +#include "opentelemetry/exporters/otlp/otlp_grpc_exporter.h" + #include "opentelemetry/exporters/otlp/otlp_grpc_metric_exporter_factory.h" #include "opentelemetry/metrics/provider.h" #include "opentelemetry/sdk/metrics/aggregation/default_aggregation.h" @@ -11,6 +14,9 @@ #include "opentelemetry/sdk/metrics/meter_provider.h" #include "opentelemetry/sdk/metrics/meter_provider_factory.h" #include "opentelemetry/sdk/metrics/provider.h" +#include "opentelemetry/sdk/metrics/view/instrument_selector_factory.h" +#include "opentelemetry/sdk/metrics/view/view_factory.h" +#include "opentelemetry/sdk/metrics/view/meter_selector_factory.h" #include #include @@ -55,6 +61,61 @@ void InitMetrics() metric_sdk::Provider::SetMeterProvider(provider); } +void InitMetrics(std::string &name) +{ + auto exporter = otlp_exporter::OtlpGrpcMetricExporterFactory::Create(exporter_options); + + std::string version{"1.2.0"}; + std::string schema{"https://opentelemetry.io/schemas/1.2.0"}; + + // Initialize and set the global MeterProvider + metric_sdk::PeriodicExportingMetricReaderOptions reader_options; + reader_options.export_interval_millis = std::chrono::milliseconds(1000); + reader_options.export_timeout_millis = std::chrono::milliseconds(500); + + auto reader = + metric_sdk::PeriodicExportingMetricReaderFactory::Create(std::move(exporter), reader_options); + + auto context = metric_sdk::MeterContextFactory::Create(); + context->AddMetricReader(std::move(reader)); + + auto provider = metric_sdk::MeterProviderFactory::Create(std::move(context)); + + // auto provider = opentelemetry::sdk::metrics::MeterProviderFactory::Create(); + + // std::shared_ptr provider(std::move(u_provider)); + + // histogram view + std::string histogram_name = name + "_histogram"; + std::string unit = "unit"; + + auto histogram_instrument_selector = metric_sdk::InstrumentSelectorFactory::Create( + metric_sdk::InstrumentType::kHistogram, histogram_name, unit); + + auto histogram_meter_selector = metric_sdk::MeterSelectorFactory::Create(name, version, schema); + + auto histogram_aggregation_config = std::unique_ptr( + new metric_sdk::Base2ExponentialHistogramAggregationConfig); + + histogram_aggregation_config->max_scale_ = 3; + + // histogram_aggregation_config->boundaries_ = std::vector{ + // 0.0, 50.0, 100.0, 250.0, 500.0, 750.0, 1000.0, 2500.0, 5000.0, 10000.0, 20000.0}; + + std::shared_ptr aggregation_config( + std::move(histogram_aggregation_config)); + + auto histogram_view = metric_sdk::ViewFactory::Create( + name, "description", unit, metric_sdk::AggregationType::kBase2ExponentialHistogram, aggregation_config); + + provider->AddView(std::move(histogram_instrument_selector), std::move(histogram_meter_selector), + std::move(histogram_view)); + + std::shared_ptr api_provider(std::move(provider)); + + metric_sdk::Provider::SetMeterProvider(api_provider); +} + void CleanupMetrics() { std::shared_ptr none; @@ -78,10 +139,19 @@ int main(int argc, char *argv[]) } } } + std::cout << "Using endpoint: " << exporter_options.endpoint << std::endl; + std::cout << "Using example type: " << example_type << std::endl; + std::cout << "Using cacert path: " << exporter_options.ssl_credentials_cacert_path << std::endl; + std::cout << "Using ssl credentials: " << exporter_options.use_ssl_credentials << std::endl; + // Removing this line will leave the default noop MetricProvider in place. - InitMetrics(); + std::string name{"otlp_grpc_metric_example"}; + InitMetrics(name); + + //InitMetrics(); + if (example_type == "counter") { foo_library::counter_example(name); @@ -93,6 +163,8 @@ int main(int argc, char *argv[]) else if (example_type == "histogram") { foo_library::histogram_example(name); + } else if (example_type == "exponential_histogram") { + foo_library::histogram_exp_example(name); } #if OPENTELEMETRY_ABI_VERSION_NO >= 2 else if (example_type == "gauge") @@ -102,15 +174,15 @@ int main(int argc, char *argv[]) #endif else { - std::thread counter_example{&foo_library::counter_example, name}; - std::thread observable_counter_example{&foo_library::observable_counter_example, name}; + //std::thread counter_example{&foo_library::counter_example, name}; + // std::thread observable_counter_example{&foo_library::observable_counter_example, name}; std::thread histogram_example{&foo_library::histogram_example, name}; #if OPENTELEMETRY_ABI_VERSION_NO >= 2 std::thread gauge_example{&foo_library::gauge_example, name}; #endif - counter_example.join(); - observable_counter_example.join(); + // counter_example.join(); + // observable_counter_example.join(); histogram_example.join(); #if OPENTELEMETRY_ABI_VERSION_NO >= 2 gauge_example.join(); diff --git a/exporters/otlp/include/opentelemetry/exporters/otlp/otlp_metric_utils.h b/exporters/otlp/include/opentelemetry/exporters/otlp/otlp_metric_utils.h index 4f92b8e665..9907f66ce8 100644 --- a/exporters/otlp/include/opentelemetry/exporters/otlp/otlp_metric_utils.h +++ b/exporters/otlp/include/opentelemetry/exporters/otlp/otlp_metric_utils.h @@ -55,6 +55,9 @@ class OtlpMetricUtils static void ConvertHistogramMetric(const opentelemetry::sdk::metrics::MetricData &metric_data, proto::metrics::v1::Histogram *const histogram) noexcept; + static void ConvertExponentialHistogramMetric(const opentelemetry::sdk::metrics::MetricData &metric_data, + proto::metrics::v1::ExponentialHistogram *const histogram) noexcept; + static void ConvertGaugeMetric(const opentelemetry::sdk::metrics::MetricData &metric_data, proto::metrics::v1::Gauge *const gauge) noexcept; diff --git a/exporters/otlp/src/otlp_metric_utils.cc b/exporters/otlp/src/otlp_metric_utils.cc index 65f86a4e47..d6b8aa3017 100644 --- a/exporters/otlp/src/otlp_metric_utils.cc +++ b/exporters/otlp/src/otlp_metric_utils.cc @@ -63,6 +63,11 @@ metric_sdk::AggregationType OtlpMetricUtils::GetAggregationType( { return metric_sdk::AggregationType::kHistogram; } + else if (nostd::holds_alternative( + point_data_with_attributes.point_data)) + { + return metric_sdk::AggregationType::kBase2ExponentialHistogram; + } else if (nostd::holds_alternative( point_data_with_attributes.point_data)) { @@ -177,6 +182,69 @@ void OtlpMetricUtils::ConvertHistogramMetric( } } +void OtlpMetricUtils::ConvertExponentialHistogramMetric( + const metric_sdk::MetricData &metric_data, + proto::metrics::v1::ExponentialHistogram *const histogram) noexcept +{ + histogram->set_aggregation_temporality( + GetProtoAggregationTemporality(metric_data.aggregation_temporality)); + auto start_ts = metric_data.start_ts.time_since_epoch().count(); + auto ts = metric_data.end_ts.time_since_epoch().count(); + for (auto &point_data_with_attributes : metric_data.point_data_attr_) + { + proto::metrics::v1::ExponentialHistogramDataPoint *proto_histogram_point_data = + histogram->add_data_points(); + proto_histogram_point_data->set_start_time_unix_nano(start_ts); + proto_histogram_point_data->set_time_unix_nano(ts); + auto histogram_data = + nostd::get(point_data_with_attributes.point_data); + // sum + proto_histogram_point_data->set_sum(histogram_data.sum_); + proto_histogram_point_data->set_count(histogram_data.count_); + if (histogram_data.record_min_max_) + { + proto_histogram_point_data->set_min(histogram_data.min_); + proto_histogram_point_data->set_max(histogram_data.max_); + } + // negative buckets + if(!histogram_data.negative_buckets_.Empty()) + { + auto negative_buckets = proto_histogram_point_data->mutable_negative(); + negative_buckets->set_offset(histogram_data.negative_buckets_.StartIndex()); + + for( auto index = histogram_data.negative_buckets_.StartIndex(); + index <= histogram_data.negative_buckets_.EndIndex(); ++index) + { + negative_buckets->add_bucket_counts(histogram_data.negative_buckets_.Get(index)); + } + } + // positive buckets + if(!histogram_data.positive_buckets_.Empty()) + { + auto positive_buckets = proto_histogram_point_data->mutable_positive(); + positive_buckets->set_offset(histogram_data.positive_buckets_.StartIndex()); + + for( auto index = histogram_data.positive_buckets_.StartIndex(); + index <= histogram_data.positive_buckets_.EndIndex(); ++index) + { + positive_buckets->add_bucket_counts(histogram_data.positive_buckets_.Get(index)); + } + } + proto_histogram_point_data->set_scale(histogram_data.scale_); + proto_histogram_point_data->set_zero_count(histogram_data.zero_count_); + + + + // attributes + for (auto &kv_attr : point_data_with_attributes.attributes) + { + OtlpPopulateAttributeUtils::PopulateAttribute(proto_histogram_point_data->add_attributes(), + kv_attr.first, kv_attr.second); + } + } +} + + void OtlpMetricUtils::ConvertGaugeMetric(const opentelemetry::sdk::metrics::MetricData &metric_data, proto::metrics::v1::Gauge *const gauge) noexcept { @@ -225,6 +293,10 @@ void OtlpMetricUtils::PopulateInstrumentInfoMetrics( ConvertHistogramMetric(metric_data, metric->mutable_histogram()); break; } + case metric_sdk::AggregationType::kBase2ExponentialHistogram: { + ConvertExponentialHistogramMetric(metric_data, metric->mutable_exponential_histogram()); + break; + } case metric_sdk::AggregationType::kLastValue: { ConvertGaugeMetric(metric_data, metric->mutable_gauge()); break; diff --git a/exporters/prometheus/src/exporter_utils.cc b/exporters/prometheus/src/exporter_utils.cc index c874faa654..7055e0d86c 100644 --- a/exporters/prometheus/src/exporter_utils.cc +++ b/exporters/prometheus/src/exporter_utils.cc @@ -562,6 +562,10 @@ metric_sdk::AggregationType PrometheusExporterUtils::getAggregationType( { return metric_sdk::AggregationType::kHistogram; } + else if (nostd::holds_alternative(point_type)) + { + return metric_sdk::AggregationType::kBase2ExponentialHistogram; + } else if (nostd::holds_alternative(point_type)) { return metric_sdk::AggregationType::kLastValue; From 423e92dc908546bf59b242f53a39b9f51ebeb20e Mon Sep 17 00:00:00 2001 From: etmcdonald Date: Fri, 4 Apr 2025 17:20:06 +0000 Subject: [PATCH 04/48] add ostream exporter and example --- examples/metrics_simple/metrics_ostream.cc | 35 ++++++++++++++++++++-- exporters/ostream/src/metric_exporter.cc | 31 +++++++++++++++++++ 2 files changed, 63 insertions(+), 3 deletions(-) diff --git a/examples/metrics_simple/metrics_ostream.cc b/examples/metrics_simple/metrics_ostream.cc index 1d85ff1692..6f05560280 100644 --- a/examples/metrics_simple/metrics_ostream.cc +++ b/examples/metrics_simple/metrics_ostream.cc @@ -17,6 +17,7 @@ #include "opentelemetry/sdk/metrics/instruments.h" #include "opentelemetry/sdk/metrics/meter_provider.h" #include "opentelemetry/sdk/metrics/meter_provider_factory.h" +#include "opentelemetry/sdk/metrics/meter_context_factory.h" #include "opentelemetry/sdk/metrics/metric_reader.h" #include "opentelemetry/sdk/metrics/provider.h" #include "opentelemetry/sdk/metrics/push_metric_exporter.h" @@ -56,9 +57,9 @@ void InitMetrics(const std::string &name) auto reader = metrics_sdk::PeriodicExportingMetricReaderFactory::Create(std::move(exporter), options); - auto provider = opentelemetry::sdk::metrics::MeterProviderFactory::Create(); - - provider->AddMetricReader(std::move(reader)); + auto context = metrics_sdk::MeterContextFactory::Create(); + context->AddMetricReader(std::move(reader)); + auto provider = opentelemetry::sdk::metrics::MeterProviderFactory::Create(std::move(context)); // counter view std::string counter_name = name + "_counter"; @@ -112,6 +113,28 @@ void InitMetrics(const std::string &name) provider->AddView(std::move(histogram_instrument_selector), std::move(histogram_meter_selector), std::move(histogram_view)); + // hisogram view with base2 exponential aggregation + std::string histogram_base2_name = name + "_exponential_histogram"; + unit = "histogram-unit"; + auto histogram_base2_instrument_selector = metrics_sdk::InstrumentSelectorFactory::Create( + metrics_sdk::InstrumentType::kHistogram, histogram_base2_name, unit); + auto histogram_base2_meter_selector = metrics_sdk::MeterSelectorFactory::Create(name, version, + schema); + auto histogram_base2_aggregation_config = std::unique_ptr( + new metrics_sdk::Base2ExponentialHistogramAggregationConfig); + histogram_base2_aggregation_config->max_scale_ = 3; + histogram_base2_aggregation_config->record_min_max_ = true; + histogram_base2_aggregation_config->max_buckets_ = 100; + + std::shared_ptr base2_aggregation_config( + std::move(histogram_base2_aggregation_config)); + + auto histogram_base2_view = metrics_sdk::ViewFactory::Create( + name, "description", unit, metrics_sdk::AggregationType::kBase2ExponentialHistogram, + base2_aggregation_config); + + provider->AddView(std::move(histogram_base2_instrument_selector), std::move(histogram_base2_meter_selector), std::move(histogram_base2_view)); + std::shared_ptr api_provider(std::move(provider)); metrics_sdk::Provider::SetMeterProvider(api_provider); @@ -147,6 +170,10 @@ int main(int argc, char **argv) { foo_library::histogram_example(name); } + else if (example_type == "exponential_histogram") + { + foo_library::histogram_exp_example(name); + } #if OPENTELEMETRY_ABI_VERSION_NO >= 2 else if (example_type == "gauge") { @@ -170,6 +197,7 @@ int main(int argc, char **argv) std::thread counter_example{&foo_library::counter_example, name}; std::thread observable_counter_example{&foo_library::observable_counter_example, name}; std::thread histogram_example{&foo_library::histogram_example, name}; + std::thread histogram_exp_example{&foo_library::histogram_exp_example, name}; #if OPENTELEMETRY_ABI_VERSION_NO >= 2 std::thread gauge_example{&foo_library::gauge_example, name}; #endif @@ -181,6 +209,7 @@ int main(int argc, char **argv) counter_example.join(); observable_counter_example.join(); histogram_example.join(); + histogram_exp_example.join(); #if OPENTELEMETRY_ABI_VERSION_NO >= 2 gauge_example.join(); #endif diff --git a/exporters/ostream/src/metric_exporter.cc b/exporters/ostream/src/metric_exporter.cc index 4bdd122492..084286329f 100644 --- a/exporters/ostream/src/metric_exporter.cc +++ b/exporters/ostream/src/metric_exporter.cc @@ -247,6 +247,37 @@ void OStreamMetricExporter::printPointData(const opentelemetry::sdk::metrics::Po sout_ << nostd::get(last_point_data.value_); } } + else if (nostd::holds_alternative(point_data)) + { + auto histogram_point_data = + nostd::get(point_data); + sout_ << "\n type: Base2ExponentialHistogramPointData"; + sout_ << "\n count: " << histogram_point_data.count_; + sout_ << "\n sum: " << histogram_point_data.sum_; + sout_ << "\n zero_count: " << histogram_point_data.zero_count_; + if (histogram_point_data.record_min_max_) + { + sout_ << "\n min: " << histogram_point_data.min_; + sout_ << "\n max: " << histogram_point_data.max_; + } + sout_ << "\n scale: " << histogram_point_data.scale_; + sout_ << "\n positive buckets: "; + if (!histogram_point_data.positive_buckets_.Empty()) + { + for (auto i = histogram_point_data.positive_buckets_.StartIndex(); i <= histogram_point_data.positive_buckets_.EndIndex(); ++i) + { + sout_ << "\n\t" << i << ": " << histogram_point_data.positive_buckets_.Get(i); + } + } + sout_ << "\n negative buckets: "; + if (!histogram_point_data.negative_buckets_.Empty()) + { + for (auto i = histogram_point_data.negative_buckets_.StartIndex(); i <= histogram_point_data.negative_buckets_.EndIndex(); ++i) + { + sout_ << "\n\t" << i << ": " << histogram_point_data.negative_buckets_.Get(i); + } + } + } } void OStreamMetricExporter::printPointAttributes( From a9737bf86b4dbc47323ab4153ca2371dbb96e475 Mon Sep 17 00:00:00 2001 From: "Felipe C. Dos Santos" Date: Fri, 4 Apr 2025 17:20:23 +0000 Subject: [PATCH 05/48] update vscode launch --- .vscode/launch.json | 47 +++++++++++++++++++++++++++------------------ 1 file changed, 28 insertions(+), 19 deletions(-) diff --git a/.vscode/launch.json b/.vscode/launch.json index 3532ca4d2a..c7ad6761dd 100644 --- a/.vscode/launch.json +++ b/.vscode/launch.json @@ -1,24 +1,33 @@ { "version": "0.2.0", "configurations": [ - { - "name": "Debug on Windows", - "type": "cppvsdbg", - "request": "launch", - "program": "${workspaceFolder}/build/", - "args": [], - "stopAtEntry": false, - "cwd": "${workspaceFolder}", - "environment": [], - "externalConsole": false - }, - { - "name": "Debug on Linux", - "type": "gdb", - "request": "launch", - "target": "${workspaceFolder}/bazel-bin/", - "cwd": "${workspaceRoot}", - "valuesFormatting": "parseText" - } + { + "name": "(ctest) Launch", + "type": "cppdbg", + "cwd": "${cmake.testWorkingDirectory}", + "request": "launch", + "program": "${cmake.testProgram}", + "args": [ "${cmake.testArgs}" ], + // other options... + }, + { + "name": "Debug on Windows", + "type": "cppvsdbg", + "request": "launch", + "program": "${workspaceFolder}/build/", + "args": [], + "stopAtEntry": false, + "cwd": "${workspaceFolder}", + "environment": [], + "externalConsole": false + }, + { + "name": "Debug on Linux", + "type": "gdb", + "request": "launch", + "target": "${workspaceFolder}/bazel-bin/", + "cwd": "${workspaceRoot}", + "valuesFormatting": "parseText" + } ] } From b2dee0124c7af4c09b1fd39932119c9704e3e410 Mon Sep 17 00:00:00 2001 From: etmcdonald Date: Fri, 4 Apr 2025 17:39:50 +0000 Subject: [PATCH 06/48] run tools/format.sh --- .../common/metrics_foo_library/foo_library.cc | 4 +- examples/metrics_simple/metrics_ostream.cc | 22 +++++---- examples/otlp/grpc_metric_main.cc | 24 ++++++---- exporters/ostream/src/metric_exporter.cc | 10 ++-- .../exporters/otlp/otlp_metric_utils.h | 5 +- exporters/otlp/src/otlp_metric_utils.cc | 31 ++++++------ .../base2_exponential_histogram_aggregation.h | 1 - .../sdk/metrics/data/metric_data.h | 8 ++-- .../sdk/metrics/data/point_data.h | 4 +- ...base2_exponential_histogram_aggregation.cc | 48 +++++++++++-------- sdk/test/metrics/aggregation_test.cc | 16 +++---- .../histogram_aggregation_benchmark.cc | 30 +++++++----- 12 files changed, 111 insertions(+), 92 deletions(-) diff --git a/examples/common/metrics_foo_library/foo_library.cc b/examples/common/metrics_foo_library/foo_library.cc index 00f1a398ff..60726bb536 100644 --- a/examples/common/metrics_foo_library/foo_library.cc +++ b/examples/common/metrics_foo_library/foo_library.cc @@ -5,9 +5,9 @@ #include #include #include +#include #include #include -#include #include #include @@ -22,10 +22,10 @@ #include "opentelemetry/metrics/sync_instruments.h" #include "opentelemetry/nostd/shared_ptr.h" #include "opentelemetry/nostd/variant.h" +#include "opentelemetry/sdk/metrics/view/view_factory.h" #include "opentelemetry/semconv/http_metrics.h" #include "opentelemetry/semconv/incubating/container_metrics.h" #include "opentelemetry/semconv/incubating/system_metrics.h" -#include "opentelemetry/sdk/metrics/view/view_factory.h" namespace metrics_api = opentelemetry::metrics; diff --git a/examples/metrics_simple/metrics_ostream.cc b/examples/metrics_simple/metrics_ostream.cc index 6f05560280..31c6199d77 100644 --- a/examples/metrics_simple/metrics_ostream.cc +++ b/examples/metrics_simple/metrics_ostream.cc @@ -15,9 +15,9 @@ #include "opentelemetry/sdk/metrics/export/periodic_exporting_metric_reader_factory.h" #include "opentelemetry/sdk/metrics/export/periodic_exporting_metric_reader_options.h" #include "opentelemetry/sdk/metrics/instruments.h" +#include "opentelemetry/sdk/metrics/meter_context_factory.h" #include "opentelemetry/sdk/metrics/meter_provider.h" #include "opentelemetry/sdk/metrics/meter_provider_factory.h" -#include "opentelemetry/sdk/metrics/meter_context_factory.h" #include "opentelemetry/sdk/metrics/metric_reader.h" #include "opentelemetry/sdk/metrics/provider.h" #include "opentelemetry/sdk/metrics/push_metric_exporter.h" @@ -114,17 +114,18 @@ void InitMetrics(const std::string &name) std::move(histogram_view)); // hisogram view with base2 exponential aggregation - std::string histogram_base2_name = name + "_exponential_histogram"; - unit = "histogram-unit"; + std::string histogram_base2_name = name + "_exponential_histogram"; + unit = "histogram-unit"; auto histogram_base2_instrument_selector = metrics_sdk::InstrumentSelectorFactory::Create( metrics_sdk::InstrumentType::kHistogram, histogram_base2_name, unit); - auto histogram_base2_meter_selector = metrics_sdk::MeterSelectorFactory::Create(name, version, - schema); - auto histogram_base2_aggregation_config = std::unique_ptr( - new metrics_sdk::Base2ExponentialHistogramAggregationConfig); - histogram_base2_aggregation_config->max_scale_ = 3; + auto histogram_base2_meter_selector = + metrics_sdk::MeterSelectorFactory::Create(name, version, schema); + auto histogram_base2_aggregation_config = + std::unique_ptr( + new metrics_sdk::Base2ExponentialHistogramAggregationConfig); + histogram_base2_aggregation_config->max_scale_ = 3; histogram_base2_aggregation_config->record_min_max_ = true; - histogram_base2_aggregation_config->max_buckets_ = 100; + histogram_base2_aggregation_config->max_buckets_ = 100; std::shared_ptr base2_aggregation_config( std::move(histogram_base2_aggregation_config)); @@ -133,7 +134,8 @@ void InitMetrics(const std::string &name) name, "description", unit, metrics_sdk::AggregationType::kBase2ExponentialHistogram, base2_aggregation_config); - provider->AddView(std::move(histogram_base2_instrument_selector), std::move(histogram_base2_meter_selector), std::move(histogram_base2_view)); + provider->AddView(std::move(histogram_base2_instrument_selector), + std::move(histogram_base2_meter_selector), std::move(histogram_base2_view)); std::shared_ptr api_provider(std::move(provider)); diff --git a/examples/otlp/grpc_metric_main.cc b/examples/otlp/grpc_metric_main.cc index 4ac40e53bb..4dc625809b 100644 --- a/examples/otlp/grpc_metric_main.cc +++ b/examples/otlp/grpc_metric_main.cc @@ -15,8 +15,8 @@ #include "opentelemetry/sdk/metrics/meter_provider_factory.h" #include "opentelemetry/sdk/metrics/provider.h" #include "opentelemetry/sdk/metrics/view/instrument_selector_factory.h" -#include "opentelemetry/sdk/metrics/view/view_factory.h" #include "opentelemetry/sdk/metrics/view/meter_selector_factory.h" +#include "opentelemetry/sdk/metrics/view/view_factory.h" #include #include @@ -87,15 +87,16 @@ void InitMetrics(std::string &name) // histogram view std::string histogram_name = name + "_histogram"; - std::string unit = "unit"; + std::string unit = "unit"; auto histogram_instrument_selector = metric_sdk::InstrumentSelectorFactory::Create( - metric_sdk::InstrumentType::kHistogram, histogram_name, unit); + metric_sdk::InstrumentType::kHistogram, histogram_name, unit); auto histogram_meter_selector = metric_sdk::MeterSelectorFactory::Create(name, version, schema); - auto histogram_aggregation_config = std::unique_ptr( - new metric_sdk::Base2ExponentialHistogramAggregationConfig); + auto histogram_aggregation_config = + std::unique_ptr( + new metric_sdk::Base2ExponentialHistogramAggregationConfig); histogram_aggregation_config->max_scale_ = 3; @@ -106,7 +107,8 @@ void InitMetrics(std::string &name) std::move(histogram_aggregation_config)); auto histogram_view = metric_sdk::ViewFactory::Create( - name, "description", unit, metric_sdk::AggregationType::kBase2ExponentialHistogram, aggregation_config); + name, "description", unit, metric_sdk::AggregationType::kBase2ExponentialHistogram, + aggregation_config); provider->AddView(std::move(histogram_instrument_selector), std::move(histogram_meter_selector), std::move(histogram_view)); @@ -150,7 +152,7 @@ int main(int argc, char *argv[]) InitMetrics(name); - //InitMetrics(); + // InitMetrics(); if (example_type == "counter") { @@ -163,7 +165,9 @@ int main(int argc, char *argv[]) else if (example_type == "histogram") { foo_library::histogram_example(name); - } else if (example_type == "exponential_histogram") { + } + else if (example_type == "exponential_histogram") + { foo_library::histogram_exp_example(name); } #if OPENTELEMETRY_ABI_VERSION_NO >= 2 @@ -174,8 +178,8 @@ int main(int argc, char *argv[]) #endif else { - //std::thread counter_example{&foo_library::counter_example, name}; - // std::thread observable_counter_example{&foo_library::observable_counter_example, name}; + // std::thread counter_example{&foo_library::counter_example, name}; + // std::thread observable_counter_example{&foo_library::observable_counter_example, name}; std::thread histogram_example{&foo_library::histogram_example, name}; #if OPENTELEMETRY_ABI_VERSION_NO >= 2 std::thread gauge_example{&foo_library::gauge_example, name}; diff --git a/exporters/ostream/src/metric_exporter.cc b/exporters/ostream/src/metric_exporter.cc index 084286329f..128526843d 100644 --- a/exporters/ostream/src/metric_exporter.cc +++ b/exporters/ostream/src/metric_exporter.cc @@ -257,14 +257,15 @@ void OStreamMetricExporter::printPointData(const opentelemetry::sdk::metrics::Po sout_ << "\n zero_count: " << histogram_point_data.zero_count_; if (histogram_point_data.record_min_max_) { - sout_ << "\n min: " << histogram_point_data.min_; - sout_ << "\n max: " << histogram_point_data.max_; + sout_ << "\n min: " << histogram_point_data.min_; + sout_ << "\n max: " << histogram_point_data.max_; } sout_ << "\n scale: " << histogram_point_data.scale_; sout_ << "\n positive buckets: "; if (!histogram_point_data.positive_buckets_.Empty()) { - for (auto i = histogram_point_data.positive_buckets_.StartIndex(); i <= histogram_point_data.positive_buckets_.EndIndex(); ++i) + for (auto i = histogram_point_data.positive_buckets_.StartIndex(); + i <= histogram_point_data.positive_buckets_.EndIndex(); ++i) { sout_ << "\n\t" << i << ": " << histogram_point_data.positive_buckets_.Get(i); } @@ -272,7 +273,8 @@ void OStreamMetricExporter::printPointData(const opentelemetry::sdk::metrics::Po sout_ << "\n negative buckets: "; if (!histogram_point_data.negative_buckets_.Empty()) { - for (auto i = histogram_point_data.negative_buckets_.StartIndex(); i <= histogram_point_data.negative_buckets_.EndIndex(); ++i) + for (auto i = histogram_point_data.negative_buckets_.StartIndex(); + i <= histogram_point_data.negative_buckets_.EndIndex(); ++i) { sout_ << "\n\t" << i << ": " << histogram_point_data.negative_buckets_.Get(i); } diff --git a/exporters/otlp/include/opentelemetry/exporters/otlp/otlp_metric_utils.h b/exporters/otlp/include/opentelemetry/exporters/otlp/otlp_metric_utils.h index 9907f66ce8..ba9b11e37d 100644 --- a/exporters/otlp/include/opentelemetry/exporters/otlp/otlp_metric_utils.h +++ b/exporters/otlp/include/opentelemetry/exporters/otlp/otlp_metric_utils.h @@ -55,8 +55,9 @@ class OtlpMetricUtils static void ConvertHistogramMetric(const opentelemetry::sdk::metrics::MetricData &metric_data, proto::metrics::v1::Histogram *const histogram) noexcept; - static void ConvertExponentialHistogramMetric(const opentelemetry::sdk::metrics::MetricData &metric_data, - proto::metrics::v1::ExponentialHistogram *const histogram) noexcept; + static void ConvertExponentialHistogramMetric( + const opentelemetry::sdk::metrics::MetricData &metric_data, + proto::metrics::v1::ExponentialHistogram *const histogram) noexcept; static void ConvertGaugeMetric(const opentelemetry::sdk::metrics::MetricData &metric_data, proto::metrics::v1::Gauge *const gauge) noexcept; diff --git a/exporters/otlp/src/otlp_metric_utils.cc b/exporters/otlp/src/otlp_metric_utils.cc index d6b8aa3017..c2e418a2c9 100644 --- a/exporters/otlp/src/otlp_metric_utils.cc +++ b/exporters/otlp/src/otlp_metric_utils.cc @@ -64,7 +64,7 @@ metric_sdk::AggregationType OtlpMetricUtils::GetAggregationType( return metric_sdk::AggregationType::kHistogram; } else if (nostd::holds_alternative( - point_data_with_attributes.point_data)) + point_data_with_attributes.point_data)) { return metric_sdk::AggregationType::kBase2ExponentialHistogram; } @@ -183,8 +183,8 @@ void OtlpMetricUtils::ConvertHistogramMetric( } void OtlpMetricUtils::ConvertExponentialHistogramMetric( - const metric_sdk::MetricData &metric_data, - proto::metrics::v1::ExponentialHistogram *const histogram) noexcept + const metric_sdk::MetricData &metric_data, + proto::metrics::v1::ExponentialHistogram *const histogram) noexcept { histogram->set_aggregation_temporality( GetProtoAggregationTemporality(metric_data.aggregation_temporality)); @@ -196,36 +196,36 @@ void OtlpMetricUtils::ConvertExponentialHistogramMetric( histogram->add_data_points(); proto_histogram_point_data->set_start_time_unix_nano(start_ts); proto_histogram_point_data->set_time_unix_nano(ts); - auto histogram_data = - nostd::get(point_data_with_attributes.point_data); + auto histogram_data = nostd::get( + point_data_with_attributes.point_data); // sum proto_histogram_point_data->set_sum(histogram_data.sum_); proto_histogram_point_data->set_count(histogram_data.count_); if (histogram_data.record_min_max_) { - proto_histogram_point_data->set_min(histogram_data.min_); - proto_histogram_point_data->set_max(histogram_data.max_); + proto_histogram_point_data->set_min(histogram_data.min_); + proto_histogram_point_data->set_max(histogram_data.max_); } // negative buckets - if(!histogram_data.negative_buckets_.Empty()) + if (!histogram_data.negative_buckets_.Empty()) { auto negative_buckets = proto_histogram_point_data->mutable_negative(); negative_buckets->set_offset(histogram_data.negative_buckets_.StartIndex()); - for( auto index = histogram_data.negative_buckets_.StartIndex(); - index <= histogram_data.negative_buckets_.EndIndex(); ++index) + for (auto index = histogram_data.negative_buckets_.StartIndex(); + index <= histogram_data.negative_buckets_.EndIndex(); ++index) { - negative_buckets->add_bucket_counts(histogram_data.negative_buckets_.Get(index)); + negative_buckets->add_bucket_counts(histogram_data.negative_buckets_.Get(index)); } } // positive buckets - if(!histogram_data.positive_buckets_.Empty()) + if (!histogram_data.positive_buckets_.Empty()) { auto positive_buckets = proto_histogram_point_data->mutable_positive(); positive_buckets->set_offset(histogram_data.positive_buckets_.StartIndex()); - for( auto index = histogram_data.positive_buckets_.StartIndex(); - index <= histogram_data.positive_buckets_.EndIndex(); ++index) + for (auto index = histogram_data.positive_buckets_.StartIndex(); + index <= histogram_data.positive_buckets_.EndIndex(); ++index) { positive_buckets->add_bucket_counts(histogram_data.positive_buckets_.Get(index)); } @@ -233,8 +233,6 @@ void OtlpMetricUtils::ConvertExponentialHistogramMetric( proto_histogram_point_data->set_scale(histogram_data.scale_); proto_histogram_point_data->set_zero_count(histogram_data.zero_count_); - - // attributes for (auto &kv_attr : point_data_with_attributes.attributes) { @@ -244,7 +242,6 @@ void OtlpMetricUtils::ConvertExponentialHistogramMetric( } } - void OtlpMetricUtils::ConvertGaugeMetric(const opentelemetry::sdk::metrics::MetricData &metric_data, proto::metrics::v1::Gauge *const gauge) noexcept { diff --git a/sdk/include/opentelemetry/sdk/metrics/aggregation/base2_exponential_histogram_aggregation.h b/sdk/include/opentelemetry/sdk/metrics/aggregation/base2_exponential_histogram_aggregation.h index 8a2cbbd4cc..92451658d4 100644 --- a/sdk/include/opentelemetry/sdk/metrics/aggregation/base2_exponential_histogram_aggregation.h +++ b/sdk/include/opentelemetry/sdk/metrics/aggregation/base2_exponential_histogram_aggregation.h @@ -24,7 +24,6 @@ class Base2ExponentialHistogramAggregation : public Aggregation Base2ExponentialHistogramAggregation(const Base2ExponentialHistogramPointData &point_data); Base2ExponentialHistogramAggregation(Base2ExponentialHistogramPointData &&point_data); - void Aggregate(int64_t value, const PointAttributes &attributes = {}) noexcept override; void Aggregate(double value, const PointAttributes &attributes = {}) noexcept override; diff --git a/sdk/include/opentelemetry/sdk/metrics/data/metric_data.h b/sdk/include/opentelemetry/sdk/metrics/data/metric_data.h index e1da0b7f95..30fa04e47f 100644 --- a/sdk/include/opentelemetry/sdk/metrics/data/metric_data.h +++ b/sdk/include/opentelemetry/sdk/metrics/data/metric_data.h @@ -19,10 +19,10 @@ namespace metrics using PointAttributes = opentelemetry::sdk::common::OrderedAttributeMap; using PointType = opentelemetry::nostd::variant; + HistogramPointData, + Base2ExponentialHistogramPointData, + LastValuePointData, + DropPointData>; struct PointDataAttributes { diff --git a/sdk/include/opentelemetry/sdk/metrics/data/point_data.h b/sdk/include/opentelemetry/sdk/metrics/data/point_data.h index cc84a3af8d..2a83c045e5 100644 --- a/sdk/include/opentelemetry/sdk/metrics/data/point_data.h +++ b/sdk/include/opentelemetry/sdk/metrics/data/point_data.h @@ -70,10 +70,10 @@ class Base2ExponentialHistogramPointData { public: // TODO: remove ctors and initializers when GCC<5 stops shipping on Ubuntu - Base2ExponentialHistogramPointData(Base2ExponentialHistogramPointData &&) = default; + Base2ExponentialHistogramPointData(Base2ExponentialHistogramPointData &&) = default; Base2ExponentialHistogramPointData &operator=(Base2ExponentialHistogramPointData &&) = default; Base2ExponentialHistogramPointData(const Base2ExponentialHistogramPointData &) = default; - Base2ExponentialHistogramPointData() = default; + Base2ExponentialHistogramPointData() = default; uint64_t count_ = {}; double sum_ = {}; diff --git a/sdk/src/metrics/aggregation/base2_exponential_histogram_aggregation.cc b/sdk/src/metrics/aggregation/base2_exponential_histogram_aggregation.cc index 41b63f10ea..37ebb8882d 100644 --- a/sdk/src/metrics/aggregation/base2_exponential_histogram_aggregation.cc +++ b/sdk/src/metrics/aggregation/base2_exponential_histogram_aggregation.cc @@ -99,11 +99,16 @@ Base2ExponentialHistogramAggregation::Base2ExponentialHistogramAggregation( Base2ExponentialHistogramAggregation::Base2ExponentialHistogramAggregation( const Base2ExponentialHistogramPointData &point_data) - : point_data_{point_data}, indexer_(point_data.scale_), record_min_max_{point_data.record_min_max_} + : point_data_{point_data}, + indexer_(point_data.scale_), + record_min_max_{point_data.record_min_max_} {} -Base2ExponentialHistogramAggregation::Base2ExponentialHistogramAggregation(Base2ExponentialHistogramPointData &&point_data) - : point_data_{std::move(point_data)}, indexer_(point_data_.scale_), record_min_max_{point_data_.record_min_max_} +Base2ExponentialHistogramAggregation::Base2ExponentialHistogramAggregation( + Base2ExponentialHistogramPointData &&point_data) + : point_data_{std::move(point_data)}, + indexer_(point_data_.scale_), + record_min_max_{point_data_.record_min_max_} {} void Base2ExponentialHistogramAggregation::Aggregate( @@ -181,12 +186,12 @@ void Base2ExponentialHistogramAggregation::Downscale(uint32_t by) noexcept std::unique_ptr Base2ExponentialHistogramAggregation::Merge( const Aggregation &delta) const noexcept { - auto left = nostd::get(ToPoint()); + auto left = nostd::get(ToPoint()); auto right = nostd::get( (static_cast(delta).ToPoint())); - auto low_res = left.scale_ < right.scale_ ? left : right; - auto high_res = left.scale_ < right.scale_ ? right : left; + auto low_res = left.scale_ < right.scale_ ? left : right; + auto high_res = left.scale_ < right.scale_ ? right : left; auto scale_reduction = high_res.scale_ - low_res.scale_; if (scale_reduction > 0) @@ -197,12 +202,12 @@ std::unique_ptr Base2ExponentialHistogramAggregation::Merge( } Base2ExponentialHistogramPointData result_value; - result_value.count_ = low_res.count_ + high_res.count_; - result_value.sum_ = low_res.sum_ + high_res.sum_; - result_value.zero_count_ = low_res.zero_count_ + high_res.zero_count_; - result_value.scale_ = std::min(low_res.scale_, high_res.scale_); - result_value.max_buckets_ = low_res.max_buckets_; - result_value.record_min_max_ = low_res.record_min_max_ && high_res.record_min_max_; + result_value.count_ = low_res.count_ + high_res.count_; + result_value.sum_ = low_res.sum_ + high_res.sum_; + result_value.zero_count_ = low_res.zero_count_ + high_res.zero_count_; + result_value.scale_ = std::min(low_res.scale_, high_res.scale_); + result_value.max_buckets_ = low_res.max_buckets_; + result_value.record_min_max_ = low_res.record_min_max_ && high_res.record_min_max_; if (result_value.record_min_max_) { result_value.min_ = std::min(low_res.min_, high_res.min_); @@ -233,14 +238,14 @@ std::unique_ptr Base2ExponentialHistogramAggregation::Merge( } std::unique_ptr Base2ExponentialHistogramAggregation::Diff( - const Aggregation &next) const noexcept + const Aggregation &next) const noexcept { - auto left = nostd::get(ToPoint()); + auto left = nostd::get(ToPoint()); auto right = nostd::get( (static_cast(next).ToPoint())); - auto low_res = left.scale_ < right.scale_ ? left : right; - auto high_res = left.scale_ < right.scale_ ? right : left; + auto low_res = left.scale_ < right.scale_ ? left : right; + auto high_res = left.scale_ < right.scale_ ? right : left; auto scale_reduction = high_res.scale_ - low_res.scale_; if (scale_reduction > 0) @@ -255,15 +260,16 @@ std::unique_ptr Base2ExponentialHistogramAggregation::Diff( result_value.max_buckets_ = low_res.max_buckets_; result_value.record_min_max_ = false; // caution for underflow - result_value.count_ = (left.count_ >= right.count_) ? (left.count_ - right.count_) : 0; - result_value.sum_ = (left.sum_ >= right.sum_) ? (left.sum_ - right.sum_) : 0.0; - result_value.zero_count_ = (left.zero_count_ >= right.zero_count_) ? (left.zero_count_ - right.zero_count_) : 0; + result_value.count_ = (left.count_ >= right.count_) ? (left.count_ - right.count_) : 0; + result_value.sum_ = (left.sum_ >= right.sum_) ? (left.sum_ - right.sum_) : 0.0; + result_value.zero_count_ = + (left.zero_count_ >= right.zero_count_) ? (left.zero_count_ - right.zero_count_) : 0; if (!high_res.positive_buckets_.Empty()) { for (int i = high_res.positive_buckets_.StartIndex(); i <= high_res.positive_buckets_.EndIndex(); i++) { - low_res.positive_buckets_.Increment(i, 0-high_res.positive_buckets_.Get(i)); + low_res.positive_buckets_.Increment(i, 0 - high_res.positive_buckets_.Get(i)); } } result_value.positive_buckets_ = std::move(low_res.positive_buckets_); @@ -273,7 +279,7 @@ std::unique_ptr Base2ExponentialHistogramAggregation::Diff( for (int i = high_res.negative_buckets_.StartIndex(); i <= high_res.negative_buckets_.EndIndex(); i++) { - low_res.negative_buckets_.Increment(i, 0-high_res.negative_buckets_.Get(i)); + low_res.negative_buckets_.Increment(i, 0 - high_res.negative_buckets_.Get(i)); } } result_value.negative_buckets_ = std::move(low_res.negative_buckets_); diff --git a/sdk/test/metrics/aggregation_test.cc b/sdk/test/metrics/aggregation_test.cc index f2721a8199..1e3e5c1be8 100644 --- a/sdk/test/metrics/aggregation_test.cc +++ b/sdk/test/metrics/aggregation_test.cc @@ -10,8 +10,8 @@ #include "opentelemetry/nostd/variant.h" #include "opentelemetry/sdk/metrics/aggregation/aggregation.h" #include "opentelemetry/sdk/metrics/aggregation/aggregation_config.h" -#include "opentelemetry/sdk/metrics/aggregation/histogram_aggregation.h" #include "opentelemetry/sdk/metrics/aggregation/base2_exponential_histogram_aggregation.h" +#include "opentelemetry/sdk/metrics/aggregation/histogram_aggregation.h" #include "opentelemetry/sdk/metrics/aggregation/lastvalue_aggregation.h" #include "opentelemetry/sdk/metrics/aggregation/sum_aggregation.h" #include "opentelemetry/sdk/metrics/data/point_data.h" @@ -228,11 +228,11 @@ TEST(Aggregation, DoubleHistogramAggregation) TEST(aggregation, Base2ExponentialHistogramAggregation) { // Low res histo - auto SCALE0 = 0; + auto SCALE0 = 0; auto MAX_BUCKETS0 = 7; Base2ExponentialHistogramAggregationConfig scale0_config; - scale0_config.max_scale_ = SCALE0; - scale0_config.max_buckets_ = MAX_BUCKETS0; + scale0_config.max_scale_ = SCALE0; + scale0_config.max_buckets_ = MAX_BUCKETS0; scale0_config.record_min_max_ = true; Base2ExponentialHistogramAggregation scale0_aggr(&scale0_config); auto point = scale0_aggr.ToPoint(); @@ -279,8 +279,8 @@ TEST(aggregation, Base2ExponentialHistogramAggregation) EXPECT_EQ(histo_point.positive_buckets_.Get(1), 2); Base2ExponentialHistogramAggregationConfig scale1_config; - scale1_config.max_scale_ = 1; - scale1_config.max_buckets_ = 14; + scale1_config.max_scale_ = 1; + scale1_config.max_buckets_ = 14; scale1_config.record_min_max_ = true; Base2ExponentialHistogramAggregation scale1_aggr(&scale1_config); @@ -295,7 +295,7 @@ TEST(aggregation, Base2ExponentialHistogramAggregation) EXPECT_EQ(scale1_point.min_, 0.0); EXPECT_EQ(scale1_point.max_, 3.5); - auto merged = scale0_aggr.Merge(scale1_aggr); + auto merged = scale0_aggr.Merge(scale1_aggr); auto merged_point = nostd::get(merged->ToPoint()); EXPECT_EQ(merged_point.count_, 8); EXPECT_EQ(merged_point.sum_, 13.0); @@ -307,7 +307,7 @@ TEST(aggregation, Base2ExponentialHistogramAggregation) EXPECT_EQ(merged_point.negative_buckets_.Get(-2), 1); EXPECT_EQ(merged_point.positive_buckets_.Get(2), 0); - auto diffd = merged->Diff(scale1_aggr); + auto diffd = merged->Diff(scale1_aggr); auto diffd_point = nostd::get(diffd->ToPoint()); EXPECT_EQ(diffd_point.count_, 4); EXPECT_EQ(diffd_point.sum_, 6.2); diff --git a/sdk/test/metrics/histogram_aggregation_benchmark.cc b/sdk/test/metrics/histogram_aggregation_benchmark.cc index 262fb5a99e..36295f25b8 100644 --- a/sdk/test/metrics/histogram_aggregation_benchmark.cc +++ b/sdk/test/metrics/histogram_aggregation_benchmark.cc @@ -99,53 +99,61 @@ BENCHMARK(BM_HistogramAggregation); // Add this helper function before your benchmark functions -void RunBase2ExponentialHistogramAggregation(benchmark::State &state, int scale) { +void RunBase2ExponentialHistogramAggregation(benchmark::State &state, int scale) +{ std::string instrument_unit = "histogram1_unit"; std::unique_ptr histogram_instrument_selector{ new InstrumentSelector(InstrumentType::kHistogram, ".*", instrument_unit)}; std::unique_ptr histogram_meter_selector{ new MeterSelector("meter1", "version1", "schema1")}; - + Base2ExponentialHistogramAggregationConfig config; config.max_scale_ = scale; - + std::unique_ptr histogram_view{ - new View("base2_expohisto", "description", instrument_unit, AggregationType::kBase2ExponentialHistogram, + new View("base2_expohisto", "description", instrument_unit, + AggregationType::kBase2ExponentialHistogram, std::make_shared(config))}; std::unique_ptr views{new ViewRegistry()}; views->AddView(std::move(histogram_instrument_selector), std::move(histogram_meter_selector), std::move(histogram_view)); - + HistogramAggregation(state, std::move(views)); } -void BM_Base2ExponentialHistogramAggregationZeroScale(benchmark::State &state) { +void BM_Base2ExponentialHistogramAggregationZeroScale(benchmark::State &state) +{ RunBase2ExponentialHistogramAggregation(state, 0); } BENCHMARK(BM_Base2ExponentialHistogramAggregationZeroScale); -void BM_Base2ExponentialHistogramAggregationOneScale(benchmark::State &state) { +void BM_Base2ExponentialHistogramAggregationOneScale(benchmark::State &state) +{ RunBase2ExponentialHistogramAggregation(state, 1); } BENCHMARK(BM_Base2ExponentialHistogramAggregationOneScale); -void BM_Base2ExponentialHistogramAggregationTwoScale(benchmark::State &state) { +void BM_Base2ExponentialHistogramAggregationTwoScale(benchmark::State &state) +{ RunBase2ExponentialHistogramAggregation(state, 2); } BENCHMARK(BM_Base2ExponentialHistogramAggregationTwoScale); -void BM_Base2ExponentialHistogramAggregationFourScale(benchmark::State &state) { +void BM_Base2ExponentialHistogramAggregationFourScale(benchmark::State &state) +{ RunBase2ExponentialHistogramAggregation(state, 4); } BENCHMARK(BM_Base2ExponentialHistogramAggregationFourScale); -void BM_Base2ExponentialHistogramAggregationEightScale(benchmark::State &state) { +void BM_Base2ExponentialHistogramAggregationEightScale(benchmark::State &state) +{ RunBase2ExponentialHistogramAggregation(state, 8); } BENCHMARK(BM_Base2ExponentialHistogramAggregationEightScale); -void BM_Base2ExponentialHistogramAggregationSixteenScale(benchmark::State &state) { +void BM_Base2ExponentialHistogramAggregationSixteenScale(benchmark::State &state) +{ RunBase2ExponentialHistogramAggregation(state, 16); } BENCHMARK(BM_Base2ExponentialHistogramAggregationSixteenScale); From 250de89a86e8476652941798f6bb58de79b8bf02 Mon Sep 17 00:00:00 2001 From: etmcdonald Date: Fri, 4 Apr 2025 19:35:27 +0000 Subject: [PATCH 07/48] add grpc exporter target switch for unix sockets --- exporters/otlp/src/otlp_grpc_client.cc | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/exporters/otlp/src/otlp_grpc_client.cc b/exporters/otlp/src/otlp_grpc_client.cc index 4d2fb6d38e..3ea5751162 100644 --- a/exporters/otlp/src/otlp_grpc_client.cc +++ b/exporters/otlp/src/otlp_grpc_client.cc @@ -333,7 +333,12 @@ std::shared_ptr OtlpGrpcClient::MakeChannel(const OtlpGrpcClientO } std::shared_ptr channel; - std::string grpc_target = url.host_ + ":" + std::to_string(static_cast(url.port_)); + std::string grpc_target; + if (url.scheme_ == "unix") { + grpc_target = "unix:" + url.path_; + } else { + grpc_target = url.host_ + ":" + std::to_string(static_cast(url.port_)); + } grpc::ChannelArguments grpc_arguments; grpc_arguments.SetUserAgentPrefix(options.user_agent); From 43761285fb07902444220e4fe16c5d5f7711ffa7 Mon Sep 17 00:00:00 2001 From: "Felipe C. Dos Santos" Date: Fri, 4 Apr 2025 20:25:37 +0000 Subject: [PATCH 08/48] Added ConvertExponentialHistogramMetric unit test --- exporters/otlp/src/otlp_grpc_client.cc | 7 +- .../test/otlp_metrics_serialization_test.cc | 109 ++++++++++++++++++ 2 files changed, 114 insertions(+), 2 deletions(-) diff --git a/exporters/otlp/src/otlp_grpc_client.cc b/exporters/otlp/src/otlp_grpc_client.cc index 3ea5751162..75bea9d4ed 100644 --- a/exporters/otlp/src/otlp_grpc_client.cc +++ b/exporters/otlp/src/otlp_grpc_client.cc @@ -334,9 +334,12 @@ std::shared_ptr OtlpGrpcClient::MakeChannel(const OtlpGrpcClientO std::shared_ptr channel; std::string grpc_target; - if (url.scheme_ == "unix") { + if (url.scheme_ == "unix") + { grpc_target = "unix:" + url.path_; - } else { + } + else + { grpc_target = url.host_ + ":" + std::to_string(static_cast(url.port_)); } grpc::ChannelArguments grpc_arguments; diff --git a/exporters/otlp/test/otlp_metrics_serialization_test.cc b/exporters/otlp/test/otlp_metrics_serialization_test.cc index 66c1376d46..add35e64ac 100644 --- a/exporters/otlp/test/otlp_metrics_serialization_test.cc +++ b/exporters/otlp/test/otlp_metrics_serialization_test.cc @@ -127,6 +127,56 @@ static metrics_sdk::MetricData CreateHistogramAggregationData() return data; } +static metrics_sdk::MetricData CreateExponentialHistogramAggregationData( + const std::chrono::system_clock::time_point &now_time) +{ + metrics_sdk::MetricData data; + data.start_ts = opentelemetry::common::SystemTimestamp(now_time); + metrics_sdk::InstrumentDescriptor inst_desc = {"Histogram", "desc", "unit", + metrics_sdk::InstrumentType::kHistogram, + metrics_sdk::InstrumentValueType::kDouble}; + metrics_sdk::Base2ExponentialHistogramPointData s_data_1, s_data_2; + s_data_1.count_ = 3; + s_data_1.sum_ = 6.5; + s_data_1.min_ = 0.0; + s_data_1.max_ = 3.5; + s_data_1.scale_ = 3; + s_data_2.record_min_max_ = true; + s_data_1.zero_count_ = 1; + s_data_1.positive_buckets_ = opentelemetry::sdk::metrics::AdaptingCircularBufferCounter(10); + s_data_1.negative_buckets_ = opentelemetry::sdk::metrics::AdaptingCircularBufferCounter(10); + s_data_1.positive_buckets_.Increment(1, 1); + s_data_1.negative_buckets_.Increment(-2, 1); + + s_data_2.count_ = 4; + s_data_2.sum_ = 6.2; + s_data_2.min_ = -0.03; + s_data_2.max_ = 3.5; + s_data_2.scale_ = 3; + s_data_2.record_min_max_ = false; + s_data_2.zero_count_ = 2; + s_data_2.positive_buckets_ = opentelemetry::sdk::metrics::AdaptingCircularBufferCounter(10); + s_data_2.negative_buckets_ = opentelemetry::sdk::metrics::AdaptingCircularBufferCounter(10); + s_data_2.positive_buckets_.Increment(3, 1); + s_data_2.negative_buckets_.Increment(-2, 1); + s_data_2.negative_buckets_.Increment(-4, 2); + + data.aggregation_temporality = metrics_sdk::AggregationTemporality::kCumulative; + data.end_ts = opentelemetry::common::SystemTimestamp(now_time); + data.instrument_descriptor = inst_desc; + metrics_sdk::PointDataAttributes point_data_attr_1, point_data_attr_2; + point_data_attr_1.attributes = {{"k1", "v1"}}; + point_data_attr_1.point_data = s_data_1; + + point_data_attr_2.attributes = {{"k2", "v2"}}; + point_data_attr_2.point_data = s_data_2; + std::vector point_data_attr; + point_data_attr.push_back(point_data_attr_1); + point_data_attr.push_back(point_data_attr_2); + data.point_data_attr_ = std::move(point_data_attr); + return data; +} + static metrics_sdk::MetricData CreateObservableGaugeAggregationData() { metrics_sdk::MetricData data; @@ -261,6 +311,65 @@ TEST(OtlpMetricSerializationTest, Histogram) EXPECT_EQ(1, 1); } +TEST(OtlpMetricSerializationTest, ExponentialHistogramAggregationData) +{ + const auto start_test_time = std::chrono::system_clock::now(); + const auto data = CreateExponentialHistogramAggregationData(start_test_time); + opentelemetry::proto::metrics::v1::ExponentialHistogram exponentialHistogram; + otlp_exporter::OtlpMetricUtils::ConvertExponentialHistogramMetric(data, &exponentialHistogram); + EXPECT_EQ(exponentialHistogram.aggregation_temporality(), + proto::metrics::v1::AggregationTemporality::AGGREGATION_TEMPORALITY_CUMULATIVE); + + EXPECT_EQ(exponentialHistogram.data_points_size(), 2); + // Point 1 + { + const auto &data_point1 = exponentialHistogram.data_points(0); + EXPECT_EQ(data_point1.count(), 3); + EXPECT_EQ(data_point1.sum(), 6.5); + EXPECT_EQ(data_point1.min(), 0.0); + EXPECT_EQ(data_point1.max(), 3.5); + EXPECT_EQ(data_point1.zero_count(), 1); + EXPECT_EQ(data_point1.scale(), 3); + EXPECT_EQ(data_point1.positive().offset(), 1); + EXPECT_EQ(data_point1.positive().bucket_counts_size(), 1); + EXPECT_EQ(data_point1.positive().bucket_counts(0), 1); + EXPECT_EQ(data_point1.negative().offset(), -2); + EXPECT_EQ(data_point1.negative().bucket_counts_size(), 1); + EXPECT_EQ(data_point1.negative().bucket_counts(0), 1); + + EXPECT_EQ(data_point1.attributes_size(), 1); + EXPECT_EQ(data_point1.attributes(0).key(), "k1"); + EXPECT_EQ(data_point1.attributes(0).value().string_value(), "v1"); + EXPECT_EQ(data_point1.start_time_unix_nano(), data.start_ts.time_since_epoch().count()); + EXPECT_EQ(data_point1.time_unix_nano(), data.end_ts.time_since_epoch().count()); + } + + // Point 2 + { + const auto &data_point2 = exponentialHistogram.data_points(1); + EXPECT_EQ(data_point2.count(), 4); + EXPECT_EQ(data_point2.sum(), 6.2); + EXPECT_EQ(data_point2.min(), 0.0); + EXPECT_EQ(data_point2.max(), 0.0); + EXPECT_EQ(data_point2.zero_count(), 2); + EXPECT_EQ(data_point2.scale(), 3); + EXPECT_EQ(data_point2.positive().offset(), 3); + EXPECT_EQ(data_point2.positive().bucket_counts_size(), 1); + EXPECT_EQ(data_point2.positive().bucket_counts(0), 1); + EXPECT_EQ(data_point2.negative().offset(), -4); + EXPECT_EQ(data_point2.negative().bucket_counts_size(), 3); + EXPECT_EQ(data_point2.negative().bucket_counts(0), 2); + EXPECT_EQ(data_point2.negative().bucket_counts(1), 0); + EXPECT_EQ(data_point2.negative().bucket_counts(2), 1); + EXPECT_EQ(data_point2.attributes(0).key(), "k2"); + EXPECT_EQ(data_point2.attributes(0).value().string_value(), "v2"); + EXPECT_EQ(data_point2.start_time_unix_nano(), data.start_ts.time_since_epoch().count()); + EXPECT_EQ(data_point2.time_unix_nano(), data.end_ts.time_since_epoch().count()); + } + + EXPECT_EQ(1, 1); +} + TEST(OtlpMetricSerializationTest, ObservableGauge) { metrics_sdk::MetricData data = CreateObservableGaugeAggregationData(); From 4408d336eb26c131e07bcdf0c3e8688c21f867d5 Mon Sep 17 00:00:00 2001 From: "Felipe C. Dos Santos" Date: Fri, 4 Apr 2025 21:16:41 +0000 Subject: [PATCH 09/48] Update the osstream_metric_test with ExponentialHistogram --- exporters/ostream/src/metric_exporter.cc | 4 +- exporters/ostream/test/ostream_metric_test.cc | 109 ++++++++++++++++++ .../test/otlp_metrics_serialization_test.cc | 2 +- 3 files changed, 112 insertions(+), 3 deletions(-) diff --git a/exporters/ostream/src/metric_exporter.cc b/exporters/ostream/src/metric_exporter.cc index 128526843d..0b3adaf0a0 100644 --- a/exporters/ostream/src/metric_exporter.cc +++ b/exporters/ostream/src/metric_exporter.cc @@ -261,7 +261,7 @@ void OStreamMetricExporter::printPointData(const opentelemetry::sdk::metrics::Po sout_ << "\n max: " << histogram_point_data.max_; } sout_ << "\n scale: " << histogram_point_data.scale_; - sout_ << "\n positive buckets: "; + sout_ << "\n positive buckets:"; if (!histogram_point_data.positive_buckets_.Empty()) { for (auto i = histogram_point_data.positive_buckets_.StartIndex(); @@ -270,7 +270,7 @@ void OStreamMetricExporter::printPointData(const opentelemetry::sdk::metrics::Po sout_ << "\n\t" << i << ": " << histogram_point_data.positive_buckets_.Get(i); } } - sout_ << "\n negative buckets: "; + sout_ << "\n negative buckets:"; if (!histogram_point_data.negative_buckets_.Empty()) { for (auto i = histogram_point_data.negative_buckets_.StartIndex(); diff --git a/exporters/ostream/test/ostream_metric_test.cc b/exporters/ostream/test/ostream_metric_test.cc index 6e7a88fa2f..59f6b7b9e6 100644 --- a/exporters/ostream/test/ostream_metric_test.cc +++ b/exporters/ostream/test/ostream_metric_test.cc @@ -179,6 +179,115 @@ TEST(OStreamMetricsExporter, ExportHistogramPointData) ASSERT_EQ(stdoutOutput.str(), expected_output); } +TEST(OStreamMetricsExporter, ExportBase2ExponentialHistogramPointData) +{ + auto exporter = + std::unique_ptr(new exportermetrics::OStreamMetricExporter); + + metric_sdk::Base2ExponentialHistogramPointData histogram_point_data1; + histogram_point_data1.count_ = 3; + histogram_point_data1.sum_ = 6.5; + histogram_point_data1.min_ = 0.0; + histogram_point_data1.max_ = 3.5; + histogram_point_data1.scale_ = 3; + histogram_point_data1.record_min_max_ = true; + histogram_point_data1.zero_count_ = 1; + histogram_point_data1.positive_buckets_ = + opentelemetry::sdk::metrics::AdaptingCircularBufferCounter(10); + histogram_point_data1.negative_buckets_ = + opentelemetry::sdk::metrics::AdaptingCircularBufferCounter(10); + histogram_point_data1.positive_buckets_.Increment(1, 1); + histogram_point_data1.negative_buckets_.Increment(-2, 1); + + metric_sdk::Base2ExponentialHistogramPointData histogram_point_data2; + histogram_point_data2.count_ = 4; + histogram_point_data2.sum_ = 6.2; + histogram_point_data2.min_ = -0.03; + histogram_point_data2.max_ = 3.5; + histogram_point_data2.scale_ = 3; + histogram_point_data2.record_min_max_ = false; + histogram_point_data2.zero_count_ = 2; + histogram_point_data2.positive_buckets_ = + opentelemetry::sdk::metrics::AdaptingCircularBufferCounter(10); + histogram_point_data2.negative_buckets_ = + opentelemetry::sdk::metrics::AdaptingCircularBufferCounter(10); + histogram_point_data2.positive_buckets_.Increment(3, 1); + histogram_point_data2.negative_buckets_.Increment(-2, 1); + histogram_point_data2.negative_buckets_.Increment(-4, 2); + + metric_sdk::ResourceMetrics data; + auto resource = opentelemetry::sdk::resource::Resource::Create( + opentelemetry::sdk::resource::ResourceAttributes{}); + data.resource_ = &resource; + auto scope = opentelemetry::sdk::instrumentationscope::InstrumentationScope::Create( + "library_name", "1.2.0"); + metric_sdk::MetricData metric_data{ + metric_sdk::InstrumentDescriptor{"library_name", "description", "unit", + metric_sdk::InstrumentType::kCounter, + metric_sdk::InstrumentValueType::kDouble}, + metric_sdk::AggregationTemporality::kDelta, opentelemetry::common::SystemTimestamp{}, + opentelemetry::common::SystemTimestamp{}, + std::vector{ + {metric_sdk::PointAttributes{{"a1", "b1"}, {"a2", "b2"}}, histogram_point_data1}, + {metric_sdk::PointAttributes{{"a1", "b1"}}, histogram_point_data2}}}; + data.scope_metric_data_ = std::vector{ + {scope.get(), std::vector{metric_data}}}; + + std::stringstream stdoutOutput; + std::streambuf *sbuf = std::cout.rdbuf(); + std::cout.rdbuf(stdoutOutput.rdbuf()); + + auto result = exporter->Export(data); + EXPECT_EQ(result, opentelemetry::sdk::common::ExportResult::kSuccess); + std::cout.rdbuf(sbuf); + + std::string expected_output = + "{" + "\n scope name\t: library_name" + "\n schema url\t: " + "\n version\t: 1.2.0" + "\n start time\t: Thu Jan 1 00:00:00 1970" + "\n end time\t: Thu Jan 1 00:00:00 1970" + "\n instrument name\t: library_name" + "\n description\t: description" + "\n unit\t\t: unit" + "\n type: Base2ExponentialHistogramPointData" + "\n count: 3" + "\n sum: 6.5" + "\n zero_count: 1" + "\n min: 0" + "\n max: 3.5" + "\n scale: 3" + "\n positive buckets:" + "\n\t1: 1" + "\n negative buckets:" + "\n\t-2: 1" + "\n attributes\t\t: " + "\n\ta1: b1" + "\n\ta2: b2" + "\n type: Base2ExponentialHistogramPointData" + "\n count: 4" + "\n sum: 6.2" + "\n zero_count: 2" + "\n scale: 3" + "\n positive buckets:" + "\n\t3: 1" + "\n negative buckets:" + "\n\t-4: 2" + "\n\t-3: 0" + "\n\t-2: 1" + "\n attributes\t\t: " + "\n\ta1: b1" + "\n resources\t:" + "\n\tservice.name: unknown_service" + "\n\ttelemetry.sdk.language: cpp" + "\n\ttelemetry.sdk.name: opentelemetry" + "\n\ttelemetry.sdk.version: "; + expected_output += OPENTELEMETRY_SDK_VERSION; + expected_output += "\n}\n"; + ASSERT_EQ(stdoutOutput.str(), expected_output); +} + TEST(OStreamMetricsExporter, ExportLastValuePointData) { auto exporter = diff --git a/exporters/otlp/test/otlp_metrics_serialization_test.cc b/exporters/otlp/test/otlp_metrics_serialization_test.cc index add35e64ac..3b5df9e04e 100644 --- a/exporters/otlp/test/otlp_metrics_serialization_test.cc +++ b/exporters/otlp/test/otlp_metrics_serialization_test.cc @@ -141,7 +141,7 @@ static metrics_sdk::MetricData CreateExponentialHistogramAggregationData( s_data_1.min_ = 0.0; s_data_1.max_ = 3.5; s_data_1.scale_ = 3; - s_data_2.record_min_max_ = true; + s_data_1.record_min_max_ = true; s_data_1.zero_count_ = 1; s_data_1.positive_buckets_ = opentelemetry::sdk::metrics::AdaptingCircularBufferCounter(10); s_data_1.negative_buckets_ = opentelemetry::sdk::metrics::AdaptingCircularBufferCounter(10); From 21e1d34553ccb11a9e82dda31726ff907129ea71 Mon Sep 17 00:00:00 2001 From: etmcdonald Date: Fri, 4 Apr 2025 20:30:22 +0000 Subject: [PATCH 10/48] update CHANGELOG.md --- CHANGELOG.md | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2c5668f161..f1a0a0cf69 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,16 @@ Increment the: * MINOR version when you add functionality in a backwards compatible manner, and * PATCH version when you make backwards compatible bug fixes. +## [1.21 2025-04-04] +* [SDK] Base2 exponential histogram aggregation + [#3175](https://github.com/open-telemetry/opentelemetry-cpp/pull/3346) + + New Features: + + Add base2 exponential histogram aggregation. Includes a new aggregation type, + ostream exporter, and otlp/grpc exporter. Updated histogram aggregation and + benchmark tests. + ## [Unreleased] * [API] Remove `WITH_ABSEIL` and `HAVE_ABSEIL` From 439788608dd09a281184d46f1350080d84425b8d Mon Sep 17 00:00:00 2001 From: etmcdonald Date: Fri, 4 Apr 2025 20:32:38 +0000 Subject: [PATCH 11/48] markdown lint --- CHANGELOG.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f1a0a0cf69..00967e78c7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,7 @@ Increment the: * PATCH version when you make backwards compatible bug fixes. ## [1.21 2025-04-04] + * [SDK] Base2 exponential histogram aggregation [#3175](https://github.com/open-telemetry/opentelemetry-cpp/pull/3346) @@ -21,7 +22,7 @@ Increment the: Add base2 exponential histogram aggregation. Includes a new aggregation type, ostream exporter, and otlp/grpc exporter. Updated histogram aggregation and - benchmark tests. + benchmark tests. ## [Unreleased] From 9b9d8c135bfcb0772db317786fd10293aa6b4b8d Mon Sep 17 00:00:00 2001 From: "Felipe C. Dos Santos" Date: Mon, 7 Apr 2025 12:03:20 +0000 Subject: [PATCH 12/48] Comment non-used function add missing includes --- .../base2_exponential_histogram_aggregation.h | 11 +++++--- .../base2_exponential_histogram_indexer.h | 4 +-- ...base2_exponential_histogram_aggregation.cc | 26 +++++++++---------- 3 files changed, 22 insertions(+), 19 deletions(-) diff --git a/sdk/include/opentelemetry/sdk/metrics/aggregation/base2_exponential_histogram_aggregation.h b/sdk/include/opentelemetry/sdk/metrics/aggregation/base2_exponential_histogram_aggregation.h index 92451658d4..d0b7a6e59d 100644 --- a/sdk/include/opentelemetry/sdk/metrics/aggregation/base2_exponential_histogram_aggregation.h +++ b/sdk/include/opentelemetry/sdk/metrics/aggregation/base2_exponential_histogram_aggregation.h @@ -3,13 +3,16 @@ #pragma once +#include +#include + #include "opentelemetry/common/spin_lock_mutex.h" -#include "opentelemetry/sdk/metrics/aggregation/aggregation.h" #include "opentelemetry/sdk/metrics/aggregation/aggregation_config.h" +#include "opentelemetry/sdk/metrics/aggregation/aggregation.h" #include "opentelemetry/sdk/metrics/aggregation/base2_exponential_histogram_indexer.h" - -#include -#include +#include "opentelemetry/sdk/metrics/data/circular_buffer.h" +#include "opentelemetry/sdk/metrics/data/metric_data.h" +#include "opentelemetry/sdk/metrics/data/point_data.h" OPENTELEMETRY_BEGIN_NAMESPACE namespace sdk diff --git a/sdk/include/opentelemetry/sdk/metrics/aggregation/base2_exponential_histogram_indexer.h b/sdk/include/opentelemetry/sdk/metrics/aggregation/base2_exponential_histogram_indexer.h index 14c00070b1..ba6904d319 100644 --- a/sdk/include/opentelemetry/sdk/metrics/aggregation/base2_exponential_histogram_indexer.h +++ b/sdk/include/opentelemetry/sdk/metrics/aggregation/base2_exponential_histogram_indexer.h @@ -3,9 +3,9 @@ #pragma once -#include "opentelemetry/version.h" +#include -#include +#include "opentelemetry/version.h" OPENTELEMETRY_BEGIN_NAMESPACE namespace sdk diff --git a/sdk/src/metrics/aggregation/base2_exponential_histogram_aggregation.cc b/sdk/src/metrics/aggregation/base2_exponential_histogram_aggregation.cc index 37ebb8882d..f11227e5db 100644 --- a/sdk/src/metrics/aggregation/base2_exponential_histogram_aggregation.cc +++ b/sdk/src/metrics/aggregation/base2_exponential_histogram_aggregation.cc @@ -37,19 +37,19 @@ uint32_t GetScaleReduction(int32_t start_index, int32_t end_index, size_t max_bu return scale_reduction; } -uint32_t GetScaleReduction(const AdaptingCircularBufferCounter &first, - const AdaptingCircularBufferCounter &second, - size_t max_buckets) -{ - if (first.Empty() || second.Empty()) - { - return 0; - } - - const int32_t start_index = std::min(first.StartIndex(), second.StartIndex()); - const int32_t end_index = std::max(first.EndIndex(), second.EndIndex()); - return GetScaleReduction(start_index, end_index, max_buckets); -} +// Commented out as it is not used +// uint32_t GetScaleReduction(const AdaptingCircularBufferCounter &first, +// const AdaptingCircularBufferCounter &second, +// size_t max_buckets) +// { +// if (first.Empty() || second.Empty()) +// { +// return 0; +// } +// const int32_t start_index = std::min(first.StartIndex(), second.StartIndex()); +// const int32_t end_index = std::max(first.EndIndex(), second.EndIndex()); +// return GetScaleReduction(start_index, end_index, max_buckets); +// } void DownscaleBuckets(AdaptingCircularBufferCounter *buckets, uint32_t by) noexcept { From 775688a0c7523bc0be852da8488dddcf17dd7303 Mon Sep 17 00:00:00 2001 From: etmcdonald Date: Tue, 8 Apr 2025 04:46:21 +0000 Subject: [PATCH 13/48] add metrics dependency to otlp exporters test build --- exporters/otlp/BUILD | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/exporters/otlp/BUILD b/exporters/otlp/BUILD index cdaba37e4d..c915dc6085 100644 --- a/exporters/otlp/BUILD +++ b/exporters/otlp/BUILD @@ -418,6 +418,7 @@ cc_test( deps = [ ":otlp_recordable", "@com_google_googletest//:gtest_main", + "//sdk/src/metrics" ], ) @@ -435,6 +436,7 @@ cc_test( ":otlp_recordable", "@com_github_opentelemetry_proto//:logs_service_proto_cc", "@com_google_googletest//:gtest_main", + "//sdk/src/metrics" ], ) @@ -452,6 +454,7 @@ cc_test( ":otlp_recordable", "//api", "@com_google_googletest//:gtest_main", + "//sdk/src/metrics" ], ) @@ -467,6 +470,7 @@ cc_test( ":otlp_grpc_exporter", "//api", "@com_google_googletest//:gtest_main", + "//sdk/src/metrics" ], ) @@ -482,6 +486,7 @@ cc_test( ":otlp_grpc_exporter", "//api", "@com_google_googletest//:gtest_main", + "//sdk/src/metrics" ], ) @@ -498,6 +503,7 @@ cc_test( "//api", "//test_common/src/http/client/nosend:http_client_nosend", "@com_google_googletest//:gtest_main", + "//sdk/src/metrics" ], ) @@ -514,6 +520,7 @@ cc_test( "//api", "//test_common/src/http/client/nosend:http_client_nosend", "@com_google_googletest//:gtest_main", + "//sdk/src/metrics" ], ) @@ -529,6 +536,7 @@ cc_test( ":otlp_file_exporter", "//api", "@com_google_googletest//:gtest_main", + "//sdk/src/metrics" ], ) @@ -544,6 +552,7 @@ cc_test( ":otlp_file_exporter", "//api", "@com_google_googletest//:gtest_main", + "//sdk/src/metrics" ], ) @@ -560,6 +569,7 @@ cc_test( "//api", "//test_common/src/http/client/nosend:http_client_nosend", "@com_google_googletest//:gtest_main", + "//sdk/src/metrics" ], ) @@ -576,6 +586,7 @@ cc_test( "//api", "//test_common/src/http/client/nosend:http_client_nosend", "@com_google_googletest//:gtest_main", + "//sdk/src/metrics" ], ) @@ -591,6 +602,7 @@ cc_test( ":otlp_file_log_record_exporter", "//api", "@com_google_googletest//:gtest_main", + "//sdk/src/metrics" ], ) @@ -606,6 +618,7 @@ cc_test( ":otlp_file_log_record_exporter", "//api", "@com_google_googletest//:gtest_main", + "//sdk/src/metrics" ], ) @@ -623,6 +636,7 @@ cc_test( "//api", "//sdk/src/logs", "@com_google_googletest//:gtest_main", + "//sdk/src/metrics" ], ) @@ -639,6 +653,7 @@ cc_test( "//api", "//sdk/src/logs", "@com_google_googletest//:gtest_main", + "//sdk/src/metrics" ], ) @@ -747,5 +762,6 @@ otel_cc_benchmark( deps = [ ":otlp_grpc_exporter", "//examples/common/foo_library:common_foo_library", + "//sdk/src/metrics" ], ) From b2d7a027a4453038c96d53fb0b5e8dde75f309b7 Mon Sep 17 00:00:00 2001 From: etmcdonald Date: Tue, 8 Apr 2025 05:00:32 +0000 Subject: [PATCH 14/48] run format --- exporters/otlp/BUILD | 32 +++++++++---------- .../base2_exponential_histogram_aggregation.h | 2 +- 2 files changed, 17 insertions(+), 17 deletions(-) diff --git a/exporters/otlp/BUILD b/exporters/otlp/BUILD index c915dc6085..ba55ba72c7 100644 --- a/exporters/otlp/BUILD +++ b/exporters/otlp/BUILD @@ -417,8 +417,8 @@ cc_test( ], deps = [ ":otlp_recordable", + "//sdk/src/metrics", "@com_google_googletest//:gtest_main", - "//sdk/src/metrics" ], ) @@ -434,9 +434,9 @@ cc_test( ], deps = [ ":otlp_recordable", + "//sdk/src/metrics", "@com_github_opentelemetry_proto//:logs_service_proto_cc", "@com_google_googletest//:gtest_main", - "//sdk/src/metrics" ], ) @@ -453,8 +453,8 @@ cc_test( ":otlp_file_exporter", ":otlp_recordable", "//api", + "//sdk/src/metrics", "@com_google_googletest//:gtest_main", - "//sdk/src/metrics" ], ) @@ -469,8 +469,8 @@ cc_test( deps = [ ":otlp_grpc_exporter", "//api", + "//sdk/src/metrics", "@com_google_googletest//:gtest_main", - "//sdk/src/metrics" ], ) @@ -485,8 +485,8 @@ cc_test( deps = [ ":otlp_grpc_exporter", "//api", + "//sdk/src/metrics", "@com_google_googletest//:gtest_main", - "//sdk/src/metrics" ], ) @@ -501,9 +501,9 @@ cc_test( deps = [ ":otlp_http_exporter", "//api", + "//sdk/src/metrics", "//test_common/src/http/client/nosend:http_client_nosend", "@com_google_googletest//:gtest_main", - "//sdk/src/metrics" ], ) @@ -518,9 +518,9 @@ cc_test( deps = [ ":otlp_http_exporter", "//api", + "//sdk/src/metrics", "//test_common/src/http/client/nosend:http_client_nosend", "@com_google_googletest//:gtest_main", - "//sdk/src/metrics" ], ) @@ -535,8 +535,8 @@ cc_test( deps = [ ":otlp_file_exporter", "//api", + "//sdk/src/metrics", "@com_google_googletest//:gtest_main", - "//sdk/src/metrics" ], ) @@ -551,8 +551,8 @@ cc_test( deps = [ ":otlp_file_exporter", "//api", + "//sdk/src/metrics", "@com_google_googletest//:gtest_main", - "//sdk/src/metrics" ], ) @@ -567,9 +567,9 @@ cc_test( deps = [ ":otlp_http_log_record_exporter", "//api", + "//sdk/src/metrics", "//test_common/src/http/client/nosend:http_client_nosend", "@com_google_googletest//:gtest_main", - "//sdk/src/metrics" ], ) @@ -584,9 +584,9 @@ cc_test( deps = [ ":otlp_http_log_record_exporter", "//api", + "//sdk/src/metrics", "//test_common/src/http/client/nosend:http_client_nosend", "@com_google_googletest//:gtest_main", - "//sdk/src/metrics" ], ) @@ -601,8 +601,8 @@ cc_test( deps = [ ":otlp_file_log_record_exporter", "//api", + "//sdk/src/metrics", "@com_google_googletest//:gtest_main", - "//sdk/src/metrics" ], ) @@ -617,8 +617,8 @@ cc_test( deps = [ ":otlp_file_log_record_exporter", "//api", + "//sdk/src/metrics", "@com_google_googletest//:gtest_main", - "//sdk/src/metrics" ], ) @@ -635,8 +635,8 @@ cc_test( ":otlp_grpc_log_record_exporter", "//api", "//sdk/src/logs", + "//sdk/src/metrics", "@com_google_googletest//:gtest_main", - "//sdk/src/metrics" ], ) @@ -652,8 +652,8 @@ cc_test( ":otlp_grpc_log_record_exporter", "//api", "//sdk/src/logs", + "//sdk/src/metrics", "@com_google_googletest//:gtest_main", - "//sdk/src/metrics" ], ) @@ -762,6 +762,6 @@ otel_cc_benchmark( deps = [ ":otlp_grpc_exporter", "//examples/common/foo_library:common_foo_library", - "//sdk/src/metrics" + "//sdk/src/metrics", ], ) diff --git a/sdk/include/opentelemetry/sdk/metrics/aggregation/base2_exponential_histogram_aggregation.h b/sdk/include/opentelemetry/sdk/metrics/aggregation/base2_exponential_histogram_aggregation.h index d0b7a6e59d..4a4dc0d165 100644 --- a/sdk/include/opentelemetry/sdk/metrics/aggregation/base2_exponential_histogram_aggregation.h +++ b/sdk/include/opentelemetry/sdk/metrics/aggregation/base2_exponential_histogram_aggregation.h @@ -7,8 +7,8 @@ #include #include "opentelemetry/common/spin_lock_mutex.h" -#include "opentelemetry/sdk/metrics/aggregation/aggregation_config.h" #include "opentelemetry/sdk/metrics/aggregation/aggregation.h" +#include "opentelemetry/sdk/metrics/aggregation/aggregation_config.h" #include "opentelemetry/sdk/metrics/aggregation/base2_exponential_histogram_indexer.h" #include "opentelemetry/sdk/metrics/data/circular_buffer.h" #include "opentelemetry/sdk/metrics/data/metric_data.h" From 712e538063b4fb20fddad722d57b4bb8fd6b40bd Mon Sep 17 00:00:00 2001 From: etmcdonald Date: Tue, 8 Apr 2025 05:57:50 +0000 Subject: [PATCH 15/48] remove unused function definition --- examples/otlp/grpc_metric_main.cc | 26 -------------------------- 1 file changed, 26 deletions(-) diff --git a/examples/otlp/grpc_metric_main.cc b/examples/otlp/grpc_metric_main.cc index 4dc625809b..6d36295dbf 100644 --- a/examples/otlp/grpc_metric_main.cc +++ b/examples/otlp/grpc_metric_main.cc @@ -37,30 +37,6 @@ namespace otlp_exporter::OtlpGrpcMetricExporterOptions exporter_options; -void InitMetrics() -{ - auto exporter = otlp_exporter::OtlpGrpcMetricExporterFactory::Create(exporter_options); - - std::string version{"1.2.0"}; - std::string schema{"https://opentelemetry.io/schemas/1.2.0"}; - - // Initialize and set the global MeterProvider - metric_sdk::PeriodicExportingMetricReaderOptions reader_options; - reader_options.export_interval_millis = std::chrono::milliseconds(1000); - reader_options.export_timeout_millis = std::chrono::milliseconds(500); - - auto reader = - metric_sdk::PeriodicExportingMetricReaderFactory::Create(std::move(exporter), reader_options); - - auto context = metric_sdk::MeterContextFactory::Create(); - context->AddMetricReader(std::move(reader)); - - auto u_provider = metric_sdk::MeterProviderFactory::Create(std::move(context)); - std::shared_ptr provider(std::move(u_provider)); - - metric_sdk::Provider::SetMeterProvider(provider); -} - void InitMetrics(std::string &name) { auto exporter = otlp_exporter::OtlpGrpcMetricExporterFactory::Create(exporter_options); @@ -152,8 +128,6 @@ int main(int argc, char *argv[]) InitMetrics(name); - // InitMetrics(); - if (example_type == "counter") { foo_library::counter_example(name); From 531ac16ec486618b92929d9f2b937dc221d3e4be Mon Sep 17 00:00:00 2001 From: "Felipe C. Dos Santos" Date: Tue, 8 Apr 2025 11:48:37 +0000 Subject: [PATCH 16/48] Added empty line in the end of a test file and removed comments --- examples/otlp/grpc_metric_main.cc | 4 ---- sdk/test/metrics/aggregation_test.cc | 2 +- 2 files changed, 1 insertion(+), 5 deletions(-) diff --git a/examples/otlp/grpc_metric_main.cc b/examples/otlp/grpc_metric_main.cc index 6d36295dbf..53b7245dd7 100644 --- a/examples/otlp/grpc_metric_main.cc +++ b/examples/otlp/grpc_metric_main.cc @@ -57,10 +57,6 @@ void InitMetrics(std::string &name) auto provider = metric_sdk::MeterProviderFactory::Create(std::move(context)); - // auto provider = opentelemetry::sdk::metrics::MeterProviderFactory::Create(); - - // std::shared_ptr provider(std::move(u_provider)); - // histogram view std::string histogram_name = name + "_histogram"; std::string unit = "unit"; diff --git a/sdk/test/metrics/aggregation_test.cc b/sdk/test/metrics/aggregation_test.cc index 1e3e5c1be8..71753c10e9 100644 --- a/sdk/test/metrics/aggregation_test.cc +++ b/sdk/test/metrics/aggregation_test.cc @@ -316,4 +316,4 @@ TEST(aggregation, Base2ExponentialHistogramAggregation) EXPECT_EQ(diffd_point.positive_buckets_.Get(1), 2); EXPECT_EQ(diffd_point.negative_buckets_.Get(-2), 1); EXPECT_EQ(diffd_point.positive_buckets_.Get(2), 0); -} \ No newline at end of file +} From 9146925a115df2e94cf27fe13507d871e1a3922e Mon Sep 17 00:00:00 2001 From: "Felipe C. Dos Santos" Date: Tue, 8 Apr 2025 18:43:40 +0000 Subject: [PATCH 17/48] Fix windows build errors --- .../sdk/metrics/data/circular_buffer.h | 4 +-- ...base2_exponential_histogram_aggregation.cc | 36 ++++++++----------- sdk/test/metrics/aggregation_test.cc | 4 +-- 3 files changed, 18 insertions(+), 26 deletions(-) diff --git a/sdk/include/opentelemetry/sdk/metrics/data/circular_buffer.h b/sdk/include/opentelemetry/sdk/metrics/data/circular_buffer.h index db4de054f4..91f4810a94 100644 --- a/sdk/include/opentelemetry/sdk/metrics/data/circular_buffer.h +++ b/sdk/include/opentelemetry/sdk/metrics/data/circular_buffer.h @@ -3,8 +3,6 @@ #pragma once -#include -#include #include #include @@ -144,7 +142,7 @@ class AdaptingCircularBufferCounter private: size_t ToBufferIndex(int32_t index) const; - static constexpr int32_t kNullIndex = std::numeric_limits::min(); + static constexpr int32_t kNullIndex = (std::numeric_limits::min)(); // Index of the first populated element, may be kNullIndex if container is empty. int32_t start_index_ = kNullIndex; diff --git a/sdk/src/metrics/aggregation/base2_exponential_histogram_aggregation.cc b/sdk/src/metrics/aggregation/base2_exponential_histogram_aggregation.cc index f11227e5db..d5d72e813f 100644 --- a/sdk/src/metrics/aggregation/base2_exponential_histogram_aggregation.cc +++ b/sdk/src/metrics/aggregation/base2_exponential_histogram_aggregation.cc @@ -1,21 +1,15 @@ // Copyright The OpenTelemetry Authors // SPDX-License-Identifier: Apache-2.0 +#include +#include +#include + #include "opentelemetry/sdk/metrics/aggregation/base2_exponential_histogram_aggregation.h" -#include "opentelemetry/sdk/metrics/aggregation/aggregation.h" #include "opentelemetry/sdk/metrics/data/circular_buffer.h" #include "opentelemetry/sdk/metrics/data/point_data.h" #include "opentelemetry/version.h" -#include -#include -#include -#include -#include -#include - -#include - OPENTELEMETRY_BEGIN_NAMESPACE namespace sdk { @@ -46,8 +40,8 @@ uint32_t GetScaleReduction(int32_t start_index, int32_t end_index, size_t max_bu // { // return 0; // } -// const int32_t start_index = std::min(first.StartIndex(), second.StartIndex()); -// const int32_t end_index = std::max(first.EndIndex(), second.EndIndex()); +// const int32_t start_index = (std::min)(first.StartIndex(), second.StartIndex()); +// const int32_t end_index = (std::max)(first.EndIndex(), second.EndIndex()); // return GetScaleReduction(start_index, end_index, max_buckets); // } @@ -91,8 +85,8 @@ Base2ExponentialHistogramAggregation::Base2ExponentialHistogramAggregation( point_data_.max_buckets_ = ac->max_buckets_; point_data_.scale_ = ac->max_scale_; point_data_.record_min_max_ = ac->record_min_max_; - point_data_.min_ = std::numeric_limits::max(); - point_data_.max_ = std::numeric_limits::min(); + point_data_.min_ = (std::numeric_limits::max)(); + point_data_.max_ = (std::numeric_limits::min)(); indexer_ = Base2ExponentialHistogramIndexer(point_data_.scale_); } @@ -128,8 +122,8 @@ void Base2ExponentialHistogramAggregation::Aggregate( if (record_min_max_) { - point_data_.min_ = std::min(point_data_.min_, value); - point_data_.max_ = std::max(point_data_.max_, value); + point_data_.min_ = (std::min)(point_data_.min_, value); + point_data_.max_ = (std::max)(point_data_.max_, value); } if (value == 0) @@ -159,8 +153,8 @@ void Base2ExponentialHistogramAggregation::AggregateIntoBuckets( const int32_t index = indexer_.ComputeIndex(value); if (!buckets->Increment(index, 1)) { - const int32_t start_index = std::min(buckets->StartIndex(), index); - const int32_t end_index = std::max(buckets->EndIndex(), index); + const int32_t start_index = (std::min)(buckets->StartIndex(), index); + const int32_t end_index = (std::max)(buckets->EndIndex(), index); const uint32_t scale_reduction = GetScaleReduction(start_index, end_index, point_data_.max_buckets_); Downscale(scale_reduction); @@ -205,13 +199,13 @@ std::unique_ptr Base2ExponentialHistogramAggregation::Merge( result_value.count_ = low_res.count_ + high_res.count_; result_value.sum_ = low_res.sum_ + high_res.sum_; result_value.zero_count_ = low_res.zero_count_ + high_res.zero_count_; - result_value.scale_ = std::min(low_res.scale_, high_res.scale_); + result_value.scale_ = (std::min)(low_res.scale_, high_res.scale_); result_value.max_buckets_ = low_res.max_buckets_; result_value.record_min_max_ = low_res.record_min_max_ && high_res.record_min_max_; if (result_value.record_min_max_) { - result_value.min_ = std::min(low_res.min_, high_res.min_); - result_value.max_ = std::max(low_res.max_, high_res.max_); + result_value.min_ = (std::min)(low_res.min_, high_res.min_); + result_value.max_ = (std::max)(low_res.max_, high_res.max_); } if (!high_res.positive_buckets_.Empty()) { diff --git a/sdk/test/metrics/aggregation_test.cc b/sdk/test/metrics/aggregation_test.cc index 71753c10e9..bd4f07392d 100644 --- a/sdk/test/metrics/aggregation_test.cc +++ b/sdk/test/metrics/aggregation_test.cc @@ -241,8 +241,8 @@ TEST(aggregation, Base2ExponentialHistogramAggregation) EXPECT_EQ(histo_point.count_, 0); EXPECT_EQ(histo_point.sum_, 0.0); EXPECT_EQ(histo_point.zero_count_, 0); - EXPECT_EQ(histo_point.min_, std::numeric_limits::max()); - EXPECT_EQ(histo_point.max_, std::numeric_limits::min()); + EXPECT_EQ(histo_point.min_, (std::numeric_limits::max)()); + EXPECT_EQ(histo_point.max_, (std::numeric_limits::min)()); EXPECT_EQ(histo_point.scale_, SCALE0); EXPECT_EQ(histo_point.max_buckets_, MAX_BUCKETS0); ASSERT_TRUE(histo_point.positive_buckets_.Empty()); From ea3190d5d378662f4ded532d4471654abe3f9912 Mon Sep 17 00:00:00 2001 From: etmcdonald Date: Tue, 8 Apr 2025 20:45:02 +0000 Subject: [PATCH 18/48] fix iwyu warnings --- .../common/metrics_foo_library/foo_library.cc | 2 -- examples/metrics_simple/metrics_ostream.cc | 1 + exporters/ostream/src/metric_exporter.cc | 1 + exporters/ostream/test/ostream_metric_test.cc | 1 + exporters/otlp/src/otlp_metric_utils.cc | 1 + .../test/otlp_metrics_serialization_test.cc | 1 + .../base2_exponential_histogram_aggregation.cc | 17 +++++++++++++---- sdk/test/metrics/aggregation_test.cc | 5 +++-- .../metrics/histogram_aggregation_benchmark.cc | 7 +++++-- 9 files changed, 26 insertions(+), 10 deletions(-) diff --git a/examples/common/metrics_foo_library/foo_library.cc b/examples/common/metrics_foo_library/foo_library.cc index 60726bb536..c8e8b17336 100644 --- a/examples/common/metrics_foo_library/foo_library.cc +++ b/examples/common/metrics_foo_library/foo_library.cc @@ -5,7 +5,6 @@ #include #include #include -#include #include #include #include @@ -22,7 +21,6 @@ #include "opentelemetry/metrics/sync_instruments.h" #include "opentelemetry/nostd/shared_ptr.h" #include "opentelemetry/nostd/variant.h" -#include "opentelemetry/sdk/metrics/view/view_factory.h" #include "opentelemetry/semconv/http_metrics.h" #include "opentelemetry/semconv/incubating/container_metrics.h" #include "opentelemetry/semconv/incubating/system_metrics.h" diff --git a/examples/metrics_simple/metrics_ostream.cc b/examples/metrics_simple/metrics_ostream.cc index 31c6199d77..a7b6bcdd08 100644 --- a/examples/metrics_simple/metrics_ostream.cc +++ b/examples/metrics_simple/metrics_ostream.cc @@ -15,6 +15,7 @@ #include "opentelemetry/sdk/metrics/export/periodic_exporting_metric_reader_factory.h" #include "opentelemetry/sdk/metrics/export/periodic_exporting_metric_reader_options.h" #include "opentelemetry/sdk/metrics/instruments.h" +#include "opentelemetry/sdk/metrics/meter_context.h" #include "opentelemetry/sdk/metrics/meter_context_factory.h" #include "opentelemetry/sdk/metrics/meter_provider.h" #include "opentelemetry/sdk/metrics/meter_provider_factory.h" diff --git a/exporters/ostream/src/metric_exporter.cc b/exporters/ostream/src/metric_exporter.cc index 0b3adaf0a0..d4a4aaabe0 100644 --- a/exporters/ostream/src/metric_exporter.cc +++ b/exporters/ostream/src/metric_exporter.cc @@ -24,6 +24,7 @@ #include "opentelemetry/sdk/common/exporter_utils.h" #include "opentelemetry/sdk/common/global_log_handler.h" #include "opentelemetry/sdk/instrumentationscope/instrumentation_scope.h" +#include "opentelemetry/sdk/metrics/data/circular_buffer.h" #include "opentelemetry/sdk/metrics/data/metric_data.h" #include "opentelemetry/sdk/metrics/data/point_data.h" #include "opentelemetry/sdk/metrics/export/metric_producer.h" diff --git a/exporters/ostream/test/ostream_metric_test.cc b/exporters/ostream/test/ostream_metric_test.cc index 59f6b7b9e6..29e6f6aa00 100644 --- a/exporters/ostream/test/ostream_metric_test.cc +++ b/exporters/ostream/test/ostream_metric_test.cc @@ -13,6 +13,7 @@ #include "opentelemetry/nostd/unique_ptr.h" #include "opentelemetry/sdk/common/exporter_utils.h" #include "opentelemetry/sdk/instrumentationscope/instrumentation_scope.h" +#include "opentelemetry/sdk/metrics/data/circular_buffer.h" #include "opentelemetry/sdk/metrics/data/metric_data.h" #include "opentelemetry/sdk/metrics/data/point_data.h" #include "opentelemetry/sdk/metrics/export/metric_producer.h" diff --git a/exporters/otlp/src/otlp_metric_utils.cc b/exporters/otlp/src/otlp_metric_utils.cc index c2e418a2c9..a36da41607 100644 --- a/exporters/otlp/src/otlp_metric_utils.cc +++ b/exporters/otlp/src/otlp_metric_utils.cc @@ -12,6 +12,7 @@ #include "opentelemetry/exporters/otlp/otlp_preferred_temporality.h" #include "opentelemetry/nostd/variant.h" #include "opentelemetry/sdk/instrumentationscope/instrumentation_scope.h" +#include "opentelemetry/sdk/metrics/data/circular_buffer.h" #include "opentelemetry/sdk/metrics/data/metric_data.h" #include "opentelemetry/sdk/metrics/data/point_data.h" #include "opentelemetry/sdk/metrics/export/metric_producer.h" diff --git a/exporters/otlp/test/otlp_metrics_serialization_test.cc b/exporters/otlp/test/otlp_metrics_serialization_test.cc index 3b5df9e04e..855051ad62 100644 --- a/exporters/otlp/test/otlp_metrics_serialization_test.cc +++ b/exporters/otlp/test/otlp_metrics_serialization_test.cc @@ -15,6 +15,7 @@ #include "opentelemetry/nostd/variant.h" #include "opentelemetry/sdk/common/attribute_utils.h" #include "opentelemetry/sdk/instrumentationscope/instrumentation_scope.h" +#include "opentelemetry/sdk/metrics/data/circular_buffer.h" #include "opentelemetry/sdk/metrics/data/metric_data.h" #include "opentelemetry/sdk/metrics/data/point_data.h" #include "opentelemetry/sdk/metrics/export/metric_producer.h" diff --git a/sdk/src/metrics/aggregation/base2_exponential_histogram_aggregation.cc b/sdk/src/metrics/aggregation/base2_exponential_histogram_aggregation.cc index d5d72e813f..c0b5b12116 100644 --- a/sdk/src/metrics/aggregation/base2_exponential_histogram_aggregation.cc +++ b/sdk/src/metrics/aggregation/base2_exponential_histogram_aggregation.cc @@ -1,12 +1,21 @@ // Copyright The OpenTelemetry Authors // SPDX-License-Identifier: Apache-2.0 -#include -#include -#include - #include "opentelemetry/sdk/metrics/aggregation/base2_exponential_histogram_aggregation.h" +#include +#include +#include +#include +#include +#include +#include +#include "opentelemetry/common/spin_lock_mutex.h" +#include "opentelemetry/nostd/variant.h" +#include "opentelemetry/sdk/metrics/aggregation/aggregation.h" +#include "opentelemetry/sdk/metrics/aggregation/aggregation_config.h" +#include "opentelemetry/sdk/metrics/aggregation/base2_exponential_histogram_indexer.h" #include "opentelemetry/sdk/metrics/data/circular_buffer.h" +#include "opentelemetry/sdk/metrics/data/metric_data.h" #include "opentelemetry/sdk/metrics/data/point_data.h" #include "opentelemetry/version.h" diff --git a/sdk/test/metrics/aggregation_test.cc b/sdk/test/metrics/aggregation_test.cc index bd4f07392d..b1476d16ff 100644 --- a/sdk/test/metrics/aggregation_test.cc +++ b/sdk/test/metrics/aggregation_test.cc @@ -1,19 +1,20 @@ // Copyright The OpenTelemetry Authors // SPDX-License-Identifier: Apache-2.0 +#include "opentelemetry/sdk/metrics/aggregation/aggregation.h" #include #include +#include #include #include - #include "opentelemetry/nostd/shared_ptr.h" #include "opentelemetry/nostd/variant.h" -#include "opentelemetry/sdk/metrics/aggregation/aggregation.h" #include "opentelemetry/sdk/metrics/aggregation/aggregation_config.h" #include "opentelemetry/sdk/metrics/aggregation/base2_exponential_histogram_aggregation.h" #include "opentelemetry/sdk/metrics/aggregation/histogram_aggregation.h" #include "opentelemetry/sdk/metrics/aggregation/lastvalue_aggregation.h" #include "opentelemetry/sdk/metrics/aggregation/sum_aggregation.h" +#include "opentelemetry/sdk/metrics/data/circular_buffer.h" #include "opentelemetry/sdk/metrics/data/point_data.h" using namespace opentelemetry::sdk::metrics; diff --git a/sdk/test/metrics/histogram_aggregation_benchmark.cc b/sdk/test/metrics/histogram_aggregation_benchmark.cc index 36295f25b8..1884167297 100644 --- a/sdk/test/metrics/histogram_aggregation_benchmark.cc +++ b/sdk/test/metrics/histogram_aggregation_benchmark.cc @@ -7,25 +7,28 @@ #include #include #include +#include #include #include #include #include "common.h" - #include "opentelemetry/context/context.h" #include "opentelemetry/metrics/meter.h" #include "opentelemetry/metrics/sync_instruments.h" -#include "opentelemetry/nostd/function_ref.h" #include "opentelemetry/nostd/shared_ptr.h" #include "opentelemetry/nostd/variant.h" #include "opentelemetry/sdk/instrumentationscope/instrumentation_scope.h" +#include "opentelemetry/sdk/metrics/aggregation/aggregation_config.h" #include "opentelemetry/sdk/metrics/data/metric_data.h" #include "opentelemetry/sdk/metrics/data/point_data.h" #include "opentelemetry/sdk/metrics/export/metric_producer.h" +#include "opentelemetry/sdk/metrics/instruments.h" #include "opentelemetry/sdk/metrics/meter_provider.h" #include "opentelemetry/sdk/metrics/metric_reader.h" #include "opentelemetry/sdk/metrics/push_metric_exporter.h" #include "opentelemetry/sdk/metrics/view/instrument_selector.h" +#include "opentelemetry/sdk/metrics/view/meter_selector.h" +#include "opentelemetry/sdk/metrics/view/view.h" #include "opentelemetry/sdk/metrics/view/view_registry.h" using namespace opentelemetry; From 0fc7071f5be0388ae1142b8de8dd6f5da911a90a Mon Sep 17 00:00:00 2001 From: ethandmd Date: Wed, 9 Apr 2025 17:21:35 +0000 Subject: [PATCH 19/48] add comment to trigger pr update --- sdk/test/metrics/aggregation_test.cc | 1 + 1 file changed, 1 insertion(+) diff --git a/sdk/test/metrics/aggregation_test.cc b/sdk/test/metrics/aggregation_test.cc index b1476d16ff..f60003f55d 100644 --- a/sdk/test/metrics/aggregation_test.cc +++ b/sdk/test/metrics/aggregation_test.cc @@ -296,6 +296,7 @@ TEST(aggregation, Base2ExponentialHistogramAggregation) EXPECT_EQ(scale1_point.min_, 0.0); EXPECT_EQ(scale1_point.max_, 3.5); + // Merge test auto merged = scale0_aggr.Merge(scale1_aggr); auto merged_point = nostd::get(merged->ToPoint()); EXPECT_EQ(merged_point.count_, 8); From 8f2130c10e5dc85506298ee5f4a5b24aac8561c2 Mon Sep 17 00:00:00 2001 From: ethandmd Date: Wed, 9 Apr 2025 18:21:42 +0000 Subject: [PATCH 20/48] fix additional iwyu warnings --- sdk/include/opentelemetry/sdk/metrics/aggregation/aggregation.h | 2 +- sdk/test/metrics/aggregation_test.cc | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/sdk/include/opentelemetry/sdk/metrics/aggregation/aggregation.h b/sdk/include/opentelemetry/sdk/metrics/aggregation/aggregation.h index a4ec9175c5..c932d5069c 100644 --- a/sdk/include/opentelemetry/sdk/metrics/aggregation/aggregation.h +++ b/sdk/include/opentelemetry/sdk/metrics/aggregation/aggregation.h @@ -3,8 +3,8 @@ #pragma once +#include #include - #include "opentelemetry/sdk/metrics/data/metric_data.h" #include "opentelemetry/version.h" diff --git a/sdk/test/metrics/aggregation_test.cc b/sdk/test/metrics/aggregation_test.cc index f60003f55d..c68a61f207 100644 --- a/sdk/test/metrics/aggregation_test.cc +++ b/sdk/test/metrics/aggregation_test.cc @@ -7,7 +7,6 @@ #include #include #include -#include "opentelemetry/nostd/shared_ptr.h" #include "opentelemetry/nostd/variant.h" #include "opentelemetry/sdk/metrics/aggregation/aggregation_config.h" #include "opentelemetry/sdk/metrics/aggregation/base2_exponential_histogram_aggregation.h" From de046031209abdd9aafbd9c0e571084f3496a5b6 Mon Sep 17 00:00:00 2001 From: "Felipe C. Dos Santos" Date: Thu, 10 Apr 2025 00:20:26 +0000 Subject: [PATCH 21/48] Fix iwyu warning --- .../aggregation/base2_exponential_histogram_aggregation.h | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/sdk/include/opentelemetry/sdk/metrics/aggregation/base2_exponential_histogram_aggregation.h b/sdk/include/opentelemetry/sdk/metrics/aggregation/base2_exponential_histogram_aggregation.h index 4a4dc0d165..6b3e55114f 100644 --- a/sdk/include/opentelemetry/sdk/metrics/aggregation/base2_exponential_histogram_aggregation.h +++ b/sdk/include/opentelemetry/sdk/metrics/aggregation/base2_exponential_histogram_aggregation.h @@ -3,16 +3,17 @@ #pragma once +#include #include -#include #include "opentelemetry/common/spin_lock_mutex.h" -#include "opentelemetry/sdk/metrics/aggregation/aggregation.h" #include "opentelemetry/sdk/metrics/aggregation/aggregation_config.h" +#include "opentelemetry/sdk/metrics/aggregation/aggregation.h" #include "opentelemetry/sdk/metrics/aggregation/base2_exponential_histogram_indexer.h" #include "opentelemetry/sdk/metrics/data/circular_buffer.h" #include "opentelemetry/sdk/metrics/data/metric_data.h" #include "opentelemetry/sdk/metrics/data/point_data.h" +#include "opentelemetry/version.h" OPENTELEMETRY_BEGIN_NAMESPACE namespace sdk From 8cf15c49d81a33addcad82f3fa570f7aa18db1b6 Mon Sep 17 00:00:00 2001 From: "Felipe C. Dos Santos" Date: Thu, 10 Apr 2025 01:14:56 +0000 Subject: [PATCH 22/48] Fix include order in base2_exponential_histogram_aggregation.h --- .../aggregation/base2_exponential_histogram_aggregation.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sdk/include/opentelemetry/sdk/metrics/aggregation/base2_exponential_histogram_aggregation.h b/sdk/include/opentelemetry/sdk/metrics/aggregation/base2_exponential_histogram_aggregation.h index 6b3e55114f..0ff4667092 100644 --- a/sdk/include/opentelemetry/sdk/metrics/aggregation/base2_exponential_histogram_aggregation.h +++ b/sdk/include/opentelemetry/sdk/metrics/aggregation/base2_exponential_histogram_aggregation.h @@ -7,8 +7,8 @@ #include #include "opentelemetry/common/spin_lock_mutex.h" -#include "opentelemetry/sdk/metrics/aggregation/aggregation_config.h" #include "opentelemetry/sdk/metrics/aggregation/aggregation.h" +#include "opentelemetry/sdk/metrics/aggregation/aggregation_config.h" #include "opentelemetry/sdk/metrics/aggregation/base2_exponential_histogram_indexer.h" #include "opentelemetry/sdk/metrics/data/circular_buffer.h" #include "opentelemetry/sdk/metrics/data/metric_data.h" From 84c09d1ca40d3234a642a7594178c2e219ec28fd Mon Sep 17 00:00:00 2001 From: etmcdonald Date: Tue, 15 Apr 2025 18:38:32 +0000 Subject: [PATCH 23/48] Add kcumulative and kdelta aggregation temporality test with collect calls for base2 histogram aggregation --- .../sync_metric_storage_histogram_test.cc | 146 ++++++++++++++++++ 1 file changed, 146 insertions(+) diff --git a/sdk/test/metrics/sync_metric_storage_histogram_test.cc b/sdk/test/metrics/sync_metric_storage_histogram_test.cc index ecaf527942..8711774e73 100644 --- a/sdk/test/metrics/sync_metric_storage_histogram_test.cc +++ b/sdk/test/metrics/sync_metric_storage_histogram_test.cc @@ -323,6 +323,152 @@ TEST_P(WritableMetricStorageHistogramTestFixture, DoubleHistogram) EXPECT_EQ(count_attributes, 2); // GET and PUT } INSTANTIATE_TEST_SUITE_P(WritableMetricStorageHistogramTestDouble, + WritableMetricStorageHistogramTestFixture, + ::testing::Values(AggregationTemporality::kCumulative, + AggregationTemporality::kDelta)); +TEST_P(WritableMetricStorageHistogramTestFixture, Base2ExponentialDoubleHistogram) +{ + AggregationTemporality temporality = GetParam(); + auto sdk_start_ts = std::chrono::system_clock::now(); + double expected_total_get_requests = 0; + double expected_total_put_requests = 0; + InstrumentDescriptor instr_desc = {"name", "desc", "1unit", InstrumentType::kHistogram, + InstrumentValueType::kDouble}; + std::map attributes_get = {{"RequestType", "GET"}}; + std::map attributes_put = {{"RequestType", "PUT"}}; + + std::unique_ptr default_attributes_processor{ + new DefaultAttributesProcessor{}}; + opentelemetry::sdk::metrics::SyncMetricStorage storage( + instr_desc, AggregationType::kBase2ExponentialHistogram, default_attributes_processor.get(), +#ifdef ENABLE_METRICS_EXEMPLAR_PREVIEW + ExemplarFilterType::kAlwaysOff, ExemplarReservoir::GetNoExemplarReservoir(), +#endif + nullptr); + + storage.RecordDouble(10.0, + KeyValueIterableView>(attributes_get), + opentelemetry::context::Context{}); + expected_total_get_requests += 10; + + storage.RecordDouble(30.0, + KeyValueIterableView>(attributes_put), + opentelemetry::context::Context{}); + expected_total_put_requests += 30; + + storage.RecordDouble(20.0, + KeyValueIterableView>(attributes_get), + opentelemetry::context::Context{}); + expected_total_get_requests += 20; + + storage.RecordDouble(40.0, + KeyValueIterableView>(attributes_put), + opentelemetry::context::Context{}); + expected_total_put_requests += 40; + + std::shared_ptr collector(new MockCollectorHandle(temporality)); + std::vector> collectors; + collectors.push_back(collector); + + // Some computation here + auto collection_ts = std::chrono::system_clock::now(); + size_t count_attributes = 0; + storage.Collect( + collector.get(), collectors, sdk_start_ts, collection_ts, [&](const MetricData &metric_data) { + for (const auto &data_attr : metric_data.point_data_attr_) + { + const auto &data = opentelemetry::nostd::get(data_attr.point_data); + if (opentelemetry::nostd::get( + data_attr.attributes.find("RequestType")->second) == "GET") + { + EXPECT_EQ(data.sum_, expected_total_get_requests); + count_attributes++; + } + else if (opentelemetry::nostd::get( + data_attr.attributes.find("RequestType")->second) == "PUT") + { + EXPECT_EQ(data.sum_, expected_total_put_requests); + count_attributes++; + } + } + return true; + }); + EXPECT_EQ(count_attributes, 2); // GET and PUT + + // In case of delta temporarily, subsequent collection would contain new data points, so resetting + // the counts + if (temporality == AggregationTemporality::kDelta) + { + expected_total_get_requests = 0; + expected_total_put_requests = 0; + } + + // collect one more time. + collection_ts = std::chrono::system_clock::now(); + count_attributes = 0; + storage.Collect( + collector.get(), collectors, sdk_start_ts, collection_ts, [&](const MetricData &metric_data) { + if (temporality == AggregationTemporality::kCumulative) + { + EXPECT_EQ(metric_data.start_ts, sdk_start_ts); + } + for (const auto &data_attr : metric_data.point_data_attr_) + { + const auto &data = opentelemetry::nostd::get(data_attr.point_data); + if (opentelemetry::nostd::get( + data_attr.attributes.find("RequestType")->second) == "GET") + { + count_attributes++; + EXPECT_EQ(data.sum_, expected_total_get_requests); + } + else if (opentelemetry::nostd::get( + data_attr.attributes.find("RequestType")->second) == "PUT") + { + count_attributes++; + EXPECT_EQ(data.sum_, expected_total_put_requests); + } + } + return true; + }); + if (temporality == AggregationTemporality::kCumulative) + { + EXPECT_EQ(count_attributes, 2); // GET AND PUT + } + + storage.RecordDouble(50.0, + KeyValueIterableView>(attributes_get), + opentelemetry::context::Context{}); + expected_total_get_requests += 50; + storage.RecordDouble(40.0, + KeyValueIterableView>(attributes_put), + opentelemetry::context::Context{}); + expected_total_put_requests += 40; + + collection_ts = std::chrono::system_clock::now(); + count_attributes = 0; + storage.Collect( + collector.get(), collectors, sdk_start_ts, collection_ts, [&](const MetricData &metric_data) { + for (const auto &data_attr : metric_data.point_data_attr_) + { + const auto &data = opentelemetry::nostd::get(data_attr.point_data); + if (opentelemetry::nostd::get( + data_attr.attributes.find("RequestType")->second) == "GET") + { + EXPECT_EQ(data.sum_, expected_total_get_requests); + count_attributes++; + } + else if (opentelemetry::nostd::get( + data_attr.attributes.find("RequestType")->second) == "PUT") + { + EXPECT_EQ(data.sum_, expected_total_put_requests); + count_attributes++; + } + } + return true; + }); + EXPECT_EQ(count_attributes, 2); // GET and PUT +} +INSTANTIATE_TEST_SUITE_P(WritableMetricStorageHistogramTestBase2ExponentialDouble, WritableMetricStorageHistogramTestFixture, ::testing::Values(AggregationTemporality::kCumulative, AggregationTemporality::kDelta)); From b9e5df6ad124d03ea2916b1973534a9461dce87e Mon Sep 17 00:00:00 2001 From: etmcdonald Date: Tue, 15 Apr 2025 18:55:37 +0000 Subject: [PATCH 24/48] add prelim bucketing checks --- .../sync_metric_storage_histogram_test.cc | 27 ++++++++++++++----- 1 file changed, 21 insertions(+), 6 deletions(-) diff --git a/sdk/test/metrics/sync_metric_storage_histogram_test.cc b/sdk/test/metrics/sync_metric_storage_histogram_test.cc index 8711774e73..14935c6393 100644 --- a/sdk/test/metrics/sync_metric_storage_histogram_test.cc +++ b/sdk/test/metrics/sync_metric_storage_histogram_test.cc @@ -322,10 +322,12 @@ TEST_P(WritableMetricStorageHistogramTestFixture, DoubleHistogram) }); EXPECT_EQ(count_attributes, 2); // GET and PUT } + INSTANTIATE_TEST_SUITE_P(WritableMetricStorageHistogramTestDouble, - WritableMetricStorageHistogramTestFixture, - ::testing::Values(AggregationTemporality::kCumulative, - AggregationTemporality::kDelta)); + WritableMetricStorageHistogramTestFixture, + ::testing::Values(AggregationTemporality::kCumulative, + AggregationTemporality::kDelta)); + TEST_P(WritableMetricStorageHistogramTestFixture, Base2ExponentialDoubleHistogram) { AggregationTemporality temporality = GetParam(); @@ -377,17 +379,28 @@ TEST_P(WritableMetricStorageHistogramTestFixture, Base2ExponentialDoubleHistogra collector.get(), collectors, sdk_start_ts, collection_ts, [&](const MetricData &metric_data) { for (const auto &data_attr : metric_data.point_data_attr_) { - const auto &data = opentelemetry::nostd::get(data_attr.point_data); + const auto &data = + opentelemetry::nostd::get(data_attr.point_data); if (opentelemetry::nostd::get( data_attr.attributes.find("RequestType")->second) == "GET") { EXPECT_EQ(data.sum_, expected_total_get_requests); + EXPECT_EQ(data.count_, 2); + EXPECT_EQ(data.min_, 10); + EXPECT_EQ(data.max_, 20); + EXPECT_EQ(data.positive_buckets_.Empty(), false); + EXPECT_EQ(data.negative_buckets_.Empty(), true); count_attributes++; } else if (opentelemetry::nostd::get( data_attr.attributes.find("RequestType")->second) == "PUT") { EXPECT_EQ(data.sum_, expected_total_put_requests); + EXPECT_EQ(data.count_, 2); + EXPECT_EQ(data.min_, 30); + EXPECT_EQ(data.max_, 40); + EXPECT_EQ(data.positive_buckets_.Empty(), false); + EXPECT_EQ(data.negative_buckets_.Empty(), true); count_attributes++; } } @@ -414,7 +427,8 @@ TEST_P(WritableMetricStorageHistogramTestFixture, Base2ExponentialDoubleHistogra } for (const auto &data_attr : metric_data.point_data_attr_) { - const auto &data = opentelemetry::nostd::get(data_attr.point_data); + const auto &data = + opentelemetry::nostd::get(data_attr.point_data); if (opentelemetry::nostd::get( data_attr.attributes.find("RequestType")->second) == "GET") { @@ -450,7 +464,8 @@ TEST_P(WritableMetricStorageHistogramTestFixture, Base2ExponentialDoubleHistogra collector.get(), collectors, sdk_start_ts, collection_ts, [&](const MetricData &metric_data) { for (const auto &data_attr : metric_data.point_data_attr_) { - const auto &data = opentelemetry::nostd::get(data_attr.point_data); + const auto &data = + opentelemetry::nostd::get(data_attr.point_data); if (opentelemetry::nostd::get( data_attr.attributes.find("RequestType")->second) == "GET") { From e4daa7b770fd2bc7f9b9183029a9f7fa9ade35c8 Mon Sep 17 00:00:00 2001 From: "Felipe C. Dos Santos" Date: Wed, 16 Apr 2025 02:24:56 +0000 Subject: [PATCH 25/48] Tested the exp2 constructor with point data --- sdk/test/metrics/aggregation_test.cc | 21 ++++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/sdk/test/metrics/aggregation_test.cc b/sdk/test/metrics/aggregation_test.cc index c68a61f207..455048cd32 100644 --- a/sdk/test/metrics/aggregation_test.cc +++ b/sdk/test/metrics/aggregation_test.cc @@ -246,9 +246,28 @@ TEST(aggregation, Base2ExponentialHistogramAggregation) EXPECT_EQ(histo_point.scale_, SCALE0); EXPECT_EQ(histo_point.max_buckets_, MAX_BUCKETS0); ASSERT_TRUE(histo_point.positive_buckets_.Empty()); + ASSERT_TRUE(histo_point.negative_buckets_.Empty()); + + // Create a new aggreagte based in point data + { + auto point_data = histo_point; + Base2ExponentialHistogramAggregation scale0_aggr2(point_data); + scale0_aggr2.Aggregate(0.0, {}); + + auto histo_point2 = nostd::get(point); + EXPECT_EQ(histo_point2.count_, 0); + EXPECT_EQ(histo_point2.sum_, 0.0); + EXPECT_EQ(histo_point2.zero_count_, 0); + EXPECT_EQ(histo_point2.min_, (std::numeric_limits::max)()); + EXPECT_EQ(histo_point2.max_, (std::numeric_limits::min)()); + EXPECT_EQ(histo_point2.scale_, SCALE0); + EXPECT_EQ(histo_point2.max_buckets_, MAX_BUCKETS0); + ASSERT_TRUE(histo_point2.positive_buckets_.Empty()); + ASSERT_TRUE(histo_point2.negative_buckets_.Empty()); + } // zero point - scale0_aggr.Aggregate(0.0, {}); + scale0_aggr.Aggregate(static_cast(0.0), {}); histo_point = nostd::get(scale0_aggr.ToPoint()); EXPECT_EQ(histo_point.count_, 1); EXPECT_EQ(histo_point.zero_count_, 1); From c7ca65047d3628772a0cbcc5024fba884b10683e Mon Sep 17 00:00:00 2001 From: "Felipe C. Dos Santos" Date: Wed, 16 Apr 2025 13:32:10 +0000 Subject: [PATCH 26/48] Fix missing include in sync histogram test --- sdk/test/metrics/sync_metric_storage_histogram_test.cc | 1 + 1 file changed, 1 insertion(+) diff --git a/sdk/test/metrics/sync_metric_storage_histogram_test.cc b/sdk/test/metrics/sync_metric_storage_histogram_test.cc index 14935c6393..50ac1b75d9 100644 --- a/sdk/test/metrics/sync_metric_storage_histogram_test.cc +++ b/sdk/test/metrics/sync_metric_storage_histogram_test.cc @@ -18,6 +18,7 @@ #include "opentelemetry/nostd/function_ref.h" #include "opentelemetry/nostd/span.h" #include "opentelemetry/nostd/variant.h" +#include "opentelemetry/sdk/metrics/data/circular_buffer.h" #include "opentelemetry/sdk/metrics/data/metric_data.h" #include "opentelemetry/sdk/metrics/data/point_data.h" #include "opentelemetry/sdk/metrics/instruments.h" From 81ea99739f1b5e6a5865e8b79a7687dd41412a1a Mon Sep 17 00:00:00 2001 From: etmcdonald Date: Thu, 17 Apr 2025 07:09:26 +0000 Subject: [PATCH 27/48] move diff and merge to use union of all buckets approach --- ...base2_exponential_histogram_aggregation.cc | 162 ++++++++++++------ 1 file changed, 108 insertions(+), 54 deletions(-) diff --git a/sdk/src/metrics/aggregation/base2_exponential_histogram_aggregation.cc b/sdk/src/metrics/aggregation/base2_exponential_histogram_aggregation.cc index c0b5b12116..39ced15c83 100644 --- a/sdk/src/metrics/aggregation/base2_exponential_histogram_aggregation.cc +++ b/sdk/src/metrics/aggregation/base2_exponential_histogram_aggregation.cc @@ -7,6 +7,7 @@ #include #include #include +#include #include #include #include "opentelemetry/common/spin_lock_mutex.h" @@ -186,6 +187,41 @@ void Base2ExponentialHistogramAggregation::Downscale(uint32_t by) noexcept indexer_ = Base2ExponentialHistogramIndexer(point_data_.scale_); } +// Merge A and B into a new circular buffer C. +// Caller must ensure that A and B are used as buckets at the same scale. +AdaptingCircularBufferCounter MergeBuckets(size_t max_buckets, const AdaptingCircularBufferCounter &A, const AdaptingCircularBufferCounter &B) +{ + AdaptingCircularBufferCounter C = AdaptingCircularBufferCounter(max_buckets); + C.Clear(); + + if (A.Empty() && B.Empty()) + { + return C; + } + if (A.Empty()) + { + return B; + } + if (B.Empty()) + { + return A; + } + + auto min_index = (std::min)(A.StartIndex(), B.StartIndex()); + auto max_index = (std::max)(A.EndIndex(), B.EndIndex()); + + for (int i = min_index; i <= max_index; i++) + { + auto count = A.Get(i) + B.Get(i); + if (count > 0) + { + C.Increment(i, count); + } + } + + return C; +} + std::unique_ptr Base2ExponentialHistogramAggregation::Merge( const Aggregation &delta) const noexcept { @@ -197,44 +233,50 @@ std::unique_ptr Base2ExponentialHistogramAggregation::Merge( auto high_res = left.scale_ < right.scale_ ? right : left; auto scale_reduction = high_res.scale_ - low_res.scale_; - if (scale_reduction > 0) - { - DownscaleBuckets(&high_res.positive_buckets_, scale_reduction); - DownscaleBuckets(&high_res.negative_buckets_, scale_reduction); - high_res.scale_ -= scale_reduction; - } - Base2ExponentialHistogramPointData result_value; result_value.count_ = low_res.count_ + high_res.count_; result_value.sum_ = low_res.sum_ + high_res.sum_; result_value.zero_count_ = low_res.zero_count_ + high_res.zero_count_; result_value.scale_ = (std::min)(low_res.scale_, high_res.scale_); - result_value.max_buckets_ = low_res.max_buckets_; + result_value.max_buckets_ = low_res.max_buckets_ >= high_res.max_buckets_ + ? low_res.max_buckets_ + : high_res.max_buckets_; result_value.record_min_max_ = low_res.record_min_max_ && high_res.record_min_max_; if (result_value.record_min_max_) { result_value.min_ = (std::min)(low_res.min_, high_res.min_); result_value.max_ = (std::max)(low_res.max_, high_res.max_); } - if (!high_res.positive_buckets_.Empty()) + + if (scale_reduction > 0) { - for (int i = high_res.positive_buckets_.StartIndex(); - i <= high_res.positive_buckets_.EndIndex(); i++) - { - low_res.positive_buckets_.Increment(i, high_res.positive_buckets_.Get(i)); - } + DownscaleBuckets(&high_res.positive_buckets_, scale_reduction); + DownscaleBuckets(&high_res.negative_buckets_, scale_reduction); + high_res.scale_ -= scale_reduction; } - result_value.positive_buckets_ = std::move(low_res.positive_buckets_); - if (!high_res.negative_buckets_.Empty()) + auto pos_min_index = (std::min)(low_res.positive_buckets_.StartIndex(), high_res.positive_buckets_.StartIndex()); + auto pos_max_index = (std::max)(low_res.positive_buckets_.EndIndex(), high_res.positive_buckets_.EndIndex()); + auto neg_min_index = (std::min)(low_res.negative_buckets_.StartIndex(), high_res.negative_buckets_.StartIndex()); + auto neg_max_index = (std::max)(low_res.negative_buckets_.EndIndex(), high_res.negative_buckets_.EndIndex()); + + if (pos_max_index > pos_min_index + result_value.max_buckets_ || neg_max_index > neg_min_index + result_value.max_buckets_) { - for (int i = high_res.negative_buckets_.StartIndex(); - i <= high_res.negative_buckets_.EndIndex(); i++) - { - low_res.negative_buckets_.Increment(i, high_res.negative_buckets_.Get(i)); - } + // We need to downscale the buckets to fit into the new max_buckets_. + const uint32_t scale_reduction = GetScaleReduction(pos_min_index, pos_max_index, result_value.max_buckets_); + DownscaleBuckets(&low_res.positive_buckets_, scale_reduction); + DownscaleBuckets(&high_res.positive_buckets_, scale_reduction); + DownscaleBuckets(&low_res.negative_buckets_, scale_reduction); + DownscaleBuckets(&high_res.negative_buckets_, scale_reduction); + low_res.scale_ -= scale_reduction; + high_res.scale_ -= scale_reduction; + result_value.scale_ -= scale_reduction; } - result_value.negative_buckets_ = std::move(low_res.negative_buckets_); + + result_value.positive_buckets_ = MergeBuckets(result_value.max_buckets_, low_res.positive_buckets_, + high_res.positive_buckets_); + result_value.negative_buckets_ = MergeBuckets(result_value.max_buckets_, low_res.negative_buckets_, + high_res.negative_buckets_); return std::unique_ptr{ new Base2ExponentialHistogramAggregation(std::move(result_value))}; @@ -258,58 +300,70 @@ std::unique_ptr Base2ExponentialHistogramAggregation::Diff( high_res.scale_ -= scale_reduction; } + auto pos_min_index = (std::min)(left.positive_buckets_.StartIndex(), right.positive_buckets_.StartIndex()); + auto pos_max_index = (std::max)(left.positive_buckets_.EndIndex(), right.positive_buckets_.EndIndex()); + auto neg_min_index = (std::min)(left.negative_buckets_.StartIndex(), right.negative_buckets_.StartIndex()); + auto neg_max_index = (std::max)(left.negative_buckets_.EndIndex(), right.negative_buckets_.EndIndex()); + + if (pos_max_index > pos_min_index + low_res.max_buckets_ || neg_max_index > neg_min_index + low_res.max_buckets_) + { + // We need to downscale the buckets to fit into the new max_buckets_. + const uint32_t scale_reduction = GetScaleReduction(pos_min_index, pos_max_index, low_res.max_buckets_); + DownscaleBuckets(&left.positive_buckets_, scale_reduction); + DownscaleBuckets(&right.positive_buckets_, scale_reduction); + DownscaleBuckets(&left.negative_buckets_, scale_reduction); + DownscaleBuckets(&right.negative_buckets_, scale_reduction); + left.scale_ -= scale_reduction; + right.scale_ -= scale_reduction; + } + Base2ExponentialHistogramPointData result_value; result_value.scale_ = low_res.scale_; result_value.max_buckets_ = low_res.max_buckets_; result_value.record_min_max_ = false; // caution for underflow - result_value.count_ = (left.count_ >= right.count_) ? (left.count_ - right.count_) : 0; - result_value.sum_ = (left.sum_ >= right.sum_) ? (left.sum_ - right.sum_) : 0.0; + // expect right.{sum, count} >= left.{sum, count} since metric points should be monotonically increasing + result_value.count_ = (right.count_ >= left.count_) ? (right.count_ - left.count_) : 0.0; + result_value.sum_ = (right.sum_ >= left.sum_) ? (right.sum_ - left.sum_) : 0.0; result_value.zero_count_ = - (left.zero_count_ >= right.zero_count_) ? (left.zero_count_ - right.zero_count_) : 0; - if (!high_res.positive_buckets_.Empty()) + (right.zero_count_ >= left.zero_count_) ? (right.zero_count_ - left.zero_count_) : 0; + result_value.positive_buckets_ = AdaptingCircularBufferCounter{right.max_buckets_}; + result_value.negative_buckets_ = AdaptingCircularBufferCounter{right.max_buckets_}; + + if (!left.positive_buckets_.Empty() && !right.positive_buckets_.Empty()) { - for (int i = high_res.positive_buckets_.StartIndex(); - i <= high_res.positive_buckets_.EndIndex(); i++) + for (auto i = pos_min_index; i <= pos_max_index; i++) { - low_res.positive_buckets_.Increment(i, 0 - high_res.positive_buckets_.Get(i)); + auto l_cnt = left.positive_buckets_.Get(i); + auto r_cnt = right.positive_buckets_.Get(i); + // expect right >= left since metric points should be monotonically increasing + auto delta = (std::max)((uint64_t)(0), r_cnt - l_cnt); + if (l_cnt > 0) + { + result_value.positive_buckets_.Increment(i, delta); + } } } - result_value.positive_buckets_ = std::move(low_res.positive_buckets_); - if (!high_res.negative_buckets_.Empty()) + if (!left.negative_buckets_.Empty() && !right.negative_buckets_.Empty()) { - for (int i = high_res.negative_buckets_.StartIndex(); - i <= high_res.negative_buckets_.EndIndex(); i++) + for (auto i = neg_min_index; i <= neg_max_index; i++) { - low_res.negative_buckets_.Increment(i, 0 - high_res.negative_buckets_.Get(i)); + auto l_cnt = left.negative_buckets_.Get(i); + auto r_cnt = right.negative_buckets_.Get(i); + // expect right >= left since metric points should be monotonically increasing + auto delta = (std::max)((uint64_t)(0), r_cnt - l_cnt); + if (delta > 0) + { + result_value.negative_buckets_.Increment(i, delta); + } } } - result_value.negative_buckets_ = std::move(low_res.negative_buckets_); return std::unique_ptr{ new Base2ExponentialHistogramAggregation(std::move(result_value))}; } -// std::unique_ptr Base2ExponentialHistogramAggregation::Diff( -// const Aggregation &next) const noexcept -// { -// auto curr_value = nostd::get(ToPoint()); -// auto next_value = nostd::get( -// (static_cast(next).ToPoint())); - -// Base2ExponentialHistogramPointData result_value; -// result_value.scale_ = curr_value.scale_; -// result_value.max_buckets_ = curr_value.max_buckets_; -// result_value.record_min_max_ = false; -// result_value.count_ = next_value.count_ - curr_value.count_; -// result_value.sum_ = next_value.sum_ - curr_value.sum_; -// result_value.zero_count_ = next_value.zero_count_ - curr_value.zero_count_; - -// return std::unique_ptr{ -// new Base2ExponentialHistogramAggregation(std::move(result_value))}; -// } - PointType Base2ExponentialHistogramAggregation::ToPoint() const noexcept { const std::lock_guard locked(lock_); From 055d3314b7215d7a84be7a2ed6ee2f6ba9fd10f4 Mon Sep 17 00:00:00 2001 From: etmcdonald Date: Thu, 17 Apr 2025 07:11:35 +0000 Subject: [PATCH 28/48] make get a const method --- sdk/include/opentelemetry/sdk/metrics/data/circular_buffer.h | 2 +- sdk/src/metrics/data/circular_buffer.cc | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/sdk/include/opentelemetry/sdk/metrics/data/circular_buffer.h b/sdk/include/opentelemetry/sdk/metrics/data/circular_buffer.h index 91f4810a94..01588f71d4 100644 --- a/sdk/include/opentelemetry/sdk/metrics/data/circular_buffer.h +++ b/sdk/include/opentelemetry/sdk/metrics/data/circular_buffer.h @@ -137,7 +137,7 @@ class AdaptingCircularBufferCounter * * @return the number of recordings for the index, or 0 if the index is out of bounds. */ - uint64_t Get(int32_t index); + uint64_t Get(int32_t index) const; private: size_t ToBufferIndex(int32_t index) const; diff --git a/sdk/src/metrics/data/circular_buffer.cc b/sdk/src/metrics/data/circular_buffer.cc index 007bedef6d..b6aab91a05 100644 --- a/sdk/src/metrics/data/circular_buffer.cc +++ b/sdk/src/metrics/data/circular_buffer.cc @@ -168,7 +168,7 @@ bool AdaptingCircularBufferCounter::Increment(int32_t index, uint64_t delta) return true; } -uint64_t AdaptingCircularBufferCounter::Get(int32_t index) +uint64_t AdaptingCircularBufferCounter::Get(int32_t index) const { if (index < start_index_ || index > end_index_) { From e5e7b20e47c5a4c6930e09e0240204a2f8d5b85c Mon Sep 17 00:00:00 2001 From: etmcdonald Date: Thu, 17 Apr 2025 07:13:59 +0000 Subject: [PATCH 29/48] update base2 expo hiso diff test --- sdk/test/metrics/aggregation_test.cc | 26 ++++++++++++++++++-------- 1 file changed, 18 insertions(+), 8 deletions(-) diff --git a/sdk/test/metrics/aggregation_test.cc b/sdk/test/metrics/aggregation_test.cc index 455048cd32..3624f31f66 100644 --- a/sdk/test/metrics/aggregation_test.cc +++ b/sdk/test/metrics/aggregation_test.cc @@ -327,13 +327,23 @@ TEST(aggregation, Base2ExponentialHistogramAggregation) EXPECT_EQ(merged_point.negative_buckets_.Get(-2), 1); EXPECT_EQ(merged_point.positive_buckets_.Get(2), 0); - auto diffd = merged->Diff(scale1_aggr); + // Diff test + Base2ExponentialHistogramAggregation scale2_aggr(&scale1_config); + Base2ExponentialHistogramAggregation scale3_aggr(&scale1_config); + scale2_aggr.Aggregate(2.0, {}); + scale2_aggr.Aggregate(4.0, {}); + scale2_aggr.Aggregate(2.5, {}); + + scale3_aggr.Aggregate(2.0, {}); + scale3_aggr.Aggregate(2.3, {}); + scale3_aggr.Aggregate(2.5, {}); + scale3_aggr.Aggregate(4.0, {}); + + auto diffd = scale2_aggr.Diff(scale3_aggr); auto diffd_point = nostd::get(diffd->ToPoint()); - EXPECT_EQ(diffd_point.count_, 4); - EXPECT_EQ(diffd_point.sum_, 6.2); - EXPECT_EQ(diffd_point.zero_count_, 1); - EXPECT_EQ(diffd_point.scale_, 0); - EXPECT_EQ(diffd_point.positive_buckets_.Get(1), 2); - EXPECT_EQ(diffd_point.negative_buckets_.Get(-2), 1); - EXPECT_EQ(diffd_point.positive_buckets_.Get(2), 0); + EXPECT_EQ(diffd_point.count_, 1); + EXPECT_NEAR(diffd_point.sum_, 2.3, 1e-9); + EXPECT_EQ(diffd_point.zero_count_, 0); + EXPECT_EQ(diffd_point.scale_, 1); + EXPECT_EQ(diffd_point.positive_buckets_.Get(2), 1); } From 556eb8901d233c97396d44270398db524d3323b0 Mon Sep 17 00:00:00 2001 From: etmcdonald Date: Thu, 17 Apr 2025 07:14:11 +0000 Subject: [PATCH 30/48] add base2 expo histo test with sync storage collect --- .../sync_metric_storage_histogram_test.cc | 55 ++++++++++++++++++- 1 file changed, 52 insertions(+), 3 deletions(-) diff --git a/sdk/test/metrics/sync_metric_storage_histogram_test.cc b/sdk/test/metrics/sync_metric_storage_histogram_test.cc index 14935c6393..5f684ed4cb 100644 --- a/sdk/test/metrics/sync_metric_storage_histogram_test.cc +++ b/sdk/test/metrics/sync_metric_storage_histogram_test.cc @@ -388,8 +388,11 @@ TEST_P(WritableMetricStorageHistogramTestFixture, Base2ExponentialDoubleHistogra EXPECT_EQ(data.count_, 2); EXPECT_EQ(data.min_, 10); EXPECT_EQ(data.max_, 20); - EXPECT_EQ(data.positive_buckets_.Empty(), false); EXPECT_EQ(data.negative_buckets_.Empty(), true); + auto start_index = data.positive_buckets_.StartIndex(); + auto end_index = data.positive_buckets_.EndIndex(); + EXPECT_EQ(data.positive_buckets_.Get(start_index), 1); + EXPECT_EQ(data.positive_buckets_.Get(end_index), 1); count_attributes++; } else if (opentelemetry::nostd::get( @@ -399,7 +402,10 @@ TEST_P(WritableMetricStorageHistogramTestFixture, Base2ExponentialDoubleHistogra EXPECT_EQ(data.count_, 2); EXPECT_EQ(data.min_, 30); EXPECT_EQ(data.max_, 40); - EXPECT_EQ(data.positive_buckets_.Empty(), false); + auto start_index = data.positive_buckets_.StartIndex(); + auto end_index = data.positive_buckets_.EndIndex(); + EXPECT_EQ(data.positive_buckets_.Get(start_index), 1); + EXPECT_EQ(data.positive_buckets_.Get(end_index), 1); EXPECT_EQ(data.negative_buckets_.Empty(), true); count_attributes++; } @@ -464,25 +470,68 @@ TEST_P(WritableMetricStorageHistogramTestFixture, Base2ExponentialDoubleHistogra collector.get(), collectors, sdk_start_ts, collection_ts, [&](const MetricData &metric_data) { for (const auto &data_attr : metric_data.point_data_attr_) { - const auto &data = + auto &data = opentelemetry::nostd::get(data_attr.point_data); if (opentelemetry::nostd::get( data_attr.attributes.find("RequestType")->second) == "GET") { EXPECT_EQ(data.sum_, expected_total_get_requests); count_attributes++; + auto start_index = data.positive_buckets_.StartIndex(); + auto end_index = data.positive_buckets_.EndIndex(); + if (temporality == AggregationTemporality::kCumulative) + { + EXPECT_EQ(data.count_, 3); + EXPECT_EQ(data.min_, 10); + EXPECT_EQ(data.max_, 50); + auto count = 0; + for (int i = start_index; i <= end_index; ++i) { + count += data.positive_buckets_.Get(i); + } + EXPECT_EQ(count, 3); + } + if (temporality == AggregationTemporality::kDelta) + { + EXPECT_EQ(data.count_, 1); + EXPECT_EQ(data.min_, 50); + EXPECT_EQ(data.max_, 50); + EXPECT_EQ(data.positive_buckets_.Get(start_index), 1); + EXPECT_EQ(end_index, start_index); + } } else if (opentelemetry::nostd::get( data_attr.attributes.find("RequestType")->second) == "PUT") { EXPECT_EQ(data.sum_, expected_total_put_requests); count_attributes++; + auto start_index = data.positive_buckets_.StartIndex(); + auto end_index = data.positive_buckets_.EndIndex(); + if (temporality == AggregationTemporality::kCumulative) + { + EXPECT_EQ(data.count_, 3); + EXPECT_EQ(data.min_, 30); + EXPECT_EQ(data.max_, 40); + auto count = 0; + for (int i = start_index; i <= end_index; ++i) { + count += data.positive_buckets_.Get(i); + } + EXPECT_EQ(count, 3); + } + if (temporality == AggregationTemporality::kDelta) + { + EXPECT_EQ(data.count_, 1); + EXPECT_EQ(data.min_, 40); + EXPECT_EQ(data.max_, 40); + EXPECT_EQ(data.positive_buckets_.Get(start_index), 1); + EXPECT_EQ(end_index, start_index);} } } return true; }); + EXPECT_EQ(count_attributes, 2); // GET and PUT } + INSTANTIATE_TEST_SUITE_P(WritableMetricStorageHistogramTestBase2ExponentialDouble, WritableMetricStorageHistogramTestFixture, ::testing::Values(AggregationTemporality::kCumulative, From e69e73ccb517715cb71e9cd5ad6b21a3e2df7851 Mon Sep 17 00:00:00 2001 From: etmcdonald Date: Thu, 17 Apr 2025 07:17:54 +0000 Subject: [PATCH 31/48] uncomment examples --- examples/otlp/grpc_metric_main.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/examples/otlp/grpc_metric_main.cc b/examples/otlp/grpc_metric_main.cc index 53b7245dd7..d8727c1042 100644 --- a/examples/otlp/grpc_metric_main.cc +++ b/examples/otlp/grpc_metric_main.cc @@ -148,8 +148,8 @@ int main(int argc, char *argv[]) #endif else { - // std::thread counter_example{&foo_library::counter_example, name}; - // std::thread observable_counter_example{&foo_library::observable_counter_example, name}; + std::thread counter_example{&foo_library::counter_example, name}; + std::thread observable_counter_example{&foo_library::observable_counter_example, name}; std::thread histogram_example{&foo_library::histogram_example, name}; #if OPENTELEMETRY_ABI_VERSION_NO >= 2 std::thread gauge_example{&foo_library::gauge_example, name}; From ca542f7a0ea37900a543254ded36e638353baf9a Mon Sep 17 00:00:00 2001 From: etmcdonald Date: Thu, 17 Apr 2025 07:20:51 +0000 Subject: [PATCH 32/48] format --- ...base2_exponential_histogram_aggregation.cc | 72 +++++++++++-------- .../sync_metric_storage_histogram_test.cc | 9 ++- 2 files changed, 49 insertions(+), 32 deletions(-) diff --git a/sdk/src/metrics/aggregation/base2_exponential_histogram_aggregation.cc b/sdk/src/metrics/aggregation/base2_exponential_histogram_aggregation.cc index 39ced15c83..8b97a2d204 100644 --- a/sdk/src/metrics/aggregation/base2_exponential_histogram_aggregation.cc +++ b/sdk/src/metrics/aggregation/base2_exponential_histogram_aggregation.cc @@ -5,9 +5,9 @@ #include #include #include +#include #include #include -#include #include #include #include "opentelemetry/common/spin_lock_mutex.h" @@ -189,7 +189,9 @@ void Base2ExponentialHistogramAggregation::Downscale(uint32_t by) noexcept // Merge A and B into a new circular buffer C. // Caller must ensure that A and B are used as buckets at the same scale. -AdaptingCircularBufferCounter MergeBuckets(size_t max_buckets, const AdaptingCircularBufferCounter &A, const AdaptingCircularBufferCounter &B) +AdaptingCircularBufferCounter MergeBuckets(size_t max_buckets, + const AdaptingCircularBufferCounter &A, + const AdaptingCircularBufferCounter &B) { AdaptingCircularBufferCounter C = AdaptingCircularBufferCounter(max_buckets); C.Clear(); @@ -217,7 +219,7 @@ AdaptingCircularBufferCounter MergeBuckets(size_t max_buckets, const AdaptingCir { C.Increment(i, count); } - } + } return C; } @@ -234,13 +236,12 @@ std::unique_ptr Base2ExponentialHistogramAggregation::Merge( auto scale_reduction = high_res.scale_ - low_res.scale_; Base2ExponentialHistogramPointData result_value; - result_value.count_ = low_res.count_ + high_res.count_; - result_value.sum_ = low_res.sum_ + high_res.sum_; - result_value.zero_count_ = low_res.zero_count_ + high_res.zero_count_; - result_value.scale_ = (std::min)(low_res.scale_, high_res.scale_); - result_value.max_buckets_ = low_res.max_buckets_ >= high_res.max_buckets_ - ? low_res.max_buckets_ - : high_res.max_buckets_; + result_value.count_ = low_res.count_ + high_res.count_; + result_value.sum_ = low_res.sum_ + high_res.sum_; + result_value.zero_count_ = low_res.zero_count_ + high_res.zero_count_; + result_value.scale_ = (std::min)(low_res.scale_, high_res.scale_); + result_value.max_buckets_ = + low_res.max_buckets_ >= high_res.max_buckets_ ? low_res.max_buckets_ : high_res.max_buckets_; result_value.record_min_max_ = low_res.record_min_max_ && high_res.record_min_max_; if (result_value.record_min_max_) { @@ -255,15 +256,21 @@ std::unique_ptr Base2ExponentialHistogramAggregation::Merge( high_res.scale_ -= scale_reduction; } - auto pos_min_index = (std::min)(low_res.positive_buckets_.StartIndex(), high_res.positive_buckets_.StartIndex()); - auto pos_max_index = (std::max)(low_res.positive_buckets_.EndIndex(), high_res.positive_buckets_.EndIndex()); - auto neg_min_index = (std::min)(low_res.negative_buckets_.StartIndex(), high_res.negative_buckets_.StartIndex()); - auto neg_max_index = (std::max)(low_res.negative_buckets_.EndIndex(), high_res.negative_buckets_.EndIndex()); - - if (pos_max_index > pos_min_index + result_value.max_buckets_ || neg_max_index > neg_min_index + result_value.max_buckets_) + auto pos_min_index = + (std::min)(low_res.positive_buckets_.StartIndex(), high_res.positive_buckets_.StartIndex()); + auto pos_max_index = + (std::max)(low_res.positive_buckets_.EndIndex(), high_res.positive_buckets_.EndIndex()); + auto neg_min_index = + (std::min)(low_res.negative_buckets_.StartIndex(), high_res.negative_buckets_.StartIndex()); + auto neg_max_index = + (std::max)(low_res.negative_buckets_.EndIndex(), high_res.negative_buckets_.EndIndex()); + + if (pos_max_index > pos_min_index + result_value.max_buckets_ || + neg_max_index > neg_min_index + result_value.max_buckets_) { // We need to downscale the buckets to fit into the new max_buckets_. - const uint32_t scale_reduction = GetScaleReduction(pos_min_index, pos_max_index, result_value.max_buckets_); + const uint32_t scale_reduction = + GetScaleReduction(pos_min_index, pos_max_index, result_value.max_buckets_); DownscaleBuckets(&low_res.positive_buckets_, scale_reduction); DownscaleBuckets(&high_res.positive_buckets_, scale_reduction); DownscaleBuckets(&low_res.negative_buckets_, scale_reduction); @@ -273,10 +280,10 @@ std::unique_ptr Base2ExponentialHistogramAggregation::Merge( result_value.scale_ -= scale_reduction; } - result_value.positive_buckets_ = MergeBuckets(result_value.max_buckets_, low_res.positive_buckets_, - high_res.positive_buckets_); - result_value.negative_buckets_ = MergeBuckets(result_value.max_buckets_, low_res.negative_buckets_, - high_res.negative_buckets_); + result_value.positive_buckets_ = MergeBuckets( + result_value.max_buckets_, low_res.positive_buckets_, high_res.positive_buckets_); + result_value.negative_buckets_ = MergeBuckets( + result_value.max_buckets_, low_res.negative_buckets_, high_res.negative_buckets_); return std::unique_ptr{ new Base2ExponentialHistogramAggregation(std::move(result_value))}; @@ -300,15 +307,21 @@ std::unique_ptr Base2ExponentialHistogramAggregation::Diff( high_res.scale_ -= scale_reduction; } - auto pos_min_index = (std::min)(left.positive_buckets_.StartIndex(), right.positive_buckets_.StartIndex()); - auto pos_max_index = (std::max)(left.positive_buckets_.EndIndex(), right.positive_buckets_.EndIndex()); - auto neg_min_index = (std::min)(left.negative_buckets_.StartIndex(), right.negative_buckets_.StartIndex()); - auto neg_max_index = (std::max)(left.negative_buckets_.EndIndex(), right.negative_buckets_.EndIndex()); - - if (pos_max_index > pos_min_index + low_res.max_buckets_ || neg_max_index > neg_min_index + low_res.max_buckets_) + auto pos_min_index = + (std::min)(left.positive_buckets_.StartIndex(), right.positive_buckets_.StartIndex()); + auto pos_max_index = + (std::max)(left.positive_buckets_.EndIndex(), right.positive_buckets_.EndIndex()); + auto neg_min_index = + (std::min)(left.negative_buckets_.StartIndex(), right.negative_buckets_.StartIndex()); + auto neg_max_index = + (std::max)(left.negative_buckets_.EndIndex(), right.negative_buckets_.EndIndex()); + + if (pos_max_index > pos_min_index + low_res.max_buckets_ || + neg_max_index > neg_min_index + low_res.max_buckets_) { // We need to downscale the buckets to fit into the new max_buckets_. - const uint32_t scale_reduction = GetScaleReduction(pos_min_index, pos_max_index, low_res.max_buckets_); + const uint32_t scale_reduction = + GetScaleReduction(pos_min_index, pos_max_index, low_res.max_buckets_); DownscaleBuckets(&left.positive_buckets_, scale_reduction); DownscaleBuckets(&right.positive_buckets_, scale_reduction); DownscaleBuckets(&left.negative_buckets_, scale_reduction); @@ -322,7 +335,8 @@ std::unique_ptr Base2ExponentialHistogramAggregation::Diff( result_value.max_buckets_ = low_res.max_buckets_; result_value.record_min_max_ = false; // caution for underflow - // expect right.{sum, count} >= left.{sum, count} since metric points should be monotonically increasing + // expect right.{sum, count} >= left.{sum, count} since metric points should be monotonically + // increasing result_value.count_ = (right.count_ >= left.count_) ? (right.count_ - left.count_) : 0.0; result_value.sum_ = (right.sum_ >= left.sum_) ? (right.sum_ - left.sum_) : 0.0; result_value.zero_count_ = diff --git a/sdk/test/metrics/sync_metric_storage_histogram_test.cc b/sdk/test/metrics/sync_metric_storage_histogram_test.cc index 29df1b2041..0813b8678a 100644 --- a/sdk/test/metrics/sync_metric_storage_histogram_test.cc +++ b/sdk/test/metrics/sync_metric_storage_histogram_test.cc @@ -486,7 +486,8 @@ TEST_P(WritableMetricStorageHistogramTestFixture, Base2ExponentialDoubleHistogra EXPECT_EQ(data.min_, 10); EXPECT_EQ(data.max_, 50); auto count = 0; - for (int i = start_index; i <= end_index; ++i) { + for (int i = start_index; i <= end_index; ++i) + { count += data.positive_buckets_.Get(i); } EXPECT_EQ(count, 3); @@ -513,7 +514,8 @@ TEST_P(WritableMetricStorageHistogramTestFixture, Base2ExponentialDoubleHistogra EXPECT_EQ(data.min_, 30); EXPECT_EQ(data.max_, 40); auto count = 0; - for (int i = start_index; i <= end_index; ++i) { + for (int i = start_index; i <= end_index; ++i) + { count += data.positive_buckets_.Get(i); } EXPECT_EQ(count, 3); @@ -524,7 +526,8 @@ TEST_P(WritableMetricStorageHistogramTestFixture, Base2ExponentialDoubleHistogra EXPECT_EQ(data.min_, 40); EXPECT_EQ(data.max_, 40); EXPECT_EQ(data.positive_buckets_.Get(start_index), 1); - EXPECT_EQ(end_index, start_index);} + EXPECT_EQ(end_index, start_index); + } } } return true; From 9040c38d8c54b3a15e7d7b0677642db7b4c7097f Mon Sep 17 00:00:00 2001 From: etmcdonald Date: Thu, 17 Apr 2025 16:16:38 +0000 Subject: [PATCH 33/48] use static_cast --- .../base2_exponential_histogram_aggregation.cc | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/sdk/src/metrics/aggregation/base2_exponential_histogram_aggregation.cc b/sdk/src/metrics/aggregation/base2_exponential_histogram_aggregation.cc index 8b97a2d204..be2503d7d4 100644 --- a/sdk/src/metrics/aggregation/base2_exponential_histogram_aggregation.cc +++ b/sdk/src/metrics/aggregation/base2_exponential_histogram_aggregation.cc @@ -265,8 +265,10 @@ std::unique_ptr Base2ExponentialHistogramAggregation::Merge( auto neg_max_index = (std::max)(low_res.negative_buckets_.EndIndex(), high_res.negative_buckets_.EndIndex()); - if (pos_max_index > pos_min_index + result_value.max_buckets_ || - neg_max_index > neg_min_index + result_value.max_buckets_) + if (static_cast(pos_max_index) > + static_cast(pos_min_index) + result_value.max_buckets_ || + static_cast(neg_max_index) > + static_cast(neg_min_index) + result_value.max_buckets_) { // We need to downscale the buckets to fit into the new max_buckets_. const uint32_t scale_reduction = @@ -316,8 +318,10 @@ std::unique_ptr Base2ExponentialHistogramAggregation::Diff( auto neg_max_index = (std::max)(left.negative_buckets_.EndIndex(), right.negative_buckets_.EndIndex()); - if (pos_max_index > pos_min_index + low_res.max_buckets_ || - neg_max_index > neg_min_index + low_res.max_buckets_) + if (static_cast(pos_max_index) > + static_cast(pos_min_index) + low_res.max_buckets_ || + static_cast(neg_max_index) > + static_cast(neg_min_index) + low_res.max_buckets_) { // We need to downscale the buckets to fit into the new max_buckets_. const uint32_t scale_reduction = @@ -351,7 +355,7 @@ std::unique_ptr Base2ExponentialHistogramAggregation::Diff( auto l_cnt = left.positive_buckets_.Get(i); auto r_cnt = right.positive_buckets_.Get(i); // expect right >= left since metric points should be monotonically increasing - auto delta = (std::max)((uint64_t)(0), r_cnt - l_cnt); + auto delta = (std::max)(static_cast(0), r_cnt - l_cnt); if (l_cnt > 0) { result_value.positive_buckets_.Increment(i, delta); @@ -366,7 +370,7 @@ std::unique_ptr Base2ExponentialHistogramAggregation::Diff( auto l_cnt = left.negative_buckets_.Get(i); auto r_cnt = right.negative_buckets_.Get(i); // expect right >= left since metric points should be monotonically increasing - auto delta = (std::max)((uint64_t)(0), r_cnt - l_cnt); + auto delta = (std::max)(static_cast(0), r_cnt - l_cnt); if (delta > 0) { result_value.negative_buckets_.Increment(i, delta); From a7a12344170a7dae61d33c07ca26b856c7564fe9 Mon Sep 17 00:00:00 2001 From: etmcdonald Date: Thu, 17 Apr 2025 20:03:10 +0000 Subject: [PATCH 34/48] fix comments --- examples/common/metrics_foo_library/foo_library.cc | 2 +- examples/otlp/grpc_metric_main.cc | 12 +++++++----- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/examples/common/metrics_foo_library/foo_library.cc b/examples/common/metrics_foo_library/foo_library.cc index c8e8b17336..7ecbbf4d27 100644 --- a/examples/common/metrics_foo_library/foo_library.cc +++ b/examples/common/metrics_foo_library/foo_library.cc @@ -116,7 +116,7 @@ void foo_library::histogram_exp_example(const std::string &name) { std::string histogram_name = name + "_exponential_histogram"; auto provider = metrics_api::Provider::GetMeterProvider(); - opentelemetry::nostd::shared_ptr meter = provider->GetMeter(name, "1.2.0"); + auto meter = provider->GetMeter(name, "1.2.0"); auto histogram_counter = meter->CreateDoubleHistogram(histogram_name, "des", "histogram-unit"); auto context = opentelemetry::context::Context{}; for (uint32_t i = 0; i < 20; ++i) diff --git a/examples/otlp/grpc_metric_main.cc b/examples/otlp/grpc_metric_main.cc index d8727c1042..6cde982880 100644 --- a/examples/otlp/grpc_metric_main.cc +++ b/examples/otlp/grpc_metric_main.cc @@ -58,8 +58,8 @@ void InitMetrics(std::string &name) auto provider = metric_sdk::MeterProviderFactory::Create(std::move(context)); // histogram view - std::string histogram_name = name + "_histogram"; - std::string unit = "unit"; + std::string histogram_name = name + "_exponential_histogram"; + std::string unit = "histogram-unit"; auto histogram_instrument_selector = metric_sdk::InstrumentSelectorFactory::Create( metric_sdk::InstrumentType::kHistogram, histogram_name, unit); @@ -79,7 +79,7 @@ void InitMetrics(std::string &name) std::move(histogram_aggregation_config)); auto histogram_view = metric_sdk::ViewFactory::Create( - name, "description", unit, metric_sdk::AggregationType::kBase2ExponentialHistogram, + name, "des", unit, metric_sdk::AggregationType::kBase2ExponentialHistogram, aggregation_config); provider->AddView(std::move(histogram_instrument_selector), std::move(histogram_meter_selector), @@ -151,13 +151,15 @@ int main(int argc, char *argv[]) std::thread counter_example{&foo_library::counter_example, name}; std::thread observable_counter_example{&foo_library::observable_counter_example, name}; std::thread histogram_example{&foo_library::histogram_example, name}; + std::thread histogram_exp_example{&foo_library::histogram_exp_example, name}; #if OPENTELEMETRY_ABI_VERSION_NO >= 2 std::thread gauge_example{&foo_library::gauge_example, name}; #endif - // counter_example.join(); - // observable_counter_example.join(); + counter_example.join(); + observable_counter_example.join(); histogram_example.join(); + histogram_exp_example.join(); #if OPENTELEMETRY_ABI_VERSION_NO >= 2 gauge_example.join(); #endif From 53e025e3a9e2b6124b9aab7ee8eb5bb29d6058ac Mon Sep 17 00:00:00 2001 From: etmcdonald Date: Thu, 17 Apr 2025 22:06:36 +0000 Subject: [PATCH 35/48] update changelog --- CHANGELOG.md | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index dfebf4d51d..0d41f10781 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,7 +13,7 @@ Increment the: * MINOR version when you add functionality in a backwards compatible manner, and * PATCH version when you make backwards compatible bug fixes. -## [1.21 2025-04-04] +## [Unreleased] * [SDK] Base2 exponential histogram aggregation [#3175](https://github.com/open-telemetry/opentelemetry-cpp/pull/3346) @@ -24,8 +24,6 @@ Increment the: ostream exporter, and otlp/grpc exporter. Updated histogram aggregation and benchmark tests. -## [Unreleased] - * [API] Remove `WITH_ABSEIL` and `HAVE_ABSEIL` [#3318](https://github.com/open-telemetry/opentelemetry-cpp/pull/3318) From 293e2dd61d12a34da21d4778cbafa9002ef57d1d Mon Sep 17 00:00:00 2001 From: "Felipe C. Dos Santos" Date: Mon, 21 Apr 2025 02:11:28 +0000 Subject: [PATCH 36/48] Fix comments and errors in the pipeline --- .../common/metrics_foo_library/foo_library.cc | 2 +- .../base2_exponential_histogram_aggregation.h | 2 +- ...base2_exponential_histogram_aggregation.cc | 91 ++++++++++--------- sdk/test/metrics/aggregation_test.cc | 2 +- .../sync_metric_storage_histogram_test.cc | 8 +- 5 files changed, 56 insertions(+), 49 deletions(-) diff --git a/examples/common/metrics_foo_library/foo_library.cc b/examples/common/metrics_foo_library/foo_library.cc index 7ecbbf4d27..5f1fb86776 100644 --- a/examples/common/metrics_foo_library/foo_library.cc +++ b/examples/common/metrics_foo_library/foo_library.cc @@ -116,7 +116,7 @@ void foo_library::histogram_exp_example(const std::string &name) { std::string histogram_name = name + "_exponential_histogram"; auto provider = metrics_api::Provider::GetMeterProvider(); - auto meter = provider->GetMeter(name, "1.2.0"); + auto meter = provider->GetMeter(name, "1.2.0"); auto histogram_counter = meter->CreateDoubleHistogram(histogram_name, "des", "histogram-unit"); auto context = opentelemetry::context::Context{}; for (uint32_t i = 0; i < 20; ++i) diff --git a/sdk/include/opentelemetry/sdk/metrics/aggregation/base2_exponential_histogram_aggregation.h b/sdk/include/opentelemetry/sdk/metrics/aggregation/base2_exponential_histogram_aggregation.h index 0ff4667092..3cb17bb7e2 100644 --- a/sdk/include/opentelemetry/sdk/metrics/aggregation/base2_exponential_histogram_aggregation.h +++ b/sdk/include/opentelemetry/sdk/metrics/aggregation/base2_exponential_histogram_aggregation.h @@ -45,7 +45,7 @@ class Base2ExponentialHistogramAggregation : public Aggregation PointType ToPoint() const noexcept override; private: - void AggregateIntoBuckets(AdaptingCircularBufferCounter *buckets, double value) noexcept; + void AggregateIntoBuckets(AdaptingCircularBufferCounter &buckets, double value) noexcept; void Downscale(uint32_t by) noexcept; mutable opentelemetry::common::SpinLockMutex lock_; diff --git a/sdk/src/metrics/aggregation/base2_exponential_histogram_aggregation.cc b/sdk/src/metrics/aggregation/base2_exponential_histogram_aggregation.cc index be2503d7d4..b30c7a5e62 100644 --- a/sdk/src/metrics/aggregation/base2_exponential_histogram_aggregation.cc +++ b/sdk/src/metrics/aggregation/base2_exponential_histogram_aggregation.cc @@ -1,19 +1,19 @@ // Copyright The OpenTelemetry Authors // SPDX-License-Identifier: Apache-2.0 -#include "opentelemetry/sdk/metrics/aggregation/base2_exponential_histogram_aggregation.h" #include #include #include -#include #include #include #include #include + #include "opentelemetry/common/spin_lock_mutex.h" #include "opentelemetry/nostd/variant.h" #include "opentelemetry/sdk/metrics/aggregation/aggregation.h" #include "opentelemetry/sdk/metrics/aggregation/aggregation_config.h" +#include "opentelemetry/sdk/metrics/aggregation/base2_exponential_histogram_aggregation.h" #include "opentelemetry/sdk/metrics/aggregation/base2_exponential_histogram_indexer.h" #include "opentelemetry/sdk/metrics/data/circular_buffer.h" #include "opentelemetry/sdk/metrics/data/metric_data.h" @@ -55,9 +55,9 @@ uint32_t GetScaleReduction(int32_t start_index, int32_t end_index, size_t max_bu // return GetScaleReduction(start_index, end_index, max_buckets); // } -void DownscaleBuckets(AdaptingCircularBufferCounter *buckets, uint32_t by) noexcept +void DownscaleBuckets(AdaptingCircularBufferCounter &buckets, uint32_t by) noexcept { - if (buckets->Empty()) + if (buckets.Empty()) { return; } @@ -66,18 +66,18 @@ void DownscaleBuckets(AdaptingCircularBufferCounter *buckets, uint32_t by) noexc // Instead of creating a new counter, we copy the existing one (for bucket size // optimisations), and clear the values before writing the new ones. // TODO(euroelessar): Do downscaling in-place. - AdaptingCircularBufferCounter new_buckets = *buckets; + auto new_buckets = buckets; new_buckets.Clear(); - for (int i = buckets->StartIndex(); i <= buckets->EndIndex(); i++) + for (auto i = buckets.StartIndex(); i <= buckets.EndIndex(); ++i) { - const uint64_t count = buckets->Get(i); + const uint64_t count = buckets.Get(i); if (count > 0) { new_buckets.Increment(i >> by, count); } } - *buckets = std::move(new_buckets); + buckets = std::move(new_buckets); } } // namespace @@ -143,33 +143,33 @@ void Base2ExponentialHistogramAggregation::Aggregate( } else if (value > 0) { - AggregateIntoBuckets(&point_data_.positive_buckets_, value); + AggregateIntoBuckets(point_data_.positive_buckets_, value); } else { - AggregateIntoBuckets(&point_data_.negative_buckets_, -value); + AggregateIntoBuckets(point_data_.negative_buckets_, -value); } } void Base2ExponentialHistogramAggregation::AggregateIntoBuckets( - AdaptingCircularBufferCounter *buckets, + AdaptingCircularBufferCounter &buckets, double value) noexcept { - if (buckets->MaxSize() == 0) + if (buckets.MaxSize() == 0) { - *buckets = AdaptingCircularBufferCounter{point_data_.max_buckets_}; + buckets = AdaptingCircularBufferCounter{point_data_.max_buckets_}; } const int32_t index = indexer_.ComputeIndex(value); - if (!buckets->Increment(index, 1)) + if (!buckets.Increment(index, 1)) { - const int32_t start_index = (std::min)(buckets->StartIndex(), index); - const int32_t end_index = (std::max)(buckets->EndIndex(), index); + const int32_t start_index = (std::min)(buckets.StartIndex(), index); + const int32_t end_index = (std::max)(buckets.EndIndex(), index); const uint32_t scale_reduction = GetScaleReduction(start_index, end_index, point_data_.max_buckets_); Downscale(scale_reduction); - buckets->Increment(index >> scale_reduction, 1); + buckets.Increment(index >> scale_reduction, 1); } } @@ -180,8 +180,8 @@ void Base2ExponentialHistogramAggregation::Downscale(uint32_t by) noexcept return; } - DownscaleBuckets(&point_data_.positive_buckets_, by); - DownscaleBuckets(&point_data_.negative_buckets_, by); + DownscaleBuckets(point_data_.positive_buckets_, by); + DownscaleBuckets(point_data_.negative_buckets_, by); point_data_.scale_ -= by; indexer_ = Base2ExponentialHistogramIndexer(point_data_.scale_); @@ -231,9 +231,8 @@ std::unique_ptr Base2ExponentialHistogramAggregation::Merge( auto right = nostd::get( (static_cast(delta).ToPoint())); - auto low_res = left.scale_ < right.scale_ ? left : right; - auto high_res = left.scale_ < right.scale_ ? right : left; - auto scale_reduction = high_res.scale_ - low_res.scale_; + auto &low_res = left.scale_ < right.scale_ ? left : right; + auto &high_res = left.scale_ < right.scale_ ? right : left; Base2ExponentialHistogramPointData result_value; result_value.count_ = low_res.count_ + high_res.count_; @@ -243,17 +242,22 @@ std::unique_ptr Base2ExponentialHistogramAggregation::Merge( result_value.max_buckets_ = low_res.max_buckets_ >= high_res.max_buckets_ ? low_res.max_buckets_ : high_res.max_buckets_; result_value.record_min_max_ = low_res.record_min_max_ && high_res.record_min_max_; + if (result_value.record_min_max_) { result_value.min_ = (std::min)(low_res.min_, high_res.min_); result_value.max_ = (std::max)(low_res.max_, high_res.max_); } - if (scale_reduction > 0) { - DownscaleBuckets(&high_res.positive_buckets_, scale_reduction); - DownscaleBuckets(&high_res.negative_buckets_, scale_reduction); - high_res.scale_ -= scale_reduction; + auto scale_reduction = high_res.scale_ - low_res.scale_; + + if (scale_reduction > 0) + { + DownscaleBuckets(high_res.positive_buckets_, scale_reduction); + DownscaleBuckets(high_res.negative_buckets_, scale_reduction); + high_res.scale_ -= scale_reduction; + } } auto pos_min_index = @@ -273,10 +277,10 @@ std::unique_ptr Base2ExponentialHistogramAggregation::Merge( // We need to downscale the buckets to fit into the new max_buckets_. const uint32_t scale_reduction = GetScaleReduction(pos_min_index, pos_max_index, result_value.max_buckets_); - DownscaleBuckets(&low_res.positive_buckets_, scale_reduction); - DownscaleBuckets(&high_res.positive_buckets_, scale_reduction); - DownscaleBuckets(&low_res.negative_buckets_, scale_reduction); - DownscaleBuckets(&high_res.negative_buckets_, scale_reduction); + DownscaleBuckets(low_res.positive_buckets_, scale_reduction); + DownscaleBuckets(high_res.positive_buckets_, scale_reduction); + DownscaleBuckets(low_res.negative_buckets_, scale_reduction); + DownscaleBuckets(high_res.negative_buckets_, scale_reduction); low_res.scale_ -= scale_reduction; high_res.scale_ -= scale_reduction; result_value.scale_ -= scale_reduction; @@ -298,15 +302,18 @@ std::unique_ptr Base2ExponentialHistogramAggregation::Diff( auto right = nostd::get( (static_cast(next).ToPoint())); - auto low_res = left.scale_ < right.scale_ ? left : right; - auto high_res = left.scale_ < right.scale_ ? right : left; - auto scale_reduction = high_res.scale_ - low_res.scale_; + auto &low_res = left.scale_ < right.scale_ ? left : right; + auto &high_res = left.scale_ < right.scale_ ? right : left; - if (scale_reduction > 0) { - DownscaleBuckets(&high_res.positive_buckets_, scale_reduction); - DownscaleBuckets(&high_res.negative_buckets_, scale_reduction); - high_res.scale_ -= scale_reduction; + const auto scale_reduction = high_res.scale_ - low_res.scale_; + + if (scale_reduction > 0) + { + DownscaleBuckets(high_res.positive_buckets_, scale_reduction); + DownscaleBuckets(high_res.negative_buckets_, scale_reduction); + high_res.scale_ -= scale_reduction; + } } auto pos_min_index = @@ -326,10 +333,10 @@ std::unique_ptr Base2ExponentialHistogramAggregation::Diff( // We need to downscale the buckets to fit into the new max_buckets_. const uint32_t scale_reduction = GetScaleReduction(pos_min_index, pos_max_index, low_res.max_buckets_); - DownscaleBuckets(&left.positive_buckets_, scale_reduction); - DownscaleBuckets(&right.positive_buckets_, scale_reduction); - DownscaleBuckets(&left.negative_buckets_, scale_reduction); - DownscaleBuckets(&right.negative_buckets_, scale_reduction); + DownscaleBuckets(left.positive_buckets_, scale_reduction); + DownscaleBuckets(right.positive_buckets_, scale_reduction); + DownscaleBuckets(left.negative_buckets_, scale_reduction); + DownscaleBuckets(right.negative_buckets_, scale_reduction); left.scale_ -= scale_reduction; right.scale_ -= scale_reduction; } @@ -341,7 +348,7 @@ std::unique_ptr Base2ExponentialHistogramAggregation::Diff( // caution for underflow // expect right.{sum, count} >= left.{sum, count} since metric points should be monotonically // increasing - result_value.count_ = (right.count_ >= left.count_) ? (right.count_ - left.count_) : 0.0; + result_value.count_ = (right.count_ >= left.count_) ? (right.count_ - left.count_) : 0; result_value.sum_ = (right.sum_ >= left.sum_) ? (right.sum_ - left.sum_) : 0.0; result_value.zero_count_ = (right.zero_count_ >= left.zero_count_) ? (right.zero_count_ - left.zero_count_) : 0; diff --git a/sdk/test/metrics/aggregation_test.cc b/sdk/test/metrics/aggregation_test.cc index 3624f31f66..98d76ec701 100644 --- a/sdk/test/metrics/aggregation_test.cc +++ b/sdk/test/metrics/aggregation_test.cc @@ -225,7 +225,7 @@ TEST(Aggregation, DoubleHistogramAggregation) EXPECT_EQ(histogram_data.counts_[7], 1); // aggr2(105.0) - aggr1(0) } -TEST(aggregation, Base2ExponentialHistogramAggregation) +TEST(Aggregation, Base2ExponentialHistogramAggregation) { // Low res histo auto SCALE0 = 0; diff --git a/sdk/test/metrics/sync_metric_storage_histogram_test.cc b/sdk/test/metrics/sync_metric_storage_histogram_test.cc index 0813b8678a..bf200cf486 100644 --- a/sdk/test/metrics/sync_metric_storage_histogram_test.cc +++ b/sdk/test/metrics/sync_metric_storage_histogram_test.cc @@ -485,8 +485,8 @@ TEST_P(WritableMetricStorageHistogramTestFixture, Base2ExponentialDoubleHistogra EXPECT_EQ(data.count_, 3); EXPECT_EQ(data.min_, 10); EXPECT_EQ(data.max_, 50); - auto count = 0; - for (int i = start_index; i <= end_index; ++i) + uint64_t count = 0; + for (auto i = start_index; i <= end_index; ++i) { count += data.positive_buckets_.Get(i); } @@ -513,8 +513,8 @@ TEST_P(WritableMetricStorageHistogramTestFixture, Base2ExponentialDoubleHistogra EXPECT_EQ(data.count_, 3); EXPECT_EQ(data.min_, 30); EXPECT_EQ(data.max_, 40); - auto count = 0; - for (int i = start_index; i <= end_index; ++i) + uint64_t count = 0; + for (auto i = start_index; i <= end_index; ++i) { count += data.positive_buckets_.Get(i); } From f184cec60f7b154a6fca22ee8c66481a018439f5 Mon Sep 17 00:00:00 2001 From: ethan <67941833+ethandmd@users.noreply.github.com> Date: Tue, 22 Apr 2025 09:53:51 -0700 Subject: [PATCH 37/48] Update sdk/src/metrics/aggregation/base2_exponential_histogram_aggregation.cc Co-authored-by: Tom Tan --- .../aggregation/base2_exponential_histogram_aggregation.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sdk/src/metrics/aggregation/base2_exponential_histogram_aggregation.cc b/sdk/src/metrics/aggregation/base2_exponential_histogram_aggregation.cc index b30c7a5e62..5216beb6ac 100644 --- a/sdk/src/metrics/aggregation/base2_exponential_histogram_aggregation.cc +++ b/sdk/src/metrics/aggregation/base2_exponential_histogram_aggregation.cc @@ -63,7 +63,7 @@ void DownscaleBuckets(AdaptingCircularBufferCounter &buckets, uint32_t by) noexc } // We want to preserve other optimisations here as well, e.g. integer size. - // Instead of creating a new counter, we copy the existing one (for bucket size + // Instead of creating a new counter, we copy the existing one (for bucket size // optimisations), and clear the values before writing the new ones. // TODO(euroelessar): Do downscaling in-place. auto new_buckets = buckets; From 50ca1c05edd5ca6662eae38581e3737afd63ac4a Mon Sep 17 00:00:00 2001 From: etmcdonald Date: Tue, 22 Apr 2025 17:10:24 +0000 Subject: [PATCH 38/48] remove unused code --- .../base2_exponential_histogram_aggregation.cc | 14 -------------- 1 file changed, 14 deletions(-) diff --git a/sdk/src/metrics/aggregation/base2_exponential_histogram_aggregation.cc b/sdk/src/metrics/aggregation/base2_exponential_histogram_aggregation.cc index 5216beb6ac..8857703645 100644 --- a/sdk/src/metrics/aggregation/base2_exponential_histogram_aggregation.cc +++ b/sdk/src/metrics/aggregation/base2_exponential_histogram_aggregation.cc @@ -41,20 +41,6 @@ uint32_t GetScaleReduction(int32_t start_index, int32_t end_index, size_t max_bu return scale_reduction; } -// Commented out as it is not used -// uint32_t GetScaleReduction(const AdaptingCircularBufferCounter &first, -// const AdaptingCircularBufferCounter &second, -// size_t max_buckets) -// { -// if (first.Empty() || second.Empty()) -// { -// return 0; -// } -// const int32_t start_index = (std::min)(first.StartIndex(), second.StartIndex()); -// const int32_t end_index = (std::max)(first.EndIndex(), second.EndIndex()); -// return GetScaleReduction(start_index, end_index, max_buckets); -// } - void DownscaleBuckets(AdaptingCircularBufferCounter &buckets, uint32_t by) noexcept { if (buckets.Empty()) From ab5441ee82d74b93d878724bc407489b3527daef Mon Sep 17 00:00:00 2001 From: etmcdonald Date: Tue, 22 Apr 2025 17:21:26 +0000 Subject: [PATCH 39/48] set max_buckets minimum to 2 --- .../aggregation/base2_exponential_histogram_aggregation.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sdk/src/metrics/aggregation/base2_exponential_histogram_aggregation.cc b/sdk/src/metrics/aggregation/base2_exponential_histogram_aggregation.cc index 8857703645..9807dcef93 100644 --- a/sdk/src/metrics/aggregation/base2_exponential_histogram_aggregation.cc +++ b/sdk/src/metrics/aggregation/base2_exponential_histogram_aggregation.cc @@ -78,7 +78,7 @@ Base2ExponentialHistogramAggregation::Base2ExponentialHistogramAggregation( ac = &default_config; } - point_data_.max_buckets_ = ac->max_buckets_; + point_data_.max_buckets_ = (std::max)(ac->max_buckets_, static_cast(2)); point_data_.scale_ = ac->max_scale_; point_data_.record_min_max_ = ac->record_min_max_; point_data_.min_ = (std::numeric_limits::max)(); From 42a904e48509d7ce62275d7d1798b109a83955ae Mon Sep 17 00:00:00 2001 From: etmcdonald Date: Tue, 22 Apr 2025 17:26:33 +0000 Subject: [PATCH 40/48] Revert otlp grpc client changes Propose change in follow up PR --- exporters/otlp/src/otlp_grpc_client.cc | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/exporters/otlp/src/otlp_grpc_client.cc b/exporters/otlp/src/otlp_grpc_client.cc index 75bea9d4ed..4d2fb6d38e 100644 --- a/exporters/otlp/src/otlp_grpc_client.cc +++ b/exporters/otlp/src/otlp_grpc_client.cc @@ -333,15 +333,7 @@ std::shared_ptr OtlpGrpcClient::MakeChannel(const OtlpGrpcClientO } std::shared_ptr channel; - std::string grpc_target; - if (url.scheme_ == "unix") - { - grpc_target = "unix:" + url.path_; - } - else - { - grpc_target = url.host_ + ":" + std::to_string(static_cast(url.port_)); - } + std::string grpc_target = url.host_ + ":" + std::to_string(static_cast(url.port_)); grpc::ChannelArguments grpc_arguments; grpc_arguments.SetUserAgentPrefix(options.user_agent); From 5641bbd8a790904175f696fed90b31cb95c827f1 Mon Sep 17 00:00:00 2001 From: etmcdonald Date: Tue, 22 Apr 2025 22:20:48 +0000 Subject: [PATCH 41/48] [wip] make buckets unique ptr --- .../base2_exponential_histogram_aggregation.h | 3 +- .../sdk/metrics/data/point_data.h | 37 +++-- ...base2_exponential_histogram_aggregation.cc | 127 +++++++++++------- 3 files changed, 101 insertions(+), 66 deletions(-) diff --git a/sdk/include/opentelemetry/sdk/metrics/aggregation/base2_exponential_histogram_aggregation.h b/sdk/include/opentelemetry/sdk/metrics/aggregation/base2_exponential_histogram_aggregation.h index 3cb17bb7e2..9e927b56a6 100644 --- a/sdk/include/opentelemetry/sdk/metrics/aggregation/base2_exponential_histogram_aggregation.h +++ b/sdk/include/opentelemetry/sdk/metrics/aggregation/base2_exponential_histogram_aggregation.h @@ -45,7 +45,8 @@ class Base2ExponentialHistogramAggregation : public Aggregation PointType ToPoint() const noexcept override; private: - void AggregateIntoBuckets(AdaptingCircularBufferCounter &buckets, double value) noexcept; + void AggregateIntoBuckets(std::unique_ptr &buckets, + double value) noexcept; void Downscale(uint32_t by) noexcept; mutable opentelemetry::common::SpinLockMutex lock_; diff --git a/sdk/include/opentelemetry/sdk/metrics/data/point_data.h b/sdk/include/opentelemetry/sdk/metrics/data/point_data.h index 2a83c045e5..62b2f1abb5 100644 --- a/sdk/include/opentelemetry/sdk/metrics/data/point_data.h +++ b/sdk/include/opentelemetry/sdk/metrics/data/point_data.h @@ -69,23 +69,34 @@ class HistogramPointData class Base2ExponentialHistogramPointData { public: - // TODO: remove ctors and initializers when GCC<5 stops shipping on Ubuntu - Base2ExponentialHistogramPointData(Base2ExponentialHistogramPointData &&) = default; - Base2ExponentialHistogramPointData &operator=(Base2ExponentialHistogramPointData &&) = default; - Base2ExponentialHistogramPointData(const Base2ExponentialHistogramPointData &) = default; - Base2ExponentialHistogramPointData() = default; + // Delete copy constructor and copy assignment operator + Base2ExponentialHistogramPointData(const Base2ExponentialHistogramPointData &) = delete; + Base2ExponentialHistogramPointData &operator=(const Base2ExponentialHistogramPointData &) = + delete; - uint64_t count_ = {}; - double sum_ = {}; - int32_t scale_ = {}; - uint64_t zero_count_ = {}; - AdaptingCircularBufferCounter positive_buckets_{0}; - AdaptingCircularBufferCounter negative_buckets_{0}; + // Default move constructor and move assignment operator + Base2ExponentialHistogramPointData(Base2ExponentialHistogramPointData &&) noexcept = default; + Base2ExponentialHistogramPointData &operator=(Base2ExponentialHistogramPointData &&) noexcept = + default; + + // Default constructor + Base2ExponentialHistogramPointData() = default; + + double sum_ = {}; double min_ = {}; double max_ = {}; double zero_threshold_ = {}; - bool record_min_max_ = true; - size_t max_buckets_ = {}; + uint64_t count_ = {}; + uint64_t zero_count_ = {}; + + std::unique_ptr positive_buckets_ = + std::make_unique(0); + std::unique_ptr negative_buckets_ = + std::make_unique(0); + + size_t max_buckets_ = {}; + int32_t scale_ = {}; + bool record_min_max_ = true; }; class DropPointData diff --git a/sdk/src/metrics/aggregation/base2_exponential_histogram_aggregation.cc b/sdk/src/metrics/aggregation/base2_exponential_histogram_aggregation.cc index 9807dcef93..03e49e07dd 100644 --- a/sdk/src/metrics/aggregation/base2_exponential_histogram_aggregation.cc +++ b/sdk/src/metrics/aggregation/base2_exponential_histogram_aggregation.cc @@ -41,9 +41,9 @@ uint32_t GetScaleReduction(int32_t start_index, int32_t end_index, size_t max_bu return scale_reduction; } -void DownscaleBuckets(AdaptingCircularBufferCounter &buckets, uint32_t by) noexcept +void DownscaleBuckets(std::unique_ptr &buckets, uint32_t by) noexcept { - if (buckets.Empty()) + if (buckets->Empty()) { return; } @@ -52,15 +52,15 @@ void DownscaleBuckets(AdaptingCircularBufferCounter &buckets, uint32_t by) noexc // Instead of creating a new counter, we copy the existing one (for bucket size // optimisations), and clear the values before writing the new ones. // TODO(euroelessar): Do downscaling in-place. - auto new_buckets = buckets; - new_buckets.Clear(); + auto new_buckets = std::make_unique(buckets->MaxSize()); + new_buckets->Clear(); - for (auto i = buckets.StartIndex(); i <= buckets.EndIndex(); ++i) + for (auto i = buckets->StartIndex(); i <= buckets->EndIndex(); ++i) { - const uint64_t count = buckets.Get(i); + const uint64_t count = buckets->Get(i); if (count > 0) { - new_buckets.Increment(i >> by, count); + new_buckets->Increment(i >> by, count); } } buckets = std::move(new_buckets); @@ -87,19 +87,19 @@ Base2ExponentialHistogramAggregation::Base2ExponentialHistogramAggregation( indexer_ = Base2ExponentialHistogramIndexer(point_data_.scale_); } -Base2ExponentialHistogramAggregation::Base2ExponentialHistogramAggregation( - const Base2ExponentialHistogramPointData &point_data) - : point_data_{point_data}, - indexer_(point_data.scale_), - record_min_max_{point_data.record_min_max_} -{} +// Base2ExponentialHistogramAggregation::Base2ExponentialHistogramAggregation( +// const Base2ExponentialHistogramPointData &point_data) +// : point_data_{point_data}, +// indexer_(point_data.scale_), +// record_min_max_{point_data.record_min_max_} +// {} -Base2ExponentialHistogramAggregation::Base2ExponentialHistogramAggregation( - Base2ExponentialHistogramPointData &&point_data) - : point_data_{std::move(point_data)}, - indexer_(point_data_.scale_), - record_min_max_{point_data_.record_min_max_} -{} +// Base2ExponentialHistogramAggregation::Base2ExponentialHistogramAggregation( +// Base2ExponentialHistogramPointData &&point_data) +// : point_data_{std::move(point_data)}, +// indexer_(point_data_.scale_), +// record_min_max_{point_data_.record_min_max_} +// {} void Base2ExponentialHistogramAggregation::Aggregate( int64_t value, @@ -138,24 +138,24 @@ void Base2ExponentialHistogramAggregation::Aggregate( } void Base2ExponentialHistogramAggregation::AggregateIntoBuckets( - AdaptingCircularBufferCounter &buckets, + std::unique_ptr &buckets, double value) noexcept { - if (buckets.MaxSize() == 0) + if (buckets->MaxSize() == 0) { - buckets = AdaptingCircularBufferCounter{point_data_.max_buckets_}; + buckets = std::make_unique(point_data_.max_buckets_); } const int32_t index = indexer_.ComputeIndex(value); - if (!buckets.Increment(index, 1)) + if (!buckets->Increment(index, 1)) { - const int32_t start_index = (std::min)(buckets.StartIndex(), index); - const int32_t end_index = (std::max)(buckets.EndIndex(), index); + const int32_t start_index = (std::min)(buckets->StartIndex(), index); + const int32_t end_index = (std::max)(buckets->EndIndex(), index); const uint32_t scale_reduction = GetScaleReduction(start_index, end_index, point_data_.max_buckets_); Downscale(scale_reduction); - buckets.Increment(index >> scale_reduction, 1); + buckets->Increment(index >> scale_reduction, 1); } } @@ -247,13 +247,13 @@ std::unique_ptr Base2ExponentialHistogramAggregation::Merge( } auto pos_min_index = - (std::min)(low_res.positive_buckets_.StartIndex(), high_res.positive_buckets_.StartIndex()); + (std::min)(low_res.positive_buckets_->StartIndex(), high_res.positive_buckets_->StartIndex()); auto pos_max_index = - (std::max)(low_res.positive_buckets_.EndIndex(), high_res.positive_buckets_.EndIndex()); + (std::max)(low_res.positive_buckets_->EndIndex(), high_res.positive_buckets_->EndIndex()); auto neg_min_index = - (std::min)(low_res.negative_buckets_.StartIndex(), high_res.negative_buckets_.StartIndex()); + (std::min)(low_res.negative_buckets_->StartIndex(), high_res.negative_buckets_->StartIndex()); auto neg_max_index = - (std::max)(low_res.negative_buckets_.EndIndex(), high_res.negative_buckets_.EndIndex()); + (std::max)(low_res.negative_buckets_->EndIndex(), high_res.negative_buckets_->EndIndex()); if (static_cast(pos_max_index) > static_cast(pos_min_index) + result_value.max_buckets_ || @@ -272,10 +272,10 @@ std::unique_ptr Base2ExponentialHistogramAggregation::Merge( result_value.scale_ -= scale_reduction; } - result_value.positive_buckets_ = MergeBuckets( - result_value.max_buckets_, low_res.positive_buckets_, high_res.positive_buckets_); - result_value.negative_buckets_ = MergeBuckets( - result_value.max_buckets_, low_res.negative_buckets_, high_res.negative_buckets_); + result_value.positive_buckets_ = std::make_unique(MergeBuckets( + result_value.max_buckets_, *low_res.positive_buckets_, *high_res.positive_buckets_)); + result_value.negative_buckets_ = std::make_unique(MergeBuckets( + result_value.max_buckets_, *low_res.negative_buckets_, *high_res.negative_buckets_)); return std::unique_ptr{ new Base2ExponentialHistogramAggregation(std::move(result_value))}; @@ -303,13 +303,13 @@ std::unique_ptr Base2ExponentialHistogramAggregation::Diff( } auto pos_min_index = - (std::min)(left.positive_buckets_.StartIndex(), right.positive_buckets_.StartIndex()); + (std::min)(left.positive_buckets_->StartIndex(), right.positive_buckets_->StartIndex()); auto pos_max_index = - (std::max)(left.positive_buckets_.EndIndex(), right.positive_buckets_.EndIndex()); + (std::max)(left.positive_buckets_->EndIndex(), right.positive_buckets_->EndIndex()); auto neg_min_index = - (std::min)(left.negative_buckets_.StartIndex(), right.negative_buckets_.StartIndex()); + (std::min)(left.negative_buckets_->StartIndex(), right.negative_buckets_->StartIndex()); auto neg_max_index = - (std::max)(left.negative_buckets_.EndIndex(), right.negative_buckets_.EndIndex()); + (std::max)(left.negative_buckets_->EndIndex(), right.negative_buckets_->EndIndex()); if (static_cast(pos_max_index) > static_cast(pos_min_index) + low_res.max_buckets_ || @@ -338,35 +338,37 @@ std::unique_ptr Base2ExponentialHistogramAggregation::Diff( result_value.sum_ = (right.sum_ >= left.sum_) ? (right.sum_ - left.sum_) : 0.0; result_value.zero_count_ = (right.zero_count_ >= left.zero_count_) ? (right.zero_count_ - left.zero_count_) : 0; - result_value.positive_buckets_ = AdaptingCircularBufferCounter{right.max_buckets_}; - result_value.negative_buckets_ = AdaptingCircularBufferCounter{right.max_buckets_}; + result_value.positive_buckets_ = + std::make_unique(right.max_buckets_); + result_value.negative_buckets_ = + std::make_unique(right.max_buckets_); - if (!left.positive_buckets_.Empty() && !right.positive_buckets_.Empty()) + if (left.positive_buckets_ && right.positive_buckets_) { for (auto i = pos_min_index; i <= pos_max_index; i++) { - auto l_cnt = left.positive_buckets_.Get(i); - auto r_cnt = right.positive_buckets_.Get(i); + auto l_cnt = left.positive_buckets_->Get(i); + auto r_cnt = right.positive_buckets_->Get(i); // expect right >= left since metric points should be monotonically increasing - auto delta = (std::max)(static_cast(0), r_cnt - l_cnt); - if (l_cnt > 0) + auto delta = std::max(static_cast(0), r_cnt - l_cnt); + if (delta > 0) { - result_value.positive_buckets_.Increment(i, delta); + result_value.positive_buckets_->Increment(i, delta); } } } - if (!left.negative_buckets_.Empty() && !right.negative_buckets_.Empty()) + if (left.negative_buckets_ && right.negative_buckets_) { for (auto i = neg_min_index; i <= neg_max_index; i++) { - auto l_cnt = left.negative_buckets_.Get(i); - auto r_cnt = right.negative_buckets_.Get(i); + auto l_cnt = left.negative_buckets_->Get(i); + auto r_cnt = right.negative_buckets_->Get(i); // expect right >= left since metric points should be monotonically increasing - auto delta = (std::max)(static_cast(0), r_cnt - l_cnt); + auto delta = std::max(static_cast(0), r_cnt - l_cnt); if (delta > 0) { - result_value.negative_buckets_.Increment(i, delta); + result_value.negative_buckets_->Increment(i, delta); } } } @@ -378,7 +380,28 @@ std::unique_ptr Base2ExponentialHistogramAggregation::Diff( PointType Base2ExponentialHistogramAggregation::ToPoint() const noexcept { const std::lock_guard locked(lock_); - return point_data_; + + Base2ExponentialHistogramPointData copy; + copy.sum_ = point_data_.sum_; + copy.min_ = point_data_.min_; + copy.max_ = point_data_.max_; + copy.zero_threshold_ = point_data_.zero_threshold_; + copy.count_ = point_data_.count_; + copy.zero_count_ = point_data_.zero_count_; + copy.max_buckets_ = point_data_.max_buckets_; + copy.scale_ = point_data_.scale_; + copy.record_min_max_ = point_data_.record_min_max_; + + if (point_data_.positive_buckets_) + { + copy.positive_buckets_ = std::make_unique(*point_data_.positive_buckets_); + } + if (point_data_.negative_buckets_) + { + copy.negative_buckets_ = std::make_unique(*point_data_.negative_buckets_); + } + + return copy; } } // namespace metrics From 20ba9353a63b278c127bcbe50b749166c0d188bb Mon Sep 17 00:00:00 2001 From: etmcdonald Date: Wed, 23 Apr 2025 21:39:07 +0000 Subject: [PATCH 42/48] refactor unique_ptr for buckets with deep copies --- exporters/ostream/src/metric_exporter.cc | 20 +-- exporters/ostream/test/ostream_metric_test.cc | 18 +-- exporters/otlp/src/otlp_metric_utils.cc | 25 ++-- .../sdk/metrics/data/point_data.h | 88 +++++++++--- ...base2_exponential_histogram_aggregation.cc | 130 +++++++++++++----- sdk/test/metrics/aggregation_test.cc | 38 +++-- .../sync_metric_storage_histogram_test.cc | 36 ++--- 7 files changed, 243 insertions(+), 112 deletions(-) diff --git a/exporters/ostream/src/metric_exporter.cc b/exporters/ostream/src/metric_exporter.cc index d4a4aaabe0..994b891684 100644 --- a/exporters/ostream/src/metric_exporter.cc +++ b/exporters/ostream/src/metric_exporter.cc @@ -252,6 +252,10 @@ void OStreamMetricExporter::printPointData(const opentelemetry::sdk::metrics::Po { auto histogram_point_data = nostd::get(point_data); + if (!histogram_point_data.positive_buckets_ && !histogram_point_data.negative_buckets_) + { + return; + } sout_ << "\n type: Base2ExponentialHistogramPointData"; sout_ << "\n count: " << histogram_point_data.count_; sout_ << "\n sum: " << histogram_point_data.sum_; @@ -263,21 +267,21 @@ void OStreamMetricExporter::printPointData(const opentelemetry::sdk::metrics::Po } sout_ << "\n scale: " << histogram_point_data.scale_; sout_ << "\n positive buckets:"; - if (!histogram_point_data.positive_buckets_.Empty()) + if (!histogram_point_data.positive_buckets_->Empty()) { - for (auto i = histogram_point_data.positive_buckets_.StartIndex(); - i <= histogram_point_data.positive_buckets_.EndIndex(); ++i) + for (auto i = histogram_point_data.positive_buckets_->StartIndex(); + i <= histogram_point_data.positive_buckets_->EndIndex(); ++i) { - sout_ << "\n\t" << i << ": " << histogram_point_data.positive_buckets_.Get(i); + sout_ << "\n\t" << i << ": " << histogram_point_data.positive_buckets_->Get(i); } } sout_ << "\n negative buckets:"; - if (!histogram_point_data.negative_buckets_.Empty()) + if (!histogram_point_data.negative_buckets_->Empty()) { - for (auto i = histogram_point_data.negative_buckets_.StartIndex(); - i <= histogram_point_data.negative_buckets_.EndIndex(); ++i) + for (auto i = histogram_point_data.negative_buckets_->StartIndex(); + i <= histogram_point_data.negative_buckets_->EndIndex(); ++i) { - sout_ << "\n\t" << i << ": " << histogram_point_data.negative_buckets_.Get(i); + sout_ << "\n\t" << i << ": " << histogram_point_data.negative_buckets_->Get(i); } } } diff --git a/exporters/ostream/test/ostream_metric_test.cc b/exporters/ostream/test/ostream_metric_test.cc index 29e6f6aa00..ff25c35427 100644 --- a/exporters/ostream/test/ostream_metric_test.cc +++ b/exporters/ostream/test/ostream_metric_test.cc @@ -194,11 +194,11 @@ TEST(OStreamMetricsExporter, ExportBase2ExponentialHistogramPointData) histogram_point_data1.record_min_max_ = true; histogram_point_data1.zero_count_ = 1; histogram_point_data1.positive_buckets_ = - opentelemetry::sdk::metrics::AdaptingCircularBufferCounter(10); + std::make_unique(10); histogram_point_data1.negative_buckets_ = - opentelemetry::sdk::metrics::AdaptingCircularBufferCounter(10); - histogram_point_data1.positive_buckets_.Increment(1, 1); - histogram_point_data1.negative_buckets_.Increment(-2, 1); + std::make_unique(10); + histogram_point_data1.positive_buckets_->Increment(1, 1); + histogram_point_data1.negative_buckets_->Increment(-2, 1); metric_sdk::Base2ExponentialHistogramPointData histogram_point_data2; histogram_point_data2.count_ = 4; @@ -209,12 +209,12 @@ TEST(OStreamMetricsExporter, ExportBase2ExponentialHistogramPointData) histogram_point_data2.record_min_max_ = false; histogram_point_data2.zero_count_ = 2; histogram_point_data2.positive_buckets_ = - opentelemetry::sdk::metrics::AdaptingCircularBufferCounter(10); + std::make_unique(10); histogram_point_data2.negative_buckets_ = - opentelemetry::sdk::metrics::AdaptingCircularBufferCounter(10); - histogram_point_data2.positive_buckets_.Increment(3, 1); - histogram_point_data2.negative_buckets_.Increment(-2, 1); - histogram_point_data2.negative_buckets_.Increment(-4, 2); + std::make_unique(10); + histogram_point_data2.positive_buckets_->Increment(3, 1); + histogram_point_data2.negative_buckets_->Increment(-2, 1); + histogram_point_data2.negative_buckets_->Increment(-4, 2); metric_sdk::ResourceMetrics data; auto resource = opentelemetry::sdk::resource::Resource::Create( diff --git a/exporters/otlp/src/otlp_metric_utils.cc b/exporters/otlp/src/otlp_metric_utils.cc index a36da41607..171d922acd 100644 --- a/exporters/otlp/src/otlp_metric_utils.cc +++ b/exporters/otlp/src/otlp_metric_utils.cc @@ -199,6 +199,11 @@ void OtlpMetricUtils::ConvertExponentialHistogramMetric( proto_histogram_point_data->set_time_unix_nano(ts); auto histogram_data = nostd::get( point_data_with_attributes.point_data); + if (histogram_data.positive_buckets_ == nullptr && + histogram_data.negative_buckets_ == nullptr) + { + continue; + } // sum proto_histogram_point_data->set_sum(histogram_data.sum_); proto_histogram_point_data->set_count(histogram_data.count_); @@ -208,27 +213,27 @@ void OtlpMetricUtils::ConvertExponentialHistogramMetric( proto_histogram_point_data->set_max(histogram_data.max_); } // negative buckets - if (!histogram_data.negative_buckets_.Empty()) + if (!histogram_data.negative_buckets_->Empty()) { auto negative_buckets = proto_histogram_point_data->mutable_negative(); - negative_buckets->set_offset(histogram_data.negative_buckets_.StartIndex()); + negative_buckets->set_offset(histogram_data.negative_buckets_->StartIndex()); - for (auto index = histogram_data.negative_buckets_.StartIndex(); - index <= histogram_data.negative_buckets_.EndIndex(); ++index) + for (auto index = histogram_data.negative_buckets_->StartIndex(); + index <= histogram_data.negative_buckets_->EndIndex(); ++index) { - negative_buckets->add_bucket_counts(histogram_data.negative_buckets_.Get(index)); + negative_buckets->add_bucket_counts(histogram_data.negative_buckets_->Get(index)); } } // positive buckets - if (!histogram_data.positive_buckets_.Empty()) + if (!histogram_data.positive_buckets_->Empty()) { auto positive_buckets = proto_histogram_point_data->mutable_positive(); - positive_buckets->set_offset(histogram_data.positive_buckets_.StartIndex()); + positive_buckets->set_offset(histogram_data.positive_buckets_->StartIndex()); - for (auto index = histogram_data.positive_buckets_.StartIndex(); - index <= histogram_data.positive_buckets_.EndIndex(); ++index) + for (auto index = histogram_data.positive_buckets_->StartIndex(); + index <= histogram_data.positive_buckets_->EndIndex(); ++index) { - positive_buckets->add_bucket_counts(histogram_data.positive_buckets_.Get(index)); + positive_buckets->add_bucket_counts(histogram_data.positive_buckets_->Get(index)); } } proto_histogram_point_data->set_scale(histogram_data.scale_); diff --git a/sdk/include/opentelemetry/sdk/metrics/data/point_data.h b/sdk/include/opentelemetry/sdk/metrics/data/point_data.h index 62b2f1abb5..22dc3bf144 100644 --- a/sdk/include/opentelemetry/sdk/metrics/data/point_data.h +++ b/sdk/include/opentelemetry/sdk/metrics/data/point_data.h @@ -69,33 +69,79 @@ class HistogramPointData class Base2ExponentialHistogramPointData { public: - // Delete copy constructor and copy assignment operator - Base2ExponentialHistogramPointData(const Base2ExponentialHistogramPointData &) = delete; - Base2ExponentialHistogramPointData &operator=(const Base2ExponentialHistogramPointData &) = - delete; - - // Default move constructor and move assignment operator Base2ExponentialHistogramPointData(Base2ExponentialHistogramPointData &&) noexcept = default; - Base2ExponentialHistogramPointData &operator=(Base2ExponentialHistogramPointData &&) noexcept = - default; + Base2ExponentialHistogramPointData &operator=(Base2ExponentialHistogramPointData &&) noexcept = default; + + Base2ExponentialHistogramPointData(const Base2ExponentialHistogramPointData &other) + : sum_(other.sum_), + min_(other.min_), + max_(other.max_), + zero_threshold_(other.zero_threshold_), + count_(other.count_), + zero_count_(other.zero_count_), + max_buckets_(other.max_buckets_), + scale_(other.scale_), + record_min_max_(other.record_min_max_) + { + if (other.positive_buckets_) + { + positive_buckets_ = std::make_unique(*other.positive_buckets_); + } + if (other.negative_buckets_) + { + negative_buckets_ = std::make_unique(*other.negative_buckets_); + } + } + + Base2ExponentialHistogramPointData &operator=(const Base2ExponentialHistogramPointData &other) + { + if (this != &other) + { + sum_ = other.sum_; + min_ = other.min_; + max_ = other.max_; + zero_threshold_ = other.zero_threshold_; + count_ = other.count_; + zero_count_ = other.zero_count_; + max_buckets_ = other.max_buckets_; + scale_ = other.scale_; + record_min_max_ = other.record_min_max_; + + if (other.positive_buckets_) + { + positive_buckets_ = std::make_unique(*other.positive_buckets_); + } + else + { + positive_buckets_.reset(); + } + + if (other.negative_buckets_) + { + negative_buckets_ = std::make_unique(*other.negative_buckets_); + } + else + { + negative_buckets_.reset(); + } + } + return *this; + } // Default constructor Base2ExponentialHistogramPointData() = default; - double sum_ = {}; - double min_ = {}; - double max_ = {}; + // Members + double sum_ = {}; + double min_ = {}; + double max_ = {}; double zero_threshold_ = {}; - uint64_t count_ = {}; - uint64_t zero_count_ = {}; - - std::unique_ptr positive_buckets_ = - std::make_unique(0); - std::unique_ptr negative_buckets_ = - std::make_unique(0); - - size_t max_buckets_ = {}; - int32_t scale_ = {}; + uint64_t count_ = {}; + uint64_t zero_count_ = {}; + std::unique_ptr positive_buckets_ = std::make_unique(0); + std::unique_ptr negative_buckets_ = std::make_unique(0); + size_t max_buckets_ = {}; + int32_t scale_ = {}; bool record_min_max_ = true; }; diff --git a/sdk/src/metrics/aggregation/base2_exponential_histogram_aggregation.cc b/sdk/src/metrics/aggregation/base2_exponential_histogram_aggregation.cc index 03e49e07dd..b41e986730 100644 --- a/sdk/src/metrics/aggregation/base2_exponential_histogram_aggregation.cc +++ b/sdk/src/metrics/aggregation/base2_exponential_histogram_aggregation.cc @@ -8,6 +8,7 @@ #include #include #include +#include #include "opentelemetry/common/spin_lock_mutex.h" #include "opentelemetry/nostd/variant.h" @@ -84,22 +85,46 @@ Base2ExponentialHistogramAggregation::Base2ExponentialHistogramAggregation( point_data_.min_ = (std::numeric_limits::max)(); point_data_.max_ = (std::numeric_limits::min)(); + // Initialize buckets + point_data_.positive_buckets_ = std::make_unique(point_data_.max_buckets_); + point_data_.negative_buckets_ = std::make_unique(point_data_.max_buckets_); + indexer_ = Base2ExponentialHistogramIndexer(point_data_.scale_); } -// Base2ExponentialHistogramAggregation::Base2ExponentialHistogramAggregation( -// const Base2ExponentialHistogramPointData &point_data) -// : point_data_{point_data}, -// indexer_(point_data.scale_), -// record_min_max_{point_data.record_min_max_} -// {} +Base2ExponentialHistogramAggregation::Base2ExponentialHistogramAggregation( + const Base2ExponentialHistogramPointData &point_data) + : point_data_{}, + indexer_(point_data.scale_), + record_min_max_{point_data.record_min_max_} +{ + point_data_.sum_ = point_data.sum_; + point_data_.min_ = point_data.min_; + point_data_.max_ = point_data.max_; + point_data_.zero_threshold_ = point_data.zero_threshold_; + point_data_.count_ = point_data.count_; + point_data_.zero_count_ = point_data.zero_count_; + point_data_.max_buckets_ = point_data.max_buckets_; + point_data_.scale_ = point_data.scale_; + point_data_.record_min_max_ = point_data.record_min_max_; + + // Deep copy the unique_ptr members + if (point_data.positive_buckets_) + { + point_data_.positive_buckets_ = std::make_unique(*point_data.positive_buckets_); + } + if (point_data.negative_buckets_) + { + point_data_.negative_buckets_ = std::make_unique(*point_data.negative_buckets_); + } +} -// Base2ExponentialHistogramAggregation::Base2ExponentialHistogramAggregation( -// Base2ExponentialHistogramPointData &&point_data) -// : point_data_{std::move(point_data)}, -// indexer_(point_data_.scale_), -// record_min_max_{point_data_.record_min_max_} -// {} +Base2ExponentialHistogramAggregation::Base2ExponentialHistogramAggregation( + Base2ExponentialHistogramPointData &&point_data) + : point_data_{std::move(point_data)}, + indexer_(point_data_.scale_), + record_min_max_{point_data_.record_min_max_} +{} void Base2ExponentialHistogramAggregation::Aggregate( int64_t value, @@ -129,11 +154,17 @@ void Base2ExponentialHistogramAggregation::Aggregate( } else if (value > 0) { - AggregateIntoBuckets(point_data_.positive_buckets_, value); + if (point_data_.positive_buckets_) + { + AggregateIntoBuckets(point_data_.positive_buckets_, value); + } } else { - AggregateIntoBuckets(point_data_.negative_buckets_, -value); + if (point_data_.negative_buckets_) + { + AggregateIntoBuckets(point_data_.negative_buckets_, -value); + } } } @@ -141,6 +172,11 @@ void Base2ExponentialHistogramAggregation::AggregateIntoBuckets( std::unique_ptr &buckets, double value) noexcept { + if (!buckets) + { + buckets = std::make_unique(point_data_.max_buckets_); + } + if (buckets->MaxSize() == 0) { buckets = std::make_unique(point_data_.max_buckets_); @@ -166,8 +202,14 @@ void Base2ExponentialHistogramAggregation::Downscale(uint32_t by) noexcept return; } - DownscaleBuckets(point_data_.positive_buckets_, by); - DownscaleBuckets(point_data_.negative_buckets_, by); + if (point_data_.positive_buckets_) + { + DownscaleBuckets(point_data_.positive_buckets_, by); + } + if (point_data_.negative_buckets_) + { + DownscaleBuckets(point_data_.negative_buckets_, by); + } point_data_.scale_ -= by; indexer_ = Base2ExponentialHistogramIndexer(point_data_.scale_); @@ -296,33 +338,61 @@ std::unique_ptr Base2ExponentialHistogramAggregation::Diff( if (scale_reduction > 0) { - DownscaleBuckets(high_res.positive_buckets_, scale_reduction); - DownscaleBuckets(high_res.negative_buckets_, scale_reduction); + if (high_res.positive_buckets_) + { + DownscaleBuckets(high_res.positive_buckets_, scale_reduction); + } + + if (high_res.negative_buckets_) + { + DownscaleBuckets(high_res.negative_buckets_, scale_reduction); + } + high_res.scale_ -= scale_reduction; } } auto pos_min_index = - (std::min)(left.positive_buckets_->StartIndex(), right.positive_buckets_->StartIndex()); + (left.positive_buckets_ && right.positive_buckets_) + ? (std::min)(left.positive_buckets_->StartIndex(), right.positive_buckets_->StartIndex()) + : 0; auto pos_max_index = - (std::max)(left.positive_buckets_->EndIndex(), right.positive_buckets_->EndIndex()); + (left.positive_buckets_ && right.positive_buckets_) + ? (std::max)(left.positive_buckets_->EndIndex(), right.positive_buckets_->EndIndex()) + : 0; auto neg_min_index = - (std::min)(left.negative_buckets_->StartIndex(), right.negative_buckets_->StartIndex()); + (left.negative_buckets_ && right.negative_buckets_) + ? (std::min)(left.negative_buckets_->StartIndex(), right.negative_buckets_->StartIndex()) + : 0; auto neg_max_index = - (std::max)(left.negative_buckets_->EndIndex(), right.negative_buckets_->EndIndex()); + (left.negative_buckets_ && right.negative_buckets_) + ? (std::max)(left.negative_buckets_->EndIndex(), right.negative_buckets_->EndIndex()) + : 0; if (static_cast(pos_max_index) > static_cast(pos_min_index) + low_res.max_buckets_ || static_cast(neg_max_index) > static_cast(neg_min_index) + low_res.max_buckets_) { - // We need to downscale the buckets to fit into the new max_buckets_. const uint32_t scale_reduction = GetScaleReduction(pos_min_index, pos_max_index, low_res.max_buckets_); - DownscaleBuckets(left.positive_buckets_, scale_reduction); - DownscaleBuckets(right.positive_buckets_, scale_reduction); - DownscaleBuckets(left.negative_buckets_, scale_reduction); - DownscaleBuckets(right.negative_buckets_, scale_reduction); + + if (left.positive_buckets_) + { + DownscaleBuckets(left.positive_buckets_, scale_reduction); + } + if (right.positive_buckets_) + { + DownscaleBuckets(right.positive_buckets_, scale_reduction); + } + if (left.negative_buckets_) + { + DownscaleBuckets(left.negative_buckets_, scale_reduction); + } + if (right.negative_buckets_) + { + DownscaleBuckets(right.negative_buckets_, scale_reduction); + } left.scale_ -= scale_reduction; right.scale_ -= scale_reduction; } @@ -331,13 +401,11 @@ std::unique_ptr Base2ExponentialHistogramAggregation::Diff( result_value.scale_ = low_res.scale_; result_value.max_buckets_ = low_res.max_buckets_; result_value.record_min_max_ = false; - // caution for underflow - // expect right.{sum, count} >= left.{sum, count} since metric points should be monotonically - // increasing result_value.count_ = (right.count_ >= left.count_) ? (right.count_ - left.count_) : 0; result_value.sum_ = (right.sum_ >= left.sum_) ? (right.sum_ - left.sum_) : 0.0; result_value.zero_count_ = (right.zero_count_ >= left.zero_count_) ? (right.zero_count_ - left.zero_count_) : 0; + result_value.positive_buckets_ = std::make_unique(right.max_buckets_); result_value.negative_buckets_ = @@ -349,7 +417,6 @@ std::unique_ptr Base2ExponentialHistogramAggregation::Diff( { auto l_cnt = left.positive_buckets_->Get(i); auto r_cnt = right.positive_buckets_->Get(i); - // expect right >= left since metric points should be monotonically increasing auto delta = std::max(static_cast(0), r_cnt - l_cnt); if (delta > 0) { @@ -364,7 +431,6 @@ std::unique_ptr Base2ExponentialHistogramAggregation::Diff( { auto l_cnt = left.negative_buckets_->Get(i); auto r_cnt = right.negative_buckets_->Get(i); - // expect right >= left since metric points should be monotonically increasing auto delta = std::max(static_cast(0), r_cnt - l_cnt); if (delta > 0) { diff --git a/sdk/test/metrics/aggregation_test.cc b/sdk/test/metrics/aggregation_test.cc index 98d76ec701..85a01af589 100644 --- a/sdk/test/metrics/aggregation_test.cc +++ b/sdk/test/metrics/aggregation_test.cc @@ -245,8 +245,10 @@ TEST(Aggregation, Base2ExponentialHistogramAggregation) EXPECT_EQ(histo_point.max_, (std::numeric_limits::min)()); EXPECT_EQ(histo_point.scale_, SCALE0); EXPECT_EQ(histo_point.max_buckets_, MAX_BUCKETS0); - ASSERT_TRUE(histo_point.positive_buckets_.Empty()); - ASSERT_TRUE(histo_point.negative_buckets_.Empty()); + ASSERT_TRUE(histo_point.positive_buckets_ != nullptr); + ASSERT_TRUE(histo_point.negative_buckets_ != nullptr); + ASSERT_TRUE(histo_point.positive_buckets_->Empty()); + ASSERT_TRUE(histo_point.negative_buckets_->Empty()); // Create a new aggreagte based in point data { @@ -262,8 +264,8 @@ TEST(Aggregation, Base2ExponentialHistogramAggregation) EXPECT_EQ(histo_point2.max_, (std::numeric_limits::min)()); EXPECT_EQ(histo_point2.scale_, SCALE0); EXPECT_EQ(histo_point2.max_buckets_, MAX_BUCKETS0); - ASSERT_TRUE(histo_point2.positive_buckets_.Empty()); - ASSERT_TRUE(histo_point2.negative_buckets_.Empty()); + ASSERT_TRUE(histo_point2.positive_buckets_->Empty()); + ASSERT_TRUE(histo_point2.negative_buckets_->Empty()); } // zero point @@ -280,12 +282,14 @@ TEST(Aggregation, Base2ExponentialHistogramAggregation) EXPECT_EQ(histo_point.sum_, 6.5); EXPECT_EQ(histo_point.min_, 0.0); EXPECT_EQ(histo_point.max_, 3.5); - ASSERT_FALSE(histo_point.positive_buckets_.Empty()); - auto start_index = histo_point.positive_buckets_.StartIndex(); - auto end_index = histo_point.positive_buckets_.EndIndex(); + ASSERT_TRUE(histo_point.positive_buckets_ != nullptr); + ASSERT_TRUE(histo_point.negative_buckets_ != nullptr); + ASSERT_FALSE(histo_point.positive_buckets_->Empty()); + auto start_index = histo_point.positive_buckets_->StartIndex(); + auto end_index = histo_point.positive_buckets_->EndIndex(); EXPECT_EQ(start_index, 1); EXPECT_EQ(end_index, 1); - EXPECT_EQ(histo_point.positive_buckets_.Get(start_index), 2); + EXPECT_EQ(histo_point.positive_buckets_->Get(start_index), 2); // Recording in a different bucket (bucket -2 at scale 0) scale0_aggr.Aggregate(-0.3, {}); @@ -294,8 +298,10 @@ TEST(Aggregation, Base2ExponentialHistogramAggregation) EXPECT_EQ(histo_point.sum_, 6.2); EXPECT_EQ(histo_point.min_, -0.3); EXPECT_EQ(histo_point.max_, 3.5); - EXPECT_EQ(histo_point.negative_buckets_.Get(-2), 1); - EXPECT_EQ(histo_point.positive_buckets_.Get(1), 2); + ASSERT_TRUE(histo_point.positive_buckets_ != nullptr); + ASSERT_TRUE(histo_point.negative_buckets_ != nullptr); + EXPECT_EQ(histo_point.negative_buckets_->Get(-2), 1); + EXPECT_EQ(histo_point.positive_buckets_->Get(1), 2); Base2ExponentialHistogramAggregationConfig scale1_config; scale1_config.max_scale_ = 1; @@ -323,9 +329,11 @@ TEST(Aggregation, Base2ExponentialHistogramAggregation) EXPECT_EQ(merged_point.min_, -0.3); EXPECT_EQ(merged_point.max_, 3.5); EXPECT_EQ(merged_point.scale_, 0); - EXPECT_EQ(merged_point.positive_buckets_.Get(1), 4); - EXPECT_EQ(merged_point.negative_buckets_.Get(-2), 1); - EXPECT_EQ(merged_point.positive_buckets_.Get(2), 0); + ASSERT_TRUE(merged_point.positive_buckets_ != nullptr); + ASSERT_TRUE(merged_point.negative_buckets_ != nullptr); + EXPECT_EQ(merged_point.positive_buckets_->Get(1), 4); + EXPECT_EQ(merged_point.negative_buckets_->Get(-2), 1); + EXPECT_EQ(merged_point.positive_buckets_->Get(2), 0); // Diff test Base2ExponentialHistogramAggregation scale2_aggr(&scale1_config); @@ -345,5 +353,7 @@ TEST(Aggregation, Base2ExponentialHistogramAggregation) EXPECT_NEAR(diffd_point.sum_, 2.3, 1e-9); EXPECT_EQ(diffd_point.zero_count_, 0); EXPECT_EQ(diffd_point.scale_, 1); - EXPECT_EQ(diffd_point.positive_buckets_.Get(2), 1); + ASSERT_TRUE(diffd_point.positive_buckets_ != nullptr); + ASSERT_TRUE(diffd_point.negative_buckets_ != nullptr); + EXPECT_EQ(diffd_point.positive_buckets_->Get(2), 1); } diff --git a/sdk/test/metrics/sync_metric_storage_histogram_test.cc b/sdk/test/metrics/sync_metric_storage_histogram_test.cc index bf200cf486..89dedb397f 100644 --- a/sdk/test/metrics/sync_metric_storage_histogram_test.cc +++ b/sdk/test/metrics/sync_metric_storage_histogram_test.cc @@ -389,11 +389,11 @@ TEST_P(WritableMetricStorageHistogramTestFixture, Base2ExponentialDoubleHistogra EXPECT_EQ(data.count_, 2); EXPECT_EQ(data.min_, 10); EXPECT_EQ(data.max_, 20); - EXPECT_EQ(data.negative_buckets_.Empty(), true); - auto start_index = data.positive_buckets_.StartIndex(); - auto end_index = data.positive_buckets_.EndIndex(); - EXPECT_EQ(data.positive_buckets_.Get(start_index), 1); - EXPECT_EQ(data.positive_buckets_.Get(end_index), 1); + EXPECT_EQ(data.negative_buckets_->Empty(), true); + auto start_index = data.positive_buckets_->StartIndex(); + auto end_index = data.positive_buckets_->EndIndex(); + EXPECT_EQ(data.positive_buckets_->Get(start_index), 1); + EXPECT_EQ(data.positive_buckets_->Get(end_index), 1); count_attributes++; } else if (opentelemetry::nostd::get( @@ -403,11 +403,11 @@ TEST_P(WritableMetricStorageHistogramTestFixture, Base2ExponentialDoubleHistogra EXPECT_EQ(data.count_, 2); EXPECT_EQ(data.min_, 30); EXPECT_EQ(data.max_, 40); - auto start_index = data.positive_buckets_.StartIndex(); - auto end_index = data.positive_buckets_.EndIndex(); - EXPECT_EQ(data.positive_buckets_.Get(start_index), 1); - EXPECT_EQ(data.positive_buckets_.Get(end_index), 1); - EXPECT_EQ(data.negative_buckets_.Empty(), true); + auto start_index = data.positive_buckets_->StartIndex(); + auto end_index = data.positive_buckets_->EndIndex(); + EXPECT_EQ(data.positive_buckets_->Get(start_index), 1); + EXPECT_EQ(data.positive_buckets_->Get(end_index), 1); + EXPECT_EQ(data.negative_buckets_->Empty(), true); count_attributes++; } } @@ -478,8 +478,8 @@ TEST_P(WritableMetricStorageHistogramTestFixture, Base2ExponentialDoubleHistogra { EXPECT_EQ(data.sum_, expected_total_get_requests); count_attributes++; - auto start_index = data.positive_buckets_.StartIndex(); - auto end_index = data.positive_buckets_.EndIndex(); + auto start_index = data.positive_buckets_->StartIndex(); + auto end_index = data.positive_buckets_->EndIndex(); if (temporality == AggregationTemporality::kCumulative) { EXPECT_EQ(data.count_, 3); @@ -488,7 +488,7 @@ TEST_P(WritableMetricStorageHistogramTestFixture, Base2ExponentialDoubleHistogra uint64_t count = 0; for (auto i = start_index; i <= end_index; ++i) { - count += data.positive_buckets_.Get(i); + count += data.positive_buckets_->Get(i); } EXPECT_EQ(count, 3); } @@ -497,7 +497,7 @@ TEST_P(WritableMetricStorageHistogramTestFixture, Base2ExponentialDoubleHistogra EXPECT_EQ(data.count_, 1); EXPECT_EQ(data.min_, 50); EXPECT_EQ(data.max_, 50); - EXPECT_EQ(data.positive_buckets_.Get(start_index), 1); + EXPECT_EQ(data.positive_buckets_->Get(start_index), 1); EXPECT_EQ(end_index, start_index); } } @@ -506,8 +506,8 @@ TEST_P(WritableMetricStorageHistogramTestFixture, Base2ExponentialDoubleHistogra { EXPECT_EQ(data.sum_, expected_total_put_requests); count_attributes++; - auto start_index = data.positive_buckets_.StartIndex(); - auto end_index = data.positive_buckets_.EndIndex(); + auto start_index = data.positive_buckets_->StartIndex(); + auto end_index = data.positive_buckets_->EndIndex(); if (temporality == AggregationTemporality::kCumulative) { EXPECT_EQ(data.count_, 3); @@ -516,7 +516,7 @@ TEST_P(WritableMetricStorageHistogramTestFixture, Base2ExponentialDoubleHistogra uint64_t count = 0; for (auto i = start_index; i <= end_index; ++i) { - count += data.positive_buckets_.Get(i); + count += data.positive_buckets_->Get(i); } EXPECT_EQ(count, 3); } @@ -525,7 +525,7 @@ TEST_P(WritableMetricStorageHistogramTestFixture, Base2ExponentialDoubleHistogra EXPECT_EQ(data.count_, 1); EXPECT_EQ(data.min_, 40); EXPECT_EQ(data.max_, 40); - EXPECT_EQ(data.positive_buckets_.Get(start_index), 1); + EXPECT_EQ(data.positive_buckets_->Get(start_index), 1); EXPECT_EQ(end_index, start_index); } } From 6468765d8a56dae3108333701ebf054095f2328d Mon Sep 17 00:00:00 2001 From: etmcdonald Date: Wed, 23 Apr 2025 21:42:31 +0000 Subject: [PATCH 43/48] format --- exporters/ostream/src/metric_exporter.cc | 2 +- exporters/ostream/test/ostream_metric_test.cc | 8 +-- exporters/otlp/src/otlp_metric_utils.cc | 3 +- .../sdk/metrics/data/point_data.h | 43 +++++++------- ...base2_exponential_histogram_aggregation.cc | 56 ++++++++++--------- 5 files changed, 60 insertions(+), 52 deletions(-) diff --git a/exporters/ostream/src/metric_exporter.cc b/exporters/ostream/src/metric_exporter.cc index 994b891684..e711a30d53 100644 --- a/exporters/ostream/src/metric_exporter.cc +++ b/exporters/ostream/src/metric_exporter.cc @@ -253,7 +253,7 @@ void OStreamMetricExporter::printPointData(const opentelemetry::sdk::metrics::Po auto histogram_point_data = nostd::get(point_data); if (!histogram_point_data.positive_buckets_ && !histogram_point_data.negative_buckets_) - { + { return; } sout_ << "\n type: Base2ExponentialHistogramPointData"; diff --git a/exporters/ostream/test/ostream_metric_test.cc b/exporters/ostream/test/ostream_metric_test.cc index ff25c35427..de79c3c757 100644 --- a/exporters/ostream/test/ostream_metric_test.cc +++ b/exporters/ostream/test/ostream_metric_test.cc @@ -194,9 +194,9 @@ TEST(OStreamMetricsExporter, ExportBase2ExponentialHistogramPointData) histogram_point_data1.record_min_max_ = true; histogram_point_data1.zero_count_ = 1; histogram_point_data1.positive_buckets_ = - std::make_unique(10); + std::make_unique(10); histogram_point_data1.negative_buckets_ = - std::make_unique(10); + std::make_unique(10); histogram_point_data1.positive_buckets_->Increment(1, 1); histogram_point_data1.negative_buckets_->Increment(-2, 1); @@ -209,9 +209,9 @@ TEST(OStreamMetricsExporter, ExportBase2ExponentialHistogramPointData) histogram_point_data2.record_min_max_ = false; histogram_point_data2.zero_count_ = 2; histogram_point_data2.positive_buckets_ = - std::make_unique(10); + std::make_unique(10); histogram_point_data2.negative_buckets_ = - std::make_unique(10); + std::make_unique(10); histogram_point_data2.positive_buckets_->Increment(3, 1); histogram_point_data2.negative_buckets_->Increment(-2, 1); histogram_point_data2.negative_buckets_->Increment(-4, 2); diff --git a/exporters/otlp/src/otlp_metric_utils.cc b/exporters/otlp/src/otlp_metric_utils.cc index 171d922acd..8c2a25b988 100644 --- a/exporters/otlp/src/otlp_metric_utils.cc +++ b/exporters/otlp/src/otlp_metric_utils.cc @@ -199,8 +199,7 @@ void OtlpMetricUtils::ConvertExponentialHistogramMetric( proto_histogram_point_data->set_time_unix_nano(ts); auto histogram_data = nostd::get( point_data_with_attributes.point_data); - if (histogram_data.positive_buckets_ == nullptr && - histogram_data.negative_buckets_ == nullptr) + if (histogram_data.positive_buckets_ == nullptr && histogram_data.negative_buckets_ == nullptr) { continue; } diff --git a/sdk/include/opentelemetry/sdk/metrics/data/point_data.h b/sdk/include/opentelemetry/sdk/metrics/data/point_data.h index 22dc3bf144..df14e2d099 100644 --- a/sdk/include/opentelemetry/sdk/metrics/data/point_data.h +++ b/sdk/include/opentelemetry/sdk/metrics/data/point_data.h @@ -70,7 +70,8 @@ class Base2ExponentialHistogramPointData { public: Base2ExponentialHistogramPointData(Base2ExponentialHistogramPointData &&) noexcept = default; - Base2ExponentialHistogramPointData &operator=(Base2ExponentialHistogramPointData &&) noexcept = default; + Base2ExponentialHistogramPointData &operator=(Base2ExponentialHistogramPointData &&) noexcept = + default; Base2ExponentialHistogramPointData(const Base2ExponentialHistogramPointData &other) : sum_(other.sum_), @@ -97,19 +98,20 @@ class Base2ExponentialHistogramPointData { if (this != &other) { - sum_ = other.sum_; - min_ = other.min_; - max_ = other.max_; + sum_ = other.sum_; + min_ = other.min_; + max_ = other.max_; zero_threshold_ = other.zero_threshold_; - count_ = other.count_; - zero_count_ = other.zero_count_; - max_buckets_ = other.max_buckets_; - scale_ = other.scale_; + count_ = other.count_; + zero_count_ = other.zero_count_; + max_buckets_ = other.max_buckets_; + scale_ = other.scale_; record_min_max_ = other.record_min_max_; if (other.positive_buckets_) { - positive_buckets_ = std::make_unique(*other.positive_buckets_); + positive_buckets_ = + std::make_unique(*other.positive_buckets_); } else { @@ -118,7 +120,8 @@ class Base2ExponentialHistogramPointData if (other.negative_buckets_) { - negative_buckets_ = std::make_unique(*other.negative_buckets_); + negative_buckets_ = + std::make_unique(*other.negative_buckets_); } else { @@ -132,16 +135,18 @@ class Base2ExponentialHistogramPointData Base2ExponentialHistogramPointData() = default; // Members - double sum_ = {}; - double min_ = {}; - double max_ = {}; + double sum_ = {}; + double min_ = {}; + double max_ = {}; double zero_threshold_ = {}; - uint64_t count_ = {}; - uint64_t zero_count_ = {}; - std::unique_ptr positive_buckets_ = std::make_unique(0); - std::unique_ptr negative_buckets_ = std::make_unique(0); - size_t max_buckets_ = {}; - int32_t scale_ = {}; + uint64_t count_ = {}; + uint64_t zero_count_ = {}; + std::unique_ptr positive_buckets_ = + std::make_unique(0); + std::unique_ptr negative_buckets_ = + std::make_unique(0); + size_t max_buckets_ = {}; + int32_t scale_ = {}; bool record_min_max_ = true; }; diff --git a/sdk/src/metrics/aggregation/base2_exponential_histogram_aggregation.cc b/sdk/src/metrics/aggregation/base2_exponential_histogram_aggregation.cc index b41e986730..286477c21e 100644 --- a/sdk/src/metrics/aggregation/base2_exponential_histogram_aggregation.cc +++ b/sdk/src/metrics/aggregation/base2_exponential_histogram_aggregation.cc @@ -4,11 +4,11 @@ #include #include #include +#include #include #include #include #include -#include #include "opentelemetry/common/spin_lock_mutex.h" #include "opentelemetry/nostd/variant.h" @@ -86,36 +86,38 @@ Base2ExponentialHistogramAggregation::Base2ExponentialHistogramAggregation( point_data_.max_ = (std::numeric_limits::min)(); // Initialize buckets - point_data_.positive_buckets_ = std::make_unique(point_data_.max_buckets_); - point_data_.negative_buckets_ = std::make_unique(point_data_.max_buckets_); + point_data_.positive_buckets_ = + std::make_unique(point_data_.max_buckets_); + point_data_.negative_buckets_ = + std::make_unique(point_data_.max_buckets_); indexer_ = Base2ExponentialHistogramIndexer(point_data_.scale_); } Base2ExponentialHistogramAggregation::Base2ExponentialHistogramAggregation( const Base2ExponentialHistogramPointData &point_data) - : point_data_{}, - indexer_(point_data.scale_), - record_min_max_{point_data.record_min_max_} + : point_data_{}, indexer_(point_data.scale_), record_min_max_{point_data.record_min_max_} { - point_data_.sum_ = point_data.sum_; - point_data_.min_ = point_data.min_; - point_data_.max_ = point_data.max_; + point_data_.sum_ = point_data.sum_; + point_data_.min_ = point_data.min_; + point_data_.max_ = point_data.max_; point_data_.zero_threshold_ = point_data.zero_threshold_; - point_data_.count_ = point_data.count_; - point_data_.zero_count_ = point_data.zero_count_; - point_data_.max_buckets_ = point_data.max_buckets_; - point_data_.scale_ = point_data.scale_; + point_data_.count_ = point_data.count_; + point_data_.zero_count_ = point_data.zero_count_; + point_data_.max_buckets_ = point_data.max_buckets_; + point_data_.scale_ = point_data.scale_; point_data_.record_min_max_ = point_data.record_min_max_; // Deep copy the unique_ptr members if (point_data.positive_buckets_) { - point_data_.positive_buckets_ = std::make_unique(*point_data.positive_buckets_); + point_data_.positive_buckets_ = + std::make_unique(*point_data.positive_buckets_); } if (point_data.negative_buckets_) { - point_data_.negative_buckets_ = std::make_unique(*point_data.negative_buckets_); + point_data_.negative_buckets_ = + std::make_unique(*point_data.negative_buckets_); } } @@ -401,8 +403,8 @@ std::unique_ptr Base2ExponentialHistogramAggregation::Diff( result_value.scale_ = low_res.scale_; result_value.max_buckets_ = low_res.max_buckets_; result_value.record_min_max_ = false; - result_value.count_ = (right.count_ >= left.count_) ? (right.count_ - left.count_) : 0; - result_value.sum_ = (right.sum_ >= left.sum_) ? (right.sum_ - left.sum_) : 0.0; + result_value.count_ = (right.count_ >= left.count_) ? (right.count_ - left.count_) : 0; + result_value.sum_ = (right.sum_ >= left.sum_) ? (right.sum_ - left.sum_) : 0.0; result_value.zero_count_ = (right.zero_count_ >= left.zero_count_) ? (right.zero_count_ - left.zero_count_) : 0; @@ -448,23 +450,25 @@ PointType Base2ExponentialHistogramAggregation::ToPoint() const noexcept const std::lock_guard locked(lock_); Base2ExponentialHistogramPointData copy; - copy.sum_ = point_data_.sum_; - copy.min_ = point_data_.min_; - copy.max_ = point_data_.max_; + copy.sum_ = point_data_.sum_; + copy.min_ = point_data_.min_; + copy.max_ = point_data_.max_; copy.zero_threshold_ = point_data_.zero_threshold_; - copy.count_ = point_data_.count_; - copy.zero_count_ = point_data_.zero_count_; - copy.max_buckets_ = point_data_.max_buckets_; - copy.scale_ = point_data_.scale_; + copy.count_ = point_data_.count_; + copy.zero_count_ = point_data_.zero_count_; + copy.max_buckets_ = point_data_.max_buckets_; + copy.scale_ = point_data_.scale_; copy.record_min_max_ = point_data_.record_min_max_; if (point_data_.positive_buckets_) { - copy.positive_buckets_ = std::make_unique(*point_data_.positive_buckets_); + copy.positive_buckets_ = + std::make_unique(*point_data_.positive_buckets_); } if (point_data_.negative_buckets_) { - copy.negative_buckets_ = std::make_unique(*point_data_.negative_buckets_); + copy.negative_buckets_ = + std::make_unique(*point_data_.negative_buckets_); } return copy; From 2bfd9dadfd56afb43f5e4633c49f15cae01bb169 Mon Sep 17 00:00:00 2001 From: etmcdonald Date: Thu, 24 Apr 2025 16:14:21 +0000 Subject: [PATCH 44/48] update otlp serialization test with unique ptr --- .../test/otlp_metrics_serialization_test.cc | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/exporters/otlp/test/otlp_metrics_serialization_test.cc b/exporters/otlp/test/otlp_metrics_serialization_test.cc index 855051ad62..dc5f0fa209 100644 --- a/exporters/otlp/test/otlp_metrics_serialization_test.cc +++ b/exporters/otlp/test/otlp_metrics_serialization_test.cc @@ -144,10 +144,10 @@ static metrics_sdk::MetricData CreateExponentialHistogramAggregationData( s_data_1.scale_ = 3; s_data_1.record_min_max_ = true; s_data_1.zero_count_ = 1; - s_data_1.positive_buckets_ = opentelemetry::sdk::metrics::AdaptingCircularBufferCounter(10); - s_data_1.negative_buckets_ = opentelemetry::sdk::metrics::AdaptingCircularBufferCounter(10); - s_data_1.positive_buckets_.Increment(1, 1); - s_data_1.negative_buckets_.Increment(-2, 1); + s_data_1.positive_buckets_ = std::make_unique(10); + s_data_1.negative_buckets_ = std::make_unique(10); + s_data_1.positive_buckets_->Increment(1, 1); + s_data_1.negative_buckets_->Increment(-2, 1); s_data_2.count_ = 4; s_data_2.sum_ = 6.2; @@ -156,11 +156,11 @@ static metrics_sdk::MetricData CreateExponentialHistogramAggregationData( s_data_2.scale_ = 3; s_data_2.record_min_max_ = false; s_data_2.zero_count_ = 2; - s_data_2.positive_buckets_ = opentelemetry::sdk::metrics::AdaptingCircularBufferCounter(10); - s_data_2.negative_buckets_ = opentelemetry::sdk::metrics::AdaptingCircularBufferCounter(10); - s_data_2.positive_buckets_.Increment(3, 1); - s_data_2.negative_buckets_.Increment(-2, 1); - s_data_2.negative_buckets_.Increment(-4, 2); + s_data_2.positive_buckets_ = std::make_unique(10); + s_data_2.negative_buckets_ = std::make_unique(10); + s_data_2.positive_buckets_->Increment(3, 1); + s_data_2.negative_buckets_->Increment(-2, 1); + s_data_2.negative_buckets_->Increment(-4, 2); data.aggregation_temporality = metrics_sdk::AggregationTemporality::kCumulative; data.end_ts = opentelemetry::common::SystemTimestamp(now_time); From 71855a98aef2a69fa87e5576848ff0920140bab6 Mon Sep 17 00:00:00 2001 From: etmcdonald Date: Thu, 24 Apr 2025 17:08:30 +0000 Subject: [PATCH 45/48] format --- .../test/otlp_metrics_serialization_test.cc | 40 ++++++++++--------- 1 file changed, 22 insertions(+), 18 deletions(-) diff --git a/exporters/otlp/test/otlp_metrics_serialization_test.cc b/exporters/otlp/test/otlp_metrics_serialization_test.cc index dc5f0fa209..c7b72aa75d 100644 --- a/exporters/otlp/test/otlp_metrics_serialization_test.cc +++ b/exporters/otlp/test/otlp_metrics_serialization_test.cc @@ -137,27 +137,31 @@ static metrics_sdk::MetricData CreateExponentialHistogramAggregationData( metrics_sdk::InstrumentType::kHistogram, metrics_sdk::InstrumentValueType::kDouble}; metrics_sdk::Base2ExponentialHistogramPointData s_data_1, s_data_2; - s_data_1.count_ = 3; - s_data_1.sum_ = 6.5; - s_data_1.min_ = 0.0; - s_data_1.max_ = 3.5; - s_data_1.scale_ = 3; - s_data_1.record_min_max_ = true; - s_data_1.zero_count_ = 1; - s_data_1.positive_buckets_ = std::make_unique(10); - s_data_1.negative_buckets_ = std::make_unique(10); + s_data_1.count_ = 3; + s_data_1.sum_ = 6.5; + s_data_1.min_ = 0.0; + s_data_1.max_ = 3.5; + s_data_1.scale_ = 3; + s_data_1.record_min_max_ = true; + s_data_1.zero_count_ = 1; + s_data_1.positive_buckets_ = + std::make_unique(10); + s_data_1.negative_buckets_ = + std::make_unique(10); s_data_1.positive_buckets_->Increment(1, 1); s_data_1.negative_buckets_->Increment(-2, 1); - s_data_2.count_ = 4; - s_data_2.sum_ = 6.2; - s_data_2.min_ = -0.03; - s_data_2.max_ = 3.5; - s_data_2.scale_ = 3; - s_data_2.record_min_max_ = false; - s_data_2.zero_count_ = 2; - s_data_2.positive_buckets_ = std::make_unique(10); - s_data_2.negative_buckets_ = std::make_unique(10); + s_data_2.count_ = 4; + s_data_2.sum_ = 6.2; + s_data_2.min_ = -0.03; + s_data_2.max_ = 3.5; + s_data_2.scale_ = 3; + s_data_2.record_min_max_ = false; + s_data_2.zero_count_ = 2; + s_data_2.positive_buckets_ = + std::make_unique(10); + s_data_2.negative_buckets_ = + std::make_unique(10); s_data_2.positive_buckets_->Increment(3, 1); s_data_2.negative_buckets_->Increment(-2, 1); s_data_2.negative_buckets_->Increment(-4, 2); From b1f54d427ef5f7bb03622211f902a03828cfa491 Mon Sep 17 00:00:00 2001 From: "Felipe C. Dos Santos" Date: Thu, 24 Apr 2025 20:32:54 +0000 Subject: [PATCH 46/48] Small pipeline fixes --- exporters/otlp/src/otlp_metric_utils.cc | 1 + .../aggregation/base2_exponential_histogram_aggregation.cc | 5 ++--- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/exporters/otlp/src/otlp_metric_utils.cc b/exporters/otlp/src/otlp_metric_utils.cc index 8c2a25b988..5383c0d563 100644 --- a/exporters/otlp/src/otlp_metric_utils.cc +++ b/exporters/otlp/src/otlp_metric_utils.cc @@ -3,6 +3,7 @@ #include #include +#include #include #include diff --git a/sdk/src/metrics/aggregation/base2_exponential_histogram_aggregation.cc b/sdk/src/metrics/aggregation/base2_exponential_histogram_aggregation.cc index 286477c21e..ea98c88a0b 100644 --- a/sdk/src/metrics/aggregation/base2_exponential_histogram_aggregation.cc +++ b/sdk/src/metrics/aggregation/base2_exponential_histogram_aggregation.cc @@ -4,7 +4,6 @@ #include #include #include -#include #include #include #include @@ -419,7 +418,7 @@ std::unique_ptr Base2ExponentialHistogramAggregation::Diff( { auto l_cnt = left.positive_buckets_->Get(i); auto r_cnt = right.positive_buckets_->Get(i); - auto delta = std::max(static_cast(0), r_cnt - l_cnt); + auto delta = (std::max)(static_cast(0), r_cnt - l_cnt); if (delta > 0) { result_value.positive_buckets_->Increment(i, delta); @@ -433,7 +432,7 @@ std::unique_ptr Base2ExponentialHistogramAggregation::Diff( { auto l_cnt = left.negative_buckets_->Get(i); auto r_cnt = right.negative_buckets_->Get(i); - auto delta = std::max(static_cast(0), r_cnt - l_cnt); + auto delta = (std::max)(static_cast(0), r_cnt - l_cnt); if (delta > 0) { result_value.negative_buckets_->Increment(i, delta); From 1d96b3e24e5c9514b8b50f9171bb1ca4d8e4b6eb Mon Sep 17 00:00:00 2001 From: etmcdonald Date: Thu, 24 Apr 2025 23:38:47 +0000 Subject: [PATCH 47/48] iwyu fix --- exporters/ostream/src/metric_exporter.cc | 1 + 1 file changed, 1 insertion(+) diff --git a/exporters/ostream/src/metric_exporter.cc b/exporters/ostream/src/metric_exporter.cc index e711a30d53..39f5b017e7 100644 --- a/exporters/ostream/src/metric_exporter.cc +++ b/exporters/ostream/src/metric_exporter.cc @@ -8,6 +8,7 @@ #include #include #include +#include #include #include #include From 17c94caed392990fad205ce1e5307b7ac33167f3 Mon Sep 17 00:00:00 2001 From: etmcdonald Date: Thu, 24 Apr 2025 23:57:17 +0000 Subject: [PATCH 48/48] remove boundary and format --- examples/otlp/grpc_metric_main.cc | 5 ----- 1 file changed, 5 deletions(-) diff --git a/examples/otlp/grpc_metric_main.cc b/examples/otlp/grpc_metric_main.cc index 6cde982880..761eabd4f1 100644 --- a/examples/otlp/grpc_metric_main.cc +++ b/examples/otlp/grpc_metric_main.cc @@ -70,11 +70,6 @@ void InitMetrics(std::string &name) std::unique_ptr( new metric_sdk::Base2ExponentialHistogramAggregationConfig); - histogram_aggregation_config->max_scale_ = 3; - - // histogram_aggregation_config->boundaries_ = std::vector{ - // 0.0, 50.0, 100.0, 250.0, 500.0, 750.0, 1000.0, 2500.0, 5000.0, 10000.0, 20000.0}; - std::shared_ptr aggregation_config( std::move(histogram_aggregation_config));