From 6819f0c847cf769af67135b6a4a1212ac848f58f Mon Sep 17 00:00:00 2001 From: Lalit Kumar Bhasin Date: Thu, 31 Mar 2022 16:03:25 -0700 Subject: [PATCH 1/4] histogram merge and diff --- .../aggregation/histogram_aggregation.h | 30 +++++++++ .../aggregation/histogram_aggregation.cc | 33 +++++++--- sdk/test/metrics/aggregation_test.cc | 62 +++++++++++++++++++ 3 files changed, 116 insertions(+), 9 deletions(-) diff --git a/sdk/include/opentelemetry/sdk/metrics/aggregation/histogram_aggregation.h b/sdk/include/opentelemetry/sdk/metrics/aggregation/histogram_aggregation.h index 8f33fa27b4..8a11119c57 100644 --- a/sdk/include/opentelemetry/sdk/metrics/aggregation/histogram_aggregation.h +++ b/sdk/include/opentelemetry/sdk/metrics/aggregation/histogram_aggregation.h @@ -6,6 +6,7 @@ # include "opentelemetry/common/spin_lock_mutex.h" # include "opentelemetry/sdk/metrics/aggregation/aggregation.h" +# include # include OPENTELEMETRY_BEGIN_NAMESPACE @@ -24,8 +25,12 @@ class LongHistogramAggregation : public Aggregation void Aggregate(double value, const PointAttributes &attributes = {}) noexcept override {} + /* Returns the result of merge of the existing aggregation with delta aggregation with same + * boundaries */ virtual 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 */ virtual std::unique_ptr Diff(const Aggregation &next) const noexcept override; PointType ToPoint() const noexcept override; @@ -56,6 +61,31 @@ class DoubleHistogramAggregation : public Aggregation mutable HistogramPointData point_data_; }; +template +void HistogramMerge(HistogramPointData ¤t, + HistogramPointData &delta, + HistogramPointData &merge) +{ + for (size_t i = 0; i < current.counts_.size(); i++) + { + merge.counts_[i] = current.counts_[i] + delta.counts_[i]; + } + merge.boundaries_ = current.boundaries_; + merge.sum_ = nostd::get(current.sum_) + nostd::get(delta.sum_); + merge.count_ = current.count_ + delta.count_; +} + +template +void HistogramDiff(HistogramPointData ¤t, HistogramPointData &next, HistogramPointData &diff) +{ + for (size_t i = 0; i < current.counts_.size(); i++) + { + diff.counts_[i] = next.counts_[i] - current.counts_[i]; + } + diff.boundaries_ = current.boundaries_; + diff.count_ = next.count_ - current.count_; +} + } // namespace metrics } // namespace sdk OPENTELEMETRY_END_NAMESPACE diff --git a/sdk/src/metrics/aggregation/histogram_aggregation.cc b/sdk/src/metrics/aggregation/histogram_aggregation.cc index 60f65c51b1..27405999c9 100644 --- a/sdk/src/metrics/aggregation/histogram_aggregation.cc +++ b/sdk/src/metrics/aggregation/histogram_aggregation.cc @@ -14,7 +14,6 @@ namespace metrics LongHistogramAggregation::LongHistogramAggregation() { - point_data_.boundaries_ = std::list{0l, 5l, 10l, 25l, 50l, 75l, 100l, 250l, 500l, 1000l}; point_data_.counts_ = std::vector(nostd::get>(point_data_.boundaries_).size() + 1, 0); @@ -47,12 +46,22 @@ void LongHistogramAggregation::Aggregate(long value, const PointAttributes &attr std::unique_ptr LongHistogramAggregation::Merge( const Aggregation &delta) const noexcept { - return nullptr; + auto curr_value = nostd::get(ToPoint()); + auto delta_value = nostd::get( + (static_cast(delta).ToPoint())); + LongHistogramAggregation *aggr = new LongHistogramAggregation(); + HistogramMerge(curr_value, delta_value, aggr->point_data_); + return std::unique_ptr(aggr); } std::unique_ptr LongHistogramAggregation::Diff(const Aggregation &next) const noexcept { - return nullptr; + auto curr_value = nostd::get(ToPoint()); + auto next_value = nostd::get( + (static_cast(next).ToPoint())); + LongHistogramAggregation *aggr = new LongHistogramAggregation(); + HistogramDiff(curr_value, next_value, aggr->point_data_); + return std::unique_ptr(aggr); } PointType LongHistogramAggregation::ToPoint() const noexcept @@ -62,7 +71,6 @@ PointType LongHistogramAggregation::ToPoint() const noexcept DoubleHistogramAggregation::DoubleHistogramAggregation() { - point_data_.boundaries_ = std::list{0.0, 5.0, 10.0, 25.0, 50.0, 75.0, 100.0, 250.0, 500.0, 1000.0}; point_data_.counts_ = @@ -96,20 +104,27 @@ void DoubleHistogramAggregation::Aggregate(double value, const PointAttributes & std::unique_ptr DoubleHistogramAggregation::Merge( const Aggregation &delta) const noexcept { - // TODO - Implement me - return nullptr; + auto curr_value = nostd::get(ToPoint()); + auto delta_value = nostd::get( + (static_cast(delta).ToPoint())); + DoubleHistogramAggregation *aggr = new DoubleHistogramAggregation(); + HistogramMerge(curr_value, delta_value, aggr->point_data_); + return std::unique_ptr(aggr); } std::unique_ptr DoubleHistogramAggregation::Diff( const Aggregation &next) const noexcept { - // TODO - Implement me - return nullptr; + auto curr_value = nostd::get(ToPoint()); + auto next_value = nostd::get( + (static_cast(next).ToPoint())); + DoubleHistogramAggregation *aggr = new DoubleHistogramAggregation(); + HistogramDiff(curr_value, next_value, aggr->point_data_); + return std::unique_ptr(aggr); } PointType DoubleHistogramAggregation::ToPoint() const noexcept { - // TODO Implement me return point_data_; } diff --git a/sdk/test/metrics/aggregation_test.cc b/sdk/test/metrics/aggregation_test.cc index a32826b145..7d55aa87d8 100644 --- a/sdk/test/metrics/aggregation_test.cc +++ b/sdk/test/metrics/aggregation_test.cc @@ -90,6 +90,37 @@ TEST(Aggregation, LongHistogramAggregation) EXPECT_EQ(histogram_data.count_, 4); EXPECT_EQ(histogram_data.counts_[3], 2); EXPECT_EQ(histogram_data.counts_[8], 1); + + // Merge + LongHistogramAggregation aggr1; + aggr1.Aggregate(1l, {}); + aggr1.Aggregate(11l, {}); + aggr1.Aggregate(26l, {}); + + LongHistogramAggregation aggr2; + aggr2.Aggregate(2l, {}); + aggr2.Aggregate(3l, {}); + aggr2.Aggregate(13l, {}); + aggr2.Aggregate(28l, {}); + aggr2.Aggregate(105l, {}); + + auto aggr3 = aggr1.Merge(aggr2); + histogram_data = nostd::get(aggr3->ToPoint()); + + EXPECT_EQ(histogram_data.count_, 8); // 3 each from aggr1 and aggr2 + EXPECT_EQ(histogram_data.counts_[1], 3); // 1, 2, 3 + EXPECT_EQ(histogram_data.counts_[3], 2); // 11, 13 + EXPECT_EQ(histogram_data.counts_[4], 2); // 25, 28 + EXPECT_EQ(histogram_data.counts_[7], 1); // 105 + + // Diff + auto aggr4 = aggr1.Diff(aggr2); + histogram_data = nostd::get(aggr4->ToPoint()); + EXPECT_EQ(histogram_data.count_, 2); // aggr2:5 - aggr1:3 + EXPECT_EQ(histogram_data.counts_[1], 1); // aggr2(2, 3) - aggr1(1) + EXPECT_EQ(histogram_data.counts_[3], 0); // aggr2(13) - aggr1(11) + EXPECT_EQ(histogram_data.counts_[4], 0); // aggr2(28) - aggr1(25) + EXPECT_EQ(histogram_data.counts_[7], 1); // aggr2(105) - aggr1(0) } TEST(Aggregation, DoubleHistogramAggregation) @@ -116,5 +147,36 @@ TEST(Aggregation, DoubleHistogramAggregation) EXPECT_EQ(histogram_data.counts_[3], 2); EXPECT_EQ(histogram_data.counts_[8], 1); EXPECT_EQ(nostd::get(histogram_data.sum_), 377); + + // Merge + DoubleHistogramAggregation aggr1; + aggr1.Aggregate(1.0, {}); + aggr1.Aggregate(11.0, {}); + aggr1.Aggregate(25.1, {}); + + DoubleHistogramAggregation aggr2; + aggr2.Aggregate(2.0, {}); + aggr2.Aggregate(3.0, {}); + aggr2.Aggregate(13.0, {}); + aggr2.Aggregate(28.1, {}); + aggr2.Aggregate(105.0, {}); + + auto aggr3 = aggr1.Merge(aggr2); + histogram_data = nostd::get(aggr3->ToPoint()); + + EXPECT_EQ(histogram_data.count_, 8); // 3 each from aggr1 and aggr2 + EXPECT_EQ(histogram_data.counts_[1], 3); // 1.0, 2.0, 3.0 + EXPECT_EQ(histogram_data.counts_[3], 2); // 11.0, 13.0 + EXPECT_EQ(histogram_data.counts_[4], 2); // 25.1, 28.1 + EXPECT_EQ(histogram_data.counts_[7], 1); // 105.0 + + // Diff + auto aggr4 = aggr1.Diff(aggr2); + histogram_data = nostd::get(aggr4->ToPoint()); + EXPECT_EQ(histogram_data.count_, 2); // aggr2:5 - aggr1:3 + EXPECT_EQ(histogram_data.counts_[1], 1); // aggr2(2.0, 3.0) - aggr1(1.0) + EXPECT_EQ(histogram_data.counts_[3], 0); // aggr2(13.0) - aggr1(11.0) + 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) } #endif \ No newline at end of file From f905ba826d21107e54b739547f7e91051e74ca16 Mon Sep 17 00:00:00 2001 From: Lalit Kumar Bhasin Date: Thu, 31 Mar 2022 16:10:24 -0700 Subject: [PATCH 2/4] add method comment --- .../sdk/metrics/aggregation/histogram_aggregation.h | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/sdk/include/opentelemetry/sdk/metrics/aggregation/histogram_aggregation.h b/sdk/include/opentelemetry/sdk/metrics/aggregation/histogram_aggregation.h index 8a11119c57..246e84e0a0 100644 --- a/sdk/include/opentelemetry/sdk/metrics/aggregation/histogram_aggregation.h +++ b/sdk/include/opentelemetry/sdk/metrics/aggregation/histogram_aggregation.h @@ -30,7 +30,10 @@ class LongHistogramAggregation : public Aggregation virtual 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 */ + * 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. + */ virtual std::unique_ptr Diff(const Aggregation &next) const noexcept override; PointType ToPoint() const noexcept override; @@ -50,8 +53,15 @@ class DoubleHistogramAggregation : public Aggregation void Aggregate(double value, const PointAttributes &attributes = {}) noexcept override; + /* Returns the result of merge of the existing aggregation with delta aggregation with same + * boundaries */ virtual 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. + */ virtual std::unique_ptr Diff(const Aggregation &next) const noexcept override; PointType ToPoint() const noexcept override; From aa9393423012aa43155cdcd07007750fbd64614c Mon Sep 17 00:00:00 2001 From: Lalit Kumar Bhasin Date: Thu, 31 Mar 2022 16:20:01 -0700 Subject: [PATCH 3/4] add newline --- sdk/test/metrics/aggregation_test.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sdk/test/metrics/aggregation_test.cc b/sdk/test/metrics/aggregation_test.cc index 7d55aa87d8..f8051776d1 100644 --- a/sdk/test/metrics/aggregation_test.cc +++ b/sdk/test/metrics/aggregation_test.cc @@ -179,4 +179,4 @@ 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) } -#endif \ No newline at end of file +#endif From b166d5d5098d130f11ac5b713a7d3e00b4317928 Mon Sep 17 00:00:00 2001 From: Lalit Kumar Bhasin Date: Thu, 14 Apr 2022 00:46:42 +0530 Subject: [PATCH 4/4] remove iostream --- .../sdk/metrics/aggregation/histogram_aggregation.h | 1 - 1 file changed, 1 deletion(-) diff --git a/sdk/include/opentelemetry/sdk/metrics/aggregation/histogram_aggregation.h b/sdk/include/opentelemetry/sdk/metrics/aggregation/histogram_aggregation.h index 246e84e0a0..b5cc2c349e 100644 --- a/sdk/include/opentelemetry/sdk/metrics/aggregation/histogram_aggregation.h +++ b/sdk/include/opentelemetry/sdk/metrics/aggregation/histogram_aggregation.h @@ -6,7 +6,6 @@ # include "opentelemetry/common/spin_lock_mutex.h" # include "opentelemetry/sdk/metrics/aggregation/aggregation.h" -# include # include OPENTELEMETRY_BEGIN_NAMESPACE