From: Pieter Lexis Date: Tue, 14 Oct 2025 11:11:05 +0000 (+0200) Subject: feat(dnsdist): Add Query info to the root span X-Git-Tag: rec-5.4.0-alpha1~187^2~4 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=2ffa2db6769d3891fccc166d3506e097523bc3b4;p=thirdparty%2Fpdns.git feat(dnsdist): Add Query info to the root span --- diff --git a/pdns/dnsdistdist/dnsdist-actions-factory.cc b/pdns/dnsdistdist/dnsdist-actions-factory.cc index de4704a820..fdfe8ef092 100644 --- a/pdns/dnsdistdist/dnsdist-actions-factory.cc +++ b/pdns/dnsdistdist/dnsdist-actions-factory.cc @@ -1681,9 +1681,8 @@ public: SetTraceAction(bool value) : d_value{value} {}; - DNSAction::Action operator()([[maybe_unused]] DNSQuestion* dnsquestion, std::string* ruleresult) const override + DNSAction::Action operator()([[maybe_unused]] DNSQuestion* dnsquestion, [[maybe_unused]] std::string* ruleresult) const override { - (void)ruleresult; #ifndef DISABLE_PROTOBUF auto tracer = dnsquestion->ids.getTracer(); if (tracer == nullptr) { @@ -1692,9 +1691,10 @@ public: } if (d_value) { tracer->activate(); - tracer->setTraceAttribute("query.qname", AnyValue{dnsquestion->ids.qname.toStringNoDot()}); - tracer->setTraceAttribute("query.qtype", AnyValue{QType(dnsquestion->ids.qtype).toString()}); - tracer->setTraceAttribute("query.remote", AnyValue{dnsquestion->ids.origRemote.toLogString()}); + tracer->setRootSpanAttribute("query.qname", AnyValue{dnsquestion->ids.qname.toStringNoDot()}); + tracer->setRootSpanAttribute("query.qtype", AnyValue{QType(dnsquestion->ids.qtype).toString()}); + tracer->setRootSpanAttribute("query.remote.address", AnyValue{dnsquestion->ids.origRemote.toString()}); + tracer->setRootSpanAttribute("query.remote.port", AnyValue{dnsquestion->ids.origRemote.getPort()}); } else { tracer->deactivate(); diff --git a/pdns/dnsdistdist/dnsdist-opentelemetry.cc b/pdns/dnsdistdist/dnsdist-opentelemetry.cc index 1cbb4ba1c3..60c64ac2d0 100644 --- a/pdns/dnsdistdist/dnsdist-opentelemetry.cc +++ b/pdns/dnsdistdist/dnsdist-opentelemetry.cc @@ -70,8 +70,17 @@ TracesData Tracer::getTracesData() .start_time_unix_nano = preActivationTrace.start_time_unix_nano, .end_time_unix_nano = preActivationTrace.end_time_unix_nano, }); + + if (preActivationTrace.parent_span_id == pdns::trace::s_emptySpanID) { + // This is the root span + otTrace.resource_spans.at(0).scope_spans.at(0).spans.back().attributes.insert( + otTrace.resource_spans.at(0).scope_spans.at(0).spans.back().attributes.cend(), + d_rootSpanAttributes.begin(), + d_rootSpanAttributes.end()); + } } } + { auto lockedPost = d_postActivationSpans.read_only_lock(); otTrace.resource_spans.at(0).scope_spans.at(0).spans.insert( @@ -178,6 +187,16 @@ void Tracer::closeSpan([[maybe_unused]] const SpanID& spanID) #endif } +void Tracer::setRootSpanAttribute([[maybe_unused]] const std::string& key, [[maybe_unused]] const AnyValue& value) +{ +#ifndef DISABLE_PROTOBUF + d_rootSpanAttributes.push_back({ + .key = key, + .value = value, + }); +#endif +} + // TODO: Figure out what to do with duplicate keys void Tracer::setSpanAttribute([[maybe_unused]] const SpanID& spanid, [[maybe_unused]] const std::string& key, [[maybe_unused]] const AnyValue& value) { diff --git a/pdns/dnsdistdist/dnsdist-opentelemetry.hh b/pdns/dnsdistdist/dnsdist-opentelemetry.hh index e4480f6a3e..aa3de14ed8 100644 --- a/pdns/dnsdistdist/dnsdist-opentelemetry.hh +++ b/pdns/dnsdistdist/dnsdist-opentelemetry.hh @@ -117,6 +117,14 @@ public: */ bool setTraceAttribute(const std::string& key, const AnyValue& value); + /** + * @brief Set an attribute on the root span + * + * @param key + * @param value + */ + void setRootSpanAttribute(const std::string& key, const AnyValue& value); + /** * @brief Set an attribute on a Span * @@ -306,6 +314,10 @@ private: * @brief All attributes related to this Trace (added to the ScopeSpan) */ std::vector d_attributes; + /** + * @brief All attributes related to the root span of this trace + */ + std::vector d_rootSpanAttributes; /** * @brief The TraceID for this Tracer. It is stable for the lifetime of the Tracer diff --git a/regression-tests.dnsdist/test_OpenTelemetryTracing.py b/regression-tests.dnsdist/test_OpenTelemetryTracing.py index a2705a9524..5738840f47 100644 --- a/regression-tests.dnsdist/test_OpenTelemetryTracing.py +++ b/regression-tests.dnsdist/test_OpenTelemetryTracing.py @@ -70,19 +70,40 @@ class DNSDistOpenTelemetryProtobufBaseTest(DNSDistOpenTelemetryProtobufTest): # Ensure the values are correct # TODO: query.remote with port - msg_scope_attrs = { - v["key"]: v["value"]["string_value"] + msg_scope_attr_keys = [ + v["key"] for v in ot_data["resource_spans"][0]["scope_spans"][0]["scope"][ "attributes" ] - if v["key"] != "query.remote" + ] + self.assertListEqual(msg_scope_attr_keys, ["hostname"]) + + root_span_attr_keys = [ + v["key"] + for v in ot_data["resource_spans"][0]["scope_spans"][0]["spans"][0][ + "attributes" + ] + ] + self.assertListEqual( + root_span_attr_keys, + ["query.qname", "query.qtype", "query.remote.address", "query.remote.port"], + ) + + # No way to guess the test port, but check the rest of the values + root_span_attrs = { + v["key"]: v["value"]["string_value"] + for v in ot_data["resource_spans"][0]["scope_spans"][0]["spans"][0][ + "attributes" + ] + if v["key"] not in ["query.remote.port"] } self.assertDictEqual( - msg_scope_attrs, { "query.qname": "query.ot.tests.powerdns.com", "query.qtype": "A", + "query.remote.address": "127.0.0.1", }, + root_span_attrs, ) msg_span_name = [