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 790cdd870363f992082ec886cb239e69e26a6909..45b58877fb29f0811f7a628f55b00996c5ed3a0f 100644 (file)
--- 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 349e5eac45fa08ef6e94de4bd65b3732e20c1650..d203128b74b8fe61ab4003dbc3411007f8b90455 100644 (file)
--- 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 77427cceb28731756f06375aeee707caa874a132..94b7fedb60b8ddcbeebf2759455b44d03d3edc50 100644 (file)
--- 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 71272eb86c5f39aa83d9e3141517619d3bceffc2..d6679149b240d65f0008a4d4d0b61bc0262d357c 100644 (file)
--- 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 d9c57f6d82d9b74683f3324030b368bfad11cb8b..0f3d0bf8f9d6254e036a1a50f9f8469424547035 100644 (file)
--- 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;