Skip to content

Commit ea75456

Browse files
committed
Make histograms cumulative
This change makes histogram collection produce cumulative histograms[1]. The previous implementation collected non-cumulative histograms, which did not conform to the prometheus specification[2]. Fixes #43. [1]: https://en.wikipedia.org/wiki/Histogram#Cumulative_histogram [2]: https://prometheus.io/docs/concepts/metric_types/#histogram
1 parent 3ae041a commit ea75456

File tree

2 files changed

+19
-21
lines changed

2 files changed

+19
-21
lines changed

lib/histogram.cc

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,10 +29,11 @@ io::prometheus::client::Metric Histogram::Collect() {
2929
histogram->set_sample_count(sample_count);
3030
histogram->set_sample_sum(sum_.Value());
3131

32+
auto cumulative_count = 0ULL;
3233
for (std::size_t i = 0; i < bucket_counts_.size(); i++) {
33-
auto& count = bucket_counts_[i];
34+
cumulative_count += bucket_counts_[i].Value();
3435
auto bucket = histogram->add_bucket();
35-
bucket->set_cumulative_count(count.Value());
36+
bucket->set_cumulative_count(cumulative_count);
3637
bucket->set_upper_bound(i == bucket_boundaries_.size()
3738
? std::numeric_limits<double>::infinity()
3839
: bucket_boundaries_[i]);

tests/histogram_test.cc

Lines changed: 16 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -47,44 +47,41 @@ TEST_F(HistogramTest, bucket_size) {
4747
EXPECT_EQ(h.bucket_size(), 3);
4848
}
4949

50-
TEST_F(HistogramTest, bucket_count) {
50+
TEST_F(HistogramTest, bucket_bounds) {
5151
Histogram histogram{{1, 2}};
52-
histogram.Observe(0);
53-
histogram.Observe(0.5);
54-
histogram.Observe(1.5);
55-
histogram.Observe(1.5);
56-
histogram.Observe(3);
5752
auto metric = histogram.Collect();
5853
ASSERT_TRUE(metric.has_histogram());
5954
auto h = metric.histogram();
6055
ASSERT_EQ(h.bucket_size(), 3);
61-
auto firstBucket = h.bucket(0);
62-
EXPECT_EQ(firstBucket.cumulative_count(), 2);
63-
auto secondBucket = h.bucket(1);
64-
EXPECT_EQ(secondBucket.cumulative_count(), 2);
65-
auto thirdBucket = h.bucket(2);
66-
EXPECT_EQ(thirdBucket.cumulative_count(), 1);
56+
EXPECT_EQ(h.bucket(0).upper_bound(), 1);
57+
EXPECT_EQ(h.bucket(1).upper_bound(), 2);
58+
EXPECT_EQ(h.bucket(2).upper_bound(), std::numeric_limits<double>::infinity());
6759
}
6860

69-
TEST_F(HistogramTest, bucket_bounds) {
61+
TEST_F(HistogramTest, bucket_counts_not_reset_by_collection) {
7062
Histogram histogram{{1, 2}};
63+
histogram.Observe(1.5);
64+
histogram.Collect();
65+
histogram.Observe(1.5);
7166
auto metric = histogram.Collect();
7267
ASSERT_TRUE(metric.has_histogram());
7368
auto h = metric.histogram();
7469
ASSERT_EQ(h.bucket_size(), 3);
75-
EXPECT_EQ(h.bucket(0).upper_bound(), 1);
76-
EXPECT_EQ(h.bucket(1).upper_bound(), 2);
77-
EXPECT_EQ(h.bucket(2).upper_bound(), std::numeric_limits<double>::infinity());
70+
EXPECT_EQ(h.bucket(1).cumulative_count(), 2);
7871
}
7972

80-
TEST_F(HistogramTest, cumulative_bucket_counts_not_reset_by_collection) {
73+
TEST_F(HistogramTest, cumulative_bucket_count) {
8174
Histogram histogram{{1, 2}};
75+
histogram.Observe(0);
76+
histogram.Observe(0.5);
8277
histogram.Observe(1.5);
83-
histogram.Collect();
8478
histogram.Observe(1.5);
79+
histogram.Observe(3);
8580
auto metric = histogram.Collect();
8681
ASSERT_TRUE(metric.has_histogram());
8782
auto h = metric.histogram();
8883
ASSERT_EQ(h.bucket_size(), 3);
89-
EXPECT_EQ(h.bucket(1).cumulative_count(), 2);
84+
EXPECT_EQ(h.bucket(0).cumulative_count(), 2);
85+
EXPECT_EQ(h.bucket(1).cumulative_count(), 4);
86+
EXPECT_EQ(h.bucket(2).cumulative_count(), 5);
9087
}

0 commit comments

Comments
 (0)