From 5aec557fdddf8fc569565ac6fb18dac5d9d032e7 Mon Sep 17 00:00:00 2001 From: Otto Moerbeek Date: Wed, 25 Jun 2025 11:49:20 +0200 Subject: [PATCH] Use a more leightweight struct to pass the initial Span data, we're only using a few fields Signed-off-by: Otto Moerbeek --- pdns/protozero-trace.cc | 2 +- pdns/protozero-trace.hh | 20 +++++++++++++++++++- pdns/recursordist/pdns_recursor.cc | 6 ++---- pdns/recursordist/rec-eventtrace.cc | 4 ++-- pdns/recursordist/rec-eventtrace.hh | 2 +- pdns/recursordist/rec-main.cc | 3 +-- pdns/recursordist/rec-main.hh | 4 ++-- pdns/recursordist/rec-tcp.cc | 2 +- pdns/recursordist/syncres.hh | 2 +- 9 files changed, 30 insertions(+), 15 deletions(-) diff --git a/pdns/protozero-trace.cc b/pdns/protozero-trace.cc index 059e2a6796..3599b688d8 100644 --- a/pdns/protozero-trace.cc +++ b/pdns/protozero-trace.cc @@ -506,7 +506,7 @@ KeyValue KeyValue::decode(protozero::pbf_reader& reader) return value; } -void extractOTraceIDs(const EDNSOptionViewMap& map, pdns::trace::Span& span) +void extractOTraceIDs(const EDNSOptionViewMap& map, pdns::trace::InitialSpanInfo& span) { // traceid gets set from edns options (if available and well-formed), otherwise random // parent_span_id gets set from edns options (if available and well-formed, otherwise it remains cleared (no parent)) diff --git a/pdns/protozero-trace.hh b/pdns/protozero-trace.hh index c07acc6ac7..1c1250adb6 100644 --- a/pdns/protozero-trace.hh +++ b/pdns/protozero-trace.hh @@ -339,6 +339,24 @@ inline uint64_t timestamp() return (1000000000ULL * now.tv_sec) + now.tv_nsec; } +// This struct is used to store the info of the initial span. As it is passed around resolving +// queries, it needs to be as small as possible, hence no full Span. +struct InitialSpanInfo +{ + TraceID trace_id{}; + SpanID span_id{}; + SpanID parent_span_id{}; + uint64_t start_time_unix_nano{0}; + + void clear() + { + pdns::trace::clear(trace_id); + pdns::trace::clear(span_id); + pdns::trace::clear(parent_span_id); + start_time_unix_nano = 0; + } +}; + struct Span { // A unique identifier for a trace. All spans from the same trace share @@ -694,6 +712,6 @@ inline KeyValueList KeyValueList::decode(protozero::pbf_reader& reader) return pdns::trace::decode(reader); } -void extractOTraceIDs(const EDNSOptionViewMap& map, pdns::trace::Span& span); +void extractOTraceIDs(const EDNSOptionViewMap& map, pdns::trace::InitialSpanInfo& span); } // namespace pdns::trace diff --git a/pdns/recursordist/pdns_recursor.cc b/pdns/recursordist/pdns_recursor.cc index 8df1a3bf47..e18c24e891 100644 --- a/pdns/recursordist/pdns_recursor.cc +++ b/pdns/recursordist/pdns_recursor.cc @@ -1875,7 +1875,6 @@ void startDoResolve(void* arg) // NOLINT(readability-function-cognitive-complexi pbMessage.addEvents(resolver.d_eventTrace); } if (resolver.d_eventTrace.enabled() && SyncRes::eventTraceEnabled(SyncRes::event_trace_to_ot)) { - resolver.d_otTrace.close(); auto otTrace = pdns::trace::TracesData::boilerPlate("rec", comboWriter->d_mdp.d_qname.toLogString() + '/' + QType(comboWriter->d_mdp.d_qtype).toString(), resolver.d_eventTrace.convertToOT(resolver.d_otTrace)); string otData = otTrace.encode(); pbMessage.setOpenTelemetryData(otData); @@ -2177,7 +2176,7 @@ bool expectProxyProtocol(const ComboAddress& from, const ComboAddress& listenAdd // source: the address we assume the query is coming from, might be set by proxy protocol // destination: the address we assume the query was sent to, might be set by proxy protocol // mappedSource: the address we assume the query is coming from. Differs from source if table based mapping has been applied -static string* doProcessUDPQuestion(const std::string& question, const ComboAddress& fromaddr, const ComboAddress& destaddr, ComboAddress source, ComboAddress destination, const ComboAddress& mappedSource, struct timeval tval, int fileDesc, std::vector& proxyProtocolValues, RecEventTrace& eventTrace, pdns::trace::Span& otTrace) // NOLINT(readability-function-cognitive-complexity): https://github.com/PowerDNS/pdns/issues/12791 +static string* doProcessUDPQuestion(const std::string& question, const ComboAddress& fromaddr, const ComboAddress& destaddr, ComboAddress source, ComboAddress destination, const ComboAddress& mappedSource, struct timeval tval, int fileDesc, std::vector& proxyProtocolValues, RecEventTrace& eventTrace, pdns::trace::InitialSpanInfo& otTrace) // NOLINT(readability-function-cognitive-complexity): https://github.com/PowerDNS/pdns/issues/12791 { RecThreadInfo::self().incNumberOfDistributedQueries(); gettimeofday(&g_now, nullptr); @@ -2475,7 +2474,7 @@ static void handleNewUDPQuestion(int fileDesc, FDMultiplexer::funcparam_t& /* va bool firstQuery = true; std::vector proxyProtocolValues; RecEventTrace eventTrace; - pdns::trace::Span otTrace; + pdns::trace::InitialSpanInfo otTrace; for (size_t queriesCounter = 0; queriesCounter < g_maxUDPQueriesPerRound; queriesCounter++) { bool proxyProto = false; @@ -2495,7 +2494,6 @@ static void handleNewUDPQuestion(int fileDesc, FDMultiplexer::funcparam_t& /* va if (SyncRes::eventTraceEnabled(SyncRes::event_trace_to_ot)) { otTrace.clear(); otTrace.start_time_unix_nano = traceTS; - otTrace.name = "RecRequest"; } firstQuery = false; diff --git a/pdns/recursordist/rec-eventtrace.cc b/pdns/recursordist/rec-eventtrace.cc index 360ad29e3d..aeb2d48078 100644 --- a/pdns/recursordist/rec-eventtrace.cc +++ b/pdns/recursordist/rec-eventtrace.cc @@ -66,7 +66,7 @@ static void addValue(const RecEventTrace::Entry& event, Span& work, bool start) // The event trace uses start-stop records which need to be mapped to OpenTelemetry Spans, which is a // list of spans. Spans can refer to other spans as their parent. -std::vector RecEventTrace::convertToOT(const Span& span) const +std::vector RecEventTrace::convertToOT(const InitialSpanInfo& span) const { timespec realtime{}; clock_gettime(CLOCK_REALTIME, &realtime); @@ -79,7 +79,7 @@ std::vector RecEventTrace::convertToOT(const Span& span) cons ret.reserve((d_events.size() / 2) + 1); // The parent of all Spans - ret.emplace_back(span); + ret.emplace_back(Span{.trace_id = span.trace_id, .span_id = span.span_id, .parent_span_id = span.parent_span_id, .end_time_unix_nano = timestamp()}); std::vector spanIDs; // mapping of span index in ret vector to SpanID std::map ids; // mapping from event record index to index in ret vector (Spans) diff --git a/pdns/recursordist/rec-eventtrace.hh b/pdns/recursordist/rec-eventtrace.hh index 1623b0c2ab..a661f5afc3 100644 --- a/pdns/recursordist/rec-eventtrace.hh +++ b/pdns/recursordist/rec-eventtrace.hh @@ -266,7 +266,7 @@ public: return d_events; } - std::vector convertToOT(const pdns::trace::Span& span) const; + std::vector convertToOT(const pdns::trace::InitialSpanInfo& span) const; size_t setParent(size_t parent) { diff --git a/pdns/recursordist/rec-main.cc b/pdns/recursordist/rec-main.cc index c4f73cb951..009940d50b 100644 --- a/pdns/recursordist/rec-main.cc +++ b/pdns/recursordist/rec-main.cc @@ -603,7 +603,7 @@ void protobufLogResponse(const DNSName& qname, QType qtype, const boost::uuids::uuid& uniqueId, const string& requestorId, const string& deviceId, const string& deviceName, const std::map& meta, const RecEventTrace& eventTrace, - pdns::trace::Span& otTrace, + pdns::trace::InitialSpanInfo& otTrace, const std::unordered_set& policyTags) { pdns::ProtoZero::RecMessage pbMessage(pbData ? pbData->d_message : "", pbData ? pbData->d_response : "", 64, 10); // The extra bytes we are going to add @@ -665,7 +665,6 @@ void protobufLogResponse(const DNSName& qname, QType qtype, pbMessage.addEvents(eventTrace); } if (eventTrace.enabled() && (SyncRes::s_event_trace_enabled & SyncRes::event_trace_to_ot) != 0) { - otTrace.close(); auto trace = pdns::trace::TracesData::boilerPlate("rec", qname.toLogString() + '/' + qtype.toString(), eventTrace.convertToOT(otTrace)); pbMessage.setOpenTelemetryData(trace.encode()); } diff --git a/pdns/recursordist/rec-main.hh b/pdns/recursordist/rec-main.hh index 4a639bf911..49576ee7bb 100644 --- a/pdns/recursordist/rec-main.hh +++ b/pdns/recursordist/rec-main.hh @@ -115,7 +115,7 @@ struct DNSComboWriter ComboAddress d_destination; // the address we assume the query is sent to, might be set by proxy protocol ComboAddress d_mappedSource; // the source address after being mapped by table based proxy mapping RecEventTrace d_eventTrace; - pdns::trace::Span d_otTrace; + pdns::trace::InitialSpanInfo d_otTrace; boost::uuids::uuid d_uuid; string d_requestorId; string d_deviceId; @@ -638,7 +638,7 @@ void protobufLogResponse(const DNSName& qname, QType qtype, const struct dnshead const boost::uuids::uuid& uniqueId, const string& requestorId, const string& deviceId, const string& deviceName, const std::map& meta, const RecEventTrace& eventTrace, - pdns::trace::Span& otTrace, + pdns::trace::InitialSpanInfo& otTrace, const std::unordered_set& policyTags); void requestWipeCaches(const DNSName& canon); void startDoResolve(void*); diff --git a/pdns/recursordist/rec-tcp.cc b/pdns/recursordist/rec-tcp.cc index 2b164c21e1..8a56f40b02 100644 --- a/pdns/recursordist/rec-tcp.cc +++ b/pdns/recursordist/rec-tcp.cc @@ -315,7 +315,7 @@ static void doProcessTCPQuestion(std::unique_ptr& comboWriter, s if (SyncRes::eventTraceEnabled(SyncRes::event_trace_to_ot)) { comboWriter->d_otTrace.clear(); comboWriter->d_otTrace.start_time_unix_nano = traceTS; - comboWriter->d_otTrace.name = "RecRequest"; + //comboWriter->d_otTrace.name = "RecRequest"; } auto luaconfsLocal = g_luaconfs.getLocal(); if (checkProtobufExport(luaconfsLocal)) { diff --git a/pdns/recursordist/syncres.hh b/pdns/recursordist/syncres.hh index 761b9879a0..09242aa09c 100644 --- a/pdns/recursordist/syncres.hh +++ b/pdns/recursordist/syncres.hh @@ -585,7 +585,7 @@ public: boost::optional d_routingTag; ComboAddress d_fromAuthIP; RecEventTrace d_eventTrace; - pdns::trace::Span d_otTrace; + pdns::trace::InitialSpanInfo d_otTrace; std::shared_ptr d_slog = g_slog->withName("syncres"); boost::optional d_extendedError; -- 2.47.2