diff --git a/core/include/prometheus/histogram.h b/core/include/prometheus/histogram.h index fc5fdc27..0cded7b2 100644 --- a/core/include/prometheus/histogram.h +++ b/core/include/prometheus/histogram.h @@ -1,6 +1,8 @@ #pragma once +#include #include +#include #include #include "prometheus/client_metric.h" @@ -44,7 +46,9 @@ class PROMETHEUS_CPP_CORE_EXPORT Histogram { /// exponential etc.. /// /// The bucket boundaries cannot be changed once the histogram is created. - Histogram(const BucketBoundaries& buckets); + explicit Histogram(const BucketBoundaries& buckets); + + explicit Histogram(BucketBoundaries&& buckets); /// \brief Observe the given amount. /// @@ -60,7 +64,28 @@ class PROMETHEUS_CPP_CORE_EXPORT Histogram { /// this function must have already sorted the values into buckets). /// Also increments the total sum of all observations by the given value. void ObserveMultiple(const std::vector& bucket_increments, - const double sum_of_values); + double sum_of_values); + + /// Same as above with custom iterator type. + template + void ObserveMultiple(InputIt from, InputIt end, double sum_of_values) { + std::lock_guard lock(mutex_); + sum_.Increment(sum_of_values); + + for (std::size_t i{0}; i < bucket_counts_.size(); ++i, ++from) { + if (from == end) { + throw std::length_error( + "The size of bucket_increments should be equal to " + "the number of buckets in the histogram."); + } + bucket_counts_[i].Increment(*from); + } + if (from != end) { + throw std::length_error( + "The size of bucket_increments should be equal to " + "the number of buckets in the histogram."); + } + } /// \brief Get the current value of the counter. /// @@ -68,7 +93,7 @@ class PROMETHEUS_CPP_CORE_EXPORT Histogram { ClientMetric Collect() const; private: - const BucketBoundaries bucket_boundaries_; + BucketBoundaries bucket_boundaries_; mutable std::mutex mutex_; std::vector bucket_counts_; Gauge sum_; diff --git a/core/include/prometheus/summary.h b/core/include/prometheus/summary.h index f13b21e7..6db295e8 100644 --- a/core/include/prometheus/summary.h +++ b/core/include/prometheus/summary.h @@ -71,9 +71,13 @@ class PROMETHEUS_CPP_CORE_EXPORT Summary { /// and how smooth the time window is moved. With only one age bucket it /// effectively results in a complete reset of the summary each time max_age /// has passed. The default value is 5. - Summary(const Quantiles& quantiles, - std::chrono::milliseconds max_age = std::chrono::seconds{60}, - int age_buckets = 5); + explicit Summary(const Quantiles& quantiles, + std::chrono::milliseconds max_age = std::chrono::seconds{60}, + int age_buckets = 5); + + explicit Summary(Quantiles&& quantiles, + std::chrono::milliseconds max_age = std::chrono::seconds{60}, + int age_buckets = 5); /// \brief Observe the given amount. void Observe(double value); @@ -84,10 +88,10 @@ class PROMETHEUS_CPP_CORE_EXPORT Summary { ClientMetric Collect() const; private: - const Quantiles quantiles_; + Quantiles quantiles_; mutable std::mutex mutex_; - std::uint64_t count_; - double sum_; + std::uint64_t count_ = 0; + double sum_ = 0; detail::TimeWindowQuantiles quantile_values_; }; diff --git a/core/src/histogram.cc b/core/src/histogram.cc index 4947b3a7..5442977a 100644 --- a/core/src/histogram.cc +++ b/core/src/histogram.cc @@ -12,18 +12,22 @@ namespace prometheus { Histogram::Histogram(const BucketBoundaries& buckets) - : bucket_boundaries_{buckets}, bucket_counts_{buckets.size() + 1}, sum_{} { + : bucket_boundaries_{buckets}, bucket_counts_{buckets.size() + 1} { assert(std::is_sorted(std::begin(bucket_boundaries_), std::end(bucket_boundaries_))); } -void Histogram::Observe(const double value) { - // TODO: determine bucket list size at which binary search would be faster - const auto bucket_index = static_cast(std::distance( - bucket_boundaries_.begin(), - std::find_if( - std::begin(bucket_boundaries_), std::end(bucket_boundaries_), - [value](const double boundary) { return boundary >= value; }))); +Histogram::Histogram(BucketBoundaries&& buckets) + : bucket_boundaries_(std::move(buckets)), + bucket_counts_{bucket_boundaries_.size() + 1} { + assert(std::is_sorted(bucket_boundaries_.begin(), bucket_boundaries_.end())); +} + +void Histogram::Observe(double value) { + 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); @@ -31,7 +35,7 @@ void Histogram::Observe(const double value) { } void Histogram::ObserveMultiple(const std::vector& bucket_increments, - const double sum_of_values) { + double sum_of_values) { if (bucket_increments.size() != bucket_counts_.size()) { throw std::length_error( "The size of bucket_increments was not equal to" @@ -49,13 +53,13 @@ void Histogram::ObserveMultiple(const std::vector& bucket_increments, ClientMetric Histogram::Collect() const { std::lock_guard lock(mutex_); - auto metric = ClientMetric{}; + ClientMetric metric{}; auto cumulative_count = 0ULL; metric.histogram.bucket.reserve(bucket_counts_.size()); for (std::size_t i{0}; i < bucket_counts_.size(); ++i) { cumulative_count += bucket_counts_[i].Value(); - auto bucket = ClientMetric::Bucket{}; + ClientMetric::Bucket bucket{}; bucket.cumulative_count = cumulative_count; bucket.upper_bound = (i == bucket_boundaries_.size() ? std::numeric_limits::infinity() diff --git a/core/src/summary.cc b/core/src/summary.cc index 877c81bf..4e24d583 100644 --- a/core/src/summary.cc +++ b/core/src/summary.cc @@ -7,11 +7,14 @@ namespace prometheus { Summary::Summary(const Quantiles& quantiles, const std::chrono::milliseconds max_age, const int age_buckets) : quantiles_{quantiles}, - count_{0}, - sum_{0}, quantile_values_{quantiles_, max_age, age_buckets} {} -void Summary::Observe(const double value) { +Summary::Summary(Quantiles&& quantiles, const std::chrono::milliseconds max_age, + const int age_buckets) + : quantiles_(std::move(quantiles)), + quantile_values_{quantiles_, max_age, age_buckets} {} + +void Summary::Observe(double value) { std::lock_guard lock(mutex_); count_ += 1; @@ -20,13 +23,13 @@ void Summary::Observe(const double value) { } ClientMetric Summary::Collect() const { - auto metric = ClientMetric{}; + ClientMetric metric{}; std::lock_guard lock(mutex_); metric.summary.quantile.reserve(quantiles_.size()); for (const auto& quantile : quantiles_) { - auto metricQuantile = ClientMetric::Quantile{}; + ClientMetric::Quantile metricQuantile{}; metricQuantile.quantile = quantile.quantile; metricQuantile.value = quantile_values_.get(quantile.quantile); metric.summary.quantile.push_back(std::move(metricQuantile)); diff --git a/core/tests/histogram_test.cc b/core/tests/histogram_test.cc index 86cc66a8..a402a098 100644 --- a/core/tests/histogram_test.cc +++ b/core/tests/histogram_test.cc @@ -2,6 +2,7 @@ #include +#include #include #include #include @@ -81,7 +82,7 @@ TEST(HistogramTest, cumulative_bucket_count) { EXPECT_EQ(h.bucket.at(2).cumulative_count, 7U); } -TEST(HistogramTest, observe_multiple_test_bucket_counts) { +TEST(HistogramTest, observe_multiple_test_bucket_counts_1) { Histogram histogram{{1, 2}}; histogram.ObserveMultiple({5, 9, 3}, 20); histogram.ObserveMultiple({0, 20, 6}, 34); @@ -93,6 +94,20 @@ TEST(HistogramTest, observe_multiple_test_bucket_counts) { EXPECT_EQ(h.bucket.at(2).cumulative_count, 43U); } +TEST(HistogramTest, observe_multiple_test_bucket_counts_2) { + Histogram histogram{{1, 2}}; + const std::forward_list values1 = {5, 9, 3}; + const std::forward_list values2 = {0, 20, 6}; + histogram.ObserveMultiple(values1.begin(), values1.end(), 20); + histogram.ObserveMultiple(values2.begin(), values2.end(), 34); + auto metric = histogram.Collect(); + auto h = metric.histogram; + ASSERT_EQ(h.bucket.size(), 3U); + EXPECT_EQ(h.bucket.at(0).cumulative_count, 5U); + EXPECT_EQ(h.bucket.at(1).cumulative_count, 34U); + EXPECT_EQ(h.bucket.at(2).cumulative_count, 43U); +} + TEST(HistogramTest, observe_multiple_test_total_sum) { Histogram histogram{{1, 2}}; histogram.ObserveMultiple({5, 9, 3}, 20); @@ -103,13 +118,23 @@ TEST(HistogramTest, observe_multiple_test_total_sum) { EXPECT_EQ(h.sample_sum, 54); } -TEST(HistogramTest, observe_multiple_test_length_error) { +TEST(HistogramTest, observe_multiple_test_length_error1) { Histogram histogram{{1, 2}}; // 2 bucket boundaries means there are 3 buckets, so giving just 2 bucket // increments should result in a length_error. ASSERT_THROW(histogram.ObserveMultiple({5, 9}, 20), std::length_error); } +TEST(HistogramTest, observe_multiple_test_length_error2) { + Histogram histogram{{1, 2}}; + const std::forward_list values1 = {5, 9}; + ASSERT_THROW(histogram.ObserveMultiple(values1.begin(), values1.end(), 20), + std::length_error); + const std::forward_list values2 = {5, 9, 5, 6}; + ASSERT_THROW(histogram.ObserveMultiple(values2.begin(), values2.end(), 20), + std::length_error); +} + TEST(HistogramTest, sum_can_go_down) { Histogram histogram{{1}}; auto metric1 = histogram.Collect();