From fe266e431c1fe5104ad16fdc4a248707ca750e23 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jupp=20M=C3=BCller?= Date: Sun, 16 Oct 2016 22:48:00 +0100 Subject: [PATCH] Add histogram metric Fixes issue #3 --- lib/BUILD | 2 + lib/exposer.cc | 17 +++++++- lib/exposer.h | 3 ++ lib/family.h | 11 +++--- lib/histogram.cc | 41 +++++++++++++++++++ lib/histogram.h | 28 +++++++++++++ lib/registry.cc | 48 +++++++++++++---------- lib/registry.h | 4 ++ tests/BUILD | 2 +- tests/family_test.cc | 40 ++++++++++++------- tests/histogram_test.cc | 78 +++++++++++++++++++++++++++++++++++++ tests/integration/scrape.sh | 3 ++ 12 files changed, 235 insertions(+), 42 deletions(-) create mode 100644 lib/histogram.cc create mode 100644 lib/histogram.h create mode 100644 tests/histogram_test.cc diff --git a/lib/BUILD b/lib/BUILD index 1c1691fa..6722223b 100644 --- a/lib/BUILD +++ b/lib/BUILD @@ -3,6 +3,7 @@ cc_library( srcs = ["counter.cc", "gauge.cc", "exposer.cc", + "histogram.cc", "registry.cc"], hdrs = ["counter.h", "gauge.h", @@ -10,6 +11,7 @@ cc_library( "metric.h", "collectable.h", "family.h", + "histogram.h", "registry.h"], visibility = ["//visibility:public"], deps = ["@protobuf//:protobuf", diff --git a/lib/exposer.cc b/lib/exposer.cc index aa883006..1cb44b99 100644 --- a/lib/exposer.cc +++ b/lib/exposer.cc @@ -25,7 +25,14 @@ MetricsHandler::MetricsHandler( numScrapesFamily_(registry.add_counter( "exposer_total_scrapes", "Number of times metrics were scraped", {{"component", "exposer"}})), - numScrapes_(numScrapesFamily_->add({})) {} + numScrapes_(numScrapesFamily_->add({})), + requestLatenciesFamily_(registry.add_histogram( + "exposer_request_latencies", + "Latencies of serving scrape requests, in milliseconds", + {{"component", "exposer"}})), + requestLatencies_(requestLatenciesFamily_->add( + {}, Histogram::BucketBoundaries{1, 5, 10, 20, 40, 80, 160, 320, 640, + 1280, 2560})) {} static std::string serializeToDelimitedProtobuf( const std::vector& metrics) { @@ -90,6 +97,8 @@ bool MetricsHandler::handleGet(CivetServer* server, struct mg_connection* conn) { using namespace io::prometheus::client; + auto startTimeOfRequest = std::chrono::steady_clock::now(); + auto acceptedEncoding = getAcceptedEncoding(conn); auto metrics = collectMetrics(); @@ -118,8 +127,12 @@ bool MetricsHandler::handleGet(CivetServer* server, mg_printf(conn, "Content-Length: %lu\r\n\r\n", body.size()); mg_write(conn, body.data(), body.size()); - bytesTransfered_->inc(body.size()); + auto stopTimeOfRequest = std::chrono::steady_clock::now(); + auto duration = std::chrono::duration_cast( + stopTimeOfRequest - startTimeOfRequest); + requestLatencies_->observe(duration.count()); + bytesTransfered_->inc(body.size()); numScrapes_->inc(); return true; } diff --git a/lib/exposer.h b/lib/exposer.h index 9e22d70d..c5a30bf4 100644 --- a/lib/exposer.h +++ b/lib/exposer.h @@ -6,6 +6,7 @@ #include "CivetServer.h" #include "registry.h" +#include "histogram.h" namespace prometheus { @@ -24,6 +25,8 @@ class MetricsHandler : public CivetHandler { Counter* bytesTransfered_; Family* numScrapesFamily_; Counter* numScrapes_; + Family *requestLatenciesFamily_; + Histogram* requestLatencies_; }; class Exposer { diff --git a/lib/family.h b/lib/family.h index 546395ec..7020e54a 100644 --- a/lib/family.h +++ b/lib/family.h @@ -7,9 +7,7 @@ #include #include -#include "counter.h" #include "collectable.h" -#include "gauge.h" #include "metric.h" namespace prometheus { @@ -19,7 +17,8 @@ class Family : public Collectable { public: Family(const std::string& name, const std::string& help, const std::map& constantLabels); - T* add(const std::map& labels); + template + T* add(const std::map& labels, Args&&... args); void remove(T* metric); // Collectable @@ -46,9 +45,11 @@ Family::Family(const std::string& name, const std::string& help, : name_(name), help_(help), constantLabels_(constantLabels) {} template -T* Family::add(const std::map& labels) { +template +T* Family::add(const std::map& labels, + Args&&... args) { auto hash = hash_labels(labels); - auto metric = new T(); + auto metric = new T(std::forward(args)...); metrics_.insert(std::make_pair(hash, std::unique_ptr{metric})); labels_.insert({hash, labels}); diff --git a/lib/histogram.cc b/lib/histogram.cc new file mode 100644 index 00000000..74f45108 --- /dev/null +++ b/lib/histogram.cc @@ -0,0 +1,41 @@ +#include +#include + +#include "histogram.h" + +namespace prometheus { + +Histogram::Histogram(const BucketBoundaries& buckets) + : bucketBoundaries_{buckets}, bucketCounts_(buckets.size() + 1) {} + +void Histogram::observe(double value) { + // TODO: determine bucket list size at which binary search would be faster + auto bucketIndex = std::max( + 0L, std::find_if(bucketBoundaries_.begin(), bucketBoundaries_.end(), + [value](double boundary) { return boundary > value; }) - + bucketBoundaries_.begin()); + sum_.inc(value); + bucketCounts_[bucketIndex].inc(); +} + +io::prometheus::client::Metric Histogram::collect() { + auto metric = io::prometheus::client::Metric{}; + auto histogram = metric.mutable_histogram(); + + auto sampleCount = std::accumulate( + bucketCounts_.begin(), bucketCounts_.end(), double{0}, + [](double sum, const Counter& counter) { return sum + counter.value(); }); + histogram->set_sample_count(sampleCount); + histogram->set_sample_sum(sum_.value()); + + for (int i = 0; i < bucketCounts_.size(); i++) { + auto& count = bucketCounts_[i]; + auto bucket = histogram->add_bucket(); + bucket->set_cumulative_count(count.value()); + bucket->set_upper_bound(i == bucketBoundaries_.size() + ? std::numeric_limits::infinity() + : bucketBoundaries_[i]); + } + return metric; +} +} diff --git a/lib/histogram.h b/lib/histogram.h new file mode 100644 index 00000000..f486f1ca --- /dev/null +++ b/lib/histogram.h @@ -0,0 +1,28 @@ +#pragma once + +#include + +#include "cpp/metrics.pb.h" + +#include "counter.h" + +namespace prometheus { +class Histogram : public Metric { + public: + using BucketBoundaries = std::vector; + + static const io::prometheus::client::MetricType metric_type = + io::prometheus::client::HISTOGRAM; + + Histogram(const BucketBoundaries& buckets); + + void observe(double value); + + io::prometheus::client::Metric collect(); + + private: + const BucketBoundaries bucketBoundaries_; + std::vector bucketCounts_; + Counter sum_; +}; +} diff --git a/lib/registry.cc b/lib/registry.cc index a4e6c4ef..b4428e9a 100644 --- a/lib/registry.cc +++ b/lib/registry.cc @@ -8,36 +8,44 @@ Registry::Registry(const std::map& constLabels) Family* Registry::add_counter( const std::string& name, const std::string& help, const std::map& labels) { - auto counterFamily = new Family(name, help, labels); - collectables_.push_back(std::unique_ptr{counterFamily}); - return counterFamily; + auto counterFamily = new Family(name, help, labels); + collectables_.push_back(std::unique_ptr{counterFamily}); + return counterFamily; } Family* Registry::add_gauge( const std::string& name, const std::string& help, const std::map& labels) { - auto gaugeFamily = new Family(name, help, labels); - collectables_.push_back(std::unique_ptr{gaugeFamily}); - return gaugeFamily; + auto gaugeFamily = new Family(name, help, labels); + collectables_.push_back(std::unique_ptr{gaugeFamily}); + return gaugeFamily; +} + +Family* Registry::add_histogram( + const std::string& name, const std::string& help, + const std::map& labels) { + auto histogramFamily = new Family(name, help, labels); + collectables_.push_back(std::unique_ptr{histogramFamily}); + return histogramFamily; } std::vector Registry::collect() { - auto results = std::vector{}; - for(auto&& collectable : collectables_) { - auto metrics = collectable->collect(); - results.insert(results.end(), metrics.begin(), metrics.end()); - } + auto results = std::vector{}; + for (auto&& collectable : collectables_) { + auto metrics = collectable->collect(); + results.insert(results.end(), metrics.begin(), metrics.end()); + } - for (auto&& metricFamily : results) { - for (auto&& metric : *metricFamily.mutable_metric()) { - for (auto&& constLabelPair : constLabels_) { - auto label = metric.add_label(); - label->set_name(constLabelPair.first); - label->set_value(constLabelPair.second); - } - } + for (auto&& metricFamily : results) { + for (auto&& metric : *metricFamily.mutable_metric()) { + for (auto&& constLabelPair : constLabels_) { + auto label = metric.add_label(); + label->set_name(constLabelPair.first); + label->set_value(constLabelPair.second); + } } + } - return results; + return results; } } diff --git a/lib/registry.h b/lib/registry.h index 55a94226..9a589fe8 100644 --- a/lib/registry.h +++ b/lib/registry.h @@ -5,6 +5,7 @@ #include "collectable.h" #include "cpp/metrics.pb.h" #include "family.h" +#include "histogram.h" namespace prometheus { @@ -20,6 +21,9 @@ class Registry : public Collectable { const std::map& labels); Family* add_gauge(const std::string& name, const std::string& help, const std::map& labels); + Family* add_histogram( + const std::string& name, const std::string& help, + const std::map& labels); // collectable std::vector collect() override; diff --git a/tests/BUILD b/tests/BUILD index 7060041d..fd96844f 100644 --- a/tests/BUILD +++ b/tests/BUILD @@ -1,6 +1,6 @@ cc_test( name = "prometheus_test", - srcs = ["counter_test.cc", "gauge_test.cc", "mock_metric.h", "family_test.cc", "registry_test.cc"], + srcs = ["counter_test.cc", "gauge_test.cc", "mock_metric.h", "family_test.cc", "registry_test.cc", "histogram_test.cc"], copts = ["-Iexternal/googletest/include"], deps = ["@googletest//:main", "//lib:prometheus-cpp"], diff --git a/tests/family_test.cc b/tests/family_test.cc index db494c22..2560d3b1 100644 --- a/tests/family_test.cc +++ b/tests/family_test.cc @@ -4,6 +4,7 @@ #include "cpp/metrics.pb.h" #include "lib/counter.h" #include "lib/family.h" +#include "lib/histogram.h" using namespace testing; using namespace prometheus; @@ -41,21 +42,32 @@ TEST_F(FamilyTest, labels) { } TEST_F(FamilyTest, counter_value) { - auto family = Family{"total_requests", "Counts all requests", {}}; - auto counter = family.add({}); - counter->inc(); - auto collected = family.collect(); - ASSERT_GE(collected.size(), 1); - ASSERT_GE(collected[0].metric_size(), 1); - EXPECT_THAT(collected[0].metric(0).counter().value(), Eq(1)); + auto family = Family{"total_requests", "Counts all requests", {}}; + auto counter = family.add({}); + counter->inc(); + auto collected = family.collect(); + ASSERT_GE(collected.size(), 1); + ASSERT_GE(collected[0].metric_size(), 1); + EXPECT_THAT(collected[0].metric(0).counter().value(), Eq(1)); } TEST_F(FamilyTest, remove) { - auto family = Family{"total_requests", "Counts all requests", {}}; - auto counter1 = family.add({{"name", "counter1"}}); - family.add({{"name", "counter2"}}); - family.remove(counter1); - auto collected = family.collect(); - ASSERT_GE(collected.size(), 1); - EXPECT_EQ(collected[0].metric_size(), 1); + auto family = Family{"total_requests", "Counts all requests", {}}; + auto counter1 = family.add({{"name", "counter1"}}); + family.add({{"name", "counter2"}}); + family.remove(counter1); + auto collected = family.collect(); + ASSERT_GE(collected.size(), 1); + EXPECT_EQ(collected[0].metric_size(), 1); +} + +TEST_F(FamilyTest, histogram) { + auto family = Family{"request_latency", "Latency Histogram", {}}; + auto histogram1 = family.add({{"name", "histogram1"}}, Histogram::BucketBoundaries{0,1,2}); + histogram1->observe(0); + auto collected = family.collect(); + ASSERT_EQ(collected.size(), 1); + ASSERT_GE(collected[0].metric_size(), 1); + ASSERT_TRUE(collected[0].metric(0).has_histogram()); + EXPECT_THAT(collected[0].metric(0).histogram().sample_count(), Eq(1)); } diff --git a/tests/histogram_test.cc b/tests/histogram_test.cc new file mode 100644 index 00000000..7a4361a1 --- /dev/null +++ b/tests/histogram_test.cc @@ -0,0 +1,78 @@ +#include + +#include "gmock/gmock.h" + +#include "lib/histogram.h" + +using namespace testing; +using namespace prometheus; + +class HistogramTest : public Test {}; + +TEST_F(HistogramTest, initialize_with_zero) { + Histogram histogram{{}}; + auto metric = histogram.collect(); + ASSERT_TRUE(metric.has_histogram()); + auto h = metric.histogram(); + EXPECT_EQ(h.sample_count(), 0); + EXPECT_EQ(h.sample_sum(), 0); +} + +TEST_F(HistogramTest, sample_count) { + Histogram histogram{{1}}; + histogram.observe(0); + histogram.observe(200); + auto metric = histogram.collect(); + ASSERT_TRUE(metric.has_histogram()); + auto h = metric.histogram(); + EXPECT_EQ(h.sample_count(), 2); +} + +TEST_F(HistogramTest, sample_sum) { + Histogram histogram{{1}}; + histogram.observe(0); + histogram.observe(1); + histogram.observe(101); + auto metric = histogram.collect(); + ASSERT_TRUE(metric.has_histogram()); + auto h = metric.histogram(); + EXPECT_EQ(h.sample_sum(), 102); +} + +TEST_F(HistogramTest, bucket_size) { + Histogram histogram{{1,2}}; + auto metric = histogram.collect(); + ASSERT_TRUE(metric.has_histogram()); + auto h = metric.histogram(); + EXPECT_EQ(h.bucket_size(), 3); +} + +TEST_F(HistogramTest, bucket_count) { + Histogram histogram{{1,2}}; + histogram.observe(0); + histogram.observe(0.5); + histogram.observe(1.5); + histogram.observe(1.5); + histogram.observe(3); + auto metric = histogram.collect(); + ASSERT_TRUE(metric.has_histogram()); + auto h = metric.histogram(); + ASSERT_EQ(h.bucket_size(), 3); + auto firstBucket = h.bucket(0); + EXPECT_EQ(firstBucket.cumulative_count(), 2); + auto secondBucket = h.bucket(1); + EXPECT_EQ(secondBucket.cumulative_count(), 2); + auto thirdBucket = h.bucket(2); + EXPECT_EQ(thirdBucket.cumulative_count(), 1); +} + +TEST_F(HistogramTest, bucket_bounds) { + Histogram histogram{{1,2}}; + auto metric = histogram.collect(); + ASSERT_TRUE(metric.has_histogram()); + auto h = metric.histogram(); + ASSERT_EQ(h.bucket_size(), 3); + EXPECT_EQ(h.bucket(0).upper_bound(), 1); + EXPECT_EQ(h.bucket(1).upper_bound(), 2); + EXPECT_EQ(h.bucket(2).upper_bound(), std::numeric_limits::infinity()); +} diff --git a/tests/integration/scrape.sh b/tests/integration/scrape.sh index b5103eac..6c6c0e01 100755 --- a/tests/integration/scrape.sh +++ b/tests/integration/scrape.sh @@ -23,4 +23,7 @@ if [[ ! $telegraf_output == *"time_running_seconds"* ]] ; then exit 1 fi +echo "Success:" +echo "${telegraf_output}" + exit 0