Process feedback from jtodd: better names and HELP and use a tag to distinguish

ipv4 and ipv6 for the new Prometheus-friendly histograms.

diff --git a/.github/actions/spell-check/expect.txt b/.github/actions/spell-check/expect.txt
index 78979a833607d62ecd7e3f26b47b9483389ed66f..beac0bb856b485018b972ae01e5c859c49010abd 100644 (file)
--- a/.github/actions/spell-check/expect.txt
+++ b/.github/actions/spell-check/expect.txt
@@ -77,6 +77,7 @@ Atomia
 aton
 attr
 atype
+authanswers
 AUTHIP
 Authoritativedoc
 auths
@@ -233,6 +234,7 @@ chroot
 chrooting
 CIDR
 classmethod
+clientanswers
 Cloos
 closesocket
 clusions

diff --git a/pdns/pdns_recursor.cc b/pdns/pdns_recursor.cc
index 3efc32837575e20c59f685b6e2c0456199d80990..a8228e74ccec72855a06807243b4c08dbda38867 100644 (file)
--- a/pdns/pdns_recursor.cc
+++ b/pdns/pdns_recursor.cc
@@ -5861,7 +5861,7 @@ int main(int argc, char **argv)
     for (size_t idx = 0; idx < 128; idx++) {
       defaultAPIDisabledStats += ", ecs-v6-response-bits-" + std::to_string(idx + 1);
     }
-    std::string defaultDisabledStats = defaultAPIDisabledStats + ", cumul-answers, cumul-auth4answers, cumul-auth6answers, policy-hits";
+    std::string defaultDisabledStats = defaultAPIDisabledStats + ", cumul-clientanswers, cumul-authanswers, policy-hits";
 
     ::arg().set("stats-api-blacklist", "List of statistics that are disabled when retrieving the complete list of statistics via the API (deprecated)")=defaultAPIDisabledStats;
     ::arg().set("stats-carbon-blacklist", "List of statistics that are prevented from being exported via Carbon (deprecated)")=defaultDisabledStats;

diff --git a/pdns/rec_channel_rec.cc b/pdns/rec_channel_rec.cc
index 360b1f54c89b4bcdb599b474af3614eec2392dae..b47807a0066164b1451de70a5218c616255f1d0f 100644 (file)
--- a/pdns/rec_channel_rec.cc
+++ b/pdns/rec_channel_rec.cc
@@ -1131,12 +1131,42 @@ static StatsMap toStatsMap(const string& name, const pdns::AtomicHistogram& hist
     snprintf(buf, sizeof(buf), "%g", 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(bucket.d_name, StatsMapEntry{pname, std::to_string(bucket.d_count)});
   }
 
   snprintf(buf, sizeof(buf), "%g", histogram.getSum() / 1e6);
-  entries.emplace(make_pair(name + "sum", StatsMapEntry{pbasename + "seconds_sum", buf}));
-  entries.emplace(make_pair(name + "count", StatsMapEntry{pbasename + "seconds_count", std::to_string(data.back().d_count)}));
+  entries.emplace(name + "sum", StatsMapEntry{pbasename + "seconds_sum", buf});
+  entries.emplace(name + "count", StatsMapEntry{pbasename + "seconds_count", std::to_string(data.back().d_count)});
+
+  return entries;
+}
+
+static StatsMap toStatsMap(const string& name, const pdns::AtomicHistogram& histogram4,  const pdns::AtomicHistogram& histogram6)
+{
+  const string pbasename = getPrometheusName(name);
+  StatsMap entries;
+  char buf[32];
+  std::string pname;
+
+  const auto& data4 = histogram4.getCumulativeBuckets();
+  for (const auto& bucket : data4) {
+    snprintf(buf, sizeof(buf), "%g", bucket.d_boundary / 1e6);
+    pname = pbasename + "seconds_bucket{ipversion=\"v4\",le=\"" +
+      (bucket.d_boundary == std::numeric_limits<uint64_t>::max() ? "+Inf" : buf) + "\"}";
+    entries.emplace(bucket.d_name + "-4", StatsMapEntry{pname, std::to_string(bucket.d_count)});
+  }
+
+  const auto& data6 = histogram6.getCumulativeBuckets();
+  for (const auto& bucket : data6) {
+    snprintf(buf, sizeof(buf), "%g", bucket.d_boundary / 1e6);
+    pname = pbasename + "seconds_bucket{ipversion=\"v6\",le=\"" +
+      (bucket.d_boundary == std::numeric_limits<uint64_t>::max() ? "+Inf" : buf) + "\"}";
+    entries.emplace(bucket.d_name + "-6", StatsMapEntry{pname, std::to_string(bucket.d_count)});
+  }
+
+  snprintf(buf, sizeof(buf), "%g", (histogram4.getSum() +  histogram6.getSum()) / 1e6);
+  entries.emplace(name + "sum", StatsMapEntry{pbasename + "seconds_sum", buf});
+  entries.emplace(name + "count", StatsMapEntry{pbasename + "seconds_count", std::to_string(data4.back().d_count + data6.back().d_count)});
 
   return entries;
 }
@@ -1424,14 +1454,11 @@ static void registerAllStats1()
     addGetStat(name, &(SyncRes::s_ecsResponsesBySubnetSize6.at(idx)));
   }
 
-  addGetStat("cumul-answers", []() {
+  addGetStat("cumul-clientanswers", []() {
     return toStatsMap(g_stats.cumulativeAnswers.getName(), g_stats.cumulativeAnswers);
   });
-  addGetStat("cumul-auth4answers", []() {
-    return toStatsMap(g_stats.cumulativeAuth4Answers.getName(), g_stats.cumulativeAuth4Answers);
-  });
-  addGetStat("cumul-auth6answers", []() {
-    return toStatsMap(g_stats.cumulativeAuth6Answers.getName(), g_stats.cumulativeAuth6Answers);
+  addGetStat("cumul-authanswers", []() {
+    return toStatsMap(g_stats.cumulativeAuth4Answers.getName(), g_stats.cumulativeAuth4Answers, g_stats.cumulativeAuth6Answers);
   });
   addGetStat("policy-hits", []() {
     return toRPZStatsMap("policy-hits", g_stats.policyHits);

diff --git a/pdns/recursordist/docs/metrics.rst b/pdns/recursordist/docs/metrics.rst
index a3a83c338cf3e0a8219599575f9ba6c38ab415a8..3dd833da8080cfbe790c68a1b2796e177d3ae624 100644 (file)
--- a/pdns/recursordist/docs/metrics.rst
+++ b/pdns/recursordist/docs/metrics.rst
@@ -284,28 +284,21 @@ cpu-steal
 
 Stolen time, which is the time spent by the whole system in other operating systems when running in a virtualized environment, in units of USER_HZ.
 
-cumul-answers-x
-^^^^^^^^^^^^^^^^^^
-.. versionadded:: 4.6
-
-Cumulative counts of answer times in buckets less or equal than x microseconds.
-These metrics include packet cache hits.
-These metrics are useful for Prometheus and not listed other outputs by default.
-
 
-cumul-auth4-answers-x
+cumul-authanswers-x
 ^^^^^^^^^^^^^^^^^^^^^
 .. versionadded:: 4.6
 
-Cumulative counts of answer times of authoritative servers over IPv4 in buckets less than x microseconds.
-These metrics are useful for Prometheus and not listed other outputs by default.
+Cumulative counts of answer times of authoritative servers in buckets less than x microseconds.
+These metrics are useful for Prometheus and not listed in other outputs by default.
 
-cumul-auth6-answers-x
-^^^^^^^^^^^^^^^^^^^^^
+cumul-clientanswers-x
+^^^^^^^^^^^^^^^^^^
 .. versionadded:: 4.6
 
-Cumulative counts of answer times of authoritative servers over IPv6 in buckets less than x microseconds.
-These metrics are useful for Prometheus and not listed other outputs by default.
+Cumulative counts of our answer times to clients in buckets less or equal than x microseconds.
+These metrics include packet cache hits.
+These metrics are useful for Prometheus and not listed in other outputs by default.
 
 dns64-prefix-answers
 ^^^^^^^^^^^^^^^^^^^^

diff --git a/pdns/syncres.hh b/pdns/syncres.hh
index 35e500791af24fb0cfb5e98846708a83bc0d985f..1e58a8622aa8a855cd6197d0d386be4491112b7c 100644 (file)
--- a/pdns/syncres.hh
+++ b/pdns/syncres.hh
@@ -1083,9 +1083,10 @@ struct RecursorStats
     auth4Answers("auth4answers", { 1000, 10000, 100000, 1000000 }),
     auth6Answers("auth6answers", { 1000, 10000, 100000, 1000000 }),
     ourtime("ourtime", { 1000, 2000, 4000, 8000, 16000, 32000 }),
-    cumulativeAnswers("cumul-answers-", 10, 19),
-    cumulativeAuth4Answers("cumul-auth4answers-", 1000, 13),
-    cumulativeAuth6Answers("cumul-auth6answers-", 1000, 13)
+    cumulativeAnswers("cumul-clientanswers-", 10, 19),
+    // These two will be merged when outputting
+    cumulativeAuth4Answers("cumul-authanswers-", 1000, 13),
+    cumulativeAuth6Answers("cumul-authanswers-", 1000, 13)
   {
   }
 };

diff --git a/pdns/ws-recursor.cc b/pdns/ws-recursor.cc
index 9df78ec3581398979b1796de0fdb304691fd53ac..a156637ceb880fc5696ba6ed5cab3f12cfdd1bd3 100644 (file)
--- a/pdns/ws-recursor.cc
+++ b/pdns/ws-recursor.cc
@@ -1061,18 +1061,13 @@ const std::map<std::string, MetricDefinition> MetricDefinitionStorage::d_metrics
                      "Number of answers synthesized from the NSEC3 aggressive cache")},
 
   // For cumulative histogram, state the xxx_count name where xxx matches the name in rec_channel_rec
-  { "cumul-answers-count",
+  { "cumul-clientanswers-count",
     MetricDefinition(PrometheusMetricType::histogram,
-                     "histogram of our answer times")},
+                     "histogram of our answer times to clients")},
   // For cumulative histogram, state the xxx_count name where xxx matches the name in rec_channel_rec
-  { "cumul-auth4answers-count",
+  { "cumul-authanswers-count",
     MetricDefinition(PrometheusMetricType::histogram,
-                     "histogram of authoritative answer times over IPv4")},
-  // For cumulative histogram, state the xxx_count name where xxx matches the name in rec_channel_rec
-  { "cumul-auth6answers-count",
-    MetricDefinition(PrometheusMetricType::histogram,
-                     "histogram of authoritative answer times over IPV6")},
-
+                     "histogram of answer times of authoritative servers")},
   { "almost-expired-pushed",
     MetricDefinition(PrometheusMetricType::counter,
                      "number of almost-expired tasks pushed")},