From: Pieter Lexis Date: Wed, 12 Nov 2025 12:22:16 +0000 (+0100) Subject: feat(dnsdist): Add `InternalQueryState::getRulesCloser` X-Git-Tag: rec-5.4.0-alpha1~72^2~2 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=24904413cfa9227e76e4b8ae2049f608e1e373bb;p=thirdparty%2Fpdns.git feat(dnsdist): Add `InternalQueryState::getRulesCloser` Also use `__func__` to prevent typos in tracer function names. --- diff --git a/pdns/dnsdistdist/dnsdist-idstate.cc b/pdns/dnsdistdist/dnsdist-idstate.cc index b572463689..1c5b3cca4c 100644 --- a/pdns/dnsdistdist/dnsdist-idstate.cc +++ b/pdns/dnsdistdist/dnsdist-idstate.cc @@ -117,3 +117,20 @@ std::optional InternalQueryState::getClose #endif return ret; } + +std::optional InternalQueryState::getRulesCloser([[maybe_unused]] const std::string& ruleName, [[maybe_unused]] const std::string& parentSpanName) +{ + std::optional ret(std::nullopt); +#ifndef DISABLE_PROTOBUF + static const std::string prefix = "Rule: "; + // getTracer returns a Tracer when tracing is globally enabled + // tracingEnabled tells us whether or not tracing is enabled for this query + // Should tracing be disabled, *but* we have not processed query rules, we will still return a closer if tracing is globally enabled + if (auto tracer = getTracer(); tracer != nullptr && (tracingEnabled || !rulesAppliedToQuery)) { + auto parentSpanID = d_OTTracer->getLastSpanIDForName(parentSpanName); + auto name = prefix + ruleName; + ret = std::optional(d_OTTracer->openSpan(name, parentSpanID)); + } +#endif + return ret; +} diff --git a/pdns/dnsdistdist/dnsdist-idstate.hh b/pdns/dnsdistdist/dnsdist-idstate.hh index 59f2ffe517..2587ec7874 100644 --- a/pdns/dnsdistdist/dnsdist-idstate.hh +++ b/pdns/dnsdistdist/dnsdist-idstate.hh @@ -150,6 +150,7 @@ struct InternalQueryState std::optional getCloser([[maybe_unused]] const std::string& name, [[maybe_unused]] const SpanID& parentSpanID); std::optional getCloser([[maybe_unused]] const std::string& name, [[maybe_unused]] const string& parentSpanName); std::optional getCloser([[maybe_unused]] const std::string& name); + std::optional getRulesCloser([[maybe_unused]] const std::string& ruleName, [[maybe_unused]] const std::string& parentSpanName); InternalQueryState() { diff --git a/pdns/dnsdistdist/dnsdist.cc b/pdns/dnsdistdist/dnsdist.cc index 3e28057ce8..6f58c7e9a9 100644 --- a/pdns/dnsdistdist/dnsdist.cc +++ b/pdns/dnsdistdist/dnsdist.cc @@ -456,8 +456,7 @@ static bool encryptResponse(PacketBuffer& response, size_t maximumSize, bool tcp bool applyRulesToResponse(const std::vector& respRuleActions, DNSResponse& dnsResponse) { - static const std::string traceEventName{"applyRulesToResponse"}; - auto closer = dnsResponse.ids.getCloser(traceEventName); + auto closer = dnsResponse.ids.getCloser(__func__); if (respRuleActions.empty()) { return true; } @@ -466,7 +465,7 @@ bool applyRulesToResponse(const std::vector& std::string ruleresult; for (const auto& rrule : respRuleActions) { - auto ruleCloser = dnsResponse.ids.getCloser("Rule: " + rrule.d_name, traceEventName); + auto ruleCloser = dnsResponse.ids.getRulesCloser(rrule.d_name, __func__); if (rrule.d_rule->matches(&dnsResponse)) { ++rrule.d_rule->d_matches; action = (*rrule.d_action)(&dnsResponse, &ruleresult); @@ -520,8 +519,7 @@ bool applyRulesToResponse(const std::vector& bool processResponseAfterRules(PacketBuffer& response, DNSResponse& dnsResponse, [[maybe_unused]] bool muted) { - static const std::string traceEventName{"processResponseAfterRules"}; - auto closer = dnsResponse.ids.getCloser(traceEventName, "processResponse"); + auto closer = dnsResponse.ids.getCloser(__func__, "processResponse"); bool zeroScope = false; if (!fixUpResponse(response, dnsResponse.ids.qname, dnsResponse.ids.origFlags, dnsResponse.ids.ednsAdded, dnsResponse.ids.ecsAdded, dnsResponse.ids.useZeroScope ? &zeroScope : nullptr)) { if (closer) { @@ -553,7 +551,7 @@ bool processResponseAfterRules(PacketBuffer& response, DNSResponse& dnsResponse, cacheKey = dnsResponse.ids.cacheKeyNoECS; } { - auto cacheInsertCloser = dnsResponse.ids.getCloser("packetCacheInsert", traceEventName); + auto cacheInsertCloser = dnsResponse.ids.getCloser("packetCacheInsert", __func__); dnsResponse.ids.packetCache->insert(cacheKey, zeroScope ? boost::none : dnsResponse.ids.subnet, dnsResponse.ids.cacheFlags, dnsResponse.ids.dnssecOK ? *dnsResponse.ids.dnssecOK : false, dnsResponse.ids.qname, dnsResponse.ids.qtype, dnsResponse.ids.qclass, response, dnsResponse.ids.forwardedOverUDP, dnsResponse.getHeader()->rcode, dnsResponse.ids.tempFailureTTL); } const auto& chains = dnsdist::configuration::getCurrentRuntimeConfiguration().d_ruleChains; @@ -585,7 +583,7 @@ bool processResponseAfterRules(PacketBuffer& response, DNSResponse& dnsResponse, bool processResponse(PacketBuffer& response, DNSResponse& dnsResponse, bool muted) { // This is a new root span - auto closer = dnsResponse.ids.getCloser("processResponse", SpanID{}); + auto closer = dnsResponse.ids.getCloser(__func__, SpanID{}); const auto& chains = dnsdist::configuration::getCurrentRuntimeConfiguration().d_ruleChains; const auto& respRuleActions = dnsdist::rules::getResponseRuleChain(chains, dnsdist::rules::ResponseRuleChain::ResponseRules); @@ -1024,11 +1022,10 @@ static bool applyRulesChainToQuery(const std::vector string ruleresult; bool drop = false; - static const std::string traceEventName{"applyRulesToChainQuery"}; - auto closer = dnsQuestion.ids.getCloser(traceEventName); + auto closer = dnsQuestion.ids.getCloser(__func__); for (const auto& rule : rules) { - auto ruleCloser = dnsQuestion.ids.getCloser("Rule: " + rule.d_name, traceEventName); + auto ruleCloser = dnsQuestion.ids.getRulesCloser(rule.d_name, __func__); if (!rule.d_rule->matches(&dnsQuestion)) { continue; @@ -1047,7 +1044,7 @@ static bool applyRulesChainToQuery(const std::vector static bool applyRulesToQuery(DNSQuestion& dnsQuestion, const timespec& now) { InternalQueryState::rulesAppliedToQuerySetter tpprs(dnsQuestion.ids.rulesAppliedToQuery); // Ensure IDS knows we are past the rules processing when we exit this function - auto closer = dnsQuestion.ids.getCloser("applyRulesToQuery"); + auto closer = dnsQuestion.ids.getCloser(__func__); if (g_rings.shouldRecordQueries()) { g_rings.insertQuery(now, dnsQuestion.ids.origRemote, dnsQuestion.ids.qname, dnsQuestion.ids.qtype, dnsQuestion.getData().size(), *dnsQuestion.getHeader(), dnsQuestion.getProtocol()); } @@ -1456,7 +1453,7 @@ static ProcessQueryResult handleQueryTurnedIntoSelfAnsweredResponse(DNSQuestion& static ServerPolicy::SelectedBackend selectBackendForOutgoingQuery(DNSQuestion& dnsQuestion, const ServerPool& serverPool) { // Not exactly processQuery, but it works for now - auto closer = dnsQuestion.ids.getCloser("selectBackendForOutgoingQuery", "processQuery"); + auto closer = dnsQuestion.ids.getCloser(__func__, "processQuery"); const auto& policy = serverPool.policy != nullptr ? *serverPool.policy : *dnsdist::configuration::getCurrentRuntimeConfiguration().d_lbPolicy; const auto& servers = serverPool.getServers(); @@ -1782,7 +1779,7 @@ std::unique_ptr getUDPCrossProtocolQueryFromDQ(DNSQuestion& ProcessQueryResult processQuery(DNSQuestion& dnsQuestion, std::shared_ptr& selectedBackend) { - auto closer = dnsQuestion.ids.getCloser("processQuery"); + auto closer = dnsQuestion.ids.getCloser(__func__); const uint16_t queryId = ntohs(dnsQuestion.getHeader()->id); try { /* we need an accurate ("real") value for the response and @@ -1814,7 +1811,7 @@ ProcessQueryResult processQuery(DNSQuestion& dnsQuestion, std::shared_ptr& downstream, uint16_t queryID, DNSQuestion& dnsQuestion, PacketBuffer& query, bool actuallySend) { - auto closer = dnsQuestion.ids.getCloser("assignOutgoingUDPQueryToBackend", "processUDPQuery"); + auto closer = dnsQuestion.ids.getCloser(__func__, "processUDPQuery"); bool doh = dnsQuestion.ids.du != nullptr; @@ -1890,7 +1887,7 @@ static void processUDPQuery(ClientState& clientState, const struct msghdr* msgh, uint16_t queryId = 0; InternalQueryState ids; - auto closer = ids.getCloser("processUDPQuery", SpanID{}); + auto closer = ids.getCloser(__func__, SpanID{}); ids.cs = &clientState; ids.origRemote = remote; diff --git a/regression-tests.dnsdist/test_OpenTelemetryTracing.py b/regression-tests.dnsdist/test_OpenTelemetryTracing.py index 8e2c20ec4b..50a8aea89d 100644 --- a/regression-tests.dnsdist/test_OpenTelemetryTracing.py +++ b/regression-tests.dnsdist/test_OpenTelemetryTracing.py @@ -138,7 +138,7 @@ class DNSDistOpenTelemetryProtobufBaseTest(DNSDistOpenTelemetryProtobufTest): "processQuery", "applyRulesToQuery", "Rule: Enable tracing", - "applyRulesToChainQuery", + "applyRulesChainToQuery", "selectBackendForOutgoingQuery", "processResponse", "applyRulesToResponse",