From: Otto Moerbeek <otto.moerbeek@open-xchange.com>
Date: Fri, 30 Oct 2020 08:12:06 +0000 (+0100)
Subject: Only prepend the pdns_recursor_ prefix if no 2nd arg to getMteric was
X-Git-Tag: dnsdist-1.6.0-alpha0~30^2
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=refs%2Fpull%2F9651%2Fhead;p=thirdparty%2Fpdns.git

Only prepend the pdns_recursor_ prefix if no 2nd arg to getMteric was
given.

This also fixes the lookup of the HELP entries since they require the
original key. So getAllStats() now returns the key, the prometheus name
and the value.
---

diff --git a/pdns/rec-carbon.cc b/pdns/rec-carbon.cc
index 1b08ef1a7b..f141c765a0 100644
--- a/pdns/rec-carbon.cc
+++ b/pdns/rec-carbon.cc
@@ -54,14 +54,13 @@ try
     s.connect(remote);  // we do the connect so the first attempt happens while we gather stats
  
     if(msg.empty()) {
-      typedef map<string,string> all_t;
-      all_t all=getAllStatsMap(StatComponent::Carbon);
+      auto all = getAllStatsMap(StatComponent::Carbon);
       
       ostringstream str;
       time_t now=time(0);
       
-      for(const all_t::value_type& val :  all) {
-        str<<namespace_name<<'.'<<hostname<<'.'<<instance_name<<'.'<<val.first<<' '<<val.second<<' '<<now<<"\r\n";
+      for(const auto& val : all) {
+        str<<namespace_name<<'.'<<hostname<<'.'<<instance_name<<'.'<<val.first<<' '<<val.second.d_value<<' '<<now<<"\r\n";
       }
       msg = str.str();
     }
diff --git a/pdns/rec_channel.hh b/pdns/rec_channel.hh
index 3862bfeed5..14b36cc0d7 100644
--- a/pdns/rec_channel.hh
+++ b/pdns/rec_channel.hh
@@ -72,7 +72,13 @@ public:
 
 enum class StatComponent { API, Carbon, RecControl, SNMP };
 
-std::map<std::string, std::string> getAllStatsMap(StatComponent component, bool prometheusName = false);
+struct StatsMapEntry {
+  std::string d_prometheusName;
+  std::string d_value;
+};
+typedef std::map<std::string, StatsMapEntry> StatsMap;
+StatsMap getAllStatsMap(StatComponent component);
+
 extern std::mutex g_carbon_config_lock;
 std::vector<std::pair<DNSName, uint16_t> >* pleaseGetQueryRing();
 std::vector<std::pair<DNSName, uint16_t> >* pleaseGetServfailQueryRing();
diff --git a/pdns/rec_channel_rec.cc b/pdns/rec_channel_rec.cc
index 7278d4eec1..6b15498076 100644
--- a/pdns/rec_channel_rec.cc
+++ b/pdns/rec_channel_rec.cc
@@ -87,6 +87,14 @@ static void addGetStat(const string& name, function<uint64_t ()> f )
   d_get64bitmembers[name]=f;
 }
 
+static std::string getPrometheusName(const std::string& arg)
+{
+  std::string name = arg;
+  std::replace_if(name.begin(), name.end(), [](char c){
+    return !isalnum(static_cast<unsigned char>(c));}, '_');
+  return "pdns_recursor_" + name;
+}
+
 std::atomic<unsigned long>* getDynMetric(const std::string& str, const std::string& prometheusName)
 {
   std::lock_guard<std::mutex> l(d_dynmetricslock);
@@ -98,8 +106,7 @@ std::atomic<unsigned long>* getDynMetric(const std::string& str, const std::stri
   if (!prometheusName.empty()) {
     name = prometheusName;
   } else {
-    std::replace_if(name.begin(), name.end(), [](char c){
-      return !isalnum(static_cast<unsigned char>(c));}, '_');
+    name = getPrometheusName(name);
   }
 
   auto ret = dynmetrics{new std::atomic<unsigned long>(), name};
@@ -131,25 +138,25 @@ optional<uint64_t> getStatByName(const std::string& name)
   return get(name);
 }
 
-map<string,string> getAllStatsMap(StatComponent component, bool prometheusNames)
+StatsMap getAllStatsMap(StatComponent component)
 {
-  map<string,string> ret;
+  StatsMap ret;
   const auto& blacklistMap = s_blacklistedStats.at(component);
 
   for(const auto& the32bits :  d_get32bitpointers) {
     if (blacklistMap.count(the32bits.first) == 0) {
-      ret.insert(make_pair(the32bits.first, std::to_string(*the32bits.second)));
+      ret.insert(make_pair(the32bits.first, StatsMapEntry{getPrometheusName(the32bits.first), std::to_string(*the32bits.second)}));
     }
   }
   for(const auto& atomic :  d_getatomics) {
     if (blacklistMap.count(atomic.first) == 0) {
-      ret.insert(make_pair(atomic.first, std::to_string(atomic.second->load())));
+      ret.insert(make_pair(atomic.first, StatsMapEntry{getPrometheusName(atomic.first), std::to_string(atomic.second->load())}));
     }
   }
 
   for(const auto& the64bitmembers :  d_get64bitmembers) {
     if (blacklistMap.count(the64bitmembers.first) == 0) {
-      ret.insert(make_pair(the64bitmembers.first, std::to_string(the64bitmembers.second())));
+      ret.insert(make_pair(the64bitmembers.first, StatsMapEntry{getPrometheusName(the64bitmembers.first), std::to_string(the64bitmembers.second())}));
     }
   }
 
@@ -157,8 +164,7 @@ map<string,string> getAllStatsMap(StatComponent component, bool prometheusNames)
     std::lock_guard<std::mutex> l(d_dynmetricslock);
     for(const auto& a : d_dynmetrics) {
       if (blacklistMap.count(a.first) == 0) {
-        ret.insert({prometheusNames ? a.second.d_prometheusName : a.first,
-            std::to_string(*a.second.d_ptr)});
+        ret.insert(make_pair(a.first, StatsMapEntry{a.second.d_prometheusName, std::to_string(*a.second.d_ptr)}));
       }
     }
   }
@@ -168,11 +174,10 @@ map<string,string> getAllStatsMap(StatComponent component, bool prometheusNames)
 
 static string getAllStats()
 {
-  typedef map<string, string> varmap_t;
-  varmap_t varmap = getAllStatsMap(StatComponent::RecControl);
+  auto varmap = getAllStatsMap(StatComponent::RecControl);
   string ret;
-  for(varmap_t::value_type& tup :  varmap) {
-    ret += tup.first + "\t" + tup.second +"\n";
+  for (const auto& tup : varmap) {
+    ret += tup.first + "\t" + tup.second.d_value + "\n";
   }
   return ret;
 }
diff --git a/pdns/ws-recursor.cc b/pdns/ws-recursor.cc
index 68755465bd..d6b5b78f0f 100644
--- a/pdns/ws-recursor.cc
+++ b/pdns/ws-recursor.cc
@@ -50,8 +50,12 @@ using json11::Json;
 
 void productServerStatisticsFetch(map<string,string>& out)
 {
-  map<string,string> stats = getAllStatsMap(StatComponent::API);
-  out.swap(stats);
+  auto stats = getAllStatsMap(StatComponent::API);
+  map<string,string> ret;
+  for (const auto& entry: stats) {
+    ret.insert(make_pair(entry.first, entry.second.d_value));
+  }
+  out.swap(ret);
 }
 
 boost::optional<uint64_t> productServerStatisticsFetch(const std::string& name)
@@ -434,17 +438,14 @@ static void prometheusMetrics(HttpRequest *req, HttpResponse *resp) {
 
     registerAllStats();
 
-
     std::ostringstream output;
-    typedef map <string, string> varmap_t;
-    
+
     // Argument controls blacklisting of any stats. So
     // stats-api-blacklist will be used to block returned stats.
-    // Second arg tells to use the prometheus names.
-    varmap_t varmap = getAllStatsMap(StatComponent::API, true);
-    for (const auto &tup :  varmap) {
+    auto varmap = getAllStatsMap(StatComponent::API);
+    for (const auto& tup : varmap) {
         std::string metricName = tup.first;
-        std::string prometheusMetricName = "pdns_recursor_" + metricName;
+        std::string prometheusMetricName = tup.second.d_prometheusName;
 
         MetricDefinition metricDetails;
 
@@ -460,7 +461,7 @@ static void prometheusMetrics(HttpRequest *req, HttpResponse *resp) {
           output << "# HELP " << prometheusMetricName << " " << metricDetails.description << "\n";
           output << "# TYPE " << prometheusMetricName << " " << prometheusTypeName << "\n";
         }
-        output << prometheusMetricName << " " << tup.second << "\n";
+        output << prometheusMetricName << " " << tup.second.d_value << "\n";
     }
 
     output << "# HELP pdns_recursor_info " << "Info from pdns_recursor, value is always 1" << "\n";
@@ -470,7 +471,6 @@ static void prometheusMetrics(HttpRequest *req, HttpResponse *resp) {
     resp->body = output.str();
     resp->headers["Content-Type"] = "text/plain";
     resp->status = 200;
-
 }