From: Otto Moerbeek Date: Mon, 19 Dec 2022 13:31:06 +0000 (+0100) Subject: Ands move policy name hits to tcounters X-Git-Tag: dnsdist-1.8.0-rc1~144^2 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=refs%2Fpull%2F12348%2Fhead;p=thirdparty%2Fpdns.git Ands move policy name hits to tcounters --- diff --git a/pdns/recursordist/pdns_recursor.cc b/pdns/recursordist/pdns_recursor.cc index 010b511cbe..8a5a54acab 100644 --- 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 680afb5929..4f91bded80 100644 --- 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(lhs.weight) + rhs.avg * static_cast(rhs.weight); - avg = weight == 0 ? 0 : avg / weight; + avg = weight == 0 ? 0 : avg / static_cast(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 9aa0996220..7d59bde46c 100644 --- 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 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(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(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 48fd3bf97e..6db7e91d8a 100644 --- 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>& map) +static StatsMap toRPZStatsMap(const string& name, const std::unordered_map& 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 83a639e002..0040eb26a9 100644 --- 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 dedceb1d93..11ac3fe49e 100644 --- a/pdns/recursordist/syncres.hh +++ b/pdns/recursordist/syncres.hh @@ -771,12 +771,6 @@ extern std::unique_ptr 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> policyHits; -}; - //! represents a running TCP/IP client session class TCPConnection : public boost::noncopyable { @@ -854,7 +848,6 @@ extern thread_local std::unique_ptr t_allowFrom; extern thread_local std::shared_ptr t_allowNotifyFrom; string doTraceRegex(vector::const_iterator begin, vector::const_iterator end); -extern RecursorStats g_stats; extern unsigned int g_networkTimeoutMsec; extern uint16_t g_outgoingEDNSBufsize; extern std::atomic g_maxCacheEntries, g_maxPacketCacheEntries; diff --git a/pdns/recursordist/test-syncres_cc.cc b/pdns/recursordist/test-syncres_cc.cc index 4d47fcb0fc..f470371095 100644 --- 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 g_luaconfs; GlobalStateHolder g_xdnssec; GlobalStateHolder g_dontThrottleNames;