From 5e08df89ff9f844ed569b14a847f57ce227a69c7 Mon Sep 17 00:00:00 2001 From: Remi Gacogne Date: Tue, 19 Feb 2019 15:31:49 +0100 Subject: [PATCH] rec: Disable the polling of expensive metrics via SNMP by default --- pdns/pdns_recursor.cc | 3 +- pdns/rec-snmp.cc | 227 +++++++++++++++------------- pdns/rec-snmp.hh | 2 +- pdns/rec_channel.hh | 8 +- pdns/rec_channel_rec.cc | 25 ++- pdns/recursordist/docs/settings.rst | 11 ++ 6 files changed, 157 insertions(+), 119 deletions(-) diff --git a/pdns/pdns_recursor.cc b/pdns/pdns_recursor.cc index 1a278125b..64bc3a751 100644 --- a/pdns/pdns_recursor.cc +++ b/pdns/pdns_recursor.cc @@ -3857,7 +3857,7 @@ static int serviceMain(int argc, char*argv[]) s_maxUDPQueriesPerRound=::arg().asNum("max-udp-queries-per-round"); if (::arg().mustDo("snmp-agent")) { - g_snmpAgent = std::make_shared("recursor", ::arg()["snmp-master-socket"]); + g_snmpAgent = std::make_shared("recursor", ::arg()["snmp-master-socket"], ::arg().mustDo("snmp-enable-expensive-stats")); g_snmpAgent->run(); } @@ -4278,6 +4278,7 @@ int main(int argc, char **argv) ::arg().setSwitch("snmp-agent", "If set, register as an SNMP agent")="no"; ::arg().set("snmp-master-socket", "If set and snmp-agent is set, the socket to use to register to the SNMP master")=""; + ::arg().setSwitch("snmp-enable-expensive-stats", "If set and snmp-agent is set, even statistics whose reporting can have an impact on production will be enabled")="no"; ::arg().set("tcp-fast-open", "Enable TCP Fast Open support on the listening sockets, using the supplied numerical value as the queue size")="0"; ::arg().set("nsec3-max-iterations", "Maximum number of iterations allowed for an NSEC3 record")="2500"; diff --git a/pdns/rec-snmp.cc b/pdns/rec-snmp.cc index e73d7e7d2..41ae6b8ca 100644 --- a/pdns/rec-snmp.cc +++ b/pdns/rec-snmp.cc @@ -148,21 +148,37 @@ static int handleCounter64Stats(netsnmp_mib_handler* handler, } } -static void registerCounter64Stat(const char* name, const oid statOID[], size_t statOIDLength) +static int handleDisabledCounter64Stats(netsnmp_mib_handler* handler, + netsnmp_handler_registration* reginfo, + netsnmp_agent_request_info* reqinfo, + netsnmp_request_info* requests) +{ + if (reqinfo->mode != MODE_GET) { + return SNMP_ERR_GENERR; + } + + if (reginfo->rootoid_len != OID_LENGTH(questionsOID) + 1) { + return SNMP_ERR_GENERR; + } + + return RecursorSNMPAgent::setCounter64Value(requests, 0); +} + +static void registerCounter64Stat(const std::string& name, const oid statOID[], size_t statOIDLength, bool expensiveStats) { if (statOIDLength != OID_LENGTH(questionsOID)) { - g_log< #include #include @@ -76,5 +77,6 @@ std::vector* pleaseGetTimeouts(); DNSName getRegisteredName(const DNSName& dom); std::atomic* getDynMetric(const std::string& str); optional getStatByName(const std::string& name); +bool isStatExpensive(const std::string& name); void registerAllStats(); -#endif + diff --git a/pdns/rec_channel_rec.cc b/pdns/rec_channel_rec.cc index 463ac9635..43de415fd 100644 --- a/pdns/rec_channel_rec.cc +++ b/pdns/rec_channel_rec.cc @@ -45,6 +45,12 @@ static map > d_get64bitmembers; static pthread_mutex_t d_dynmetricslock = PTHREAD_MUTEX_INITIALIZER; static map* > d_dynmetrics; +bool isStatExpensive(const string& name) +{ + static const std::set expensiveStats = { "cache-bytes", "packetcache-bytes", "special-memory-usage" }; + return expensiveStats.count(name) != 0; +} + static void addGetStat(const string& name, const uint32_t* place) { d_get32bitpointers[name]=place; @@ -101,19 +107,22 @@ map getAllStatsMap() map ret; for(const auto& the32bits : d_get32bitpointers) { - ret.insert(make_pair(the32bits.first, std::to_string(*the32bits.second))); + if (!isStatExpensive(the32bits.first)) { + ret.insert(make_pair(the32bits.first, std::to_string(*the32bits.second))); + } } for(const auto& atomic : d_getatomics) { - ret.insert(make_pair(atomic.first, std::to_string(atomic.second->load()))); + if (!isStatExpensive(atomic.first)) { + ret.insert(make_pair(atomic.first, std::to_string(atomic.second->load()))); + } } - for(const auto& the64bitmembers : d_get64bitmembers) { - if(the64bitmembers.first == "cache-bytes" || the64bitmembers.first=="packetcache-bytes") - continue; // too slow for 'get-all' - if(the64bitmembers.first == "special-memory-usage") - continue; // too slow for 'get-all' - ret.insert(make_pair(the64bitmembers.first, std::to_string(the64bitmembers.second()))); + for(const auto& the64bitmembers : d_get64bitmembers) { + if (!isStatExpensive(the64bitmembers.first)) { + ret.insert(make_pair(the64bitmembers.first, std::to_string(the64bitmembers.second()))); + } } + Lock l(&d_dynmetricslock); for(const auto& a : d_dynmetrics) ret.insert({a.first, std::to_string(*a.second)}); diff --git a/pdns/recursordist/docs/settings.rst b/pdns/recursordist/docs/settings.rst index 2e6200539..803c6fede 100644 --- a/pdns/recursordist/docs/settings.rst +++ b/pdns/recursordist/docs/settings.rst @@ -1225,6 +1225,17 @@ Use only a single socket for outgoing queries. If set to true and PowerDNS has been compiled with SNMP support, it will register as an SNMP agent to provide statistics and be able to send traps. +.. _setting-snmp-enable-expensive-stats: + +``snmp-enable-expensive-stats`` +---------------------- +.. versionadded:: 4.2.0 + +- Boolean +- Default: no + +If set and snmp-agent is set, even statistics whose reporting can have an impact on production will be enabled + .. _setting-snmp-master-socket: ``snmp-master-socket`` -- 2.49.0