From: Pieter Lexis Date: Thu, 5 Feb 2026 10:54:23 +0000 (+0100) Subject: chore(dnsdist): Use TRACEPARENT nomenclature consistently X-Git-Tag: dnsdist-2.1.0-beta1~3^2~13 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=9fce5232c108c608e24df1f4988378e2d1db306e;p=thirdparty%2Fpdns.git chore(dnsdist): Use TRACEPARENT nomenclature consistently --- diff --git a/pdns/dnsdistdist/dnsdist-actions-definitions.yml b/pdns/dnsdistdist/dnsdist-actions-definitions.yml index 58d4b9902e..c137aa33e8 100644 --- a/pdns/dnsdistdist/dnsdist-actions-definitions.yml +++ b/pdns/dnsdistdist/dnsdist-actions-definitions.yml @@ -421,22 +421,22 @@ are processed after this action" type: "Vec" default: "" description: "The names of all the loggers that should be sent the protobuf messages that includes the tracing information. This message will be sent once dnsdist is finished with the DNS transaction (i.e. the response has been sent, the query was dropped, or when a timeout occured)." - - name: "use_incoming_traceid" + - name: "use_incoming_traceparent" type: "bool" default: "false" - description: "When set in the EDNS section, use the query's Trace ID and Span ID" - - name: "trace_edns_option" + description: "When set in the EDNS section, use the Trace ID and Span ID from the TRACEPARENT option" + - name: "traceparent_edns_option_code" type: "u16" default: 65500 - description: "The EDNS Option code to take the Trace ID from" - - name: "strip_incoming_traceid" + description: "The EDNS Option code for TRACEPARENT" + - name: "strip_incoming_traceparent" type: "bool" default: "false" description: "Remove any existing TRACEPARENT EDNS option from the query. When use_incoming_traceid is set to true, the ID will be picked set before removing the option" - - name: "downstream_edns_traceid_option" + - name: "downstream_edns_traceparent_option_code" type: "u16" default: "0" - description: "Set to the EDNS option number to add the TRACEPARENT option to the EDNS options of the query forwarded to the downstream server" + description: "Set to the EDNS option code to add the TRACEPARENT option to the EDNS options of the query forwarded to the downstream server. When ``strip_incoming_traceparent`` is ``false`` and this option is not set, any TRACEPARENT option in the original query will be sent downstream unaltered" - name: "SNMPTrap" description: "Send an SNMP trap, adding the message string as the query description. Subsequent rules are processed after this action" parameters: diff --git a/pdns/dnsdistdist/dnsdist-actions-factory.cc b/pdns/dnsdistdist/dnsdist-actions-factory.cc index 2e8c6bea60..169caf5f11 100644 --- a/pdns/dnsdistdist/dnsdist-actions-factory.cc +++ b/pdns/dnsdistdist/dnsdist-actions-factory.cc @@ -1717,7 +1717,7 @@ class SetTraceAction : public DNSAction { public: SetTraceAction(SetTraceActionConfiguration& config) : - d_loggers(config.remote_loggers), d_incomingTraceIDOptionCode(config.trace_edns_option), d_downstreamTraceIDOptionCode(config.downstream_trace_edns_option), d_value{config.value}, d_useIncomingTraceID(config.use_incoming_traceid), d_stripIncomingTraceID(config.strip_incoming_traceid) {}; + d_loggers(config.remote_loggers), d_incomingTraceparentOptionCode(config.incomingTraceparentOptionCode), d_downstreamTraceparentOptionCode(config.downstreamTraceparentOptionCode), d_value{config.value}, d_useIncomingTraceparent(config.useIncomingTraceparent), d_stripIncomingTraceparent(config.stripIncomingTraceparent) {}; DNSAction::Action operator()([[maybe_unused]] DNSQuestion* dnsquestion, [[maybe_unused]] std::string* ruleresult) const override { @@ -1742,11 +1742,11 @@ public: tracer->setRootSpanAttribute("query.remote.address", AnyValue{dnsquestion->ids.origRemote.toString()}); tracer->setRootSpanAttribute("query.remote.port", AnyValue{dnsquestion->ids.origRemote.getPort()}); - if (d_downstreamTraceIDOptionCode != 0) { - dnsquestion->ids.sendTraceParentToDownstreamID = d_downstreamTraceIDOptionCode; + if (d_downstreamTraceparentOptionCode != 0) { + dnsquestion->ids.sendTraceParentToDownstreamID = d_downstreamTraceparentOptionCode; } - if (!d_useIncomingTraceID && !d_stripIncomingTraceID) { + if (!d_useIncomingTraceparent && !d_stripIncomingTraceparent) { // No need to check EDNS return Action::None; } @@ -1763,10 +1763,10 @@ public: return Action::None; } - if (d_useIncomingTraceID) { + if (d_useIncomingTraceparent) { pdns::trace::TraceID traceID; pdns::trace::SpanID spanID; - if (pdns::trace::extractOTraceIDs(*(dnsquestion->ednsOptions), EDNSOptionCode::EDNSOptionCodeEnum(d_incomingTraceIDOptionCode), traceID, spanID)) { + if (pdns::trace::extractOTraceIDs(*(dnsquestion->ednsOptions), EDNSOptionCode::EDNSOptionCodeEnum(d_incomingTraceparentOptionCode), traceID, spanID)) { tracer->setTraceID(traceID); if (spanID != pdns::trace::s_emptySpanID) { tracer->setRootSpanID(spanID); @@ -1774,7 +1774,7 @@ public: } } - if (d_stripIncomingTraceID) { + if (d_stripIncomingTraceparent) { uint16_t optStart; size_t optLen; bool last; @@ -1791,7 +1791,7 @@ public: } size_t existingOptLen = optLen; - removeEDNSOptionFromOPT(reinterpret_cast(&dnsquestion->getMutableData().at(optStart)), &optLen, d_incomingTraceIDOptionCode); + removeEDNSOptionFromOPT(reinterpret_cast(&dnsquestion->getMutableData().at(optStart)), &optLen, d_incomingTraceparentOptionCode); dnsquestion->getMutableData().resize(dnsquestion->getData().size() - (existingOptLen - optLen)); // Ensure the EDNS Option View is not out of date dnsquestion->ednsOptions.reset(); @@ -1807,12 +1807,12 @@ public: private: std::vector> d_loggers; - short unsigned int d_incomingTraceIDOptionCode; - short unsigned int d_downstreamTraceIDOptionCode; + short unsigned int d_incomingTraceparentOptionCode; + short unsigned int d_downstreamTraceparentOptionCode; bool d_value; - bool d_useIncomingTraceID; - bool d_stripIncomingTraceID; + bool d_useIncomingTraceparent; + bool d_stripIncomingTraceparent; }; #endif diff --git a/pdns/dnsdistdist/dnsdist-actions-factory.hh b/pdns/dnsdistdist/dnsdist-actions-factory.hh index ef3cf1da02..0080cdd870 100644 --- a/pdns/dnsdistdist/dnsdist-actions-factory.hh +++ b/pdns/dnsdistdist/dnsdist-actions-factory.hh @@ -127,11 +127,11 @@ std::shared_ptr getDnstapLogResponseAction(const std::string& struct SetTraceActionConfiguration { std::vector> remote_loggers; - std::uint16_t trace_edns_option = 0; - std::uint16_t downstream_trace_edns_option = 0; + std::uint16_t incomingTraceparentOptionCode = 0; + std::uint16_t downstreamTraceparentOptionCode = 0; bool value = false; - bool use_incoming_traceid = false; - bool strip_incoming_traceid = false; + bool useIncomingTraceparent = false; + bool stripIncomingTraceparent = false; }; std::shared_ptr getSetTraceAction(SetTraceActionConfiguration& config); #endif /* DISABLE_PROTOBUF */ diff --git a/pdns/dnsdistdist/dnsdist-configuration-yaml.cc b/pdns/dnsdistdist/dnsdist-configuration-yaml.cc index 04f11d47a7..8ba5fe2af3 100644 --- a/pdns/dnsdistdist/dnsdist-configuration-yaml.cc +++ b/pdns/dnsdistdist/dnsdist-configuration-yaml.cc @@ -1776,11 +1776,11 @@ std::shared_ptr getSetTraceAction(const SetTraceActionConfigur dnsdist::actions::SetTraceActionConfiguration actionConfig{ .remote_loggers = std::move(loggers), - .trace_edns_option = config.trace_edns_option, - .downstream_trace_edns_option = config.downstream_edns_traceid_option, + .incomingTraceparentOptionCode = config.traceparent_edns_option_code, + .downstreamTraceparentOptionCode = config.downstream_edns_traceparent_option_code, .value = config.value, - .use_incoming_traceid = config.use_incoming_traceid, - .strip_incoming_traceid = config.strip_incoming_traceid, + .useIncomingTraceparent = config.use_incoming_traceparent, + .stripIncomingTraceparent = config.strip_incoming_traceparent, }; auto action = dnsdist::actions::getSetTraceAction(actionConfig); diff --git a/pdns/dnsdistdist/dnsdist-lua-actions.cc b/pdns/dnsdistdist/dnsdist-lua-actions.cc index ef50168a33..581db91ae1 100644 --- a/pdns/dnsdistdist/dnsdist-lua-actions.cc +++ b/pdns/dnsdistdist/dnsdist-lua-actions.cc @@ -239,7 +239,7 @@ void setupLuaActions(LuaContext& luaCtx) }); #ifndef DISABLE_PROTOBUF - luaCtx.writeFunction("SetTraceAction", [](bool value, std::optional>> remote_loggers, std::optional use_incoming_traceid, std::optional trace_edns_option, std::optional downstream_trace_edns_option, std::optional strip_incoming_traceid) { + luaCtx.writeFunction("SetTraceAction", [](bool value, std::optional>> remote_loggers, std::optional use_incoming_traceparent, std::optional traceparent_option_code, std::optional downstream_traceparent_option_code, std::optional strip_incoming_traceparent) { dnsdist::actions::SetTraceActionConfiguration config; if (remote_loggers) { @@ -258,10 +258,10 @@ void setupLuaActions(LuaContext& luaCtx) config.remote_loggers = std::move(loggers); } config.value = value; - config.trace_edns_option = trace_edns_option.value_or(65500); - config.downstream_trace_edns_option = downstream_trace_edns_option.value_or(0); - config.use_incoming_traceid = use_incoming_traceid.value_or(false); - config.strip_incoming_traceid = strip_incoming_traceid.value_or(false); + config.incomingTraceparentOptionCode = traceparent_option_code.value_or(65500); + config.downstreamTraceparentOptionCode = downstream_traceparent_option_code.value_or(0); + config.useIncomingTraceparent = use_incoming_traceparent.value_or(false); + config.stripIncomingTraceparent = strip_incoming_traceparent.value_or(false); return dnsdist::actions::getSetTraceAction(config); }); diff --git a/pdns/dnsdistdist/docs/reference/ottrace.rst b/pdns/dnsdistdist/docs/reference/ottrace.rst index 879b8f0844..1c4d66850e 100644 --- a/pdns/dnsdistdist/docs/reference/ottrace.rst +++ b/pdns/dnsdistdist/docs/reference/ottrace.rst @@ -78,7 +78,7 @@ Passing Trace ID and Span ID to downstream servers ================================================== When storing traces, it is beneficial to correlate traces of the same query through different applications. -The `PowerDNS Recursor `__ (since 5.3.0) supports the experimental `draft-edns-otel-trace-ids `__ EDNS option to pass the trace identifier. +The `PowerDNS Recursor `__ (since 5.3.0) supports the experimental `TRACEPARENT `__ EDNS option to pass the trace identifier. The :doc:`DNSQuestion object ` supports the :func:`getTraceID ` method to retrieve the trace identifier as a binary string. Combining all this, a :func:`LuaAction` can be used to add this EDNS option to the query. @@ -138,8 +138,8 @@ Should there be no ID in the incoming query, a random ID will be generated. action: type: SetTrace value: true - use_incoming_traceid: true + use_incoming_traceparent: true As :program:`dnsdist` keeps EDNS existing options in the query, the Trace ID option is passed as-is to the backend, which might not be desirable. -Using the ``strip_incoming_traceid`` boolean option, the EDNS option will be removed from the query. +Using the ``strip_incoming_traceparent`` boolean option, the EDNS option will be removed from the query. Note that this will only happen when ``value`` is set to ``true``. diff --git a/regression-tests.dnsdist/test_OpenTelemetryTracing.py b/regression-tests.dnsdist/test_OpenTelemetryTracing.py index 6eceee3a83..522ff3c29e 100644 --- a/regression-tests.dnsdist/test_OpenTelemetryTracing.py +++ b/regression-tests.dnsdist/test_OpenTelemetryTracing.py @@ -2,17 +2,17 @@ import base64 import binascii +import threading +import time + +import dns.edns import dns.message -import dns.rrset import dns.rcode import dns.rdataclass import dns.rdatatype -import dns.edns -import time -import threading - -import opentelemetry.proto.trace.v1.trace_pb2 +import dns.rrset import google.protobuf.json_format +import opentelemetry.proto.trace.v1.trace_pb2 import test_Protobuf @@ -415,7 +415,7 @@ query_rules: action: type: SetTrace value: true - use_incoming_traceid: true + use_incoming_traceparent: true response_rules: - name: Do PB logging @@ -772,7 +772,7 @@ query_rules: action: type: SetTrace value: true - strip_incoming_traceid: true + strip_incoming_traceparent: true """ @classmethod @@ -814,6 +814,7 @@ query_rules: ottrace = dns.edns.GenericOption(str(65500), "\x00\x00") ottrace.data += binascii.a2b_hex("12345678901234567890123456789012") ottrace.data += binascii.a2b_hex("1234567890123456") + ottrace.data += binascii.a2b_hex("00") query = dns.message.make_query( name, "A", "IN", use_edns=True, options=[ottrace] ) @@ -832,3 +833,129 @@ query_rules: def testStripIncomingTraceIDTCP(self): self.doQuery(True) + + +def verifyTraceparentInQuery(request: dns.message.Message): + print(request) + response = dns.message.make_response(request) + + traceparent : dns.edns.Option|None = next((i for i in request.options if i.otype == 65500), None) + + print(traceparent) + + if traceparent is None: + response.set_rcode(dns.rcode.SERVFAIL) + return response.to_wire() + + # Ensure the SpanID is not 1234567890123456 + if traceparent.to_text()[-17:-1] == "1234567890123456": + response.set_rcode(dns.rcode.SERVFAIL) + return response.to_wire() + + # technically wrong, as we include the TRACEPARENT in the response + return response.to_wire() + + + +class TestOpenTelemetryTracingSendTraceparentDownstream( + DNSDistOpenTelemetryProtobufTest +): + _yaml_config_params = [ + "_testServerPort", + ] + _yaml_config_template = """--- +logging: + open_telemetry_tracing: true + +backends: + - address: 127.0.0.1:%d + protocol: Do53 + health_checks: + mode: up + +query_rules: + - name: Enable tracing + selector: + type: All + action: + type: SetTrace + value: true + downstream_edns_traceparent_option_code: 65500 +""" + + @classmethod + def startResponders(cls): + print("Launching responders..") + + cls._UDPResponder = threading.Thread( + name="UDP Responder", + target=cls.UDPResponder, + args=[ + cls._testServerPort, + cls._toResponderQueue, + cls._fromResponderQueue, + False, + verifyTraceparentInQuery, + ], + ) + cls._UDPResponder.daemon = True + cls._UDPResponder.start() + + cls._TCPResponder = threading.Thread( + name="TCP Responder", + target=cls.TCPResponder, + args=[ + cls._testServerPort, + cls._toResponderQueue, + cls._fromResponderQueue, + False, + False, + verifyTraceparentInQuery, + ], + ) + cls._TCPResponder.daemon = True + cls._TCPResponder.start() + + def doQuery(self, useTCP=False): + name = "query.ot.tests.powerdns.com." + + ottrace = dns.edns.GenericOption(str(65500), "\x00\x00") + ottrace.data += binascii.a2b_hex("12345678901234567890123456789012") + ottrace.data += binascii.a2b_hex("1234567890123456") + ottrace.data += binascii.a2b_hex("00") + query = dns.message.make_query( + name, "A", "IN", use_edns=True, options=[ottrace] + ) + + if useTCP: + (_, receivedResponse) = self.sendTCPQuery(query, response=None) + else: + (_, receivedResponse) = self.sendUDPQuery(query, response=None) + + self.assertIsNotNone(receivedResponse) + # If we stripped the OpenTelemetry Trace ID from the query, we should not get a SERVFAIL + self.assertEqual(receivedResponse.rcode(), dns.rcode.NOERROR) + + def testSetDownstreamTraceparentUDP(self): + self.doQuery() + + def testSetDownstreamTraceparentTCP(self): + self.doQuery(True) + +class TestOpenTelemetryTracingSendTraceparentDownstreamLua( + TestOpenTelemetryTracingSendTraceparentDownstream +): + _yaml_config_template = None + _config_params = [ + "_testServerPort", + "_protobufServerPort", + ] + _config_template = """ +newServer{address="127.0.0.1:%d"} +getServer(0):setUp() +rl = newRemoteLogger('127.0.0.1:%d') +setOpenTelemetryTracing(true) + +addAction(AllRule(), SetTraceAction(true, {rl}, false, 65500, 65500, false), {name="Enable tracing"}) +addResponseAction(AllRule(), RemoteLogResponseAction(rl), {name="Do PB logging"}) + """