Skip to content

Commit

Permalink
[Metrics SDK] Histogram min/max support (#1540)
Browse files Browse the repository at this point in the history
  • Loading branch information
@esigo
esigo authored Aug 6, 2022
1 parent 22f07a0 commit 124b198
Show file tree
Hide file tree
Showing 12 changed files with 119 additions and 18 deletions.
6 changes: 3 additions & 3 deletions bazel/repository.bzl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -87,10 +87,10 @@ def opentelemetry_cpp_deps():
http_archive,
name = "com_github_opentelemetry_proto",
build_file = "@io_opentelemetry_cpp//bazel:opentelemetry_proto.BUILD",
sha256 = "f269fbcb30e17b03caa1decd231ce826e59d7651c0f71c3b28eb5140b4bb5412",
strip_prefix = "opentelemetry-proto-0.17.0",
sha256 = "134ce87f0a623daac19b9507b92da0d9b82929e3db796bba631e422f6ea8d3b3",
strip_prefix = "opentelemetry-proto-0.18.0",
urls = [
"https://github.com/open-telemetry/opentelemetry-proto/archive/v0.17.0.tar.gz",
"https://github.com/open-telemetry/opentelemetry-proto/archive/v0.18.0.tar.gz",
],
)

Expand Down
20 changes: 20 additions & 0 deletions exporters/ostream/src/metric_exporter.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,26 @@ void OStreamMetricExporter::printPointData(const opentelemetry::sdk::metrics::Po
sout_ << nostd::get<long>(histogram_point_data.sum_);
}

if (histogram_point_data.record_min_max_)
{
if (nostd::holds_alternative<long>(histogram_point_data.min_))
{
sout_ << "\n min : " << nostd::get<long>(histogram_point_data.min_);
}
else if (nostd::holds_alternative<double>(histogram_point_data.min_))
{
sout_ << "\n min : " << nostd::get<double>(histogram_point_data.min_);
}
if (nostd::holds_alternative<long>(histogram_point_data.max_))
{
sout_ << "\n max : " << nostd::get<long>(histogram_point_data.max_);
}
if (nostd::holds_alternative<double>(histogram_point_data.max_))
{
sout_ << "\n max : " << nostd::get<double>(histogram_point_data.max_);
}
}

sout_ << "\n buckets : ";
if (nostd::holds_alternative<std::list<double>>(histogram_point_data.boundaries_))
{
Expand Down
6 changes: 6 additions & 0 deletions exporters/ostream/test/ostream_metric_test.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,8 @@ TEST(OStreamMetricsExporter, ExportHistogramPointData)
histogram_point_data.count_ = 3;
histogram_point_data.counts_ = {200, 300, 400, 500};
histogram_point_data.sum_ = 900.5;
histogram_point_data.min_ = 1.8;
histogram_point_data.max_ = 12.0;
metric_sdk::HistogramPointData histogram_point_data2{};
histogram_point_data2.boundaries_ = std::list<long>{10, 20, 30};
histogram_point_data2.count_ = 3;
Expand Down Expand Up @@ -146,6 +148,8 @@ TEST(OStreamMetricsExporter, ExportHistogramPointData)
"\n type : HistogramPointData"
"\n count : 3"
"\n sum : 900.5"
"\n min : 1.8"
"\n max : 12"
"\n buckets : [10.1, 20.2, 30.2, ]"
"\n counts : [200, 300, 400, 500, ]"
"\n attributes\t\t: "
Expand All @@ -154,6 +158,8 @@ TEST(OStreamMetricsExporter, ExportHistogramPointData)
"\n type : HistogramPointData"
"\n count : 3"
"\n sum : 900"
"\n min : 0"
"\n max : 0"
"\n buckets : [10, 20, 30, ]"
"\n counts : [200, 300, 400, 500, ]"
"\n attributes\t\t: "
Expand Down
19 changes: 19 additions & 0 deletions exporters/otlp/src/otlp_metric_utils.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,25 @@ void OtlpMetricUtils::ConvertHistogramMetric(
}
// count
proto_histogram_point_data->set_count(histogram_data.count_);
if (histogram_data.record_min_max_)
{
if (nostd::holds_alternative<long>(histogram_data.min_))
{
proto_histogram_point_data->set_min(nostd::get<long>(histogram_data.min_));
}
else
{
proto_histogram_point_data->set_min(nostd::get<double>(histogram_data.min_));
}
if (nostd::holds_alternative<long>(histogram_data.max_))
{
proto_histogram_point_data->set_min(nostd::get<long>(histogram_data.max_));
}
else
{
proto_histogram_point_data->set_max(nostd::get<double>(histogram_data.max_));
}
}
// buckets
if ((nostd::holds_alternative<std::list<double>>(histogram_data.boundaries_)))
{
Expand Down
4 changes: 4 additions & 0 deletions exporters/otlp/test/otlp_http_metric_exporter_test.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -502,6 +502,8 @@ class OtlpHttpMetricExporterTestPeer : public ::testing::Test
histogram_point_data.count_ = 3;
histogram_point_data.counts_ = {200, 300, 400, 500};
histogram_point_data.sum_ = 900.5;
histogram_point_data.min_ = 1.8;
histogram_point_data.max_ = 19.0;
opentelemetry::sdk::metrics::HistogramPointData histogram_point_data2{};
histogram_point_data2.boundaries_ = std::list<long>{10, 20, 30};
histogram_point_data2.count_ = 3;
Expand Down Expand Up @@ -551,6 +553,8 @@ class OtlpHttpMetricExporterTestPeer : public ::testing::Test
auto data_points = metric["histogram"]["data_points"];
EXPECT_EQ(3, JsonToInteger<int64_t>(data_points[0]["count"]));
EXPECT_EQ(900.5, data_points[0]["sum"].get<double>());
EXPECT_EQ(1.8, data_points[0]["min"].get<double>());
EXPECT_EQ(19, data_points[0]["max"].get<double>());
EXPECT_EQ(4, data_points[0]["bucket_counts"].size());
if (4 == data_points[0]["bucket_counts"].size())
{
Expand Down
1 change: 1 addition & 0 deletions sdk/include/opentelemetry/sdk/metrics/aggregation/aggregation_config.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ class HistogramAggregationConfig : public AggregationConfig
{
public:
std::list<T> boundaries_;
bool record_min_max_ = true;
};
} // namespace metrics
} // namespace sdk
Expand Down
19 changes: 14 additions & 5 deletions sdk/include/opentelemetry/sdk/metrics/aggregation/histogram_aggregation.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ class LongHistogramAggregation : public Aggregation
private:
opentelemetry::common::SpinLockMutex lock_;
HistogramPointData point_data_;
bool record_min_max_ = true;
};

class DoubleHistogramAggregation : public Aggregation
Expand Down Expand Up @@ -72,6 +73,7 @@ class DoubleHistogramAggregation : public Aggregation
private:
mutable opentelemetry::common::SpinLockMutex lock_;
mutable HistogramPointData point_data_;
bool record_min_max_ = true;
};

template <class T>
Expand All @@ -83,9 +85,15 @@ void HistogramMerge(HistogramPointData &current,
{
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_;
merge.boundaries_ = current.boundaries_;
merge.sum_ = nostd::get<T>(current.sum_) + nostd::get<T>(delta.sum_);
merge.count_ = current.count_ + delta.count_;
merge.record_min_max_ = current.record_min_max_ && delta.record_min_max_;
if (merge.record_min_max_)
{
merge.min_ = std::min(nostd::get<T>(current.min_), nostd::get<T>(delta.min_));
merge.max_ = std::max(nostd::get<T>(current.max_), nostd::get<T>(delta.max_));
}
}

template <class T>
Expand All @@ -95,8 +103,9 @@ void HistogramDiff(HistogramPointData &current, HistogramPointData &next, Histog
{
diff.counts_[i] = next.counts_[i] - current.counts_[i];
}
diff.boundaries_ = current.boundaries_;
diff.count_ = next.count_ - current.count_;
diff.boundaries_ = current.boundaries_;
diff.count_ = next.count_ - current.count_;
diff.record_min_max_ = false;
}

} // namespace metrics
Expand Down
3 changes: 3 additions & 0 deletions sdk/include/opentelemetry/sdk/metrics/data/point_data.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,8 +58,11 @@ class HistogramPointData

ListType boundaries_ = {};
ValueType sum_ = {};
ValueType min_ = {};
ValueType max_ = {};
std::vector<uint64_t> counts_ = {};
uint64_t count_ = {};
bool record_min_max_ = true;
};

class DropPointData
Expand Down
43 changes: 35 additions & 8 deletions sdk/src/metrics/aggregation/histogram_aggregation.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,9 @@

#ifndef ENABLE_METRICS_PREVIEW
# include "opentelemetry/sdk/metrics/aggregation/histogram_aggregation.h"
# include <algorithm>
# include <iomanip>
# include <limits>
# include "opentelemetry/version.h"

# include <mutex>
Expand All @@ -23,26 +26,38 @@ LongHistogramAggregation::LongHistogramAggregation(
{
point_data_.boundaries_ = std::list<long>{0l, 5l, 10l, 25l, 50l, 75l, 100l, 250l, 500l, 1000l};
}
if (aggregation_config)
{
record_min_max_ = aggregation_config->record_min_max_;
}
point_data_.counts_ =
std::vector<uint64_t>(nostd::get<std::list<long>>(point_data_.boundaries_).size() + 1, 0);
point_data_.sum_ = 0l;
point_data_.count_ = 0;
point_data_.sum_ = 0l;
point_data_.count_ = 0;
point_data_.record_min_max_ = record_min_max_;
point_data_.min_ = std::numeric_limits<long>::max();
point_data_.max_ = std::numeric_limits<long>::min();
}

LongHistogramAggregation::LongHistogramAggregation(HistogramPointData &&data)
: point_data_{std::move(data)}
: point_data_{std::move(data)}, record_min_max_{point_data_.record_min_max_}
{}

LongHistogramAggregation::LongHistogramAggregation(const HistogramPointData &data)
: point_data_{data}
: point_data_{data}, record_min_max_{point_data_.record_min_max_}
{}

void LongHistogramAggregation::Aggregate(long value, const PointAttributes &attributes) noexcept
{
const std::lock_guard<opentelemetry::common::SpinLockMutex> locked(lock_);
point_data_.count_ += 1;
point_data_.sum_ = nostd::get<long>(point_data_.sum_) + value;
size_t index = 0;
if (record_min_max_)
{
point_data_.min_ = std::min(nostd::get<long>(point_data_.min_), value);
point_data_.max_ = std::max(nostd::get<long>(point_data_.max_), value);
}
size_t index = 0;
for (auto it = nostd::get<std::list<long>>(point_data_.boundaries_).begin();
it != nostd::get<std::list<long>>(point_data_.boundaries_).end(); ++it)
{
Expand Down Expand Up @@ -93,10 +108,17 @@ 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};
}
if (aggregation_config)
{
record_min_max_ = aggregation_config->record_min_max_;
}
point_data_.counts_ =
std::vector<uint64_t>(nostd::get<std::list<double>>(point_data_.boundaries_).size() + 1, 0);
point_data_.sum_ = 0.0;
point_data_.count_ = 0;
point_data_.sum_ = 0.0;
point_data_.count_ = 0;
point_data_.record_min_max_ = record_min_max_;
point_data_.min_ = std::numeric_limits<double>::max();
point_data_.max_ = std::numeric_limits<double>::min();
}

DoubleHistogramAggregation::DoubleHistogramAggregation(HistogramPointData &&data)
Expand All @@ -112,7 +134,12 @@ void DoubleHistogramAggregation::Aggregate(double value, const PointAttributes &
const std::lock_guard<opentelemetry::common::SpinLockMutex> locked(lock_);
point_data_.count_ += 1;
point_data_.sum_ = nostd::get<double>(point_data_.sum_) + value;
size_t index = 0;
if (record_min_max_)
{
point_data_.min_ = std::min(nostd::get<double>(point_data_.min_), value);
point_data_.max_ = std::max(nostd::get<double>(point_data_.max_), value);
}
size_t index = 0;
for (auto it = nostd::get<std::list<double>>(point_data_.boundaries_).begin();
it != nostd::get<std::list<double>>(point_data_.boundaries_).end(); ++it)
{
Expand Down
12 changes: 12 additions & 0 deletions sdk/test/metrics/aggregation_test.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,8 @@ TEST(Aggregation, LongHistogramAggregation)
EXPECT_NO_THROW(aggr.Aggregate(12l, {})); // lies in fourth bucket
EXPECT_NO_THROW(aggr.Aggregate(100l, {})); // lies in eight bucket
histogram_data = nostd::get<HistogramPointData>(aggr.ToPoint());
EXPECT_EQ(nostd::get<long>(histogram_data.min_), 12);
EXPECT_EQ(nostd::get<long>(histogram_data.max_), 100);
EXPECT_EQ(nostd::get<long>(histogram_data.sum_), 112);
EXPECT_EQ(histogram_data.count_, 2);
EXPECT_EQ(histogram_data.counts_[3], 1);
Expand All @@ -91,6 +93,8 @@ TEST(Aggregation, LongHistogramAggregation)
EXPECT_EQ(histogram_data.count_, 4);
EXPECT_EQ(histogram_data.counts_[3], 2);
EXPECT_EQ(histogram_data.counts_[8], 1);
EXPECT_EQ(nostd::get<long>(histogram_data.min_), 12);
EXPECT_EQ(nostd::get<long>(histogram_data.max_), 252);

// Merge
LongHistogramAggregation aggr1;
Expand All @@ -113,6 +117,8 @@ TEST(Aggregation, LongHistogramAggregation)
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
EXPECT_EQ(nostd::get<long>(histogram_data.min_), 1);
EXPECT_EQ(nostd::get<long>(histogram_data.max_), 105);

// Diff
auto aggr4 = aggr1.Diff(aggr2);
Expand Down Expand Up @@ -170,13 +176,17 @@ TEST(Aggregation, DoubleHistogramAggregation)
EXPECT_EQ(histogram_data.count_, 2);
EXPECT_EQ(histogram_data.counts_[3], 1);
EXPECT_EQ(histogram_data.counts_[7], 1);
EXPECT_EQ(nostd::get<double>(histogram_data.min_), 12);
EXPECT_EQ(nostd::get<double>(histogram_data.max_), 100);
EXPECT_NO_THROW(aggr.Aggregate(13.0, {})); // lies in fourth bucket
EXPECT_NO_THROW(aggr.Aggregate(252.0, {})); // lies in ninth bucket
histogram_data = nostd::get<HistogramPointData>(aggr.ToPoint());
EXPECT_EQ(histogram_data.count_, 4);
EXPECT_EQ(histogram_data.counts_[3], 2);
EXPECT_EQ(histogram_data.counts_[8], 1);
EXPECT_EQ(nostd::get<double>(histogram_data.sum_), 377);
EXPECT_EQ(nostd::get<double>(histogram_data.min_), 12);
EXPECT_EQ(nostd::get<double>(histogram_data.max_), 252);

// Merge
DoubleHistogramAggregation aggr1;
Expand All @@ -199,6 +209,8 @@ TEST(Aggregation, DoubleHistogramAggregation)
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
EXPECT_EQ(nostd::get<double>(histogram_data.min_), 1);
EXPECT_EQ(nostd::get<double>(histogram_data.max_), 105);

// Diff
auto aggr4 = aggr1.Diff(aggr2);
Expand Down
2 changes: 1 addition & 1 deletion third_party/opentelemetry-proto
Submodule opentelemetry-proto updated 3 files
+12 −1 CHANGELOG.md
+1 −1 README.md
+1 −1 opentelemetry/proto/metrics/v1/metrics.proto
2 changes: 1 addition & 1 deletion third_party_release
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,6 @@ benchmark=v1.5.3
googletest=release-1.10.0-459-ga6dfd3ac
ms-gsl=v3.1.0-67-g6f45293
nlohmann-json=v3.10.5
opentelemetry-proto=v0.17.0
opentelemetry-proto=v0.18.0
prometheus-cpp=v1.0.0
vcpkg=2022.04.12

1 comment on commit 124b198

@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 sdk Benchmark'.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 2.

Benchmark suite Current: 124b198 Previous: 22f07a0 Ratio
BM_BaselineBuffer/2 16343913.078308105 ns/iter 4603675.127029419 ns/iter 3.55
BM_LockFreeBuffer/1 3578179.132238637 ns/iter 301135.54716507817 ns/iter 11.88
BM_LockFreeBuffer/2 4491815.7160282135 ns/iter 973275.1846313477 ns/iter 4.62

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

Please sign in to comment.