Skip to content

Implement Merge and Diff operation for Histogram Aggregation #1303

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
ThomsonTan merged 7 commits into from
Apr 14, 2022
Merged

Implement Merge and Diff operation for Histogram Aggregation #1303

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
6819f0c
histogram merge and diff
lalitb Mar 31, 2022
f905ba8
add method comment
lalitb Mar 31, 2022
aa93934
add newline
lalitb Mar 31, 2022
9457ab1
Merge branch 'main' into histogram-aggregation
ThomsonTan Apr 11, 2022
814d30f
Merge branch 'main' into histogram-aggregation
esigo Apr 13, 2022
b166d5d
remove iostream
lalitb Apr 13, 2022
346bc09
Merge branch 'main' into histogram-aggregation
esigo Apr 14, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 39 additions & 0 deletions sdk/include/opentelemetry/sdk/metrics/aggregation/histogram_aggregation.h
View file
Open in desktop
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++)
Comment thread
esigo marked this conversation as resolved.
{
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++)
Comment thread
esigo marked this conversation as resolved.
{
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
Open in desktop
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
Comment thread
esigo marked this conversation as resolved.
{
// TODO Implement me
return point_data_;
}

Expand Down
64 changes: 63 additions & 1 deletion sdk/test/metrics/aggregation_test.cc
View file
Open in desktop
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