From 5773b1c694c7fb746d3e0ab4cc5294cf46e3a403 Mon Sep 17 00:00:00 2001 From: Otto Moerbeek Date: Thu, 5 Sep 2019 13:32:56 +0200 Subject: [PATCH] Review comments: change prefix to pdns_recursor, some type changes --- .../recursordist/docs/http-api/prometheus.rst | 22 +++++----- pdns/recursordist/rec_metrics.hh | 40 ++++++++++--------- pdns/ws-recursor.cc | 6 +-- regression-tests.api/test_Servers.py | 4 +- 4 files changed, 37 insertions(+), 35 deletions(-) diff --git a/pdns/recursordist/docs/http-api/prometheus.rst b/pdns/recursordist/docs/http-api/prometheus.rst index 7ba1e55f4..1fd9fd859 100644 --- 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 ec1580fa5..69bae90f0 100644 --- 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 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 dc4980dba..1d6c4d50a 100644 --- a/pdns/ws-recursor.cc +++ b/pdns/ws-recursor.cc @@ -435,11 +435,11 @@ static void prometheusMetrics(HttpRequest *req, HttpResponse *resp) { typedef map 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 66a183058..c9b8355e3 100644 --- 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): -- 2.40.0