From: Otto Moerbeek Date: Tue, 21 Oct 2025 11:33:30 +0000 (+0200) Subject: Handle traceid_only and other conditions correctly X-Git-Tag: rec-5.4.0-alpha1~103^2~13 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=50e1653c844797efb7cc22c5dde43f99330a9371;p=thirdparty%2Fpdns.git Handle traceid_only and other conditions correctly Signed-off-by: Otto Moerbeek --- diff --git a/pdns/recursordist/pdns_recursor.cc b/pdns/recursordist/pdns_recursor.cc index 45b58877fb..77f595bea4 100644 --- a/pdns/recursordist/pdns_recursor.cc +++ b/pdns/recursordist/pdns_recursor.cc @@ -1861,7 +1861,8 @@ void startDoResolve(void* arg) // NOLINT(readability-function-cognitive-complexi if (resolver.d_eventTrace.enabled() && SyncRes::eventTraceEnabled(SyncRes::event_trace_to_pb)) { pbMessage.addEvents(resolver.d_eventTrace); } - if (resolver.d_eventTrace.enabled() && SyncRes::eventTraceEnabled(SyncRes::event_trace_to_ot)) { + + if (resolver.d_eventTrace.enabled() && resolver.d_eventTrace.getThisOTTraceEnabled() && SyncRes::eventTraceEnabled(SyncRes::event_trace_to_ot)) { auto otTrace = pdns::trace::TracesData::boilerPlate("rec", resolver.d_eventTrace.convertToOT(resolver.d_otTrace), { {"query.qname", {comboWriter->d_mdp.d_qname.toLogString()}}, {"query.qtype", {QType(comboWriter->d_mdp.d_qtype).toString()}}, @@ -2158,7 +2159,7 @@ bool matchOTConditions(const std::unique_ptr& cond return true; } -bool matchOTConditions(const std::unique_ptr& conditions, const ComboAddress& source, const DNSName& qname, QType qtype, uint16_t qid, bool edns_option_present) +bool matchOTConditions(RecEventTrace& eventTrace, const std::unique_ptr& conditions, const ComboAddress& source, const DNSName& qname, QType qtype, uint16_t qid, bool edns_option_present) { if (conditions == nullptr || conditions->size() == 0) { return false; @@ -2181,6 +2182,7 @@ bool matchOTConditions(const std::unique_ptr& cond return false; } } + eventTrace.setThisOTTraceEnabled(); return true; } @@ -2290,7 +2292,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 (SyncRes::eventTraceEnabledOnly(SyncRes::event_trace_to_ot) && !matchOTConditions(t_OTConditions, mappedSource, qname, qtype, ntohs(headerdata->id), ednsFound)) { + if (!matchOTConditions(eventTrace, t_OTConditions, mappedSource, qname, qtype, ntohs(headerdata->id), ednsFound) && SyncRes::eventTraceEnabledOnly(SyncRes::event_trace_to_ot)) { eventTrace.setEnabled(false); } } diff --git a/pdns/recursordist/rec-eventtrace.hh b/pdns/recursordist/rec-eventtrace.hh index 31bb509f0c..34fe36067d 100644 --- a/pdns/recursordist/rec-eventtrace.hh +++ b/pdns/recursordist/rec-eventtrace.hh @@ -71,7 +71,8 @@ public: RecEventTrace(const RecEventTrace& old) : d_events(old.d_events), d_base(old.d_base), - d_status(old.d_status) + d_status(old.d_status), + d_OTTrace(old.d_OTTrace) { // An RecEventTrace object can be copied, but the original will be marked invalid. // This is do detect (very likely) unintended modifications to the original after @@ -82,7 +83,8 @@ public: RecEventTrace(RecEventTrace&& old) noexcept : d_events(std::move(old.d_events)), d_base(old.d_base), - d_status(old.d_status) + d_status(old.d_status), + d_OTTrace(old.d_OTTrace) { // An RecEventTrace object can be moved, but the original will be marked invalid. // This is do detect (very likely) unintended modifications to the original after @@ -96,6 +98,7 @@ public: d_events = std::move(old.d_events); d_base = old.d_base; d_status = old.d_status; + d_OTTrace = old.d_OTTrace; old.d_status = Invalid; return *this; } @@ -260,6 +263,7 @@ public: struct timespec theTime{}; clock_gettime(CLOCK_MONOTONIC, &theTime); d_base = theTime.tv_nsec + theTime.tv_sec * 1000000000; + d_OTTrace = false; d_status = Disabled; } @@ -298,6 +302,16 @@ public: return old; } + bool getThisOTTraceEnabled() const + { + return d_OTTrace; + } + + void setThisOTTraceEnabled() + { + d_OTTrace = true; + } + // The EventScope class is used to close (add an end event) automatically upon the scope object // going out of scope. It is also possible to manually close it, specifying a value to be registered // at the close event. In that case the dt call will become a no-op. @@ -355,4 +369,5 @@ private: Enabled }; mutable Status d_status{Disabled}; + bool d_OTTrace{false}; }; diff --git a/pdns/recursordist/rec-main.cc b/pdns/recursordist/rec-main.cc index d203128b74..03f3cde46e 100644 --- a/pdns/recursordist/rec-main.cc +++ b/pdns/recursordist/rec-main.cc @@ -639,7 +639,8 @@ void protobufLogResponse(const DNSName& qname, QType qtype, if (eventTrace.enabled() && SyncRes::eventTraceEnabled(SyncRes::event_trace_to_pb)) { pbMessage.addEvents(eventTrace); } - if (eventTrace.enabled() && SyncRes::eventTraceEnabled(SyncRes::event_trace_to_ot)) { + + if (eventTrace.enabled() && eventTrace.getThisOTTraceEnabled() && 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 94b7fedb60..d5361b8234 100644 --- a/pdns/recursordist/rec-main.hh +++ b/pdns/recursordist/rec-main.hh @@ -647,7 +647,7 @@ void requestWipeCaches(const DNSName& canon); void startDoResolve(void*); bool expectProxyProtocol(const ComboAddress& from, const ComboAddress& listenAddress); bool matchOTConditions(const std::unique_ptr& conditions, const ComboAddress& source); -bool matchOTConditions(const std::unique_ptr& conditions, const ComboAddress& source, const DNSName& qname, QType qtype, uint16_t qid, bool edns_option_present); +bool matchOTConditions(RecEventTrace& eventTrace, const std::unique_ptr& conditions, const ComboAddress& source, const DNSName& qname, QType qtype, uint16_t qid, bool edns_option_present); void finishTCPReply(std::unique_ptr&, 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 d6679149b2..cb1be7c0fd 100644 --- a/pdns/recursordist/rec-tcp.cc +++ b/pdns/recursordist/rec-tcp.cc @@ -302,7 +302,7 @@ static void doProcessTCPQuestion(std::unique_ptr& comboWriter, s boost::optional 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)) { + if (!matchOTConditions(t_OTConditions, comboWriter->d_mappedSource) && SyncRes::eventTraceEnabledOnly(SyncRes::event_trace_to_ot)) { comboWriter->d_eventTrace.setEnabled(false); } @@ -341,7 +341,7 @@ static void doProcessTCPQuestion(std::unique_ptr& comboWriter, s if (SyncRes::eventTraceEnabled(SyncRes::event_trace_to_ot)) { 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)) { + if (SyncRes::eventTraceEnabledOnly(SyncRes::event_trace_to_ot) && !matchOTConditions(comboWriter->d_eventTrace, t_OTConditions, comboWriter->d_mappedSource, qname, qtype, ntohs(comboWriter->d_mdp.d_header.id), ednsFound)) { comboWriter->d_eventTrace.setEnabled(false); } }