From b51f86701152b8972cc4637cbcbcf52ec4b73811 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ensar=20Saraj=C4=8Di=C4=87?= Date: Thu, 12 Dec 2024 09:04:56 +0100 Subject: [PATCH 01/16] dnsdist: add support for labels on custom metrics This adds support for prometheus labels on custom metrics. Changes the metrics maps to hold a map of labels to metric values for each metric. Metrics without labels have an empty string label combination value. Adds an optional options table as the last parameter of `incMetric`, `decMetric`, `setMetric` and `getMetric`, which can be used to set labels by using the `label` key or to set the previously used parameter (either `step` or `value`). Closes: #13359 --- pdns/dnsdistdist/dnsdist-lua-ffi.cc | 15 +- pdns/dnsdistdist/dnsdist-lua.cc | 52 +++++- pdns/dnsdistdist/dnsdist-metrics.cc | 267 +++++++++++++++++----------- pdns/dnsdistdist/dnsdist-metrics.hh | 14 +- pdns/dnsdistdist/dnsdist-web.cc | 4 + 5 files changed, 225 insertions(+), 127 deletions(-) diff --git a/pdns/dnsdistdist/dnsdist-lua-ffi.cc b/pdns/dnsdistdist/dnsdist-lua-ffi.cc index 192e8f1cfd10..c7a0c055f682 100644 --- a/pdns/dnsdistdist/dnsdist-lua-ffi.cc +++ b/pdns/dnsdistdist/dnsdist-lua-ffi.cc @@ -1811,7 +1811,8 @@ bool dnsdist_ffi_metric_declare(const char* name, size_t nameLen, const char* ty void dnsdist_ffi_metric_inc(const char* metricName, size_t metricNameLen) { - auto result = dnsdist::metrics::incrementCustomCounter(std::string_view(metricName, metricNameLen), 1U); + // TODO: add labels? + auto result = dnsdist::metrics::incrementCustomCounter(std::string_view(metricName, metricNameLen), 1U, {}); if (std::get_if(&result) != nullptr) { return; } @@ -1819,7 +1820,8 @@ void dnsdist_ffi_metric_inc(const char* metricName, size_t metricNameLen) void dnsdist_ffi_metric_inc_by(const char* metricName, size_t metricNameLen, uint64_t value) { - auto result = dnsdist::metrics::incrementCustomCounter(std::string_view(metricName, metricNameLen), value); + // TODO: add labels? + auto result = dnsdist::metrics::incrementCustomCounter(std::string_view(metricName, metricNameLen), value, {}); if (std::get_if(&result) != nullptr) { return; } @@ -1827,7 +1829,8 @@ void dnsdist_ffi_metric_inc_by(const char* metricName, size_t metricNameLen, uin void dnsdist_ffi_metric_dec(const char* metricName, size_t metricNameLen) { - auto result = dnsdist::metrics::decrementCustomCounter(std::string_view(metricName, metricNameLen), 1U); + // TODO: add labels? + auto result = dnsdist::metrics::decrementCustomCounter(std::string_view(metricName, metricNameLen), 1U, {}); if (std::get_if(&result) != nullptr) { return; } @@ -1835,7 +1838,8 @@ void dnsdist_ffi_metric_dec(const char* metricName, size_t metricNameLen) void dnsdist_ffi_metric_set(const char* metricName, size_t metricNameLen, double value) { - auto result = dnsdist::metrics::setCustomGauge(std::string_view(metricName, metricNameLen), value); + // TODO: add labels? + auto result = dnsdist::metrics::setCustomGauge(std::string_view(metricName, metricNameLen), value, {}); if (std::get_if(&result) != nullptr) { return; } @@ -1843,7 +1847,8 @@ void dnsdist_ffi_metric_set(const char* metricName, size_t metricNameLen, double double dnsdist_ffi_metric_get(const char* metricName, size_t metricNameLen, bool isCounter) { - auto result = dnsdist::metrics::getCustomMetric(std::string_view(metricName, metricNameLen)); + // TODO: add labels? + auto result = dnsdist::metrics::getCustomMetric(std::string_view(metricName, metricNameLen), {}); if (std::get_if(&result) != nullptr) { return 0.; } diff --git a/pdns/dnsdistdist/dnsdist-lua.cc b/pdns/dnsdistdist/dnsdist-lua.cc index 09818d982107..53e1038c135c 100644 --- a/pdns/dnsdistdist/dnsdist-lua.cc +++ b/pdns/dnsdistdist/dnsdist-lua.cc @@ -85,6 +85,8 @@ using std::thread; +using update_metric_opts_t = LuaAssociativeTable>>; + static boost::tribool s_noLuaSideEffect; /* this is a best effort way to prevent logging calls with no side-effects in the output of delta() @@ -3419,8 +3421,20 @@ static void setupLuaConfig(LuaContext& luaCtx, bool client, bool configCheck) } return true; }); - luaCtx.writeFunction("incMetric", [](const std::string& name, boost::optional step) { - auto result = dnsdist::metrics::incrementCustomCounter(name, step ? *step : 1); + luaCtx.writeFunction("incMetric", [](const std::string& name, boost::optional>> opts) { + auto incOpts = opts.get_value_or(1); + uint64_t step = 1; + std::unordered_map labels; + if (auto* custom_step = boost::get(&incOpts)) { + step = *custom_step; + } + else { + boost::optional vars = boost::get>(incOpts); + getOptionalValue(vars, "step", step); + getOptionalValue>(vars, "labels", labels); + checkAllParametersConsumed("incMetric", vars); + } + auto result = dnsdist::metrics::incrementCustomCounter(name, step, labels); if (const auto* errorStr = std::get_if(&result)) { g_outputBuffer = *errorStr + "'\n"; errlog("Error in incMetric: %s", *errorStr); @@ -3428,8 +3442,20 @@ static void setupLuaConfig(LuaContext& luaCtx, bool client, bool configCheck) } return std::get(result); }); - luaCtx.writeFunction("decMetric", [](const std::string& name, boost::optional step) { - auto result = dnsdist::metrics::decrementCustomCounter(name, step ? *step : 1); + luaCtx.writeFunction("decMetric", [](const std::string& name, boost::optional>> opts) { + auto decOpts = opts.get_value_or(1); + uint64_t step = 1; + std::unordered_map labels; + if (auto custom_step = boost::get(&decOpts)) { + step = *custom_step; + } + else { + boost::optional vars = boost::get>(decOpts); + getOptionalValue(vars, "step", step); + getOptionalValue>(vars, "labels", labels); + checkAllParametersConsumed("decMetric", vars); + } + auto result = dnsdist::metrics::decrementCustomCounter(name, step, labels); if (const auto* errorStr = std::get_if(&result)) { g_outputBuffer = *errorStr + "'\n"; errlog("Error in decMetric: %s", *errorStr); @@ -3437,8 +3463,13 @@ static void setupLuaConfig(LuaContext& luaCtx, bool client, bool configCheck) } return std::get(result); }); - luaCtx.writeFunction("setMetric", [](const std::string& name, const double value) -> double { - auto result = dnsdist::metrics::setCustomGauge(name, value); + luaCtx.writeFunction("setMetric", [](const std::string& name, const double value, boost::optional opts) -> double { + std::unordered_map labels; + if (opts) { + getOptionalValue>(opts, "labels", labels); + } + checkAllParametersConsumed("setMetric", opts); + auto result = dnsdist::metrics::setCustomGauge(name, value, labels); if (const auto* errorStr = std::get_if(&result)) { g_outputBuffer = *errorStr + "'\n"; errlog("Error in setMetric: %s", *errorStr); @@ -3446,8 +3477,13 @@ static void setupLuaConfig(LuaContext& luaCtx, bool client, bool configCheck) } return std::get(result); }); - luaCtx.writeFunction("getMetric", [](const std::string& name) { - auto result = dnsdist::metrics::getCustomMetric(name); + luaCtx.writeFunction("getMetric", [](const std::string& name, boost::optional opts) { + std::unordered_map labels; + if (opts) { + getOptionalValue>(opts, "labels", labels); + } + checkAllParametersConsumed("getMetric", opts); + auto result = dnsdist::metrics::getCustomMetric(name, labels); if (const auto* errorStr = std::get_if(&result)) { g_outputBuffer = *errorStr + "'\n"; errlog("Error in getMetric: %s", *errorStr); diff --git a/pdns/dnsdistdist/dnsdist-metrics.cc b/pdns/dnsdistdist/dnsdist-metrics.cc index 434088df4e2f..039b1ffc8924 100644 --- a/pdns/dnsdistdist/dnsdist-metrics.cc +++ b/pdns/dnsdistdist/dnsdist-metrics.cc @@ -19,6 +19,8 @@ * along with this program; if not, write to the Free Software * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. */ +#include +#include #include #include "dnsdist-metrics.hh" @@ -67,100 +69,100 @@ struct MutableGauge mutable pdns::stat_double_t d_value{0}; }; -static SharedLockGuarded>> s_customCounters; -static SharedLockGuarded>> s_customGauges; +static SharedLockGuarded, std::less<>>> s_customCounters; +static SharedLockGuarded, std::less<>>> s_customGauges; Stats::Stats() : - entries{std::vector{ - {"responses", &responses}, - {"servfail-responses", &servfailResponses}, - {"queries", &queries}, - {"frontend-nxdomain", &frontendNXDomain}, - {"frontend-servfail", &frontendServFail}, - {"frontend-noerror", &frontendNoError}, - {"acl-drops", &aclDrops}, - {"rule-drop", &ruleDrop}, - {"rule-nxdomain", &ruleNXDomain}, - {"rule-refused", &ruleRefused}, - {"rule-servfail", &ruleServFail}, - {"rule-truncated", &ruleTruncated}, - {"self-answered", &selfAnswered}, - {"downstream-timeouts", &downstreamTimeouts}, - {"downstream-send-errors", &downstreamSendErrors}, - {"trunc-failures", &truncFail}, - {"no-policy", &noPolicy}, - {"latency0-1", &latency0_1}, - {"latency1-10", &latency1_10}, - {"latency10-50", &latency10_50}, - {"latency50-100", &latency50_100}, - {"latency100-1000", &latency100_1000}, - {"latency-slow", &latencySlow}, - {"latency-avg100", &latencyAvg100}, - {"latency-avg1000", &latencyAvg1000}, - {"latency-avg10000", &latencyAvg10000}, - {"latency-avg1000000", &latencyAvg1000000}, - {"latency-tcp-avg100", &latencyTCPAvg100}, - {"latency-tcp-avg1000", &latencyTCPAvg1000}, - {"latency-tcp-avg10000", &latencyTCPAvg10000}, - {"latency-tcp-avg1000000", &latencyTCPAvg1000000}, - {"latency-dot-avg100", &latencyDoTAvg100}, - {"latency-dot-avg1000", &latencyDoTAvg1000}, - {"latency-dot-avg10000", &latencyDoTAvg10000}, - {"latency-dot-avg1000000", &latencyDoTAvg1000000}, - {"latency-doh-avg100", &latencyDoHAvg100}, - {"latency-doh-avg1000", &latencyDoHAvg1000}, - {"latency-doh-avg10000", &latencyDoHAvg10000}, - {"latency-doh-avg1000000", &latencyDoHAvg1000000}, - {"latency-doq-avg100", &latencyDoQAvg100}, - {"latency-doq-avg1000", &latencyDoQAvg1000}, - {"latency-doq-avg10000", &latencyDoQAvg10000}, - {"latency-doq-avg1000000", &latencyDoQAvg1000000}, - {"latency-doh3-avg100", &latencyDoH3Avg100}, - {"latency-doh3-avg1000", &latencyDoH3Avg1000}, - {"latency-doh3-avg10000", &latencyDoH3Avg10000}, - {"latency-doh3-avg1000000", &latencyDoH3Avg1000000}, - {"uptime", uptimeOfProcess}, - {"real-memory-usage", getRealMemoryUsage}, - {"special-memory-usage", getSpecialMemoryUsage}, - {"udp-in-errors", [](const std::string&) { return udpErrorStats("udp-in-errors"); }}, - {"udp-noport-errors", [](const std::string&) { return udpErrorStats("udp-noport-errors"); }}, - {"udp-recvbuf-errors", [](const std::string&) { return udpErrorStats("udp-recvbuf-errors"); }}, - {"udp-sndbuf-errors", [](const std::string&) { return udpErrorStats("udp-sndbuf-errors"); }}, - {"udp-in-csum-errors", [](const std::string&) { return udpErrorStats("udp-in-csum-errors"); }}, - {"udp6-in-errors", [](const std::string&) { return udp6ErrorStats("udp6-in-errors"); }}, - {"udp6-recvbuf-errors", [](const std::string&) { return udp6ErrorStats("udp6-recvbuf-errors"); }}, - {"udp6-sndbuf-errors", [](const std::string&) { return udp6ErrorStats("udp6-sndbuf-errors"); }}, - {"udp6-noport-errors", [](const std::string&) { return udp6ErrorStats("udp6-noport-errors"); }}, - {"udp6-in-csum-errors", [](const std::string&) { return udp6ErrorStats("udp6-in-csum-errors"); }}, - {"tcp-listen-overflows", [](const std::string&) { return tcpErrorStats("ListenOverflows"); }}, - {"noncompliant-queries", &nonCompliantQueries}, - {"noncompliant-responses", &nonCompliantResponses}, - {"proxy-protocol-invalid", &proxyProtocolInvalid}, - {"rdqueries", &rdQueries}, - {"empty-queries", &emptyQueries}, - {"cache-hits", &cacheHits}, - {"cache-misses", &cacheMisses}, - {"cpu-iowait", getCPUIOWait}, - {"cpu-steal", getCPUSteal}, - {"cpu-sys-msec", getCPUTimeSystem}, - {"cpu-user-msec", getCPUTimeUser}, - {"fd-usage", getOpenFileDescriptors}, - {"dyn-blocked", &dynBlocked}, + entries{std::vector{ + {"responses", "", &responses}, + {"servfail-responses", "", &servfailResponses}, + {"queries", "", &queries}, + {"frontend-nxdomain", "", &frontendNXDomain}, + {"frontend-servfail", "", &frontendServFail}, + {"frontend-noerror", "", &frontendNoError}, + {"acl-drops", "", &aclDrops}, + {"rule-drop", "", &ruleDrop}, + {"rule-nxdomain", "", &ruleNXDomain}, + {"rule-refused", "", &ruleRefused}, + {"rule-servfail", "", &ruleServFail}, + {"rule-truncated", "", &ruleTruncated}, + {"self-answered", "", &selfAnswered}, + {"downstream-timeouts", "", &downstreamTimeouts}, + {"downstream-send-errors", "", &downstreamSendErrors}, + {"trunc-failures", "", &truncFail}, + {"no-policy", "", &noPolicy}, + {"latency0-1", "", &latency0_1}, + {"latency1-10", "", &latency1_10}, + {"latency10-50", "", &latency10_50}, + {"latency50-100", "", &latency50_100}, + {"latency100-1000", "", &latency100_1000}, + {"latency-slow", "", &latencySlow}, + {"latency-avg100", "", &latencyAvg100}, + {"latency-avg1000", "", &latencyAvg1000}, + {"latency-avg10000", "", &latencyAvg10000}, + {"latency-avg1000000", "", &latencyAvg1000000}, + {"latency-tcp-avg100", "", &latencyTCPAvg100}, + {"latency-tcp-avg1000", "", &latencyTCPAvg1000}, + {"latency-tcp-avg10000", "", &latencyTCPAvg10000}, + {"latency-tcp-avg1000000", "", &latencyTCPAvg1000000}, + {"latency-dot-avg100", "", &latencyDoTAvg100}, + {"latency-dot-avg1000", "", &latencyDoTAvg1000}, + {"latency-dot-avg10000", "", &latencyDoTAvg10000}, + {"latency-dot-avg1000000", "", &latencyDoTAvg1000000}, + {"latency-doh-avg100", "", &latencyDoHAvg100}, + {"latency-doh-avg1000", "", &latencyDoHAvg1000}, + {"latency-doh-avg10000", "", &latencyDoHAvg10000}, + {"latency-doh-avg1000000", "", &latencyDoHAvg1000000}, + {"latency-doq-avg100", "", &latencyDoQAvg100}, + {"latency-doq-avg1000", "", &latencyDoQAvg1000}, + {"latency-doq-avg10000", "", &latencyDoQAvg10000}, + {"latency-doq-avg1000000", "", &latencyDoQAvg1000000}, + {"latency-doh3-avg100", "", &latencyDoH3Avg100}, + {"latency-doh3-avg1000", "", &latencyDoH3Avg1000}, + {"latency-doh3-avg10000", "", &latencyDoH3Avg10000}, + {"latency-doh3-avg1000000", "", &latencyDoH3Avg1000000}, + {"uptime", "", uptimeOfProcess}, + {"real-memory-usage", "", getRealMemoryUsage}, + {"special-memory-usage", "", getSpecialMemoryUsage}, + {"udp-in-errors", "", [](const std::string&) { return udpErrorStats("udp-in-errors"); }}, + {"udp-noport-errors", "", [](const std::string&) { return udpErrorStats("udp-noport-errors"); }}, + {"udp-recvbuf-errors", "", [](const std::string&) { return udpErrorStats("udp-recvbuf-errors"); }}, + {"udp-sndbuf-errors", "", [](const std::string&) { return udpErrorStats("udp-sndbuf-errors"); }}, + {"udp-in-csum-errors", "", [](const std::string&) { return udpErrorStats("udp-in-csum-errors"); }}, + {"udp6-in-errors", "", [](const std::string&) { return udp6ErrorStats("udp6-in-errors"); }}, + {"udp6-recvbuf-errors", "", [](const std::string&) { return udp6ErrorStats("udp6-recvbuf-errors"); }}, + {"udp6-sndbuf-errors", "", [](const std::string&) { return udp6ErrorStats("udp6-sndbuf-errors"); }}, + {"udp6-noport-errors", "", [](const std::string&) { return udp6ErrorStats("udp6-noport-errors"); }}, + {"udp6-in-csum-errors", "", [](const std::string&) { return udp6ErrorStats("udp6-in-csum-errors"); }}, + {"tcp-listen-overflows", "", [](const std::string&) { return tcpErrorStats("ListenOverflows"); }}, + {"noncompliant-queries", "", &nonCompliantQueries}, + {"noncompliant-responses", "", &nonCompliantResponses}, + {"proxy-protocol-invalid", "", &proxyProtocolInvalid}, + {"rdqueries", "", &rdQueries}, + {"empty-queries", "", &emptyQueries}, + {"cache-hits", "", &cacheHits}, + {"cache-misses", "", &cacheMisses}, + {"cpu-iowait", "", getCPUIOWait}, + {"cpu-steal", "", getCPUSteal}, + {"cpu-sys-msec", "", getCPUTimeSystem}, + {"cpu-user-msec", "", getCPUTimeUser}, + {"fd-usage", "", getOpenFileDescriptors}, + {"dyn-blocked", "", &dynBlocked}, #ifndef DISABLE_DYNBLOCKS - {"dyn-block-nmg-size", [](const std::string&) { return dnsdist::DynamicBlocks::getClientAddressDynamicRules().size(); }}, + {"dyn-block-nmg-size", "", [](const std::string&) { return dnsdist::DynamicBlocks::getClientAddressDynamicRules().size(); }}, #endif /* DISABLE_DYNBLOCKS */ - {"security-status", &securityStatus}, - {"doh-query-pipe-full", &dohQueryPipeFull}, - {"doh-response-pipe-full", &dohResponsePipeFull}, - {"doq-response-pipe-full", &doqResponsePipeFull}, - {"doh3-response-pipe-full", &doh3ResponsePipeFull}, - {"outgoing-doh-query-pipe-full", &outgoingDoHQueryPipeFull}, - {"tcp-query-pipe-full", &tcpQueryPipeFull}, - {"tcp-cross-protocol-query-pipe-full", &tcpCrossProtocolQueryPipeFull}, - {"tcp-cross-protocol-response-pipe-full", &tcpCrossProtocolResponsePipeFull}, + {"security-status", "", &securityStatus}, + {"doh-query-pipe-full", "", &dohQueryPipeFull}, + {"doh-response-pipe-full", "", &dohResponsePipeFull}, + {"doq-response-pipe-full", "", &doqResponsePipeFull}, + {"doh3-response-pipe-full", "", &doh3ResponsePipeFull}, + {"outgoing-doh-query-pipe-full", "", &outgoingDoHQueryPipeFull}, + {"tcp-query-pipe-full", "", &tcpQueryPipeFull}, + {"tcp-cross-protocol-query-pipe-full", "", &tcpCrossProtocolQueryPipeFull}, + {"tcp-cross-protocol-response-pipe-full", "", &tcpCrossProtocolResponsePipeFull}, // Latency histogram - {"latency-sum", &latencySum}, - {"latency-count", &latencyCount}, + {"latency-sum", "", &latencySum}, + {"latency-count", "", &latencyCount}, }} { } @@ -176,18 +178,16 @@ std::optional declareCustomMetric(const std::string& name, const st const std::string finalCustomName(customName ? *customName : ""); if (type == "counter") { auto customCounters = s_customCounters.write_lock(); - auto itp = customCounters->insert({name, MutableCounter()}); + auto itp = customCounters->emplace(name, std::map()); if (itp.second) { - g_stats.entries.write_lock()->emplace_back(Stats::EntryPair{name, &(*customCounters)[name].d_value}); dnsdist::prometheus::PrometheusMetricDefinition def{name, type, description, finalCustomName}; dnsdist::webserver::addMetricDefinition(def); } } else if (type == "gauge") { auto customGauges = s_customGauges.write_lock(); - auto itp = customGauges->insert({name, MutableGauge()}); + auto itp = customGauges->emplace(name, std::map()); if (itp.second) { - g_stats.entries.write_lock()->emplace_back(Stats::EntryPair{name, &(*customGauges)[name].d_value}); dnsdist::prometheus::PrometheusMetricDefinition def{name, type, description, finalCustomName}; dnsdist::webserver::addMetricDefinition(def); } @@ -198,57 +198,108 @@ std::optional declareCustomMetric(const std::string& name, const st return std::nullopt; } -std::variant incrementCustomCounter(const std::string_view& name, uint64_t step) +static string prometheusLabelValueEscape(const string& value) { - auto customCounters = s_customCounters.read_lock(); + string ret; + + for (char i : value) { + if (i == '"' || i == '\\') { + ret += '\\'; + ret += i; + } + else if (i == '\n') { + ret += '\\'; + ret += 'n'; + } + else + ret += i; + } + return ret; +} + +static std::string generateCombinationOfLabels(const std::unordered_map& labels) +{ + return std::accumulate(labels.begin(), labels.end(), std::string(), [](const std::string& acc, const std::pair& l) { + return acc + (acc.empty() ? std::string() : ",") + l.first + "=" + "\"" + prometheusLabelValueEscape(l.second) + "\""; + }); +} + +std::variant incrementCustomCounter(const std::string_view& name, uint64_t step, const std::unordered_map& labels) +{ + auto customCounters = s_customCounters.write_lock(); auto metric = customCounters->find(name); if (metric != customCounters->end()) { - metric->second.d_value += step; - return metric->second.d_value.load(); + auto combinationOfLabels = generateCombinationOfLabels(labels); + auto metricEntry = metric->second.find(combinationOfLabels); + if (metricEntry == metric->second.end()) { + metricEntry = metric->second.emplace(combinationOfLabels, MutableCounter()).first; + g_stats.entries.write_lock()->emplace_back(Stats::EntryTriple{std::string(name), combinationOfLabels, &metricEntry->second.d_value}); + } + metricEntry->second.d_value += step; + return metricEntry->second.d_value.load(); } return std::string("Unable to increment custom metric '") + std::string(name) + "': no such counter"; } -std::variant decrementCustomCounter(const std::string_view& name, uint64_t step) +std::variant decrementCustomCounter(const std::string_view& name, uint64_t step, const std::unordered_map& labels) { - auto customCounters = s_customCounters.read_lock(); + auto customCounters = s_customCounters.write_lock(); auto metric = customCounters->find(name); if (metric != customCounters->end()) { - metric->second.d_value -= step; - return metric->second.d_value.load(); + auto combinationOfLabels = generateCombinationOfLabels(labels); + auto metricEntry = metric->second.find(combinationOfLabels); + if (metricEntry == metric->second.end()) { + metricEntry = metric->second.emplace(combinationOfLabels, MutableCounter()).first; + g_stats.entries.write_lock()->emplace_back(Stats::EntryTriple{std::string(name), combinationOfLabels, &metricEntry->second.d_value}); + } + metricEntry->second.d_value -= step; + return metricEntry->second.d_value.load(); } return std::string("Unable to decrement custom metric '") + std::string(name) + "': no such counter"; } -std::variant setCustomGauge(const std::string_view& name, const double value) +std::variant setCustomGauge(const std::string_view& name, const double value, const std::unordered_map& labels) { - auto customGauges = s_customGauges.read_lock(); + auto customGauges = s_customGauges.write_lock(); auto metric = customGauges->find(name); if (metric != customGauges->end()) { - metric->second.d_value = value; + auto combinationOfLabels = generateCombinationOfLabels(labels); + auto metricEntry = metric->second.find(combinationOfLabels); + if (metricEntry == metric->second.end()) { + metricEntry = metric->second.emplace(combinationOfLabels, MutableGauge()).first; + g_stats.entries.write_lock()->emplace_back(Stats::EntryTriple{std::string(name), combinationOfLabels, &metricEntry->second.d_value}); + } + metricEntry->second.d_value = value; return value; } return std::string("Unable to set metric '") + std::string(name) + "': no such gauge"; } -std::variant getCustomMetric(const std::string_view& name) +std::variant getCustomMetric(const std::string_view& name, const std::unordered_map& labels) { { auto customCounters = s_customCounters.read_lock(); auto counter = customCounters->find(name); if (counter != customCounters->end()) { - return static_cast(counter->second.d_value.load()); + auto combinationOfLabels = generateCombinationOfLabels(labels); + auto metricEntry = counter->second.find(combinationOfLabels); + if (metricEntry != counter->second.end()) { + return static_cast(metricEntry->second.d_value.load()); + } } } { auto customGauges = s_customGauges.read_lock(); auto gauge = customGauges->find(name); if (gauge != customGauges->end()) { - return gauge->second.d_value.load(); + auto combinationOfLabels = generateCombinationOfLabels(labels); + auto metricEntry = gauge->second.find(combinationOfLabels); + if (metricEntry != gauge->second.end()) { + return metricEntry->second.d_value.load(); + } } } return std::string("Unable to get metric '") + std::string(name) + "': no such metric"; } - } diff --git a/pdns/dnsdistdist/dnsdist-metrics.hh b/pdns/dnsdistdist/dnsdist-metrics.hh index 47a3fb84078b..8afb67509d20 100644 --- a/pdns/dnsdistdist/dnsdist-metrics.hh +++ b/pdns/dnsdistdist/dnsdist-metrics.hh @@ -25,6 +25,7 @@ #include #include #include +#include #include #include "lock.hh" @@ -35,10 +36,10 @@ namespace dnsdist::metrics using Error = std::string; [[nodiscard]] std::optional declareCustomMetric(const std::string& name, const std::string& type, const std::string& description, std::optional customName); -[[nodiscard]] std::variant incrementCustomCounter(const std::string_view& name, uint64_t step); -[[nodiscard]] std::variant decrementCustomCounter(const std::string_view& name, uint64_t step); -[[nodiscard]] std::variant setCustomGauge(const std::string_view& name, const double value); -[[nodiscard]] std::variant getCustomMetric(const std::string_view& name); +[[nodiscard]] std::variant incrementCustomCounter(const std::string_view& name, uint64_t step, const std::unordered_map& labels); +[[nodiscard]] std::variant decrementCustomCounter(const std::string_view& name, uint64_t step, const std::unordered_map& labels); +[[nodiscard]] std::variant setCustomGauge(const std::string_view& name, const double value, const std::unordered_map& labels); +[[nodiscard]] std::variant getCustomMetric(const std::string_view& name, const std::unordered_map& labels); using pdns::stat_t; @@ -89,13 +90,14 @@ struct Stats pdns::stat_double_t latencyDoH3Avg100{0}, latencyDoH3Avg1000{0}, latencyDoH3Avg10000{0}, latencyDoH3Avg1000000{0}; using statfunction_t = std::function; using entry_t = std::variant; - struct EntryPair + struct EntryTriple { std::string d_name; + std::string d_labels; entry_t d_value; }; - SharedLockGuarded> entries; + SharedLockGuarded> entries; }; extern struct Stats g_stats; diff --git a/pdns/dnsdistdist/dnsdist-web.cc b/pdns/dnsdistdist/dnsdist-web.cc index 5cdfead09cd0..1f38516e8d63 100644 --- a/pdns/dnsdistdist/dnsdist-web.cc +++ b/pdns/dnsdistdist/dnsdist-web.cc @@ -500,6 +500,10 @@ static void handlePrometheus(const YaHTTP::Request& req, YaHTTP::Response& resp) prometheusMetricName = metricDetails.customName; } + if (!entry.d_labels.empty()) { + prometheusMetricName += "{" + entry.d_labels + "}"; + } + // for these we have the help and types encoded in the sources // but we need to be careful about labels in custom metrics std::string helpName = prometheusMetricName.substr(0, prometheusMetricName.find('{')); From 0dff6be4145aadd48cfd9f20f6ac48ac8b3d08df Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ensar=20Saraj=C4=8Di=C4=87?= Date: Thu, 12 Dec 2024 16:01:14 +0100 Subject: [PATCH 02/16] dnsdist: add options to `declareMetric` to skip initialization if labels are used --- pdns/dnsdistdist/dnsdist-lua-ffi.cc | 3 ++- pdns/dnsdistdist/dnsdist-lua.cc | 21 ++++++++++++++++++--- pdns/dnsdistdist/dnsdist-metrics.cc | 10 +++++++++- pdns/dnsdistdist/dnsdist-metrics.hh | 2 +- 4 files changed, 30 insertions(+), 6 deletions(-) diff --git a/pdns/dnsdistdist/dnsdist-lua-ffi.cc b/pdns/dnsdistdist/dnsdist-lua-ffi.cc index c7a0c055f682..d6991323b090 100644 --- a/pdns/dnsdistdist/dnsdist-lua-ffi.cc +++ b/pdns/dnsdistdist/dnsdist-lua-ffi.cc @@ -1805,7 +1805,8 @@ bool dnsdist_ffi_metric_declare(const char* name, size_t nameLen, const char* ty if (name == nullptr || nameLen == 0 || type == nullptr || description == nullptr) { return false; } - auto result = dnsdist::metrics::declareCustomMetric(name, type, description, customName != nullptr ? std::optional(customName) : std::nullopt); + // TODO: add labels options? + auto result = dnsdist::metrics::declareCustomMetric(name, type, description, customName != nullptr ? std::optional(customName) : std::nullopt, false); return !result; } diff --git a/pdns/dnsdistdist/dnsdist-lua.cc b/pdns/dnsdistdist/dnsdist-lua.cc index 53e1038c135c..cf366cb69987 100644 --- a/pdns/dnsdistdist/dnsdist-lua.cc +++ b/pdns/dnsdistdist/dnsdist-lua.cc @@ -85,7 +85,8 @@ using std::thread; -using update_metric_opts_t = LuaAssociativeTable>>; +using update_metric_opts_t = LuaAssociativeTable>>; +using declare_metric_opts_t = LuaAssociativeTable>; static boost::tribool s_noLuaSideEffect; @@ -3412,8 +3413,22 @@ static void setupLuaConfig(LuaContext& luaCtx, bool client, bool configCheck) newThread.detach(); }); - luaCtx.writeFunction("declareMetric", [](const std::string& name, const std::string& type, const std::string& description, boost::optional customName) { - auto result = dnsdist::metrics::declareCustomMetric(name, type, description, customName ? std::optional(*customName) : std::nullopt); + luaCtx.writeFunction("declareMetric", [](const std::string& name, const std::string& type, const std::string& description, boost::optional>> opts) { + bool withLabels = false; + std::optional customName = std::nullopt; + if (opts) { + auto* optCustomName = boost::get(&opts.get()); + if (optCustomName) { + customName = std::optional(*optCustomName); + } + if (!customName) { + boost::optional vars = boost::get>(opts.get()); + getOptionalValue(vars, "customName", customName); + getOptionalValue(vars, "withLabels", withLabels); + checkAllParametersConsumed("declareMetric", vars); + } + } + auto result = dnsdist::metrics::declareCustomMetric(name, type, description, customName, withLabels); if (result) { g_outputBuffer += *result + "\n"; errlog("Error in declareMetric: %s", *result); diff --git a/pdns/dnsdistdist/dnsdist-metrics.cc b/pdns/dnsdistdist/dnsdist-metrics.cc index 039b1ffc8924..18ac63aab226 100644 --- a/pdns/dnsdistdist/dnsdist-metrics.cc +++ b/pdns/dnsdistdist/dnsdist-metrics.cc @@ -169,7 +169,7 @@ Stats::Stats() : struct Stats g_stats; -std::optional declareCustomMetric(const std::string& name, const std::string& type, const std::string& description, std::optional customName) +std::optional declareCustomMetric(const std::string& name, const std::string& type, const std::string& description, std::optional customName, bool withLabels) { if (!std::regex_match(name, std::regex("^[a-z0-9-]+$"))) { return std::string("Unable to declare metric '") + std::string(name) + std::string("': invalid name\n"); @@ -180,6 +180,10 @@ std::optional declareCustomMetric(const std::string& name, const st auto customCounters = s_customCounters.write_lock(); auto itp = customCounters->emplace(name, std::map()); if (itp.second) { + if (!withLabels) { + auto counter = itp.first->second.emplace("", MutableCounter()); + g_stats.entries.write_lock()->emplace_back(Stats::EntryTriple{name, "", &counter.first->second.d_value}); + } dnsdist::prometheus::PrometheusMetricDefinition def{name, type, description, finalCustomName}; dnsdist::webserver::addMetricDefinition(def); } @@ -188,6 +192,10 @@ std::optional declareCustomMetric(const std::string& name, const st auto customGauges = s_customGauges.write_lock(); auto itp = customGauges->emplace(name, std::map()); if (itp.second) { + if (!withLabels) { + auto gauge = itp.first->second.emplace("", MutableGauge()); + g_stats.entries.write_lock()->emplace_back(Stats::EntryTriple{name, "", &gauge.first->second.d_value}); + } dnsdist::prometheus::PrometheusMetricDefinition def{name, type, description, finalCustomName}; dnsdist::webserver::addMetricDefinition(def); } diff --git a/pdns/dnsdistdist/dnsdist-metrics.hh b/pdns/dnsdistdist/dnsdist-metrics.hh index 8afb67509d20..6cab90527284 100644 --- a/pdns/dnsdistdist/dnsdist-metrics.hh +++ b/pdns/dnsdistdist/dnsdist-metrics.hh @@ -35,7 +35,7 @@ namespace dnsdist::metrics { using Error = std::string; -[[nodiscard]] std::optional declareCustomMetric(const std::string& name, const std::string& type, const std::string& description, std::optional customName); +[[nodiscard]] std::optional declareCustomMetric(const std::string& name, const std::string& type, const std::string& description, std::optional customName, bool withLabels); [[nodiscard]] std::variant incrementCustomCounter(const std::string_view& name, uint64_t step, const std::unordered_map& labels); [[nodiscard]] std::variant decrementCustomCounter(const std::string_view& name, uint64_t step, const std::unordered_map& labels); [[nodiscard]] std::variant setCustomGauge(const std::string_view& name, const double value, const std::unordered_map& labels); From 138a0fb776a3de2a9013c70cea8f459dd9889fe9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ensar=20Saraj=C4=8Di=C4=87?= Date: Thu, 12 Dec 2024 16:17:01 +0100 Subject: [PATCH 03/16] chore: reformat metrics related files --- pdns/dnsdistdist/dnsdist-lua-ffi.cc | 17 ++-- pdns/dnsdistdist/dnsdist-lua-ffi.hh | 120 ++++++++++++++++------------ pdns/dnsdistdist/dnsdist-lua.cc | 71 ++++++++-------- pdns/dnsdistdist/dnsdist-metrics.cc | 3 +- pdns/dnsdistdist/dnsdist-web.cc | 2 +- 5 files changed, 115 insertions(+), 98 deletions(-) diff --git a/pdns/dnsdistdist/dnsdist-lua-ffi.cc b/pdns/dnsdistdist/dnsdist-lua-ffi.cc index d6991323b090..d3f9a3982998 100644 --- a/pdns/dnsdistdist/dnsdist-lua-ffi.cc +++ b/pdns/dnsdistdist/dnsdist-lua-ffi.cc @@ -260,7 +260,7 @@ void dnsdist_ffi_dnsquestion_get_sni(const dnsdist_ffi_dnsquestion_t* dq, const const char* dnsdist_ffi_dnsquestion_get_tag(const dnsdist_ffi_dnsquestion_t* dq, const char* label) { - const char * result = nullptr; + const char* result = nullptr; if (dq != nullptr && dq->dq != nullptr && dq->dq->ids.qTag != nullptr) { const auto it = dq->dq->ids.qTag->find(label); @@ -456,7 +456,6 @@ size_t dnsdist_ffi_dnsquestion_get_tag_array(dnsdist_ffi_dnsquestion_t* dq, cons ++pos; } - if (!dq->tagsVect->empty()) { *out = dq->tagsVect->data(); } @@ -1007,7 +1006,7 @@ static constexpr char s_lua_ffi_code[] = R"FFICodeContent( ffi.cdef[[ )FFICodeContent" #include "dnsdist-lua-ffi-interface.inc" -R"FFICodeContent( + R"FFICodeContent( ]] )FFICodeContent"; @@ -1057,7 +1056,7 @@ size_t dnsdist_ffi_generate_proxy_protocol_payload(const size_t addrSize, const if (valuesCount > 0) { valuesVect.reserve(valuesCount); for (size_t idx = 0; idx < valuesCount; idx++) { - valuesVect.push_back({ std::string(values[idx].value, values[idx].size), values[idx].type }); + valuesVect.push_back({std::string(values[idx].value, values[idx].size), values[idx].type}); } } @@ -1086,7 +1085,7 @@ size_t dnsdist_ffi_dnsquestion_generate_proxy_protocol_payload(const dnsdist_ffi if (valuesCount > 0) { valuesVect.reserve(valuesCount); for (size_t idx = 0; idx < valuesCount; idx++) { - valuesVect.push_back({ std::string(values[idx].value, values[idx].size), values[idx].type }); + valuesVect.push_back({std::string(values[idx].value, values[idx].size), values[idx].type}); } } @@ -1113,7 +1112,7 @@ bool dnsdist_ffi_dnsquestion_add_proxy_protocol_values(dnsdist_ffi_dnsquestion_t dnsQuestion->dq->proxyProtocolValues->reserve(dnsQuestion->dq->proxyProtocolValues->size() + valuesCount); for (size_t idx = 0; idx < valuesCount; idx++) { // NOLINTNEXTLINE(cppcoreguidelines-pro-bounds-pointer-arithmetic): the Lua FFI API is a C API.. - dnsQuestion->dq->proxyProtocolValues->push_back({ std::string(values[idx].value, values[idx].size), values[idx].type }); + dnsQuestion->dq->proxyProtocolValues->push_back({std::string(values[idx].value, values[idx].size), values[idx].type}); } return true; @@ -1146,7 +1145,8 @@ struct dnsdist_ffi_domain_list_t { std::vector d_domains; }; -struct dnsdist_ffi_address_list_t { +struct dnsdist_ffi_address_list_t +{ std::vector d_addresses; }; @@ -1476,7 +1476,8 @@ void dnsdist_ffi_ring_entry_list_free(dnsdist_ffi_ring_entry_list_t* list) delete list; } -template static void addRingEntryToList(std::unique_ptr& list, const struct timespec& now, const T& entry) +template +static void addRingEntryToList(std::unique_ptr& list, const struct timespec& now, const T& entry) { auto age = DiffTime(entry.when, now); diff --git a/pdns/dnsdistdist/dnsdist-lua-ffi.hh b/pdns/dnsdistdist/dnsdist-lua-ffi.hh index 1369c2a07cd1..644114d69971 100644 --- a/pdns/dnsdistdist/dnsdist-lua-ffi.hh +++ b/pdns/dnsdistdist/dnsdist-lua-ffi.hh @@ -23,27 +23,31 @@ #include "dnsdist.hh" -extern "C" { +extern "C" +{ #include "dnsdist-lua-ffi-interface.h" } #include "ext/luawrapper/include/LuaContext.hpp" // dnsdist_ffi_dnsquestion_t is a lightuserdata -template<> -struct LuaContext::Pusher { - static const int minSize = 1; - static const int maxSize = 1; - - static PushedObject push(lua_State* state, dnsdist_ffi_dnsquestion_t* ptr) noexcept { - lua_pushlightuserdata(state, ptr); - return PushedObject{state, 1}; - } +template <> +struct LuaContext::Pusher +{ + static const int minSize = 1; + static const int maxSize = 1; + + static PushedObject push(lua_State* state, dnsdist_ffi_dnsquestion_t* ptr) noexcept + { + lua_pushlightuserdata(state, ptr); + return PushedObject{state, 1}; + } }; struct dnsdist_ffi_dnsquestion_t { - dnsdist_ffi_dnsquestion_t(DNSQuestion* dq_): dq(dq_) + dnsdist_ffi_dnsquestion_t(DNSQuestion* dq_) : + dq(dq_) { } @@ -63,20 +67,23 @@ struct dnsdist_ffi_dnsquestion_t }; // dnsdist_ffi_dnsresponse_t is a lightuserdata -template<> -struct LuaContext::Pusher { - static const int minSize = 1; - static const int maxSize = 1; - - static PushedObject push(lua_State* state, dnsdist_ffi_dnsresponse_t* ptr) noexcept { - lua_pushlightuserdata(state, ptr); - return PushedObject{state, 1}; - } +template <> +struct LuaContext::Pusher +{ + static const int minSize = 1; + static const int maxSize = 1; + + static PushedObject push(lua_State* state, dnsdist_ffi_dnsresponse_t* ptr) noexcept + { + lua_pushlightuserdata(state, ptr); + return PushedObject{state, 1}; + } }; struct dnsdist_ffi_dnsresponse_t { - dnsdist_ffi_dnsresponse_t(DNSResponse* dr_): dr(dr_) + dnsdist_ffi_dnsresponse_t(DNSResponse* dr_) : + dr(dr_) { } @@ -85,20 +92,23 @@ struct dnsdist_ffi_dnsresponse_t }; // dnsdist_ffi_server_t is a lightuserdata -template<> -struct LuaContext::Pusher { - static const int minSize = 1; - static const int maxSize = 1; - - static PushedObject push(lua_State* state, dnsdist_ffi_server_t* ptr) noexcept { - lua_pushlightuserdata(state, ptr); - return PushedObject{state, 1}; - } +template <> +struct LuaContext::Pusher +{ + static const int minSize = 1; + static const int maxSize = 1; + + static PushedObject push(lua_State* state, dnsdist_ffi_server_t* ptr) noexcept + { + lua_pushlightuserdata(state, ptr); + return PushedObject{state, 1}; + } }; struct dnsdist_ffi_server_t { - dnsdist_ffi_server_t(const std::shared_ptr& server_): server(server_) + dnsdist_ffi_server_t(const std::shared_ptr& server_) : + server(server_) { } @@ -106,23 +116,26 @@ struct dnsdist_ffi_server_t }; // dnsdist_ffi_servers_list_t is a lightuserdata -template<> -struct LuaContext::Pusher { - static const int minSize = 1; - static const int maxSize = 1; - - static PushedObject push(lua_State* state, dnsdist_ffi_servers_list_t* ptr) noexcept { - lua_pushlightuserdata(state, ptr); - return PushedObject{state, 1}; - } +template <> +struct LuaContext::Pusher +{ + static const int minSize = 1; + static const int maxSize = 1; + + static PushedObject push(lua_State* state, dnsdist_ffi_servers_list_t* ptr) noexcept + { + lua_pushlightuserdata(state, ptr); + return PushedObject{state, 1}; + } }; struct dnsdist_ffi_servers_list_t { - dnsdist_ffi_servers_list_t(const ServerPolicy::NumberedServerVector& servers_): servers(servers_) + dnsdist_ffi_servers_list_t(const ServerPolicy::NumberedServerVector& servers_) : + servers(servers_) { ffiServers.reserve(servers.size()); - for (const auto& server: servers) { + for (const auto& server : servers) { ffiServers.push_back(dnsdist_ffi_server_t(server.second)); } } @@ -132,20 +145,23 @@ struct dnsdist_ffi_servers_list_t }; // dnsdist_ffi_network_message_t is a lightuserdata -template<> -struct LuaContext::Pusher { - static const int minSize = 1; - static const int maxSize = 1; - - static PushedObject push(lua_State* state, dnsdist_ffi_network_message_t* ptr) noexcept { - lua_pushlightuserdata(state, ptr); - return PushedObject{state, 1}; - } +template <> +struct LuaContext::Pusher +{ + static const int minSize = 1; + static const int maxSize = 1; + + static PushedObject push(lua_State* state, dnsdist_ffi_network_message_t* ptr) noexcept + { + lua_pushlightuserdata(state, ptr); + return PushedObject{state, 1}; + } }; struct dnsdist_ffi_network_message_t { - dnsdist_ffi_network_message_t(const std::string& payload_ ,const std::string& from_, uint16_t endpointID_): payload(payload_), from(from_), endpointID(endpointID_) + dnsdist_ffi_network_message_t(const std::string& payload_, const std::string& from_, uint16_t endpointID_) : + payload(payload_), from(from_), endpointID(endpointID_) { } diff --git a/pdns/dnsdistdist/dnsdist-lua.cc b/pdns/dnsdistdist/dnsdist-lua.cc index cf366cb69987..7da861d9db36 100644 --- a/pdns/dnsdistdist/dnsdist-lua.cc +++ b/pdns/dnsdistdist/dnsdist-lua.cc @@ -886,29 +886,28 @@ static void setupLuaConfig(LuaContext& luaCtx, bool client, bool configCheck) const std::function mutator; const size_t maximumValue{std::numeric_limits::max()}; }; - static const std::vector unsignedIntegerImmutableConfigItems - { + static const std::vector unsignedIntegerImmutableConfigItems{ {"setMaxTCPQueuedConnections", [](dnsdist::configuration::ImmutableConfiguration& config, uint64_t newValue) { config.d_maxTCPQueuedConnections = newValue; }, std::numeric_limits::max()}, - {"setMaxTCPClientThreads", [](dnsdist::configuration::ImmutableConfiguration& config, uint64_t newValue) { config.d_maxTCPClientThreads = newValue; }, std::numeric_limits::max()}, - {"setMaxTCPConnectionsPerClient", [](dnsdist::configuration::ImmutableConfiguration& config, uint64_t newValue) { config.d_maxTCPConnectionsPerClient = newValue; }, std::numeric_limits::max()}, - {"setTCPInternalPipeBufferSize", [](dnsdist::configuration::ImmutableConfiguration& config, uint64_t newValue) { config.d_tcpInternalPipeBufferSize = newValue; }, std::numeric_limits::max()}, - {"setMaxCachedTCPConnectionsPerDownstream", [](dnsdist::configuration::ImmutableConfiguration& config, uint64_t newValue) { config.d_outgoingTCPMaxIdlePerBackend = newValue; }, std::numeric_limits::max()}, - {"setTCPDownstreamCleanupInterval", [](dnsdist::configuration::ImmutableConfiguration& config, uint64_t newValue) { config.d_outgoingTCPCleanupInterval = newValue; }, std::numeric_limits::max()}, - {"setTCPDownstreamMaxIdleTime", [](dnsdist::configuration::ImmutableConfiguration& config, uint64_t newValue) { config.d_outgoingTCPMaxIdleTime = newValue; }, std::numeric_limits::max()}, + {"setMaxTCPClientThreads", [](dnsdist::configuration::ImmutableConfiguration& config, uint64_t newValue) { config.d_maxTCPClientThreads = newValue; }, std::numeric_limits::max()}, + {"setMaxTCPConnectionsPerClient", [](dnsdist::configuration::ImmutableConfiguration& config, uint64_t newValue) { config.d_maxTCPConnectionsPerClient = newValue; }, std::numeric_limits::max()}, + {"setTCPInternalPipeBufferSize", [](dnsdist::configuration::ImmutableConfiguration& config, uint64_t newValue) { config.d_tcpInternalPipeBufferSize = newValue; }, std::numeric_limits::max()}, + {"setMaxCachedTCPConnectionsPerDownstream", [](dnsdist::configuration::ImmutableConfiguration& config, uint64_t newValue) { config.d_outgoingTCPMaxIdlePerBackend = newValue; }, std::numeric_limits::max()}, + {"setTCPDownstreamCleanupInterval", [](dnsdist::configuration::ImmutableConfiguration& config, uint64_t newValue) { config.d_outgoingTCPCleanupInterval = newValue; }, std::numeric_limits::max()}, + {"setTCPDownstreamMaxIdleTime", [](dnsdist::configuration::ImmutableConfiguration& config, uint64_t newValue) { config.d_outgoingTCPMaxIdleTime = newValue; }, std::numeric_limits::max()}, #if defined(HAVE_DNS_OVER_HTTPS) && defined(HAVE_NGHTTP2) - {"setOutgoingDoHWorkerThreads", [](dnsdist::configuration::ImmutableConfiguration& config, uint64_t newValue) { config.d_outgoingDoHWorkers = newValue; }, std::numeric_limits::max()}, - {"setMaxIdleDoHConnectionsPerDownstream", [](dnsdist::configuration::ImmutableConfiguration& config, uint64_t newValue) { config.d_outgoingDoHMaxIdlePerBackend = newValue; }, std::numeric_limits::max()}, - {"setDoHDownstreamCleanupInterval", [](dnsdist::configuration::ImmutableConfiguration& config, uint64_t newValue) { config.d_outgoingDoHCleanupInterval = newValue; }, std::numeric_limits::max()}, - {"setDoHDownstreamMaxIdleTime", [](dnsdist::configuration::ImmutableConfiguration& config, uint64_t newValue) { config.d_outgoingDoHMaxIdleTime = newValue; }, std::numeric_limits::max()}, + {"setOutgoingDoHWorkerThreads", [](dnsdist::configuration::ImmutableConfiguration& config, uint64_t newValue) { config.d_outgoingDoHWorkers = newValue; }, std::numeric_limits::max()}, + {"setMaxIdleDoHConnectionsPerDownstream", [](dnsdist::configuration::ImmutableConfiguration& config, uint64_t newValue) { config.d_outgoingDoHMaxIdlePerBackend = newValue; }, std::numeric_limits::max()}, + {"setDoHDownstreamCleanupInterval", [](dnsdist::configuration::ImmutableConfiguration& config, uint64_t newValue) { config.d_outgoingDoHCleanupInterval = newValue; }, std::numeric_limits::max()}, + {"setDoHDownstreamMaxIdleTime", [](dnsdist::configuration::ImmutableConfiguration& config, uint64_t newValue) { config.d_outgoingDoHMaxIdleTime = newValue; }, std::numeric_limits::max()}, #endif /* HAVE_DNS_OVER_HTTPS && HAVE_NGHTTP2 */ - {"setMaxUDPOutstanding", [](dnsdist::configuration::ImmutableConfiguration& config, uint64_t newValue) { config.d_maxUDPOutstanding = newValue; }, std::numeric_limits::max()}, - {"setWHashedPertubation", [](dnsdist::configuration::ImmutableConfiguration& config, uint64_t newValue) { config.d_hashPerturbation = newValue; }, std::numeric_limits::max()}, + {"setMaxUDPOutstanding", [](dnsdist::configuration::ImmutableConfiguration& config, uint64_t newValue) { config.d_maxUDPOutstanding = newValue; }, std::numeric_limits::max()}, + {"setWHashedPertubation", [](dnsdist::configuration::ImmutableConfiguration& config, uint64_t newValue) { config.d_hashPerturbation = newValue; }, std::numeric_limits::max()}, #ifndef DISABLE_RECVMMSG - {"setUDPMultipleMessagesVectorSize", [](dnsdist::configuration::ImmutableConfiguration& config, uint64_t newValue) { config.d_udpVectorSize = newValue; }, std::numeric_limits::max()}, + {"setUDPMultipleMessagesVectorSize", [](dnsdist::configuration::ImmutableConfiguration& config, uint64_t newValue) { config.d_udpVectorSize = newValue; }, std::numeric_limits::max()}, #endif /* DISABLE_RECVMMSG */ - {"setUDPTimeout", [](dnsdist::configuration::ImmutableConfiguration& config, uint64_t newValue) { config.d_udpTimeout = newValue; }, std::numeric_limits::max()}, - {"setConsoleMaximumConcurrentConnections", [](dnsdist::configuration::ImmutableConfiguration& config, uint64_t newValue) { config.d_consoleMaxConcurrentConnections = newValue; }, std::numeric_limits::max()}, - {"setRingBuffersLockRetries", [](dnsdist::configuration::ImmutableConfiguration& config, uint64_t newValue) { config.d_ringsNbLockTries = newValue; }, std::numeric_limits::max()}, + {"setUDPTimeout", [](dnsdist::configuration::ImmutableConfiguration& config, uint64_t newValue) { config.d_udpTimeout = newValue; }, std::numeric_limits::max()}, + {"setConsoleMaximumConcurrentConnections", [](dnsdist::configuration::ImmutableConfiguration& config, uint64_t newValue) { config.d_consoleMaxConcurrentConnections = newValue; }, std::numeric_limits::max()}, + {"setRingBuffersLockRetries", [](dnsdist::configuration::ImmutableConfiguration& config, uint64_t newValue) { config.d_ringsNbLockTries = newValue; }, std::numeric_limits::max()}, }; struct DoubleImmutableConfigurationItems @@ -1267,7 +1266,7 @@ static void setupLuaConfig(LuaContext& luaCtx, bool client, bool configCheck) }); luaCtx.writeFunction("getPoolServers", [](const string& pool) { - //coverity[auto_causes_copy] + // coverity[auto_causes_copy] const auto poolServers = getDownstreamCandidates(pool); return *poolServers; }); @@ -1603,7 +1602,7 @@ static void setupLuaConfig(LuaContext& luaCtx, bool client, bool configCheck) g_outputBuffer = "Crypto failed..\n"; } #else - g_outputBuffer = "Crypto not available.\n"; + g_outputBuffer = "Crypto not available.\n"; #endif }); @@ -1876,9 +1875,9 @@ static void setupLuaConfig(LuaContext& luaCtx, bool client, bool configCheck) // 1 2 3 4 ret << (fmt % "Name" % "Cache" % "ServerPolicy" % "Servers") << endl; - //coverity[auto_causes_copy] + // coverity[auto_causes_copy] const auto defaultPolicyName = dnsdist::configuration::getCurrentRuntimeConfiguration().d_lbPolicy->getName(); - //coverity[auto_causes_copy] + // coverity[auto_causes_copy] const auto pools = dnsdist::configuration::getCurrentRuntimeConfiguration().d_pools; for (const auto& entry : pools) { const string& name = entry.first; @@ -2461,7 +2460,7 @@ static void setupLuaConfig(LuaContext& luaCtx, bool client, bool configCheck) #ifdef HAVE_NGHTTP2 frontend->d_library = "nghttp2"; #else /* HAVE_NGHTTP2 */ - frontend->d_library = "h2o"; + frontend->d_library = "h2o"; #endif /* HAVE_NGHTTP2 */ } if (frontend->d_library == "h2o") { @@ -2470,8 +2469,8 @@ static void setupLuaConfig(LuaContext& luaCtx, bool client, bool configCheck) // we _really_ need to set it again, as we just replaced the generic frontend by a new one frontend->d_library = "h2o"; #else /* HAVE_LIBH2OEVLOOP */ - errlog("DOH bind %s is configured to use libh2o but the library is not available", addr); - return; + errlog("DOH bind %s is configured to use libh2o but the library is not available", addr); + return; #endif /* HAVE_LIBH2OEVLOOP */ } else if (frontend->d_library == "nghttp2") { @@ -2588,7 +2587,7 @@ static void setupLuaConfig(LuaContext& luaCtx, bool client, bool configCheck) #ifdef HAVE_LIBSSL const std::string provider("openssl"); #else - const std::string provider("gnutls"); + const std::string provider("gnutls"); #endif vinfolog("Loading default TLS provider '%s'", provider); } @@ -2609,7 +2608,7 @@ static void setupLuaConfig(LuaContext& luaCtx, bool client, bool configCheck) config.d_frontends.push_back(std::move(clientState)); }); #else /* HAVE_DNS_OVER_HTTPS */ - throw std::runtime_error("addDOHLocal() called but DNS over HTTPS support is not present!"); + throw std::runtime_error("addDOHLocal() called but DNS over HTTPS support is not present!"); #endif /* HAVE_DNS_OVER_HTTPS */ }); @@ -2686,7 +2685,7 @@ static void setupLuaConfig(LuaContext& luaCtx, bool client, bool configCheck) config.d_frontends.push_back(std::move(clientState)); }); #else - throw std::runtime_error("addDOH3Local() called but DNS over HTTP/3 support is not present!"); + throw std::runtime_error("addDOH3Local() called but DNS over HTTP/3 support is not present!"); #endif }); @@ -2763,7 +2762,7 @@ static void setupLuaConfig(LuaContext& luaCtx, bool client, bool configCheck) config.d_frontends.push_back(std::move(clientState)); }); #else - throw std::runtime_error("addDOQLocal() called but DNS over QUIC support is not present!"); + throw std::runtime_error("addDOQLocal() called but DNS over QUIC support is not present!"); #endif }); @@ -2786,7 +2785,7 @@ static void setupLuaConfig(LuaContext& luaCtx, bool client, bool configCheck) throw; } #else - g_outputBuffer = "DNS over QUIC support is not present!\n"; + g_outputBuffer = "DNS over QUIC support is not present!\n"; #endif }); @@ -2845,7 +2844,7 @@ static void setupLuaConfig(LuaContext& luaCtx, bool client, bool configCheck) throw; } #else - g_outputBuffer = "DNS over HTTPS support is not present!\n"; + g_outputBuffer = "DNS over HTTPS support is not present!\n"; #endif }); @@ -2868,7 +2867,7 @@ static void setupLuaConfig(LuaContext& luaCtx, bool client, bool configCheck) throw; } #else - g_outputBuffer = "DNS over HTTP3 support is not present!\n"; + g_outputBuffer = "DNS over HTTP3 support is not present!\n"; #endif }); @@ -2938,7 +2937,7 @@ static void setupLuaConfig(LuaContext& luaCtx, bool client, bool configCheck) throw; } #else - g_outputBuffer = "DNS over HTTPS support is not present!\n"; + g_outputBuffer = "DNS over HTTPS support is not present!\n"; #endif }); @@ -3117,7 +3116,7 @@ static void setupLuaConfig(LuaContext& luaCtx, bool client, bool configCheck) #ifdef HAVE_LIBSSL const std::string provider("openssl"); #else - const std::string provider("gnutls"); + const std::string provider("gnutls"); #endif vinfolog("Loading default TLS provider '%s'", provider); } @@ -3143,7 +3142,7 @@ static void setupLuaConfig(LuaContext& luaCtx, bool client, bool configCheck) g_outputBuffer = "Error: " + string(e.what()) + "\n"; } #else - throw std::runtime_error("addTLSLocal() called but DNS over TLS support is not present!"); + throw std::runtime_error("addTLSLocal() called but DNS over TLS support is not present!"); #endif }); @@ -3167,7 +3166,7 @@ static void setupLuaConfig(LuaContext& luaCtx, bool client, bool configCheck) throw; } #else - g_outputBuffer = "DNS over TLS support is not present!\n"; + g_outputBuffer = "DNS over TLS support is not present!\n"; #endif }); diff --git a/pdns/dnsdistdist/dnsdist-metrics.cc b/pdns/dnsdistdist/dnsdist-metrics.cc index 18ac63aab226..4f9c92b94f93 100644 --- a/pdns/dnsdistdist/dnsdist-metrics.cc +++ b/pdns/dnsdistdist/dnsdist-metrics.cc @@ -227,7 +227,8 @@ static string prometheusLabelValueEscape(const string& value) static std::string generateCombinationOfLabels(const std::unordered_map& labels) { - return std::accumulate(labels.begin(), labels.end(), std::string(), [](const std::string& acc, const std::pair& l) { + auto ordered = std::map(labels.begin(), labels.end()); + return std::accumulate(ordered.begin(), ordered.end(), std::string(), [](const std::string& acc, const std::pair& l) { return acc + (acc.empty() ? std::string() : ",") + l.first + "=" + "\"" + prometheusLabelValueEscape(l.second) + "\""; }); } diff --git a/pdns/dnsdistdist/dnsdist-web.cc b/pdns/dnsdistdist/dnsdist-web.cc index 1f38516e8d63..772af6490286 100644 --- a/pdns/dnsdistdist/dnsdist-web.cc +++ b/pdns/dnsdistdist/dnsdist-web.cc @@ -1912,7 +1912,7 @@ void setMaxConcurrentConnections(size_t max) void WebserverThread(Socket sock) { setThreadName("dnsdist/webserv"); - //coverity[auto_causes_copy] + // coverity[auto_causes_copy] const auto local = *dnsdist::configuration::getCurrentRuntimeConfiguration().d_webServerAddress; infolog("Webserver launched on %s", local.toStringWithPort()); From 7d075382ad52b6c4e784b9c0c398c164d1482655 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ensar=20Saraj=C4=8Di=C4=87?= Date: Thu, 12 Dec 2024 16:36:54 +0100 Subject: [PATCH 04/16] dnsdist: remove unnecessary optional around variant opts --- pdns/dnsdistdist/dnsdist-lua.cc | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/pdns/dnsdistdist/dnsdist-lua.cc b/pdns/dnsdistdist/dnsdist-lua.cc index 7da861d9db36..1e432d31b0a4 100644 --- a/pdns/dnsdistdist/dnsdist-lua.cc +++ b/pdns/dnsdistdist/dnsdist-lua.cc @@ -3412,7 +3412,7 @@ static void setupLuaConfig(LuaContext& luaCtx, bool client, bool configCheck) newThread.detach(); }); - luaCtx.writeFunction("declareMetric", [](const std::string& name, const std::string& type, const std::string& description, boost::optional>> opts) { + luaCtx.writeFunction("declareMetric", [](const std::string& name, const std::string& type, const std::string& description, boost::optional> opts) { bool withLabels = false; std::optional customName = std::nullopt; if (opts) { @@ -3421,7 +3421,7 @@ static void setupLuaConfig(LuaContext& luaCtx, bool client, bool configCheck) customName = std::optional(*optCustomName); } if (!customName) { - boost::optional vars = boost::get>(opts.get()); + boost::optional vars = {boost::get(opts.get())}; getOptionalValue(vars, "customName", customName); getOptionalValue(vars, "withLabels", withLabels); checkAllParametersConsumed("declareMetric", vars); @@ -3435,7 +3435,7 @@ static void setupLuaConfig(LuaContext& luaCtx, bool client, bool configCheck) } return true; }); - luaCtx.writeFunction("incMetric", [](const std::string& name, boost::optional>> opts) { + luaCtx.writeFunction("incMetric", [](const std::string& name, boost::optional> opts) { auto incOpts = opts.get_value_or(1); uint64_t step = 1; std::unordered_map labels; @@ -3443,7 +3443,7 @@ static void setupLuaConfig(LuaContext& luaCtx, bool client, bool configCheck) step = *custom_step; } else { - boost::optional vars = boost::get>(incOpts); + boost::optional vars = {boost::get(incOpts)}; getOptionalValue(vars, "step", step); getOptionalValue>(vars, "labels", labels); checkAllParametersConsumed("incMetric", vars); @@ -3456,7 +3456,7 @@ static void setupLuaConfig(LuaContext& luaCtx, bool client, bool configCheck) } return std::get(result); }); - luaCtx.writeFunction("decMetric", [](const std::string& name, boost::optional>> opts) { + luaCtx.writeFunction("decMetric", [](const std::string& name, boost::optional> opts) { auto decOpts = opts.get_value_or(1); uint64_t step = 1; std::unordered_map labels; @@ -3464,7 +3464,7 @@ static void setupLuaConfig(LuaContext& luaCtx, bool client, bool configCheck) step = *custom_step; } else { - boost::optional vars = boost::get>(decOpts); + boost::optional vars = {boost::get(decOpts)}; getOptionalValue(vars, "step", step); getOptionalValue>(vars, "labels", labels); checkAllParametersConsumed("decMetric", vars); From 423251d25d553d85fb87bbd388bec3b251b95d6c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ensar=20Saraj=C4=8Di=C4=87?= Date: Thu, 12 Dec 2024 16:51:33 +0100 Subject: [PATCH 05/16] dnsdist: fix formatting in dnsdist-lua.cc --- pdns/dnsdistdist/dnsdist-lua.cc | 65 +++++++++++++++++---------------- 1 file changed, 33 insertions(+), 32 deletions(-) diff --git a/pdns/dnsdistdist/dnsdist-lua.cc b/pdns/dnsdistdist/dnsdist-lua.cc index 1e432d31b0a4..bf7e179ed0b3 100644 --- a/pdns/dnsdistdist/dnsdist-lua.cc +++ b/pdns/dnsdistdist/dnsdist-lua.cc @@ -886,28 +886,29 @@ static void setupLuaConfig(LuaContext& luaCtx, bool client, bool configCheck) const std::function mutator; const size_t maximumValue{std::numeric_limits::max()}; }; - static const std::vector unsignedIntegerImmutableConfigItems{ + static const std::vector unsignedIntegerImmutableConfigItems + { {"setMaxTCPQueuedConnections", [](dnsdist::configuration::ImmutableConfiguration& config, uint64_t newValue) { config.d_maxTCPQueuedConnections = newValue; }, std::numeric_limits::max()}, - {"setMaxTCPClientThreads", [](dnsdist::configuration::ImmutableConfiguration& config, uint64_t newValue) { config.d_maxTCPClientThreads = newValue; }, std::numeric_limits::max()}, - {"setMaxTCPConnectionsPerClient", [](dnsdist::configuration::ImmutableConfiguration& config, uint64_t newValue) { config.d_maxTCPConnectionsPerClient = newValue; }, std::numeric_limits::max()}, - {"setTCPInternalPipeBufferSize", [](dnsdist::configuration::ImmutableConfiguration& config, uint64_t newValue) { config.d_tcpInternalPipeBufferSize = newValue; }, std::numeric_limits::max()}, - {"setMaxCachedTCPConnectionsPerDownstream", [](dnsdist::configuration::ImmutableConfiguration& config, uint64_t newValue) { config.d_outgoingTCPMaxIdlePerBackend = newValue; }, std::numeric_limits::max()}, - {"setTCPDownstreamCleanupInterval", [](dnsdist::configuration::ImmutableConfiguration& config, uint64_t newValue) { config.d_outgoingTCPCleanupInterval = newValue; }, std::numeric_limits::max()}, - {"setTCPDownstreamMaxIdleTime", [](dnsdist::configuration::ImmutableConfiguration& config, uint64_t newValue) { config.d_outgoingTCPMaxIdleTime = newValue; }, std::numeric_limits::max()}, + {"setMaxTCPClientThreads", [](dnsdist::configuration::ImmutableConfiguration& config, uint64_t newValue) { config.d_maxTCPClientThreads = newValue; }, std::numeric_limits::max()}, + {"setMaxTCPConnectionsPerClient", [](dnsdist::configuration::ImmutableConfiguration& config, uint64_t newValue) { config.d_maxTCPConnectionsPerClient = newValue; }, std::numeric_limits::max()}, + {"setTCPInternalPipeBufferSize", [](dnsdist::configuration::ImmutableConfiguration& config, uint64_t newValue) { config.d_tcpInternalPipeBufferSize = newValue; }, std::numeric_limits::max()}, + {"setMaxCachedTCPConnectionsPerDownstream", [](dnsdist::configuration::ImmutableConfiguration& config, uint64_t newValue) { config.d_outgoingTCPMaxIdlePerBackend = newValue; }, std::numeric_limits::max()}, + {"setTCPDownstreamCleanupInterval", [](dnsdist::configuration::ImmutableConfiguration& config, uint64_t newValue) { config.d_outgoingTCPCleanupInterval = newValue; }, std::numeric_limits::max()}, + {"setTCPDownstreamMaxIdleTime", [](dnsdist::configuration::ImmutableConfiguration& config, uint64_t newValue) { config.d_outgoingTCPMaxIdleTime = newValue; }, std::numeric_limits::max()}, #if defined(HAVE_DNS_OVER_HTTPS) && defined(HAVE_NGHTTP2) - {"setOutgoingDoHWorkerThreads", [](dnsdist::configuration::ImmutableConfiguration& config, uint64_t newValue) { config.d_outgoingDoHWorkers = newValue; }, std::numeric_limits::max()}, - {"setMaxIdleDoHConnectionsPerDownstream", [](dnsdist::configuration::ImmutableConfiguration& config, uint64_t newValue) { config.d_outgoingDoHMaxIdlePerBackend = newValue; }, std::numeric_limits::max()}, - {"setDoHDownstreamCleanupInterval", [](dnsdist::configuration::ImmutableConfiguration& config, uint64_t newValue) { config.d_outgoingDoHCleanupInterval = newValue; }, std::numeric_limits::max()}, - {"setDoHDownstreamMaxIdleTime", [](dnsdist::configuration::ImmutableConfiguration& config, uint64_t newValue) { config.d_outgoingDoHMaxIdleTime = newValue; }, std::numeric_limits::max()}, + {"setOutgoingDoHWorkerThreads", [](dnsdist::configuration::ImmutableConfiguration& config, uint64_t newValue) { config.d_outgoingDoHWorkers = newValue; }, std::numeric_limits::max()}, + {"setMaxIdleDoHConnectionsPerDownstream", [](dnsdist::configuration::ImmutableConfiguration& config, uint64_t newValue) { config.d_outgoingDoHMaxIdlePerBackend = newValue; }, std::numeric_limits::max()}, + {"setDoHDownstreamCleanupInterval", [](dnsdist::configuration::ImmutableConfiguration& config, uint64_t newValue) { config.d_outgoingDoHCleanupInterval = newValue; }, std::numeric_limits::max()}, + {"setDoHDownstreamMaxIdleTime", [](dnsdist::configuration::ImmutableConfiguration& config, uint64_t newValue) { config.d_outgoingDoHMaxIdleTime = newValue; }, std::numeric_limits::max()}, #endif /* HAVE_DNS_OVER_HTTPS && HAVE_NGHTTP2 */ - {"setMaxUDPOutstanding", [](dnsdist::configuration::ImmutableConfiguration& config, uint64_t newValue) { config.d_maxUDPOutstanding = newValue; }, std::numeric_limits::max()}, - {"setWHashedPertubation", [](dnsdist::configuration::ImmutableConfiguration& config, uint64_t newValue) { config.d_hashPerturbation = newValue; }, std::numeric_limits::max()}, + {"setMaxUDPOutstanding", [](dnsdist::configuration::ImmutableConfiguration& config, uint64_t newValue) { config.d_maxUDPOutstanding = newValue; }, std::numeric_limits::max()}, + {"setWHashedPertubation", [](dnsdist::configuration::ImmutableConfiguration& config, uint64_t newValue) { config.d_hashPerturbation = newValue; }, std::numeric_limits::max()}, #ifndef DISABLE_RECVMMSG - {"setUDPMultipleMessagesVectorSize", [](dnsdist::configuration::ImmutableConfiguration& config, uint64_t newValue) { config.d_udpVectorSize = newValue; }, std::numeric_limits::max()}, + {"setUDPMultipleMessagesVectorSize", [](dnsdist::configuration::ImmutableConfiguration& config, uint64_t newValue) { config.d_udpVectorSize = newValue; }, std::numeric_limits::max()}, #endif /* DISABLE_RECVMMSG */ - {"setUDPTimeout", [](dnsdist::configuration::ImmutableConfiguration& config, uint64_t newValue) { config.d_udpTimeout = newValue; }, std::numeric_limits::max()}, - {"setConsoleMaximumConcurrentConnections", [](dnsdist::configuration::ImmutableConfiguration& config, uint64_t newValue) { config.d_consoleMaxConcurrentConnections = newValue; }, std::numeric_limits::max()}, - {"setRingBuffersLockRetries", [](dnsdist::configuration::ImmutableConfiguration& config, uint64_t newValue) { config.d_ringsNbLockTries = newValue; }, std::numeric_limits::max()}, + {"setUDPTimeout", [](dnsdist::configuration::ImmutableConfiguration& config, uint64_t newValue) { config.d_udpTimeout = newValue; }, std::numeric_limits::max()}, + {"setConsoleMaximumConcurrentConnections", [](dnsdist::configuration::ImmutableConfiguration& config, uint64_t newValue) { config.d_consoleMaxConcurrentConnections = newValue; }, std::numeric_limits::max()}, + {"setRingBuffersLockRetries", [](dnsdist::configuration::ImmutableConfiguration& config, uint64_t newValue) { config.d_ringsNbLockTries = newValue; }, std::numeric_limits::max()}, }; struct DoubleImmutableConfigurationItems @@ -1602,7 +1603,7 @@ static void setupLuaConfig(LuaContext& luaCtx, bool client, bool configCheck) g_outputBuffer = "Crypto failed..\n"; } #else - g_outputBuffer = "Crypto not available.\n"; + g_outputBuffer = "Crypto not available.\n"; #endif }); @@ -2460,7 +2461,7 @@ static void setupLuaConfig(LuaContext& luaCtx, bool client, bool configCheck) #ifdef HAVE_NGHTTP2 frontend->d_library = "nghttp2"; #else /* HAVE_NGHTTP2 */ - frontend->d_library = "h2o"; + frontend->d_library = "h2o"; #endif /* HAVE_NGHTTP2 */ } if (frontend->d_library == "h2o") { @@ -2469,8 +2470,8 @@ static void setupLuaConfig(LuaContext& luaCtx, bool client, bool configCheck) // we _really_ need to set it again, as we just replaced the generic frontend by a new one frontend->d_library = "h2o"; #else /* HAVE_LIBH2OEVLOOP */ - errlog("DOH bind %s is configured to use libh2o but the library is not available", addr); - return; + errlog("DOH bind %s is configured to use libh2o but the library is not available", addr); + return; #endif /* HAVE_LIBH2OEVLOOP */ } else if (frontend->d_library == "nghttp2") { @@ -2587,7 +2588,7 @@ static void setupLuaConfig(LuaContext& luaCtx, bool client, bool configCheck) #ifdef HAVE_LIBSSL const std::string provider("openssl"); #else - const std::string provider("gnutls"); + const std::string provider("gnutls"); #endif vinfolog("Loading default TLS provider '%s'", provider); } @@ -2608,7 +2609,7 @@ static void setupLuaConfig(LuaContext& luaCtx, bool client, bool configCheck) config.d_frontends.push_back(std::move(clientState)); }); #else /* HAVE_DNS_OVER_HTTPS */ - throw std::runtime_error("addDOHLocal() called but DNS over HTTPS support is not present!"); + throw std::runtime_error("addDOHLocal() called but DNS over HTTPS support is not present!"); #endif /* HAVE_DNS_OVER_HTTPS */ }); @@ -2685,7 +2686,7 @@ static void setupLuaConfig(LuaContext& luaCtx, bool client, bool configCheck) config.d_frontends.push_back(std::move(clientState)); }); #else - throw std::runtime_error("addDOH3Local() called but DNS over HTTP/3 support is not present!"); + throw std::runtime_error("addDOH3Local() called but DNS over HTTP/3 support is not present!"); #endif }); @@ -2762,7 +2763,7 @@ static void setupLuaConfig(LuaContext& luaCtx, bool client, bool configCheck) config.d_frontends.push_back(std::move(clientState)); }); #else - throw std::runtime_error("addDOQLocal() called but DNS over QUIC support is not present!"); + throw std::runtime_error("addDOQLocal() called but DNS over QUIC support is not present!"); #endif }); @@ -2785,7 +2786,7 @@ static void setupLuaConfig(LuaContext& luaCtx, bool client, bool configCheck) throw; } #else - g_outputBuffer = "DNS over QUIC support is not present!\n"; + g_outputBuffer = "DNS over QUIC support is not present!\n"; #endif }); @@ -2844,7 +2845,7 @@ static void setupLuaConfig(LuaContext& luaCtx, bool client, bool configCheck) throw; } #else - g_outputBuffer = "DNS over HTTPS support is not present!\n"; + g_outputBuffer = "DNS over HTTPS support is not present!\n"; #endif }); @@ -2867,7 +2868,7 @@ static void setupLuaConfig(LuaContext& luaCtx, bool client, bool configCheck) throw; } #else - g_outputBuffer = "DNS over HTTP3 support is not present!\n"; + g_outputBuffer = "DNS over HTTP3 support is not present!\n"; #endif }); @@ -2937,7 +2938,7 @@ static void setupLuaConfig(LuaContext& luaCtx, bool client, bool configCheck) throw; } #else - g_outputBuffer = "DNS over HTTPS support is not present!\n"; + g_outputBuffer = "DNS over HTTPS support is not present!\n"; #endif }); @@ -3116,7 +3117,7 @@ static void setupLuaConfig(LuaContext& luaCtx, bool client, bool configCheck) #ifdef HAVE_LIBSSL const std::string provider("openssl"); #else - const std::string provider("gnutls"); + const std::string provider("gnutls"); #endif vinfolog("Loading default TLS provider '%s'", provider); } @@ -3142,7 +3143,7 @@ static void setupLuaConfig(LuaContext& luaCtx, bool client, bool configCheck) g_outputBuffer = "Error: " + string(e.what()) + "\n"; } #else - throw std::runtime_error("addTLSLocal() called but DNS over TLS support is not present!"); + throw std::runtime_error("addTLSLocal() called but DNS over TLS support is not present!"); #endif }); @@ -3166,7 +3167,7 @@ static void setupLuaConfig(LuaContext& luaCtx, bool client, bool configCheck) throw; } #else - g_outputBuffer = "DNS over TLS support is not present!\n"; + g_outputBuffer = "DNS over TLS support is not present!\n"; #endif }); From 657caa55a4c3ae37a039c7de24762553b08ed8f3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ensar=20Saraj=C4=8Di=C4=87?= Date: Thu, 12 Dec 2024 17:23:05 +0100 Subject: [PATCH 06/16] dnsdist: use template function to simplify metrics mutation functions --- pdns/dnsdistdist/dnsdist-metrics.cc | 44 ++++++++++++++--------------- 1 file changed, 21 insertions(+), 23 deletions(-) diff --git a/pdns/dnsdistdist/dnsdist-metrics.cc b/pdns/dnsdistdist/dnsdist-metrics.cc index 4f9c92b94f93..10d18dc2923a 100644 --- a/pdns/dnsdistdist/dnsdist-metrics.cc +++ b/pdns/dnsdistdist/dnsdist-metrics.cc @@ -22,6 +22,7 @@ #include #include #include +#include #include "dnsdist-metrics.hh" #include "dnsdist.hh" @@ -233,19 +234,26 @@ static std::string generateCombinationOfLabels(const std::unordered_map +static T& initializeOrGetMetric(const std::string_view& name, std::map& metricEntries, const std::unordered_map& labels) +{ + auto combinationOfLabels = generateCombinationOfLabels(labels); + auto metricEntry = metricEntries.find(combinationOfLabels); + if (metricEntry == metricEntries.end()) { + metricEntry = metricEntries.emplace(std::piecewise_construct, std::forward_as_tuple(combinationOfLabels), std::forward_as_tuple()).first; + g_stats.entries.write_lock()->emplace_back(Stats::EntryTriple{std::string(name), combinationOfLabels, &metricEntry->second.d_value}); + } + return metricEntry->second; +} + std::variant incrementCustomCounter(const std::string_view& name, uint64_t step, const std::unordered_map& labels) { auto customCounters = s_customCounters.write_lock(); auto metric = customCounters->find(name); if (metric != customCounters->end()) { - auto combinationOfLabels = generateCombinationOfLabels(labels); - auto metricEntry = metric->second.find(combinationOfLabels); - if (metricEntry == metric->second.end()) { - metricEntry = metric->second.emplace(combinationOfLabels, MutableCounter()).first; - g_stats.entries.write_lock()->emplace_back(Stats::EntryTriple{std::string(name), combinationOfLabels, &metricEntry->second.d_value}); - } - metricEntry->second.d_value += step; - return metricEntry->second.d_value.load(); + auto& metricEntry = initializeOrGetMetric(name, metric->second, labels); + metricEntry.d_value += step; + return metricEntry.d_value.load(); } return std::string("Unable to increment custom metric '") + std::string(name) + "': no such counter"; } @@ -255,14 +263,9 @@ std::variant decrementCustomCounter(const std::string_view& nam auto customCounters = s_customCounters.write_lock(); auto metric = customCounters->find(name); if (metric != customCounters->end()) { - auto combinationOfLabels = generateCombinationOfLabels(labels); - auto metricEntry = metric->second.find(combinationOfLabels); - if (metricEntry == metric->second.end()) { - metricEntry = metric->second.emplace(combinationOfLabels, MutableCounter()).first; - g_stats.entries.write_lock()->emplace_back(Stats::EntryTriple{std::string(name), combinationOfLabels, &metricEntry->second.d_value}); - } - metricEntry->second.d_value -= step; - return metricEntry->second.d_value.load(); + auto& metricEntry = initializeOrGetMetric(name, metric->second, labels); + metricEntry.d_value -= step; + return metricEntry.d_value.load(); } return std::string("Unable to decrement custom metric '") + std::string(name) + "': no such counter"; } @@ -272,13 +275,8 @@ std::variant setCustomGauge(const std::string_view& name, const d auto customGauges = s_customGauges.write_lock(); auto metric = customGauges->find(name); if (metric != customGauges->end()) { - auto combinationOfLabels = generateCombinationOfLabels(labels); - auto metricEntry = metric->second.find(combinationOfLabels); - if (metricEntry == metric->second.end()) { - metricEntry = metric->second.emplace(combinationOfLabels, MutableGauge()).first; - g_stats.entries.write_lock()->emplace_back(Stats::EntryTriple{std::string(name), combinationOfLabels, &metricEntry->second.d_value}); - } - metricEntry->second.d_value = value; + auto& metricEntry = initializeOrGetMetric(name, metric->second, labels); + metricEntry.d_value = value; return value; } From ec216cdb5f74e3947ebefeb1effb3de767c63d96 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ensar=20Saraj=C4=8Di=C4=87?= Date: Thu, 12 Dec 2024 17:38:43 +0100 Subject: [PATCH 07/16] dnsdist: clean up clang-tidy warnings in lua and metrics --- pdns/dnsdistdist/dnsdist-lua.cc | 10 +++++----- pdns/dnsdistdist/dnsdist-metrics.cc | 17 +++++++++-------- 2 files changed, 14 insertions(+), 13 deletions(-) diff --git a/pdns/dnsdistdist/dnsdist-lua.cc b/pdns/dnsdistdist/dnsdist-lua.cc index bf7e179ed0b3..ff4e05587e5c 100644 --- a/pdns/dnsdistdist/dnsdist-lua.cc +++ b/pdns/dnsdistdist/dnsdist-lua.cc @@ -3413,12 +3413,12 @@ static void setupLuaConfig(LuaContext& luaCtx, bool client, bool configCheck) newThread.detach(); }); - luaCtx.writeFunction("declareMetric", [](const std::string& name, const std::string& type, const std::string& description, boost::optional> opts) { + luaCtx.writeFunction("declareMetric", [](const std::string& name, const std::string& type, const std::string& description, const boost::optional>& opts) { bool withLabels = false; std::optional customName = std::nullopt; if (opts) { auto* optCustomName = boost::get(&opts.get()); - if (optCustomName) { + if (optCustomName != nullptr) { customName = std::optional(*optCustomName); } if (!customName) { @@ -3436,7 +3436,7 @@ static void setupLuaConfig(LuaContext& luaCtx, bool client, bool configCheck) } return true; }); - luaCtx.writeFunction("incMetric", [](const std::string& name, boost::optional> opts) { + luaCtx.writeFunction("incMetric", [](const std::string& name, const boost::optional>& opts) { auto incOpts = opts.get_value_or(1); uint64_t step = 1; std::unordered_map labels; @@ -3457,11 +3457,11 @@ static void setupLuaConfig(LuaContext& luaCtx, bool client, bool configCheck) } return std::get(result); }); - luaCtx.writeFunction("decMetric", [](const std::string& name, boost::optional> opts) { + luaCtx.writeFunction("decMetric", [](const std::string& name, const boost::optional>& opts) { auto decOpts = opts.get_value_or(1); uint64_t step = 1; std::unordered_map labels; - if (auto custom_step = boost::get(&decOpts)) { + if (auto* custom_step = boost::get(&decOpts)) { step = *custom_step; } else { diff --git a/pdns/dnsdistdist/dnsdist-metrics.cc b/pdns/dnsdistdist/dnsdist-metrics.cc index 10d18dc2923a..293be6d95c4d 100644 --- a/pdns/dnsdistdist/dnsdist-metrics.cc +++ b/pdns/dnsdistdist/dnsdist-metrics.cc @@ -211,17 +211,18 @@ static string prometheusLabelValueEscape(const string& value) { string ret; - for (char i : value) { - if (i == '"' || i == '\\') { + for (char lblchar : value) { + if (lblchar == '"' || lblchar == '\\') { ret += '\\'; - ret += i; + ret += lblchar; } - else if (i == '\n') { + else if (lblchar == '\n') { ret += '\\'; ret += 'n'; } - else - ret += i; + else { + ret += lblchar; + } } return ret; } @@ -229,8 +230,8 @@ static string prometheusLabelValueEscape(const string& value) static std::string generateCombinationOfLabels(const std::unordered_map& labels) { auto ordered = std::map(labels.begin(), labels.end()); - return std::accumulate(ordered.begin(), ordered.end(), std::string(), [](const std::string& acc, const std::pair& l) { - return acc + (acc.empty() ? std::string() : ",") + l.first + "=" + "\"" + prometheusLabelValueEscape(l.second) + "\""; + return std::accumulate(ordered.begin(), ordered.end(), std::string(), [](const std::string& acc, const std::pair& label) { + return acc + (acc.empty() ? std::string() : ",") + label.first + "=" + "\"" + prometheusLabelValueEscape(label.second) + "\""; }); } From 64e93522ade211912b79ba27758254811113aaf8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ensar=20Saraj=C4=8Di=C4=87?= Date: Thu, 12 Dec 2024 19:36:41 +0100 Subject: [PATCH 08/16] dnsdist: skip metrics with labels for carbon export and stats dump --- pdns/dnsdistdist/dnsdist-carbon.cc | 5 +++++ pdns/dnsdistdist/dnsdist-lua-inspection.cc | 14 +++++++++++--- pdns/dnsdistdist/dnsdist-lua.cc | 6 +++--- 3 files changed, 19 insertions(+), 6 deletions(-) diff --git a/pdns/dnsdistdist/dnsdist-carbon.cc b/pdns/dnsdistdist/dnsdist-carbon.cc index 040783b68a95..66ec734250e4 100644 --- a/pdns/dnsdistdist/dnsdist-carbon.cc +++ b/pdns/dnsdistdist/dnsdist-carbon.cc @@ -57,6 +57,11 @@ static bool doOneCarbonExport(const Carbon::Endpoint& endpoint) { auto entries = dnsdist::metrics::g_stats.entries.read_lock(); for (const auto& entry : *entries) { + // Skip non-empty labels, since labels are not supported in Carbon + if (!entry.d_labels.empty()) { + continue; + } + str << namespace_name << "." << hostname << "." << instance_name << "." << entry.d_name << ' '; if (const auto& val = std::get_if(&entry.d_value)) { str << (*val)->load(); diff --git a/pdns/dnsdistdist/dnsdist-lua-inspection.cc b/pdns/dnsdistdist/dnsdist-lua-inspection.cc index 2ad3999f7c2e..26d37f712254 100644 --- a/pdns/dnsdistdist/dnsdist-lua-inspection.cc +++ b/pdns/dnsdistdist/dnsdist-lua-inspection.cc @@ -19,7 +19,9 @@ * along with this program; if not, write to the Free Software * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. */ +#include #include +#include #include "dnsdist.hh" #include "dnsdist-console.hh" @@ -811,12 +813,18 @@ void setupLuaInspection(LuaContext& luaCtx) boost::format fmt("%-35s\t%+11s"); g_outputBuffer.clear(); auto entries = *dnsdist::metrics::g_stats.entries.read_lock(); - sort(entries.begin(), entries.end(), + + // Filter entries to just the ones without label, for clearer output + std::vector> unlabeledEntries; + std::copy_if(entries.begin(), entries.end(), std::back_inserter(unlabeledEntries), [](const decltype(entries)::value_type& triple) { return triple.d_labels.empty(); }); + + sort(unlabeledEntries.begin(), unlabeledEntries.end(), [](const decltype(entries)::value_type& lhs, const decltype(entries)::value_type& rhs) { return lhs.d_name < rhs.d_name; }); boost::format flt(" %9.1f"); - for (const auto& entry : entries) { + for (const auto& entryRef : unlabeledEntries) { + const auto& entry = entryRef.get(); string second; if (const auto& val = std::get_if(&entry.d_value)) { second = std::to_string((*val)->load()); @@ -828,7 +836,7 @@ void setupLuaInspection(LuaContext& luaCtx) second = std::to_string((*func)(entry.d_name)); } - if (leftcolumn.size() < entries.size() / 2) { + if (leftcolumn.size() < unlabeledEntries.size() / 2) { leftcolumn.push_back((fmt % entry.d_name % second).str()); } else { diff --git a/pdns/dnsdistdist/dnsdist-lua.cc b/pdns/dnsdistdist/dnsdist-lua.cc index ff4e05587e5c..eec7e4dc97f4 100644 --- a/pdns/dnsdistdist/dnsdist-lua.cc +++ b/pdns/dnsdistdist/dnsdist-lua.cc @@ -3413,7 +3413,7 @@ static void setupLuaConfig(LuaContext& luaCtx, bool client, bool configCheck) newThread.detach(); }); - luaCtx.writeFunction("declareMetric", [](const std::string& name, const std::string& type, const std::string& description, const boost::optional>& opts) { + luaCtx.writeFunction("declareMetric", [](const std::string& name, const std::string& type, const std::string& description, boost::optional> opts) { bool withLabels = false; std::optional customName = std::nullopt; if (opts) { @@ -3436,7 +3436,7 @@ static void setupLuaConfig(LuaContext& luaCtx, bool client, bool configCheck) } return true; }); - luaCtx.writeFunction("incMetric", [](const std::string& name, const boost::optional>& opts) { + luaCtx.writeFunction("incMetric", [](const std::string& name, boost::optional> opts) { auto incOpts = opts.get_value_or(1); uint64_t step = 1; std::unordered_map labels; @@ -3457,7 +3457,7 @@ static void setupLuaConfig(LuaContext& luaCtx, bool client, bool configCheck) } return std::get(result); }); - luaCtx.writeFunction("decMetric", [](const std::string& name, const boost::optional>& opts) { + luaCtx.writeFunction("decMetric", [](const std::string& name, boost::optional> opts) { auto decOpts = opts.get_value_or(1); uint64_t step = 1; std::unordered_map labels; From 9e9c3e54a91189a4b1434cdce3296e94a4e4dfa8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ensar=20Saraj=C4=8Di=C4=87?= Date: Fri, 13 Dec 2024 12:16:13 +0100 Subject: [PATCH 09/16] dnsdist: remove todo comments from `dnsdist-lua-ffi` --- pdns/dnsdistdist/dnsdist-lua-ffi.cc | 6 ------ 1 file changed, 6 deletions(-) diff --git a/pdns/dnsdistdist/dnsdist-lua-ffi.cc b/pdns/dnsdistdist/dnsdist-lua-ffi.cc index d3f9a3982998..92e042e28506 100644 --- a/pdns/dnsdistdist/dnsdist-lua-ffi.cc +++ b/pdns/dnsdistdist/dnsdist-lua-ffi.cc @@ -1806,14 +1806,12 @@ bool dnsdist_ffi_metric_declare(const char* name, size_t nameLen, const char* ty if (name == nullptr || nameLen == 0 || type == nullptr || description == nullptr) { return false; } - // TODO: add labels options? auto result = dnsdist::metrics::declareCustomMetric(name, type, description, customName != nullptr ? std::optional(customName) : std::nullopt, false); return !result; } void dnsdist_ffi_metric_inc(const char* metricName, size_t metricNameLen) { - // TODO: add labels? auto result = dnsdist::metrics::incrementCustomCounter(std::string_view(metricName, metricNameLen), 1U, {}); if (std::get_if(&result) != nullptr) { return; @@ -1822,7 +1820,6 @@ void dnsdist_ffi_metric_inc(const char* metricName, size_t metricNameLen) void dnsdist_ffi_metric_inc_by(const char* metricName, size_t metricNameLen, uint64_t value) { - // TODO: add labels? auto result = dnsdist::metrics::incrementCustomCounter(std::string_view(metricName, metricNameLen), value, {}); if (std::get_if(&result) != nullptr) { return; @@ -1831,7 +1828,6 @@ void dnsdist_ffi_metric_inc_by(const char* metricName, size_t metricNameLen, uin void dnsdist_ffi_metric_dec(const char* metricName, size_t metricNameLen) { - // TODO: add labels? auto result = dnsdist::metrics::decrementCustomCounter(std::string_view(metricName, metricNameLen), 1U, {}); if (std::get_if(&result) != nullptr) { return; @@ -1840,7 +1836,6 @@ void dnsdist_ffi_metric_dec(const char* metricName, size_t metricNameLen) void dnsdist_ffi_metric_set(const char* metricName, size_t metricNameLen, double value) { - // TODO: add labels? auto result = dnsdist::metrics::setCustomGauge(std::string_view(metricName, metricNameLen), value, {}); if (std::get_if(&result) != nullptr) { return; @@ -1849,7 +1844,6 @@ void dnsdist_ffi_metric_set(const char* metricName, size_t metricNameLen, double double dnsdist_ffi_metric_get(const char* metricName, size_t metricNameLen, bool isCounter) { - // TODO: add labels? auto result = dnsdist::metrics::getCustomMetric(std::string_view(metricName, metricNameLen), {}); if (std::get_if(&result) != nullptr) { return 0.; From 60872f6741955289de6f41407f2d80aece07b5bf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ensar=20Saraj=C4=8Di=C4=87?= Date: Fri, 13 Dec 2024 16:02:24 +0100 Subject: [PATCH 10/16] dnsdist: ignore clangd-tidy warnings Co-authored-by: Remi Gacogne --- pdns/dnsdistdist/dnsdist-lua-ffi.cc | 2 ++ pdns/dnsdistdist/dnsdist-lua.cc | 2 ++ 2 files changed, 4 insertions(+) diff --git a/pdns/dnsdistdist/dnsdist-lua-ffi.cc b/pdns/dnsdistdist/dnsdist-lua-ffi.cc index 92e042e28506..03ffb98a2602 100644 --- a/pdns/dnsdistdist/dnsdist-lua-ffi.cc +++ b/pdns/dnsdistdist/dnsdist-lua-ffi.cc @@ -1056,6 +1056,7 @@ size_t dnsdist_ffi_generate_proxy_protocol_payload(const size_t addrSize, const if (valuesCount > 0) { valuesVect.reserve(valuesCount); for (size_t idx = 0; idx < valuesCount; idx++) { + // NOLINTNEXTLINE(cppcoreguidelines-pro-bounds-pointer-arithmetic) valuesVect.push_back({std::string(values[idx].value, values[idx].size), values[idx].type}); } } @@ -1085,6 +1086,7 @@ size_t dnsdist_ffi_dnsquestion_generate_proxy_protocol_payload(const dnsdist_ffi if (valuesCount > 0) { valuesVect.reserve(valuesCount); for (size_t idx = 0; idx < valuesCount; idx++) { + // NOLINTNEXTLINE(cppcoreguidelines-pro-bounds-pointer-arithmetic) valuesVect.push_back({std::string(values[idx].value, values[idx].size), values[idx].type}); } } diff --git a/pdns/dnsdistdist/dnsdist-lua.cc b/pdns/dnsdistdist/dnsdist-lua.cc index eec7e4dc97f4..d97c8329e0de 100644 --- a/pdns/dnsdistdist/dnsdist-lua.cc +++ b/pdns/dnsdistdist/dnsdist-lua.cc @@ -3436,6 +3436,7 @@ static void setupLuaConfig(LuaContext& luaCtx, bool client, bool configCheck) } return true; }); + // NOLINTNEXTLINE(performance-unnecessary-value-param) luaCtx.writeFunction("incMetric", [](const std::string& name, boost::optional> opts) { auto incOpts = opts.get_value_or(1); uint64_t step = 1; @@ -3457,6 +3458,7 @@ static void setupLuaConfig(LuaContext& luaCtx, bool client, bool configCheck) } return std::get(result); }); + // NOLINTNEXTLINE(performance-unnecessary-value-param) luaCtx.writeFunction("decMetric", [](const std::string& name, boost::optional> opts) { auto decOpts = opts.get_value_or(1); uint64_t step = 1; From ce95774ffbba6053a2208a60c758f604142aa84f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ensar=20Saraj=C4=8Di=C4=87?= Date: Mon, 16 Dec 2024 11:10:32 +0100 Subject: [PATCH 11/16] dnsdist: update metrics related regression tests --- pdns/dnsdistdist/dnsdist-web.cc | 3 +++ regression-tests.dnsdist/test_API.py | 9 +++++++++ regression-tests.dnsdist/test_Prometheus.py | 20 ++++++++++++++++---- 3 files changed, 28 insertions(+), 4 deletions(-) diff --git a/pdns/dnsdistdist/dnsdist-web.cc b/pdns/dnsdistdist/dnsdist-web.cc index 772af6490286..55cbc1588dd3 100644 --- a/pdns/dnsdistdist/dnsdist-web.cc +++ b/pdns/dnsdistdist/dnsdist-web.cc @@ -931,6 +931,9 @@ static void addStatsToJSONObject(Json::object& obj) if (entry.d_name == "special-memory-usage") { continue; // Too expensive for get-all } + if (entry.d_labels != "") { + continue; // Skip labeled metrics to prevent duplicates + } if (const auto& val = std::get_if(&entry.d_value)) { obj.emplace(entry.d_name, (double)(*val)->load()); } diff --git a/regression-tests.dnsdist/test_API.py b/regression-tests.dnsdist/test_API.py index dd0e9fde85a7..dfdda817f8a3 100644 --- a/regression-tests.dnsdist/test_API.py +++ b/regression-tests.dnsdist/test_API.py @@ -878,6 +878,10 @@ class TestAPICustomStatistics(APITestsBase): declareMetric("my-custom-metric", "counter", "Number of statistics") declareMetric("my-other-metric", "counter", "Another number of statistics") declareMetric("my-gauge", "gauge", "Current memory usage") + declareMetric("my-labeled-gauge", "gauge", "Custom gauge with labels", { withLabels = true }) + setMetric("my-labeled-gauge", 123, { labels = { foo = "bar" } }) + declareMetric("my-labeled-counter", "counter", "Custom counter with labels", { withLabels = true }) + incMetric("my-labeled-counter", { labels = { foo = "bar" } }) setWebserverConfig({password="%s", apiKey="%s"}) """ @@ -899,3 +903,8 @@ def testCustomStats(self): for key in expected: self.assertIn(key, content) self.assertTrue(content[key] >= 0) + + unexpected = ['my-labeled-gauge', 'my-labeled-counter'] + + for key in unexpected: + self.assertNotIn(key, content) diff --git a/regression-tests.dnsdist/test_Prometheus.py b/regression-tests.dnsdist/test_Prometheus.py index f6d334038b0f..c4559f5d9b4e 100644 --- a/regression-tests.dnsdist/test_Prometheus.py +++ b/regression-tests.dnsdist/test_Prometheus.py @@ -30,8 +30,9 @@ class TestPrometheus(DNSDistTest): declareMetric('custom-metric3', 'counter', 'Custom counter', 'custom_prometheus_name') -- test prometheus labels in custom metrics - declareMetric('custom-metric-foo-x-bar-y-xyz', 'counter', 'Custom counter with labels', 'custom_metric_foo{x="bar",y="xyz"}') - declareMetric('custom-metric-foo-x-baz-y-abc', 'counter', 'Custom counter with labels', 'custom_metric_foo{x="baz",y="abc"}') + declareMetric('custom-metric-foo', 'counter', 'Custom counter with labels', { withLabels = true }) + incMetric('custom-metric-foo', { labels = { x = 'bar', y = 'xyz' } }) + incMetric('custom-metric-foo', { labels = { x = 'baz', y = 'abc' } }) """ def checkPrometheusContentBasic(self, content): @@ -47,12 +48,16 @@ def checkPrometheusContentBasic(self, content): tokens = line.split(' ') self.assertEqual(len(tokens), 2) if not line.startswith('dnsdist_') and not line.startswith('custom_'): - raise AssertionError('Expecting prometheus metric to be prefixed by \'dnsdist_\', got: "%s"' % (line)) + raise AssertionError( + 'Expecting prometheus metric to be prefixed by \'dnsdist_\', got: "%s"' % (line)) - def checkMetric(self, content, name, expectedType, expectedValue): + def checkMetric(self, content, name, expectedType, expectedValue, expectedLabels=""): typeFound = False helpFound = False valueFound = False + labelsFound = False + if expectedLabels == "": + labelsFound = True for line in content.splitlines(): if name in str(line): tokens = line.split(' ') @@ -70,10 +75,15 @@ def checkMetric(self, content, name, expectedType, expectedValue): if tokens[0] == name: valueFound = True self.assertEqual(int(tokens[1]), expectedValue) + elif tokens[0] == name + expectedLabels: + valueFound = True + labelsFound = True + self.assertEqual(int(tokens[1]), expectedValue) self.assertTrue(typeFound) self.assertTrue(helpFound) self.assertTrue(valueFound) + self.assertTrue(labelsFound) def checkPrometheusContentPromtool(self, content): output = None @@ -106,3 +116,5 @@ def testMetrics(self): self.checkMetric(r.text, 'dnsdist_custom_metric1', 'counter', 1) self.checkMetric(r.text, 'dnsdist_custom_metric2', 'gauge', 0) self.checkMetric(r.text, 'custom_prometheus_name', 'counter', 0) + self.checkMetric(r.text, 'dnsdist_custom_metric_foo', 'counter', 1, '{x="bar",y="xyz"}') + self.checkMetric(r.text, 'dnsdist_custom_metric_foo', 'counter', 1, '{x="baz",y="abc"}') From 15bee38419a91c4247c3c46e083e7b3966e847fc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ensar=20Saraj=C4=8Di=C4=87?= Date: Mon, 16 Dec 2024 11:31:01 +0100 Subject: [PATCH 12/16] dnsdist: add docs for new arguments for custom metric functions --- .../docs/reference/custommetrics.rst | 72 +++++++++++++++++-- 1 file changed, 65 insertions(+), 7 deletions(-) diff --git a/pdns/dnsdistdist/docs/reference/custommetrics.rst b/pdns/dnsdistdist/docs/reference/custommetrics.rst index b2f5b517be3d..1cda8f2329dc 100644 --- a/pdns/dnsdistdist/docs/reference/custommetrics.rst +++ b/pdns/dnsdistdist/docs/reference/custommetrics.rst @@ -10,13 +10,19 @@ Then you can update those at runtime using the following functions, depending on * manipulate counters using :func:`incMetric` and :func:`decMetric` * update a gauge using :func:`setMetric` -.. function:: declareMetric(name, type, description [, prometheusName]) -> bool +.. function:: declareMetric(name, type, description [, prometheusName|options]) -> bool .. versionadded:: 1.8.0 .. versionchanged:: 1.8.1 This function can now be used at runtime, instead of only at configuration time. + .. versionchanged:: ? + This function now takes options, with ``withLabels`` option added. ``prometheusName`` can now be provided in options. + + .. note:: + Labels are only available for prometheus. Metrics with labels are otherwise ignored. + Re-declaring an existing metric with the same name and type will not reset it. Re-declaring with the same name but a different type will cause one of them to be masked. @@ -25,51 +31,103 @@ Then you can update those at runtime using the following functions, depending on :param str name: The name of the metric, lowercase alphanumerical characters and dashes (-) only :param str type: The desired type in ``gauge`` or ``counter`` :param str description: The description of the metric - :param str prometheusName: The name to use in the prometheus metrics, if supplied. Otherwise the regular name will be used, prefixed with ``dnsdist_`` and ``-`` replaced by ``_``. + :param str prometheusName: The name to use in the prometheus metrics, if supplied. Otherwise the regular name will be used, prefixed with ``dnsdist_`` and ``-`` replaced by ``_`` + :param table options: A table with key: value pairs with metric options. + + Options: -.. function:: incMetric(name [, step]) -> int + * ``name``: str - The name to use in the prometheus metrics, if supplied. Otherwise the regular name will be used, prefixed with ``dnsdist_`` and ``-`` replaced by ``_`` + * ``withLabels=false``: bool - If set to true, labels will be expected when updating this metric and it will not be automatically created without labels. Defaults to ``false``, which automatically creates this metric without labels with default value. + +.. function:: incMetric(name [, step|options]) -> int .. versionadded:: 1.8.0 .. versionchanged:: 1.8.1 Optional ``step`` parameter added. + .. versionchanged:: ? + This function now takes options, with ``labels`` option added. ``step`` can now be provided in options. + + .. note:: + Labels are only available for prometheus. Metrics with labels are otherwise ignored. + Increment counter by one (or more, see the ``step`` parameter), will issue an error if the metric is not declared or not a ``counter``. Returns the new value. :param str name: The name of the metric - :param int step: By how much the counter should be incremented, default to 1. + :param int step: By how much the counter should be incremented, default to 1 + :param table options: A table with key: value pairs with metric options. + + Options: + + * ``step``: int - By ow much the counter should be incremented, default to 1 + * ``labels={}``: table - Set of key: value pairs with labels and their values that should be used to increment the metric. Different combinations of labels have different metric values. -.. function:: decMetric(name) -> int +.. function:: decMetric(name [, step|options]) -> int .. versionadded:: 1.8.0 .. versionchanged:: 1.8.1 Optional ``step`` parameter added. + .. versionchanged:: ? + This function now takes options, with ``labels`` option added. ``step`` can now be provided in options. + + .. note:: + Labels are only available for prometheus. Metrics with labels are otherwise ignored. + Decrement counter by one (or more, see the ``step`` parameter), will issue an error if the metric is not declared or not a ``counter``. Returns the new value. :param str name: The name of the metric :param int step: By how much the counter should be decremented, default to 1. + :param table options: A table with key: value pairs with metric options. -.. function:: getMetric(name) -> double + Options: + + * ``step``: int - By ow much the counter should be decremented, default to 1 + * ``labels={}``: table - Set of key: value pairs with labels and their values that should be used to decrement the metric. Different combinations of labels have different metric values. + +.. function:: getMetric(name [, options]) -> double .. versionadded:: 1.8.0 + .. versionchanged:: ? + This function now takes options, with ``labels`` option added. + + .. note:: + Labels are only available for prometheus. Metrics with labels are otherwise ignored. + Get metric value. :param str name: The name of the metric + :param table options: A table with key: value pairs with metric options. + + Options: -.. function:: setMetric(name, value) -> double + * ``labels={}``: table - Set of key: value pairs with labels and their values that should be used to read the metric. Different combinations of labels have different metric values. + +.. function:: setMetric(name, value [, options]) -> double .. versionadded:: 1.8.0 + .. versionchanged:: ? + This function now takes options, with ``labels`` option added. + + .. note:: + Labels are only available for prometheus. Metrics with labels are otherwise ignored. + Set the new value, will issue an error if the metric is not declared or not a ``gauge``. Return the new value. :param str name: The name of the metric :param double value: The new value + :param table options: A table with key: value pairs with metric options. + + Options: + + * ``labels={}``: table - Set of key: value pairs with labels and their values that should be used to set the metric. Different combinations of labels have different metric values. From 882718e14f10fe5fac4d5b6e29ee556a8ce4b514 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ensar=20Saraj=C4=8Di=C4=87?= Date: Mon, 16 Dec 2024 11:42:44 +0100 Subject: [PATCH 13/16] dnsdist: use `empty()` instead of "" check --- pdns/dnsdistdist/dnsdist-web.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pdns/dnsdistdist/dnsdist-web.cc b/pdns/dnsdistdist/dnsdist-web.cc index 55cbc1588dd3..91ad9110a7f4 100644 --- a/pdns/dnsdistdist/dnsdist-web.cc +++ b/pdns/dnsdistdist/dnsdist-web.cc @@ -931,7 +931,7 @@ static void addStatsToJSONObject(Json::object& obj) if (entry.d_name == "special-memory-usage") { continue; // Too expensive for get-all } - if (entry.d_labels != "") { + if (entry.d_labels.empty()) { continue; // Skip labeled metrics to prevent duplicates } if (const auto& val = std::get_if(&entry.d_value)) { From fc8822a4cf3ca36978c8d0b5d8ed61582a56462c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ensar=20Saraj=C4=8Di=C4=87?= Date: Mon, 16 Dec 2024 12:22:04 +0100 Subject: [PATCH 14/16] dnsdist: fix empty labels check in json stats endpoint --- pdns/dnsdistdist/dnsdist-web.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pdns/dnsdistdist/dnsdist-web.cc b/pdns/dnsdistdist/dnsdist-web.cc index 91ad9110a7f4..26c34edc4d14 100644 --- a/pdns/dnsdistdist/dnsdist-web.cc +++ b/pdns/dnsdistdist/dnsdist-web.cc @@ -931,7 +931,7 @@ static void addStatsToJSONObject(Json::object& obj) if (entry.d_name == "special-memory-usage") { continue; // Too expensive for get-all } - if (entry.d_labels.empty()) { + if (!entry.d_labels.empty()) { continue; // Skip labeled metrics to prevent duplicates } if (const auto& val = std::get_if(&entry.d_value)) { From c6258397632ec4de3e332d444878f642d2c1ef29 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ensar=20Saraj=C4=8Di=C4=87?= Date: Mon, 16 Dec 2024 17:10:28 +0100 Subject: [PATCH 15/16] dnsdist: set version changed for new custom metrics options in docs --- pdns/dnsdistdist/docs/reference/custommetrics.rst | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/pdns/dnsdistdist/docs/reference/custommetrics.rst b/pdns/dnsdistdist/docs/reference/custommetrics.rst index 1cda8f2329dc..a2b91e41413c 100644 --- a/pdns/dnsdistdist/docs/reference/custommetrics.rst +++ b/pdns/dnsdistdist/docs/reference/custommetrics.rst @@ -17,7 +17,7 @@ Then you can update those at runtime using the following functions, depending on .. versionchanged:: 1.8.1 This function can now be used at runtime, instead of only at configuration time. - .. versionchanged:: ? + .. versionchanged:: 2.0.0 This function now takes options, with ``withLabels`` option added. ``prometheusName`` can now be provided in options. .. note:: @@ -46,7 +46,7 @@ Then you can update those at runtime using the following functions, depending on .. versionchanged:: 1.8.1 Optional ``step`` parameter added. - .. versionchanged:: ? + .. versionchanged:: 2.0.0 This function now takes options, with ``labels`` option added. ``step`` can now be provided in options. .. note:: @@ -72,7 +72,7 @@ Then you can update those at runtime using the following functions, depending on .. versionchanged:: 1.8.1 Optional ``step`` parameter added. - .. versionchanged:: ? + .. versionchanged:: 2.0.0 This function now takes options, with ``labels`` option added. ``step`` can now be provided in options. .. note:: @@ -95,7 +95,7 @@ Then you can update those at runtime using the following functions, depending on .. versionadded:: 1.8.0 - .. versionchanged:: ? + .. versionchanged:: 2.0.0 This function now takes options, with ``labels`` option added. .. note:: @@ -114,7 +114,7 @@ Then you can update those at runtime using the following functions, depending on .. versionadded:: 1.8.0 - .. versionchanged:: ? + .. versionchanged:: 2.0.0 This function now takes options, with ``labels`` option added. .. note:: From b3f572fe3e421cdc1e0e3edaabd3bc4ec809914f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ensar=20Saraj=C4=8Di=C4=87?= Date: Mon, 16 Dec 2024 17:11:35 +0100 Subject: [PATCH 16/16] dnsdist: fix typos in custom metrics docs Co-authored-by: Remi Gacogne --- pdns/dnsdistdist/docs/reference/custommetrics.rst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pdns/dnsdistdist/docs/reference/custommetrics.rst b/pdns/dnsdistdist/docs/reference/custommetrics.rst index a2b91e41413c..d688a0a1c16c 100644 --- a/pdns/dnsdistdist/docs/reference/custommetrics.rst +++ b/pdns/dnsdistdist/docs/reference/custommetrics.rst @@ -62,7 +62,7 @@ Then you can update those at runtime using the following functions, depending on Options: - * ``step``: int - By ow much the counter should be incremented, default to 1 + * ``step``: int - By how much the counter should be incremented, default to 1 * ``labels={}``: table - Set of key: value pairs with labels and their values that should be used to increment the metric. Different combinations of labels have different metric values. .. function:: decMetric(name [, step|options]) -> int @@ -88,7 +88,7 @@ Then you can update those at runtime using the following functions, depending on Options: - * ``step``: int - By ow much the counter should be decremented, default to 1 + * ``step``: int - By how much the counter should be decremented, default to 1 * ``labels={}``: table - Set of key: value pairs with labels and their values that should be used to decrement the metric. Different combinations of labels have different metric values. .. function:: getMetric(name [, options]) -> double