From 80a1a9b4e8e9d78a174690e31021056c0ae9c2da Mon Sep 17 00:00:00 2001 From: David Wagner Date: Fri, 24 Feb 2023 11:49:15 +0100 Subject: [PATCH] feat(core): histograms: support observing values a non-integral number of times Observing a given value multiple times can be performed either by calling Observe multiple times or calling ObserveMultiple. The latter option is not very convenient because the caller has to keep track of existing buckets and perform bucketization. Observing a given value a non-integral number of times can also be done using ObserveMultiple (even though that was probably not intended) because the bucket_increments argument is a vector of doubles. It can't be done by calling Observe because you can't call a function a fraction of a time. Accumulating non-integral counts in buckets makes sense if what you are measuring is dimensionful: e.g. keeping track of how long (in seconds) an object is moving at certain (bucketed) speeds. If you observe that the object has moved at speed X for 1.5s, you'll want to increase the bucket corresponding to X by 1.5. This commit adds an argument to Observe (defaulting to 1.0 for backward compatibility) that makes it possible increment the bucket corresponding to the value by a non-integral quantity. It can be seen as a generalization of calling Observe in a loop, for a non-integral number of times. The count is incremented by the quantity and sum is incremented by value * quantity to preserve the semantic of the count and of the sum (and then of the computed mean), which is also consistent with what would happen if Observe was called in a loop. --- core/include/prometheus/histogram.h | 22 ++++++++++++++++------ core/src/histogram.cc | 8 ++++---- core/tests/histogram_test.cc | 25 +++++++++++++++++++++++++ 3 files changed, 45 insertions(+), 10 deletions(-) diff --git a/core/include/prometheus/histogram.h b/core/include/prometheus/histogram.h index f5e7aae3..7de0e482 100644 --- a/core/include/prometheus/histogram.h +++ b/core/include/prometheus/histogram.h @@ -49,19 +49,29 @@ class PROMETHEUS_CPP_CORE_EXPORT Histogram { /// \copydoc Histogram::Histogram(const BucketBoundaries&) explicit Histogram(BucketBoundaries&& buckets); - /// \brief Observe the given amount. + /// \brief Observe the given value. /// - /// The given amount selects the 'observed' bucket. The observed bucket is - /// chosen for which the given amount falls into the half-open interval [b_n, - /// b_n+1). The counter of the observed bucket is incremented. Also the total - /// sum of all observations is incremented. - void Observe(double value); + /// The given value selects the 'observed' bucket. The observed bucket is + /// chosen for which the given value falls into the half-open interval [b_n, + /// b_n+1). The counter of the observed bucket is incremented by the given + /// quantity. Also the total sum of all observations is incremented by + /// value*quantity. + /// + /// Passing a quantity!=1 can be seen as a generalization for real numbers of + /// calling Observe(value, 1) in a loop. The collected bucket counts will be + /// truncated but resulting rounding errors (other than those caused by + /// summing floating points) will not accumulate over time nor over cumulative + /// bucket counts. Same for the cumulative sum of the histogram. + void Observe(double value, double quantity = 1.0); /// \brief Observe multiple data points. /// /// Increments counters given a count for each bucket. (i.e. the caller of /// this function must have already sorted the values into buckets). /// Also increments the total sum of all observations by the given value. + /// + /// See the details in Observe(double, double) about floats when passing + /// non-integer values as bucket increments. void ObserveMultiple(const std::vector& bucket_increments, double sum_of_values); diff --git a/core/src/histogram.cc b/core/src/histogram.cc index e4377386..29a40554 100644 --- a/core/src/histogram.cc +++ b/core/src/histogram.cc @@ -37,15 +37,15 @@ Histogram::Histogram(BucketBoundaries&& buckets) } } -void Histogram::Observe(const double value) { +void Histogram::Observe(const double value, const double quantity) { const auto bucket_index = static_cast( std::distance(bucket_boundaries_.begin(), std::lower_bound(bucket_boundaries_.begin(), bucket_boundaries_.end(), value))); std::lock_guard lock(mutex_); - sum_.Increment(value); - bucket_counts_[bucket_index].Increment(); + sum_.Increment(quantity * value); + bucket_counts_[bucket_index].Increment(quantity); } void Histogram::ObserveMultiple(const std::vector& bucket_increments, @@ -77,7 +77,7 @@ ClientMetric Histogram::Collect() const { auto metric = ClientMetric{}; - auto cumulative_count = 0ULL; + auto cumulative_count = 0.0; metric.histogram.bucket.reserve(bucket_counts_.size()); for (std::size_t i{0}; i < bucket_counts_.size(); ++i) { cumulative_count += bucket_counts_[i].Value(); diff --git a/core/tests/histogram_test.cc b/core/tests/histogram_test.cc index ef8ecda6..73c58221 100644 --- a/core/tests/histogram_test.cc +++ b/core/tests/histogram_test.cc @@ -89,6 +89,31 @@ TEST(HistogramTest, cumulative_bucket_count) { EXPECT_EQ(h.bucket.at(2).cumulative_count, 7U); } +TEST(HistogramTest, observe_float_test_bucket_counts) { + Histogram histogram{{1, 2}}; + histogram.Observe(0.1, 2.6); + histogram.Observe(1.2, 3.6); + histogram.Observe(2.3, 4.6); + auto metric = histogram.Collect(); + auto h = metric.histogram; + ASSERT_EQ(h.bucket.size(), 3U); + // rounding errors do not accumulate in the cumulative count: + EXPECT_EQ(h.bucket.at(0).cumulative_count, 2U); // 2.6, truncated + EXPECT_EQ(h.bucket.at(1).cumulative_count, 6U); // 2.6 + 3.6, truncated + EXPECT_EQ(h.bucket.at(2).cumulative_count, 10U); // 2.6 + 3.6 + 4.6, truncated +} + +TEST(HistogramTest, observe_float_test_total_sum) { + Histogram histogram{{1, 2}}; + histogram.Observe(0.1, 2.6); + histogram.Observe(1.2, 3.6); + histogram.Observe(2.3, 4.6); + auto metric = histogram.Collect(); + auto h = metric.histogram; + EXPECT_EQ(h.sample_count, 10U); + EXPECT_FLOAT_EQ(h.sample_sum, 15.16); +} + TEST(HistogramTest, observe_multiple_test_bucket_counts) { Histogram histogram{{1, 2}}; histogram.ObserveMultiple({5, 9, 3}, 20);