Skip to content

Commit

Permalink
Implement Merge and Diff operation for Histogram Aggregation (#1303)
Browse files Browse the repository at this point in the history
  • Loading branch information
@lalitb
lalitb authored Apr 14, 2022
1 parent e7f051e commit 29d68f1
Show file tree
Hide file tree
Showing 3 changed files with 126 additions and 10 deletions.
39 changes: 39 additions & 0 deletions sdk/include/opentelemetry/sdk/metrics/aggregation/histogram_aggregation.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,15 @@ 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<Aggregation> 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<Aggregation> Diff(const Aggregation &next) const noexcept override;

PointType ToPoint() const noexcept override;
Expand All @@ -45,8 +52,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<Aggregation> 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<Aggregation> Diff(const Aggregation &next) const noexcept override;

PointType ToPoint() const noexcept override;
Expand All @@ -56,6 +70,31 @@ class DoubleHistogramAggregation : public Aggregation
mutable HistogramPointData point_data_;
};

template <class T>
void HistogramMerge(HistogramPointData &current,
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<T>(current.sum_) + nostd::get<T>(delta.sum_);
merge.count_ = current.count_ + delta.count_;
}

template <class T>
void HistogramDiff(HistogramPointData &current, 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
Expand Down
33 changes: 24 additions & 9 deletions sdk/src/metrics/aggregation/histogram_aggregation.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ namespace metrics

LongHistogramAggregation::LongHistogramAggregation()
{

point_data_.boundaries_ = std::list<long>{0l, 5l, 10l, 25l, 50l, 75l, 100l, 250l, 500l, 1000l};
point_data_.counts_ =
std::vector<uint64_t>(nostd::get<std::list<long>>(point_data_.boundaries_).size() + 1, 0);
Expand Down Expand Up @@ -47,12 +46,22 @@ void LongHistogramAggregation::Aggregate(long value, const PointAttributes &attr
std::unique_ptr<Aggregation> LongHistogramAggregation::Merge(
const Aggregation &delta) const noexcept
{
return nullptr;
auto curr_value = nostd::get<HistogramPointData>(ToPoint());
auto delta_value = nostd::get<HistogramPointData>(
(static_cast<const LongHistogramAggregation &>(delta).ToPoint()));
LongHistogramAggregation *aggr = new LongHistogramAggregation();
HistogramMerge<long>(curr_value, delta_value, aggr->point_data_);
return std::unique_ptr<Aggregation>(aggr);
}

std::unique_ptr<Aggregation> LongHistogramAggregation::Diff(const Aggregation &next) const noexcept
{
return nullptr;
auto curr_value = nostd::get<HistogramPointData>(ToPoint());
auto next_value = nostd::get<HistogramPointData>(
(static_cast<const LongHistogramAggregation &>(next).ToPoint()));
LongHistogramAggregation *aggr = new LongHistogramAggregation();
HistogramDiff<long>(curr_value, next_value, aggr->point_data_);
return std::unique_ptr<Aggregation>(aggr);
}

PointType LongHistogramAggregation::ToPoint() const noexcept
Expand All @@ -62,7 +71,6 @@ PointType LongHistogramAggregation::ToPoint() const noexcept

DoubleHistogramAggregation::DoubleHistogramAggregation()
{

point_data_.boundaries_ =
std::list<double>{0.0, 5.0, 10.0, 25.0, 50.0, 75.0, 100.0, 250.0, 500.0, 1000.0};
point_data_.counts_ =
Expand Down Expand Up @@ -96,20 +104,27 @@ void DoubleHistogramAggregation::Aggregate(double value, const PointAttributes &
std::unique_ptr<Aggregation> DoubleHistogramAggregation::Merge(
const Aggregation &delta) const noexcept
{
// TODO - Implement me
return nullptr;
auto curr_value = nostd::get<HistogramPointData>(ToPoint());
auto delta_value = nostd::get<HistogramPointData>(
(static_cast<const DoubleHistogramAggregation &>(delta).ToPoint()));
DoubleHistogramAggregation *aggr = new DoubleHistogramAggregation();
HistogramMerge<double>(curr_value, delta_value, aggr->point_data_);
return std::unique_ptr<Aggregation>(aggr);
}

std::unique_ptr<Aggregation> DoubleHistogramAggregation::Diff(
const Aggregation &next) const noexcept
{
// TODO - Implement me
return nullptr;
auto curr_value = nostd::get<HistogramPointData>(ToPoint());
auto next_value = nostd::get<HistogramPointData>(
(static_cast<const DoubleHistogramAggregation &>(next).ToPoint()));
DoubleHistogramAggregation *aggr = new DoubleHistogramAggregation();
HistogramDiff<double>(curr_value, next_value, aggr->point_data_);
return std::unique_ptr<Aggregation>(aggr);
}

PointType DoubleHistogramAggregation::ToPoint() const noexcept
{
// TODO Implement me
return point_data_;
}

Expand Down
64 changes: 63 additions & 1 deletion sdk/test/metrics/aggregation_test.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<HistogramPointData>(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<HistogramPointData>(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)
Expand All @@ -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<double>(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<HistogramPointData>(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<HistogramPointData>(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
#endif

1 comment on commit 29d68f1

@github-actions
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Performance Alert ⚠️

Possible performance regression was detected for benchmark 'OpenTelemetry-cpp api Benchmark'.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 2.

Benchmark suite Current: 29d68f1 Previous: e7f051e Ratio
BM_SpinLockThrashing/1/process_time/real_time 2.189164161682129 ms/iter 0.12842557276156466 ms/iter 17.05

This comment was automatically generated by workflow using github-action-benchmark.

Please sign in to comment.