From: Otto <otto.moerbeek@open-xchange.com>
Date: Tue, 8 Jun 2021 10:34:24 +0000 (+0200)
Subject: Align with Prometheus way of doing things and simplify
X-Git-Tag: dnsdist-1.7.0-alpha1~116^2~8
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=8a8852648c40d8ecfdc7b3de6e0e107354e19fc4;p=thirdparty%2Fpdns.git

Align with Prometheus way of doing things and simplify
template a bit.
---

diff --git a/pdns/histogram.hh b/pdns/histogram.hh
index 0e04944f12..03cae4fc42 100644
--- a/pdns/histogram.hh
+++ b/pdns/histogram.hh
@@ -53,7 +53,7 @@ struct AtomicBucket
   mutable std::atomic<uint64_t> d_count{0};
 };
 
-template <class B>
+template <class B, class SumType>
 class BaseHistogram
 {
 public:
@@ -94,6 +94,11 @@ public:
     return d_name;
   }
 
+  uint64_t getSum() const
+  {
+    return d_sum;
+  }
+
   const std::vector<B>& getRawData() const
   {
     return d_buckets;
@@ -138,11 +143,13 @@ public:
     auto index = std::upper_bound(d_buckets.begin(), d_buckets.end(), d, lessOrEqual);
     // our index is always valid
     ++index->d_count;
+    d_sum += d;
   }
 
 private:
   std::vector<B> d_buckets;
   std::string d_name;
+  mutable SumType d_sum{0};
 
   std::vector<uint64_t> to125(uint64_t start, int num)
   {
@@ -169,10 +176,8 @@ private:
   }
 };
 
-template <class T>
-using Histogram = BaseHistogram<Bucket>;
+using Histogram = BaseHistogram<Bucket, uint64_t>;
 
-template <class T>
-using AtomicHistogram = BaseHistogram<AtomicBucket>;
+using AtomicHistogram = BaseHistogram<AtomicBucket, std::atomic<uint64_t>>;
 
 } // namespace pdns
diff --git a/pdns/rec_channel_rec.cc b/pdns/rec_channel_rec.cc
index fb3f3bd7c8..2bd594f92d 100644
--- a/pdns/rec_channel_rec.cc
+++ b/pdns/rec_channel_rec.cc
@@ -1111,14 +1111,22 @@ static uint64_t doGetMallocated()
   return 0;
 }
 
-static StatsMap toStatsMap(const string& name, const vector<pdns::AtomicBucket>& data)
+static StatsMap toStatsMap(const string& name, const pdns::AtomicHistogram& histogram)
 {
+  const auto& data = histogram.getCumulativeBuckets();
+  const string pbasename = getPrometheusName(name);
   StatsMap entries;
+
   for (const auto& bucket : data) {
-    std::string pname = getPrometheusName(name) + '{' + "le=\"" +
-      (bucket.d_boundary == std::numeric_limits<uint64_t>::max() ? "+Inf" : std::to_string(bucket.d_boundary)) + "\"}";
+    char buf[32];
+    snprintf(buf, sizeof(buf), "%.0e", bucket.d_boundary / 1e6);
+    std::string pname = pbasename + "seconds_bucket{" + "le=\"" +
+      (bucket.d_boundary == std::numeric_limits<uint64_t>::max() ? "+Inf" : buf) + "\"}";
     entries.emplace(make_pair(bucket.d_name, StatsMapEntry{pname, std::to_string(bucket.d_count)}));
   }
+  entries.emplace(make_pair(name + "sum", StatsMapEntry{pbasename + "sum", std::to_string(histogram.getSum())}));
+  entries.emplace(make_pair(name + "count", StatsMapEntry{pbasename + "count", std::to_string(data.back().d_count)}));
+
   return entries;
 }
 
@@ -1366,13 +1374,13 @@ static void registerAllStats1()
   }
 
   addGetStat("cumulativeAnswers-usec-", []() {
-    return toStatsMap(g_stats.cumulativeAnswers.getName(), g_stats.cumulativeAnswers.getCumulativeBuckets());
+    return toStatsMap(g_stats.cumulativeAnswers.getName(), g_stats.cumulativeAnswers);
   });
   addGetStat("cumulativeAuth4Answers-usec-", []() {
-    return toStatsMap(g_stats.cumulativeAuth4Answers.getName(), g_stats.cumulativeAuth4Answers.getCumulativeBuckets());
+    return toStatsMap(g_stats.cumulativeAuth4Answers.getName(), g_stats.cumulativeAuth4Answers);
   });
   addGetStat("cumulativeAuth6Answers-usec-", []() {
-    return toStatsMap(g_stats.cumulativeAuth6Answers.getName(), g_stats.cumulativeAuth6Answers.getCumulativeBuckets());
+    return toStatsMap(g_stats.cumulativeAuth6Answers.getName(), g_stats.cumulativeAuth6Answers);
   });
 }
 
diff --git a/pdns/recursordist/test-histogram_hh.cc b/pdns/recursordist/test-histogram_hh.cc
index b0570d6d05..3858a6ec74 100644
--- a/pdns/recursordist/test-histogram_hh.cc
+++ b/pdns/recursordist/test-histogram_hh.cc
@@ -34,7 +34,7 @@ BOOST_AUTO_TEST_SUITE(histogram_hh)
 
 BOOST_AUTO_TEST_CASE(test_simple)
 {
-  auto h = pdns::AtomicHistogram<uint64_t>("myname-", {1, 3, 5, 10, 100});
+  auto h = pdns::AtomicHistogram("myname-", {1, 3, 5, 10, 100});
 
   h(0);
   h(1);
diff --git a/pdns/responsestats.hh b/pdns/responsestats.hh
index 421c814cfc..62f94e5841 100644
--- a/pdns/responsestats.hh
+++ b/pdns/responsestats.hh
@@ -47,7 +47,7 @@ private:
 
   std::array<Counter, 65535> d_qtypecounters;
   std::array<Counter, 256> d_rcodecounters;
-  pdns::AtomicHistogram<uint64_t> d_sizecounters;
+  pdns::AtomicHistogram d_sizecounters;
 };
 
 extern ResponseStats g_rs;
diff --git a/pdns/syncres.hh b/pdns/syncres.hh
index df61d54955..e9c6ee1156 100644
--- a/pdns/syncres.hh
+++ b/pdns/syncres.hh
@@ -1003,13 +1003,13 @@ struct RecursorStats
   std::atomic<uint64_t> servFails;
   std::atomic<uint64_t> nxDomains;
   std::atomic<uint64_t> noErrors;
-  pdns::AtomicHistogram<uint64_t> answers;
-  pdns::AtomicHistogram<uint64_t> auth4Answers;
-  pdns::AtomicHistogram<uint64_t> auth6Answers;
-  pdns::AtomicHistogram<uint64_t> ourtime;
-  pdns::AtomicHistogram<uint64_t> cumulativeAnswers;
-  pdns::AtomicHistogram<uint64_t> cumulativeAuth4Answers;
-  pdns::AtomicHistogram<uint64_t> cumulativeAuth6Answers;
+  pdns::AtomicHistogram answers;
+  pdns::AtomicHistogram auth4Answers;
+  pdns::AtomicHistogram auth6Answers;
+  pdns::AtomicHistogram ourtime;
+  pdns::AtomicHistogram cumulativeAnswers;
+  pdns::AtomicHistogram cumulativeAuth4Answers;
+  pdns::AtomicHistogram cumulativeAuth6Answers;
   std::atomic<double> avgLatencyUsec;
   std::atomic<double> avgLatencyOursUsec;
   std::atomic<uint64_t> qcounter;     // not increased for unauth packets
@@ -1058,9 +1058,9 @@ struct RecursorStats
     auth4Answers("auth4answers", { 1000, 10000, 100000, 1000000 }),
     auth6Answers("auth6answers", { 1000, 10000, 100000, 1000000 }),
     ourtime("ourtime", { 1000, 2000, 4000, 8000, 16000, 32000 }),
-    cumulativeAnswers("cumulAnswers-us", 10, 19),
-    cumulativeAuth4Answers("cumulAuth4Answers-us", 1000, 13),
-    cumulativeAuth6Answers("cumulAuth6Answers-us", 1000, 13)
+    cumulativeAnswers("cumulAnswers-", 10, 19),
+    cumulativeAuth4Answers("cumulAuth4Answers-", 1000, 13),
+    cumulativeAuth6Answers("cumulAuth6Answers-", 1000, 13)
   {
   }
 };