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 b572463689f90fb35aa0e4736ff7271629ed5f62..1c5b3cca4c3d68d100adaeb23fed3fa85f548ac3 100644 (file)
--- a/pdns/dnsdistdist/dnsdist-idstate.cc
+++ b/pdns/dnsdistdist/dnsdist-idstate.cc
@@ -117,3 +117,20 @@ std::optional<pdns::trace::dnsdist::Tracer::Closer> InternalQueryState::getClose
 #endif
   return ret;
 }
+
+std::optional<pdns::trace::dnsdist::Tracer::Closer> InternalQueryState::getRulesCloser([[maybe_unused]] const std::string& ruleName, [[maybe_unused]] const std::string& parentSpanName)
+{
+  std::optional<pdns::trace::dnsdist::Tracer::Closer> 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<pdns::trace::dnsdist::Tracer::Closer>(d_OTTracer->openSpan(name, parentSpanID));
+  }
+#endif
+  return ret;
+}

diff --git a/pdns/dnsdistdist/dnsdist-idstate.hh b/pdns/dnsdistdist/dnsdist-idstate.hh
index 59f2ffe5170ec0486e615f4b7046f929a63b3280..2587ec7874a2efe085050f68c58abd7ae8e73031 100644 (file)
--- a/pdns/dnsdistdist/dnsdist-idstate.hh
+++ b/pdns/dnsdistdist/dnsdist-idstate.hh
@@ -150,6 +150,7 @@ struct InternalQueryState
   std::optional<pdns::trace::dnsdist::Tracer::Closer> getCloser([[maybe_unused]] const std::string& name, [[maybe_unused]] const SpanID& parentSpanID);
   std::optional<pdns::trace::dnsdist::Tracer::Closer> getCloser([[maybe_unused]] const std::string& name, [[maybe_unused]] const string& parentSpanName);
   std::optional<pdns::trace::dnsdist::Tracer::Closer> getCloser([[maybe_unused]] const std::string& name);
+  std::optional<pdns::trace::dnsdist::Tracer::Closer> 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 3e28057ce8816a0803163f7a286b9860474146e5..6f58c7e9a985ab84031cf9e456a72dffafd1002c 100644 (file)
--- 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<dnsdist::rules::ResponseRuleAction>& 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<dnsdist::rules::ResponseRuleAction>&
   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<dnsdist::rules::ResponseRuleAction>&
 
 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<dnsdist::rules::RuleAction>
   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<dnsdist::rules::RuleAction>
 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<CrossProtocolQuery> getUDPCrossProtocolQueryFromDQ(DNSQuestion&
 ProcessQueryResult processQuery(DNSQuestion& dnsQuestion, std::shared_ptr<DownstreamState>& 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<Downst
 
 bool assignOutgoingUDPQueryToBackend(std::shared_ptr<DownstreamState>& 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 8e2c20ec4b68ce6fa30cbe9aa0f17a41e701e6b8..50a8aea89d936f9b3db208c2d440aa686da009b1 100644 (file)
--- 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",