From: Otto Moerbeek <otto.moerbeek@open-xchange.com>
Date: Tue, 21 Oct 2025 10:14:55 +0000 (+0200)
Subject: Add TCP case, do early match when we know the source
X-Git-Tag: rec-5.4.0-alpha1~103^2~14
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=343159f3e3c22922f84c32ccdd3a4a665ad6ce6a;p=thirdparty%2Fpdns.git

Add TCP case, do early match when we know the source

One case goes wrong: old-style tracing enabled and OT tracing enabled, but traceid_only is set

Signed-off-by: Otto Moerbeek <otto.moerbeek@open-xchange.com>
---

diff --git a/pdns/recursordist/pdns_recursor.cc b/pdns/recursordist/pdns_recursor.cc
index 790cdd8703..45b58877fb 100644
--- a/pdns/recursordist/pdns_recursor.cc
+++ b/pdns/recursordist/pdns_recursor.cc
@@ -2144,37 +2144,43 @@ bool expectProxyProtocol(const ComboAddress& from, const ComboAddress& listenAdd
   return false;
 }
 
-static bool match(const std::unique_ptr<OpenTelemetryTraceConditions>& conditions, const ComboAddress& source, const DNSName& qname, QType qtype, uint16_t qid, bool edns_option_present)
+bool matchOTConditions(const std::unique_ptr<OpenTelemetryTraceConditions>& conditions, const ComboAddress& source)
+{
+  if (conditions == nullptr || conditions->size() == 0) {
+    return false;
+  }
+  if (auto const* match = conditions->lookup(source); match != nullptr) {
+    const auto& condition = match->second;
+    if (condition.d_traceid_only) {
+      return false;
+    }
+  }
+  return true;
+}
+
+bool matchOTConditions(const std::unique_ptr<OpenTelemetryTraceConditions>& conditions, const ComboAddress& source, const DNSName& qname, QType qtype, uint16_t qid, bool edns_option_present)
 {
   if (conditions == nullptr || conditions->size() == 0) {
-    cerr << "match 0 false" << endl;
     return false;
   }
   if (auto const* match = conditions->lookup(source); match != nullptr) {
-    cerr << "match 1" << endl;
     const auto& condition = match->second;
     if (condition.d_traceid_only) {
-      cerr << "match 2 false" << endl;
       return false;
     }
     if (condition.d_edns_option_required && !edns_option_present) {
-      cerr << "match 3 false" << endl;
       return false;
     }
     if (condition.d_qid && condition.d_qid != qid) {
-    cerr << "match 4 false" << endl;
       return false;
     }
     if (condition.d_qtypes && condition.d_qtypes->count(qtype) == 0) {
-    cerr << "match 5 false" << endl;
       return false;
     }
     if (condition.d_qnames && !condition.d_qnames->check(qname)) {
-    cerr << "match 6 false" << endl;
       return false;
     }
   }
-  cerr << "match return true" << endl;
   return true;
 }
 
@@ -2284,7 +2290,7 @@ static string* doProcessUDPQuestion(const std::string& question, const ComboAddr
 
         if (SyncRes::eventTraceEnabled(SyncRes::event_trace_to_ot)) {
           bool ednsFound = pdns::trace::extractOTraceIDs(ednsOptions, otTrace);
-          if (!match(t_OTConditions, source, qname, qtype, ntohs(headerdata->id), ednsFound)) {
+          if (SyncRes::eventTraceEnabledOnly(SyncRes::event_trace_to_ot) && !matchOTConditions(t_OTConditions, mappedSource, qname, qtype, ntohs(headerdata->id), ednsFound)) {
             eventTrace.setEnabled(false);
           }
         }
@@ -2648,6 +2654,9 @@ static void handleNewUDPQuestion(int fileDesc, FDMultiplexer::funcparam_t& /* va
             destination = destaddr;
           }
 
+          if (SyncRes::eventTraceEnabledOnly(SyncRes::event_trace_to_ot) && !matchOTConditions(t_OTConditions, mappedSource)) {
+              eventTrace.setEnabled(false);
+          }
           eventTrace.add(RecEventTrace::ReqRecv, 0, false, match);
           if (RecThreadInfo::weDistributeQueries()) {
             std::string localdata = data;
diff --git a/pdns/recursordist/rec-main.cc b/pdns/recursordist/rec-main.cc
index 349e5eac45..d203128b74 100644
--- a/pdns/recursordist/rec-main.cc
+++ b/pdns/recursordist/rec-main.cc
@@ -636,10 +636,10 @@ void protobufLogResponse(const DNSName& qname, QType qtype,
     pbMessage.setNewlyObservedDomain(false);
   }
 #endif
-  if (eventTrace.enabled() && (SyncRes::s_event_trace_enabled & SyncRes::event_trace_to_pb) != 0) {
+  if (eventTrace.enabled() && SyncRes::eventTraceEnabled(SyncRes::event_trace_to_pb)) {
     pbMessage.addEvents(eventTrace);
   }
-  if (eventTrace.enabled() && (SyncRes::s_event_trace_enabled & SyncRes::event_trace_to_ot) != 0) {
+  if (eventTrace.enabled() && SyncRes::eventTraceEnabled(SyncRes::event_trace_to_ot)) {
     auto trace = pdns::trace::TracesData::boilerPlate("rec", eventTrace.convertToOT(otTrace),
                                                       {{"query.qname", {qname.toLogString()}},
                                                        {"query.qtype", {qtype.toString()}}},
diff --git a/pdns/recursordist/rec-main.hh b/pdns/recursordist/rec-main.hh
index 77427cceb2..94b7fedb60 100644
--- a/pdns/recursordist/rec-main.hh
+++ b/pdns/recursordist/rec-main.hh
@@ -646,6 +646,8 @@ void protobufLogResponse(const DNSName& qname, QType qtype, const struct dnshead
 void requestWipeCaches(const DNSName& canon);
 void startDoResolve(void*);
 bool expectProxyProtocol(const ComboAddress& from, const ComboAddress& listenAddress);
+bool matchOTConditions(const std::unique_ptr<OpenTelemetryTraceConditions>& conditions, const ComboAddress& source);
+bool matchOTConditions(const std::unique_ptr<OpenTelemetryTraceConditions>& conditions, const ComboAddress& source, const DNSName& qname, QType qtype, uint16_t qid, bool edns_option_present);
 void finishTCPReply(std::unique_ptr<DNSComboWriter>&, bool hadError, bool updateInFlight);
 void checkFastOpenSysctl(bool active, Logr::log_t);
 void checkTFOconnect(Logr::log_t);
diff --git a/pdns/recursordist/rec-tcp.cc b/pdns/recursordist/rec-tcp.cc
index 71272eb86c..d6679149b2 100644
--- a/pdns/recursordist/rec-tcp.cc
+++ b/pdns/recursordist/rec-tcp.cc
@@ -302,6 +302,10 @@ static void doProcessTCPQuestion(std::unique_ptr<DNSComboWriter>& comboWriter, s
   boost::optional<uint32_t> ednsVersion;
 
   comboWriter->d_eventTrace.setEnabled(SyncRes::s_event_trace_enabled != 0);
+  if (SyncRes::eventTraceEnabledOnly(SyncRes::event_trace_to_ot) && !matchOTConditions(t_OTConditions, comboWriter->d_mappedSource)) {
+    comboWriter->d_eventTrace.setEnabled(false);
+  }
+
   // eventTrace uses monotonic time, while OpenTelemetry uses absolute time. setEnabled()
   // established the reference point, get an absolute TS as close as possible to the
   // eventTrace start of trace time.
@@ -336,7 +340,10 @@ static void doProcessTCPQuestion(std::unique_ptr<DNSComboWriter>& comboWriter, s
       qnameParsed = true;
 
       if (SyncRes::eventTraceEnabled(SyncRes::event_trace_to_ot)) {
-        pdns::trace::extractOTraceIDs(ednsOptions, comboWriter->d_otTrace);
+        bool ednsFound = pdns::trace::extractOTraceIDs(ednsOptions, comboWriter->d_otTrace);
+        if (SyncRes::eventTraceEnabledOnly(SyncRes::event_trace_to_ot) && !matchOTConditions(t_OTConditions, comboWriter->d_mappedSource, qname, qtype, ntohs(comboWriter->d_mdp.d_header.id), ednsFound)) {
+          comboWriter->d_eventTrace.setEnabled(false);
+        }
       }
       if (t_pdl) {
         try {
diff --git a/pdns/recursordist/syncres.hh b/pdns/recursordist/syncres.hh
index d9c57f6d82..0f3d0bf8f9 100644
--- a/pdns/recursordist/syncres.hh
+++ b/pdns/recursordist/syncres.hh
@@ -582,6 +582,10 @@ public:
   {
     return (s_event_trace_enabled & flag) != 0;
   }
+  static bool eventTraceEnabledOnly(int flag)
+  {
+    return s_event_trace_enabled == flag;
+  }
   std::unordered_map<std::string, bool> d_discardedPolicies;
   DNSFilterEngine::Policy d_appliedPolicy;
   std::unordered_set<std::string> d_policyTags;