Review comments: change prefix to pdns_recursor, some type changes

diff --git a/pdns/recursordist/docs/http-api/prometheus.rst b/pdns/recursordist/docs/http-api/prometheus.rst
index 7ba1e55f491eef8bb2f00a849cb079aeb9db28e0..1fd9fd859e95868f0c081974c239187905aeb4b1 100644 (file)
--- a/pdns/recursordist/docs/http-api/prometheus.rst
+++ b/pdns/recursordist/docs/http-api/prometheus.rst
@@ -21,17 +21,17 @@ Prometheus Data Endpoint
     Content-Type: text/plain
     Server: PowerDNS/0.0.16480.0.g876dd46192
 
-    # HELP pdnsrecursor_all_outqueries Number of outgoing UDP queries since starting
-    # TYPE pdnsrecursor_all_outqueries counter
-    pdnsrecursor_all_outqueries 20
-    # HELP pdnsrecursor_answers_slow Number of queries answered after 1 second
-    # TYPE pdnsrecursor_answers_slow counter
-    pdnsrecursor_answers_slow 0
-    # HELP pdnsrecursor_answers0_1 Number of queries answered within 1 millisecond
-    # TYPE pdnsrecursor_answers0_1 counter
-    pdnsrecursor_answers0_1 0
-    # HELP pdnsrecursor_answers1_10 Number of queries answered within 10 milliseconds
-    # TYPE pdnsrecursor_answers1_10 counter
+    # HELP pdns_recursor_all_outqueries Number of outgoing UDP queries since starting
+    # TYPE pdns_recursor_all_outqueries counter
+    pdns_recursor_all_outqueries 20
+    # HELP pdns_recursor_answers_slow Number of queries answered after 1 second
+    # TYPE pdns_recursor_answers_slow counter
+    pdns_recursor_answers_slow 0
+    # HELP pdns_recursor_answers0_1 Number of queries answered within 1 millisecond
+    # TYPE pdns_recursor_answers0_1 counter
+    pdns_recursor_answers0_1 0
+    # HELP pdns_recursor_answers1_10 Number of queries answered within 10 milliseconds
+    # TYPE pdns_recursor_answers1_10 counter
 
     ...
 

diff --git a/pdns/recursordist/rec_metrics.hh b/pdns/recursordist/rec_metrics.hh
index ec1580fa5f7694553f3ff97586be54d0ba2dcf7f..69bae90f0671846f14dbe99b68cc353386697ce1 100644 (file)
--- a/pdns/recursordist/rec_metrics.hh
+++ b/pdns/recursordist/rec_metrics.hh
@@ -37,7 +37,7 @@ enum class PrometheusMetricType: int {
 
 // Keeps additional information about metrics
 struct MetricDefinition {
-    MetricDefinition(PrometheusMetricType prometheusType, const std::string& description) {
+    MetricDefinition(const PrometheusMetricType& prometheusType, const std::string& description) {
       this->prometheusType = prometheusType;
       this->description = description;
     }
@@ -50,9 +50,10 @@ struct MetricDefinition {
     PrometheusMetricType prometheusType;
 };
 
-struct MetricDefinitionStorage {
+class MetricDefinitionStorage {
+public:
     // Return metric definition by name
-    bool getMetricDetails(std::string metricName, MetricDefinition& metric) {
+    bool getMetricDetails(const std::string& metricName, MetricDefinition& metric) {
       auto metricDetailsIter = metrics.find(metricName);
 
       if (metricDetailsIter == metrics.end()) {
@@ -64,7 +65,7 @@ struct MetricDefinitionStorage {
     };
 
     // Return string representation of Prometheus metric type
-    std::string getPrometheusStringMetricType(PrometheusMetricType metricType) {
+    std::string getPrometheusStringMetricType(const PrometheusMetricType& metricType) {
       switch (metricType) {
         case PrometheusMetricType::counter:
           return "counter";
@@ -78,6 +79,7 @@ struct MetricDefinitionStorage {
       }
     };
 
+private:
     // Description and types for prometheus output of stats
     std::map<std::string, MetricDefinition> metrics = {
             { "all-outqueries",              MetricDefinition(PrometheusMetricType::counter, "Number of outgoing UDP queries since starting") },
@@ -88,17 +90,17 @@ struct MetricDefinitionStorage {
             { "answers10-100",              MetricDefinition(PrometheusMetricType::counter, "Number of queries answered within 100 milliseconds") },
             { "answers100-1000",              MetricDefinition(PrometheusMetricType::counter, "Number of queries answered within 1 second") },
 
-            { "auth4-answers-slow",              MetricDefinition(PrometheusMetricType::counter, "Number of queries answered by auth4s after 1 second") },
-            { "auth4-answers0-1",              MetricDefinition(PrometheusMetricType::counter, "Number of queries answered by auth4s within 1 millisecond") },
-            { "auth4-answers1-10",              MetricDefinition(PrometheusMetricType::counter, "Number of queries answered by auth4s within 10 milliseconds") },
-            { "auth4-answers10-100",              MetricDefinition(PrometheusMetricType::counter, "Number of queries answered by auth4s within 100 milliseconds") },
-            { "auth4-answers100-1000",              MetricDefinition(PrometheusMetricType::counter, "Number of queries answered by auth4s within 1 second") },
+            { "auth4-answers-slow",              MetricDefinition(PrometheusMetricType::counter, "Number of queries answered by authoritatives over IPv4 after 1 second") },
+            { "auth4-answers0-1",              MetricDefinition(PrometheusMetricType::counter, "Number of queries answered by authoritatives over IPv4within 1 millisecond") },
+            { "auth4-answers1-10",              MetricDefinition(PrometheusMetricType::counter, "Number of queries answered by authoritatives over IPv4 within 10 milliseconds") },
+            { "auth4-answers10-100",              MetricDefinition(PrometheusMetricType::counter, "Number of queries answered by authoritatives over IPv4 within 100 milliseconds") },
+            { "auth4-answers100-1000",              MetricDefinition(PrometheusMetricType::counter, "Number of queries answered by authoritatives over IPv4 within 1 second") },
 
-            { "auth6-answers-slow",              MetricDefinition(PrometheusMetricType::counter, "Number of queries answered by auth6s after 1 second") },
-            { "auth6-answers0-1",              MetricDefinition(PrometheusMetricType::counter, "Number of queries answered by auth6s within 1 millisecond") },
-            { "auth6-answers1-10",              MetricDefinition(PrometheusMetricType::counter, "Number of queries answered by auth6s within 10 milliseconds") },
-            { "auth6-answers10-100",              MetricDefinition(PrometheusMetricType::counter, "Number of queries answered by auth6s within 100 milliseconds") },
-            { "auth6-answers100-1000",              MetricDefinition(PrometheusMetricType::counter, "Number of queries answered by auth6s within 1 second") },
+            { "auth6-answers-slow",              MetricDefinition(PrometheusMetricType::counter, "Number of queries answered by authoritatives over IPv6 after 1 second") },
+            { "auth6-answers0-1",              MetricDefinition(PrometheusMetricType::counter, "Number of queries answered by authoritatives over IPv6 within 1 millisecond") },
+            { "auth6-answers1-10",              MetricDefinition(PrometheusMetricType::counter, "Number of queries answered by authoritatives over IPv6 within 10 milliseconds") },
+            { "auth6-answers10-100",              MetricDefinition(PrometheusMetricType::counter, "Number of queries answered by authoritatives over IPv6 within 100 milliseconds") },
+            { "auth6-answers100-1000",              MetricDefinition(PrometheusMetricType::counter, "Number of queries answered by authoritatives over IPv6 within 1 second") },
 
 
 
@@ -108,7 +110,7 @@ struct MetricDefinitionStorage {
             { "cache-hits",              MetricDefinition(PrometheusMetricType::counter, "Number of of cache hits since starting, this does **not** include hits that got answered from the packet-cache") },
             { "cache-misses",              MetricDefinition(PrometheusMetricType::counter, "Number of cache misses since starting") },
             { "case-mismatches",              MetricDefinition(PrometheusMetricType::counter, "Number of mismatches in character case since starting") },
-            { "chain-resends",              MetricDefinition(PrometheusMetricType::gauge, "Number of queries chained to existing outstanding") },
+            { "chain-resends",              MetricDefinition(PrometheusMetricType::counter, "Number of queries chained to existing outstanding") },
             { "client-parse-errors",              MetricDefinition(PrometheusMetricType::counter, "Number of client packets that could not be parsed") },
             { "concurrent-queries",              MetricDefinition(PrometheusMetricType::gauge, "Number of MThreads currently running") },
             { "cpu-msec-thread-0",              MetricDefinition(PrometheusMetricType::counter, "Number of milliseconds spent in thread n") },
@@ -135,8 +137,8 @@ struct MetricDefinitionStorage {
             { "ipv6-questions",              MetricDefinition(PrometheusMetricType::counter, "Number of end-user initiated queries with the RD bit set, received over IPv6 UDP") },
             { "malloc-bytes",              MetricDefinition(PrometheusMetricType::counter, "Number of bytes allocated by the process (broken, always returns 0)") },
             { "max-cache-entries",              MetricDefinition(PrometheusMetricType::gauge, "Currently configured maximum number of cache entries") },
-            { "max-packetcache-entries",              MetricDefinition(PrometheusMetricType::counter, "Currently configured maximum number of packet cache entries") },
-            { "max-mthread-stack",              MetricDefinition(PrometheusMetricType::counter, "Maximum amount of thread stack ever used") },
+            { "max-packetcache-entries",              MetricDefinition(PrometheusMetricType::gauge, "Currently configured maximum number of packet cache entries") },
+            { "max-mthread-stack",              MetricDefinition(PrometheusMetricType::gauge, "Maximum amount of thread stack ever used") },
 
 
             { "negcache-entries",              MetricDefinition(PrometheusMetricType::gauge, "Number of entries in the negative answer cache") },
@@ -169,7 +171,7 @@ struct MetricDefinitionStorage {
             { "questions",              MetricDefinition(PrometheusMetricType::counter, "Counts all end-user initiated queries with the RD bit set") },
             { "rebalanced-queries",              MetricDefinition(PrometheusMetricType::counter, "Number of queries balanced to a different worker thread because the first selected one was above the target load configured with 'distribution-load-factor'") },
             { "resource-limits",              MetricDefinition(PrometheusMetricType::counter, "Number of queries that could not be performed because of resource limits") },
-            { "security-status",              MetricDefinition(PrometheusMetricType::counter, "security status based on `securitypolling`") },
+            { "security-status",              MetricDefinition(PrometheusMetricType::gauge, "security status based on `securitypolling`") },
             { "server-parse-errors",              MetricDefinition(PrometheusMetricType::counter, "Number of server replied packets that could not be parsed") },
             { "servfail-answers",              MetricDefinition(PrometheusMetricType::counter, "Number of SERVFAIL answers since starting") },
             { "spoof-prevents",              MetricDefinition(PrometheusMetricType::counter, "Number of times PowerDNS considered itself spoofed, and dropped the data") },
@@ -202,7 +204,7 @@ struct MetricDefinitionStorage {
             { "x-ourtime-slow",              MetricDefinition(PrometheusMetricType::counter, "Counts responses where more than 32 milliseconds was spent within the Recursor") },
 
             { "fd-usage",              MetricDefinition(PrometheusMetricType::gauge, "Number of open file descriptors") },
-            { "real-memory-usage",              MetricDefinition(PrometheusMetricType::counter, "Number of bytes real process memory usage") },
+            { "real-memory-usage",              MetricDefinition(PrometheusMetricType::gauge, "Number of bytes real process memory usage") },
             { "udp-in-errors",              MetricDefinition(PrometheusMetricType::counter, "From /proc/net/snmp InErrors") },
             { "udp-noport-errors",              MetricDefinition(PrometheusMetricType::counter, "From /proc/net/snmp NoPorts") },
             { "udp-recvbuf-errors",              MetricDefinition(PrometheusMetricType::counter, "From /proc/net/snmp RcvbufErrors") },

diff --git a/pdns/ws-recursor.cc b/pdns/ws-recursor.cc
index dc4980dbae0eeec0813512c67fff619a2bab2db1..1d6c4d50a82e26d48ccbdfd233d1b1adea435ae8 100644 (file)
--- a/pdns/ws-recursor.cc
+++ b/pdns/ws-recursor.cc
@@ -435,11 +435,11 @@ static void prometheusMetrics(HttpRequest *req, HttpResponse *resp) {
     typedef map <string, string> varmap_t;
     varmap_t varmap = getAllStatsMap(
             StatComponent::API); // Argument controls blacklisting of any stats. So stats-api-blacklist will be used to block returned stats.
-    for (const varmap_t::value_type &tup :  varmap) {
+    for (const auto &tup :  varmap) {
         std::string metricName = tup.first;
 
         // Prometheus suggest using '_' instead of '-'
-        std::string prometheusMetricName = "pdnsrecursor_" + boost::replace_all_copy(metricName, "-", "_");
+        std::string prometheusMetricName = "pdns_recursor_" + boost::replace_all_copy(metricName, "-", "_");
 
         MetricDefinition metricDetails;
 
@@ -447,7 +447,7 @@ static void prometheusMetrics(HttpRequest *req, HttpResponse *resp) {
           std::string prometheusTypeName = g_metricDefinitions.getPrometheusStringMetricType(
                   metricDetails.prometheusType);
 
-          if (prometheusTypeName == "") {
+          if (prometheusTypeName.empty()) {
             continue;
           }
 

diff --git a/regression-tests.api/test_Servers.py b/regression-tests.api/test_Servers.py
index 66a183058ff8e514181c76ad237316a00e77cadc..c9b8355e3a00e61fb75d24cfea31206a4a4d2de4 100644 (file)
--- a/regression-tests.api/test_Servers.py
+++ b/regression-tests.api/test_Servers.py
@@ -80,9 +80,9 @@ class Servers(ApiTestCase):
             for line in res.text.splitlines():
                 if line[0] == "#":
                     continue
-                if line.split(" ")[0] == "pdnsrecursor_uptime":
+                if line.split(" ")[0] == "pdns_recursor_uptime":
                     found = True
-            self.assertTrue(found,"pdnsrecursor_uptime is missing")
+            self.assertTrue(found,"pdns_recursor_uptime is missing")
             
     @unittest.skipIf(is_auth(), "Not applicable")
     def test_read_statistics_using_password(self):