Ands move policy name hits to tcounters

diff --git a/pdns/recursordist/pdns_recursor.cc b/pdns/recursordist/pdns_recursor.cc
index 010b511cbe78089044f3d9aa40d0acb9384f17fe..8a5a54acab4f6cb37806dfba25a413469ec3f85a 100644 (file)
--- a/pdns/recursordist/pdns_recursor.cc
+++ b/pdns/recursordist/pdns_recursor.cc
@@ -84,7 +84,6 @@ uint16_t g_minUdpSourcePort;
 uint16_t g_maxUdpSourcePort;
 double g_balancingFactor;
 
-RecursorStats g_stats;
 bool g_lowercaseOutgoing;
 unsigned int g_networkTimeoutMsec;
 uint16_t g_outgoingEDNSBufsize;
@@ -483,9 +482,8 @@ static PolicyResult handlePolicyHit(const DNSFilterEngine::Policy& appliedPolicy
 {
   /* don't account truncate actions for TCP queries, since they are not applied */
   if (appliedPolicy.d_kind != DNSFilterEngine::PolicyKind::Truncate || !dc->d_tcp) {
-    //++g_stats.policyResults[appliedPolicy.d_kind];
-    t_Counters.at(rec::PolicyHistogram::policy).at(appliedPolicy.d_kind)++;
-    ++(g_stats.policyHits.lock()->operator[](appliedPolicy.getName()));
+    ++t_Counters.at(rec::PolicyHistogram::policy).at(appliedPolicy.d_kind);
+    ++t_Counters.at(rec::PolicyNameHits::policyName).counts[appliedPolicy.getName()];
   }
 
   if (sr.doLog() && appliedPolicy.d_type != DNSFilterEngine::PolicyType::None) {

diff --git a/pdns/recursordist/rec-tcounters.cc b/pdns/recursordist/rec-tcounters.cc
index 680afb5929cbfa336b02db51c25ceeb20b41ec85..4f91bded80a6d94ec12471f41734bd4c6daa5320 100644 (file)
--- a/pdns/recursordist/rec-tcounters.cc
+++ b/pdns/recursordist/rec-tcounters.cc
@@ -39,18 +39,22 @@ Counters& Counters::merge(const Counters& data)
     const auto& rhs = data.doubleWAvg.at(i);
     auto weight = lhs.weight + rhs.weight;
     auto avg = lhs.avg * static_cast<double>(lhs.weight) + rhs.avg * static_cast<double>(rhs.weight);
-    avg = weight == 0 ? 0 : avg / weight;
+    avg = weight == 0 ? 0 : avg / static_cast<double>(weight);
     lhs.avg = avg;
     lhs.weight = weight;
   }
+
   // Rcode Counters are simply added
   for (size_t i = 0; i < auth.rcodeCounters.size(); i++) {
     auth.rcodeCounters.at(i) += data.auth.rcodeCounters.at(i);
   }
+
   // Histograms counts are added by += operator on Histograms
   for (size_t i = 0; i < histograms.size(); i++) {
     histograms.at(i) += data.histograms.at(i);
   }
+
+  // ResponseStats knows how to add
   responseStats += data.responseStats;
 
   // DNSSEC histograms: add individual entries
@@ -61,10 +65,15 @@ Counters& Counters::merge(const Counters& data)
       lhs.counts.at(j) += rhs.counts.at(j);
     }
   }
+
+  // policy kind counters: add individual entries
   for (size_t i = 0; i < policyCounters.counts.size(); i++) {
     policyCounters.counts.at(i) += data.policyCounters.counts.at(i);
   }
 
+  // Policy name counts knows how to add
+  policyNameHits += data.policyNameHits;
+
   return *this;
 }
 
@@ -85,10 +94,14 @@ std::string Counters::toString() const
   }
   stream << "Histograms: ";
   for (const auto& element : histograms) {
-    stream << element.getName() << ": NYI";
+    stream << element.getName() << ": NYI ";
   }
   stream << "DNSSEC Histograms: ";
-  stream << "NYI";
+  stream << "NYI ";
+  stream << "Policy Counters: ";
+  stream << "NYI ";
+  stream << "Policy Name Counters: ";
+  stream << "NYI ";
 
   stream << std::endl;
   return stream.str();

diff --git a/pdns/recursordist/rec-tcounters.hh b/pdns/recursordist/rec-tcounters.hh
index 9aa09962203439d93baaba112802781a905a8b05..7d59bde46cfc91d4429d444c13bf65a45be438d6 100644 (file)
--- a/pdns/recursordist/rec-tcounters.hh
+++ b/pdns/recursordist/rec-tcounters.hh
@@ -147,7 +147,14 @@ enum class DNSSECHistogram : uint8_t
 enum class PolicyHistogram : uint8_t
 {
   policy,
-  
+
+  numberOfCounters
+};
+
+enum class PolicyNameHits : uint8_t
+{
+  policyName,
+
   numberOfCounters
 };
 
@@ -238,6 +245,20 @@ struct Counters
   };
   PolicyCounters policyCounters{};
 
+  // Policy hits by name
+  struct PolicyNameCounters
+  {
+    PolicyNameCounters& operator+=(const PolicyNameCounters& rhs)
+    {
+      for (const auto& [name, count] : rhs.counts) {
+        counts[name] += count;
+      }
+      return *this;
+    }
+    std::unordered_map<std::string, uint64_t> counts;
+  };
+  PolicyNameCounters policyNameHits;
+
   Counters()
   {
     for (auto& elem : uint64Count) {
@@ -257,6 +278,7 @@ struct Counters
     for (auto& elem : policyCounters.counts) {
       elem = 0;
     }
+    // PolicyNameCounters has a default constuctor that initializes
   }
 
   // Merge a set of counters into an existing set of counters. For simple counters, that will be additions
@@ -274,13 +296,13 @@ struct Counters
     return doubleWAvg.at(static_cast<size_t>(index));
   }
 
-  RCodeCounters& at(RCode index)
+  RCodeCounters& at(RCode /*unused*/)
   {
     // We only have a single RCode indexed Histogram, so no need to select a specific one
     return auth;
   }
 
-  RecResponseStats& at(ResponseStats index)
+  RecResponseStats& at(ResponseStats /*unused*/)
   {
     // We only have a single ResponseStats indexed RecResponseStats, so no need to select a specific one
     return responseStats;
@@ -296,11 +318,18 @@ struct Counters
     return dnssecCounters.at(static_cast<size_t>(index));
   }
 
-  PolicyCounters& at(PolicyHistogram index)
+  // We only have a single PolicyHistogram indexed PolicyCounters, so no need to select a specific one
+  PolicyCounters& at(PolicyHistogram)
   {
     return policyCounters;
   }
 
+  // We only have a single policyNameHits indexed PolicyNameCounters, so no need to select a specific one
+  PolicyNameCounters& at(PolicyNameHits /*unused*/)
+  {
+    return policyNameHits;
+  }
+
   // Mainly for debugging purposes
   [[nodiscard]] std::string toString() const;
 };

diff --git a/pdns/recursordist/rec_channel_rec.cc b/pdns/recursordist/rec_channel_rec.cc
index 48fd3bf97ea47cbbcb2add69767d72377da6073b..6db7e91d8a88f3c8cd943fde9ea4099c5233ba05 100644 (file)
--- a/pdns/recursordist/rec_channel_rec.cc
+++ b/pdns/recursordist/rec_channel_rec.cc
@@ -1223,15 +1223,15 @@ static StatsMap toCPUStatsMap(const string& name)
   return entries;
 }
 
-static StatsMap toRPZStatsMap(const string& name, LockGuarded<std::unordered_map<std::string, pdns::stat_t>>& map)
+static StatsMap toRPZStatsMap(const string& name, const std::unordered_map<std::string, uint64_t>& map)
 {
   const string pbasename = getPrometheusName(name);
   StatsMap entries;
 
   uint64_t total = 0;
-  for (const auto& entry : *map.lock()) {
-    auto& key = entry.first;
-    auto count = entry.second.load();
+  for (const auto& entry : map) {
+    const auto& key = entry.first;
+    auto count = entry.second;
     std::string sname, pname;
     if (key.empty()) {
       sname = name + "-filter";
@@ -1576,7 +1576,7 @@ static void registerAllStats1()
     return toStatsMap(t_Counters.at(rec::Histogram::cumulativeAuth4Answers).getName(), g_Counters.sum(rec::Histogram::cumulativeAuth4Answers), g_Counters.sum(rec::Histogram::cumulativeAuth6Answers));
   });
   addGetStat("policy-hits", []() {
-    return toRPZStatsMap("policy-hits", g_stats.policyHits);
+    return toRPZStatsMap("policy-hits", g_Counters.sum(rec::PolicyNameHits::policyName).counts);
   });
   addGetStat("proxy-mapping-total", []() {
     return toProxyMappingStatsMap("proxy-mapping-total");

diff --git a/pdns/recursordist/syncres.cc b/pdns/recursordist/syncres.cc
index 83a639e002e5a9d011fe1403c5c97019fb867a6c..0040eb26a94b458b045e4bcc3244a616d38deba1 100644 (file)
--- a/pdns/recursordist/syncres.cc
+++ b/pdns/recursordist/syncres.cc
@@ -3237,9 +3237,8 @@ void SyncRes::handlePolicyHit(const std::string& prefix, const DNSName& qname, c
 
   /* don't account truncate actions for TCP queries, since they are not applied */
   if (d_appliedPolicy.d_kind != DNSFilterEngine::PolicyKind::Truncate || !d_queryReceivedOverTCP) {
-    t_Counters.at(rec::PolicyHistogram::policy).at(d_appliedPolicy.d_kind)++;
-    //++g_stats.policyResults[d_appliedPolicy.d_kind];
-    ++(g_stats.policyHits.lock()->operator[](d_appliedPolicy.getName()));
+    ++t_Counters.at(rec::PolicyHistogram::policy).at(d_appliedPolicy.d_kind);
+    ++t_Counters.at(rec::PolicyNameHits::policyName).counts[d_appliedPolicy.getName()];
   }
 
   if (d_appliedPolicy.d_type != DNSFilterEngine::PolicyType::None) {

diff --git a/pdns/recursordist/syncres.hh b/pdns/recursordist/syncres.hh
index dedceb1d9338935898867a8f1a06fb817d949ab7..11ac3fe49efa66f1a69176ba1f53041675c7fc77 100644 (file)
--- a/pdns/recursordist/syncres.hh
+++ b/pdns/recursordist/syncres.hh
@@ -771,12 +771,6 @@ extern std::unique_ptr<MemRecursorCache> g_recCache;
 extern rec::GlobalCounters g_Counters;
 extern thread_local rec::TCounters t_Counters;
 
-struct RecursorStats
-{
-  // XXX Convert counters below to be part of rec::Counters
-  LockGuarded<std::unordered_map<std::string, pdns::stat_t>> policyHits;
-};
-
 //! represents a running TCP/IP client session
 class TCPConnection : public boost::noncopyable
 {
@@ -854,7 +848,6 @@ extern thread_local std::unique_ptr<boost::circular_buffer<pair<DNSName, uint16_
 extern thread_local std::shared_ptr<NetmaskGroup> t_allowFrom;
 extern thread_local std::shared_ptr<NetmaskGroup> t_allowNotifyFrom;
 string doTraceRegex(vector<string>::const_iterator begin, vector<string>::const_iterator end);
-extern RecursorStats g_stats;
 extern unsigned int g_networkTimeoutMsec;
 extern uint16_t g_outgoingEDNSBufsize;
 extern std::atomic<uint32_t> g_maxCacheEntries, g_maxPacketCacheEntries;

diff --git a/pdns/recursordist/test-syncres_cc.cc b/pdns/recursordist/test-syncres_cc.cc
index 4d47fcb0fcf9b31abd7e12de0b787a62cb79f948..f47037109536381e86e26f65e977b59d1ab4f40a 100644 (file)
--- a/pdns/recursordist/test-syncres_cc.cc
+++ b/pdns/recursordist/test-syncres_cc.cc
@@ -8,7 +8,6 @@
 #include "rec-taskqueue.hh"
 #include "test-syncres_cc.hh"
 
-RecursorStats g_stats;
 GlobalStateHolder<LuaConfigItems> g_luaconfs;
 GlobalStateHolder<SuffixMatchNode> g_xdnssec;
 GlobalStateHolder<SuffixMatchNode> g_dontThrottleNames;