From: Pieter Lexis Date: Thu, 12 Feb 2026 15:17:37 +0000 (+0100) Subject: feat(dnsdist): Use only one TRACEPARENT option code for in and out X-Git-Tag: dnsdist-2.1.0-beta1~3^2~3 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=fa835d5a32b3a4e376797a1e9239fcf258e44979;p=thirdparty%2Fpdns.git feat(dnsdist): Use only one TRACEPARENT option code for in and out --- diff --git a/pdns/dnsdistdist/dnsdist-actions-definitions.yml b/pdns/dnsdistdist/dnsdist-actions-definitions.yml index c137aa33e8..f84d9969ef 100644 --- a/pdns/dnsdistdist/dnsdist-actions-definitions.yml +++ b/pdns/dnsdistdist/dnsdist-actions-definitions.yml @@ -433,10 +433,10 @@ are processed after this action" 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_traceparent_option_code" - type: "u16" - default: "0" - 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: "send_downstream_traceparent" + type: "bool" + default: "false" + description: "Set to true to add (or update) the TRACEPARENT option in the EDNS options of the query forwarded to the downstream server. When ``strip_incoming_traceparent`` is ``false`` and this option is ``false``, 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 169caf5f11..f96f843a67 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_incomingTraceparentOptionCode(config.incomingTraceparentOptionCode), d_downstreamTraceparentOptionCode(config.downstreamTraceparentOptionCode), d_value{config.value}, d_useIncomingTraceparent(config.useIncomingTraceparent), d_stripIncomingTraceparent(config.stripIncomingTraceparent) {}; + d_loggers(config.remote_loggers), d_traceparentOptionCode(config.traceparentOptionCode), d_value{config.value}, d_useIncomingTraceparent(config.useIncomingTraceparent), d_stripIncomingTraceparent(config.stripIncomingTraceparent), d_sendDownstreamTraceparent(config.sendDownstreamTraceparent) {}; DNSAction::Action operator()([[maybe_unused]] DNSQuestion* dnsquestion, [[maybe_unused]] std::string* ruleresult) const override { @@ -1742,8 +1742,8 @@ public: tracer->setRootSpanAttribute("query.remote.address", AnyValue{dnsquestion->ids.origRemote.toString()}); tracer->setRootSpanAttribute("query.remote.port", AnyValue{dnsquestion->ids.origRemote.getPort()}); - if (d_downstreamTraceparentOptionCode != 0) { - dnsquestion->ids.sendTraceParentToDownstreamID = d_downstreamTraceparentOptionCode; + if (d_sendDownstreamTraceparent) { + dnsquestion->ids.sendTraceParentToDownstreamID = d_traceparentOptionCode; } if (!d_useIncomingTraceparent && !d_stripIncomingTraceparent) { @@ -1766,7 +1766,7 @@ public: if (d_useIncomingTraceparent) { pdns::trace::TraceID traceID; pdns::trace::SpanID spanID; - if (pdns::trace::extractOTraceIDs(*(dnsquestion->ednsOptions), EDNSOptionCode::EDNSOptionCodeEnum(d_incomingTraceparentOptionCode), traceID, spanID)) { + if (pdns::trace::extractOTraceIDs(*(dnsquestion->ednsOptions), EDNSOptionCode::EDNSOptionCodeEnum(d_traceparentOptionCode), traceID, spanID)) { tracer->setTraceID(traceID); if (spanID != pdns::trace::s_emptySpanID) { tracer->setRootSpanID(spanID); @@ -1791,7 +1791,7 @@ public: } size_t existingOptLen = optLen; - removeEDNSOptionFromOPT(reinterpret_cast(&dnsquestion->getMutableData().at(optStart)), &optLen, d_incomingTraceparentOptionCode); + removeEDNSOptionFromOPT(reinterpret_cast(&dnsquestion->getMutableData().at(optStart)), &optLen, d_traceparentOptionCode); 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_incomingTraceparentOptionCode; - short unsigned int d_downstreamTraceparentOptionCode; + short unsigned int d_traceparentOptionCode; bool d_value; bool d_useIncomingTraceparent; bool d_stripIncomingTraceparent; + bool d_sendDownstreamTraceparent; }; #endif diff --git a/pdns/dnsdistdist/dnsdist-actions-factory.hh b/pdns/dnsdistdist/dnsdist-actions-factory.hh index 0080cdd870..0eeecbb8ff 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 incomingTraceparentOptionCode = 0; - std::uint16_t downstreamTraceparentOptionCode = 0; + std::uint16_t traceparentOptionCode = 65500; bool value = false; bool useIncomingTraceparent = false; bool stripIncomingTraceparent = false; + bool sendDownstreamTraceparent = 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 8ba5fe2af3..79c733f644 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), - .incomingTraceparentOptionCode = config.traceparent_edns_option_code, - .downstreamTraceparentOptionCode = config.downstream_edns_traceparent_option_code, + .traceparentOptionCode = config.traceparent_edns_option_code, .value = config.value, .useIncomingTraceparent = config.use_incoming_traceparent, .stripIncomingTraceparent = config.strip_incoming_traceparent, + .sendDownstreamTraceparent = config.send_downstream_traceparent, }; auto action = dnsdist::actions::getSetTraceAction(actionConfig); diff --git a/pdns/dnsdistdist/dnsdist-lua-actions.cc b/pdns/dnsdistdist/dnsdist-lua-actions.cc index 39978d9ce0..276df3f144 100644 --- a/pdns/dnsdistdist/dnsdist-lua-actions.cc +++ b/pdns/dnsdistdist/dnsdist-lua-actions.cc @@ -261,14 +261,14 @@ void setupLuaActions(LuaContext& luaCtx) } } config.remote_loggers = std::move(loggers); - if (getOptionalValue(options, "incomingTraceparentOptionCode", config.incomingTraceparentOptionCode) < 0) { - throw std::runtime_error("incomingTraceparentOptionCode in SetTraceAction is not a number"); + if (getOptionalValue(options, "traceparentOptionCode", config.traceparentOptionCode) < 0) { + throw std::runtime_error("TraceparentOptionCode in SetTraceAction is not a number"); } - if (config.useIncomingTraceparent == 0) { - config.useIncomingTraceparent = 65500; + if (config.traceparentOptionCode == 0) { + config.traceparentOptionCode = EDNSOptionCode::TRACEPARENT; } - if (getOptionalValue(options, "downstreamTraceparentOptionCode", config.downstreamTraceparentOptionCode) < 0) { - throw std::runtime_error("downstreamTraceparentOptionCode in SetTraceAction is not a number"); + if (getOptionalValue(options, "sendDownstreamTraceparent", config.sendDownstreamTraceparent) < 0) { + throw std::runtime_error("sendDownstreamTraceparent in SetTraceAction is not a bool"); } if (getOptionalValue(options, "useIncomingTraceparent", config.useIncomingTraceparent) < 0) { throw std::runtime_error("useIncomingTraceparent in SetTraceAction is not a bool"); diff --git a/pdns/dnsdistdist/docs/reference/actions.rst b/pdns/dnsdistdist/docs/reference/actions.rst index 0f1111fcb8..52774a0202 100644 --- a/pdns/dnsdistdist/docs/reference/actions.rst +++ b/pdns/dnsdistdist/docs/reference/actions.rst @@ -802,11 +802,11 @@ The following actions exist. Options: - * ``remoteLoggers``: A table of :func:`remoteLogger ` objects to send the traces to. Note that these log messages are empty apart from the trace data. + * ``remoteLoggers``: A table of :func:`remoteLogger ` objects to send the traces to. Note that these log messages will be empty, apart from the trace data. * ``useIncomingTraceparent``: boolean, default false. Use the information in the TRACEPARENT EDNS option (if any) from the query as the Trace ID and initial Span ID. * ``stripIncomingTraceparent``: boolean, default false. Remove the TRACEPARENT EDNS option from the DNS query so it is not passed to the backend server. - * ``incomingTraceparentOptionCode``: integer, default 65500. The EDNS option code for TRACEPARENT in the incoming query. - * ``downstreamTraceparentOptionCode``: integer, default unset. When set, a TRACEPARENT EDNS option is added to the downstream query with the active Trace ID and the last Span ID of the trace. + * ``traceparentOptionCode``: integer, default 65500. The EDNS option code for TRACEPARENT EDNS option. + * ``sendDownstreamTraceparent``: boolean, default false. When true, a TRACEPARENT EDNS option is added to the downstream query with the active Trace ID and the last Span ID of the trace. .. function:: SkipCacheAction() diff --git a/pdns/dnsdistdist/docs/reference/ottrace.rst b/pdns/dnsdistdist/docs/reference/ottrace.rst index 80f83d4554..cb3b7e6926 100644 --- a/pdns/dnsdistdist/docs/reference/ottrace.rst +++ b/pdns/dnsdistdist/docs/reference/ottrace.rst @@ -80,7 +80,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 `TRACEPARENT `__ EDNS option to pass the trace identifier. -This can be easily achieved by adding the `downstream_edns_traceparent_option_code` option with the desired EDNS OptionCode. +This can be easily achieved by adding the `send_downstream_traceparent` option with the desired EDNS OptionCode. .. code-block:: yaml @@ -91,7 +91,7 @@ This can be easily achieved by adding the `downstream_edns_traceparent_option_co action: type: SetTrace value: true - downstream_edns_traceparent_option_code: 65500 + send_downstream_traceparent: true Accepting TRACEPARENT from upstream servers =========================================== @@ -135,5 +135,5 @@ The following example makes :program:`dnsdist` accept a TRACEPARENT, and update action: type: SetTrace value: true - downstream_edns_traceparent_option_code: 65500 + send_downstream_traceparent: true use_incoming_traceparent: true diff --git a/regression-tests.dnsdist/test_OpenTelemetryTracing.py b/regression-tests.dnsdist/test_OpenTelemetryTracing.py index 93b1ce15c4..f3a24da22d 100644 --- a/regression-tests.dnsdist/test_OpenTelemetryTracing.py +++ b/regression-tests.dnsdist/test_OpenTelemetryTracing.py @@ -41,7 +41,7 @@ class DNSDistOpenTelemetryProtobufTest(test_Protobuf.DNSDistProtobufTest): ottrace.data += binascii.a2b_hex(traceID) if spanID != "": ottrace.data += binascii.a2b_hex(spanID) - ottrace.data += b"\x00" # flags + ottrace.data += b"\x00" # flags query = dns.message.make_query( name, "A", "IN", use_edns=True, options=[ottrace] ) @@ -59,9 +59,9 @@ class DNSDistOpenTelemetryProtobufTest(test_Protobuf.DNSDistProtobufTest): response.answer.append(rrset) if useTCP: - (receivedQuery, receivedResponse) = self.sendTCPQuery(query, response) + receivedQuery, receivedResponse = self.sendTCPQuery(query, response) else: - (receivedQuery, receivedResponse) = self.sendUDPQuery(query, response) + receivedQuery, receivedResponse = self.sendUDPQuery(query, response) if querySentByDNSDist: self.assertTrue(receivedQuery) @@ -820,9 +820,9 @@ query_rules: ) if useTCP: - (_, receivedResponse) = self.sendTCPQuery(query, response=None) + _, receivedResponse = self.sendTCPQuery(query, response=None) else: - (_, receivedResponse) = self.sendUDPQuery(query, response=None) + _, 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 @@ -839,7 +839,9 @@ 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) + traceparent: dns.edns.Option | None = next( + (i for i in request.options if i.otype == 65500), None + ) print(traceparent) @@ -856,7 +858,6 @@ def verifyTraceparentInQuery(request: dns.message.Message): return response.to_wire() - class TestOpenTelemetryTracingSendTraceparentDownstream( DNSDistOpenTelemetryProtobufTest ): @@ -880,7 +881,7 @@ query_rules: action: type: SetTrace value: true - downstream_edns_traceparent_option_code: 65500 + send_downstream_traceparent: true """ @classmethod @@ -919,14 +920,12 @@ query_rules: def doQuery(self, useTCP=False): name = "query.ot.tests.powerdns.com." - query = dns.message.make_query( - name, "A", "IN", use_edns=True - ) + query = dns.message.make_query(name, "A", "IN", use_edns=True) if useTCP: - (_, receivedResponse) = self.sendTCPQuery(query, response=None) + _, receivedResponse = self.sendTCPQuery(query, response=None) else: - (_, receivedResponse) = self.sendUDPQuery(query, response=None) + _, 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 @@ -938,6 +937,7 @@ query_rules: def testSetDownstreamTraceparentTCP(self): self.doQuery(True) + class TestOpenTelemetryTracingSendTraceparentDownstreamLua( TestOpenTelemetryTracingSendTraceparentDownstream ): @@ -952,6 +952,6 @@ getServer(0):setUp() rl = newRemoteLogger('127.0.0.1:%d') setOpenTelemetryTracing(true) -addAction(AllRule(), SetTraceAction(true, {remoteLoggers={rl}, downstreamTraceparentOptionCode=65500}), {name="Enable tracing"}) +addAction(AllRule(), SetTraceAction(true, {remoteLoggers={rl}, sendDownstreamTraceparent=true}), {name="Enable tracing"}) addResponseAction(AllRule(), RemoteLogResponseAction(rl), {name="Do PB logging"}) """ diff --git a/regression-tests.dnsdist/test_OutgoingTLS.py b/regression-tests.dnsdist/test_OutgoingTLS.py index 39a0d0a84f..24f6ed4227 100644 --- a/regression-tests.dnsdist/test_OutgoingTLS.py +++ b/regression-tests.dnsdist/test_OutgoingTLS.py @@ -336,7 +336,7 @@ query_rules: action: type: SetTrace value: true - downstream_edns_traceparent_option_code: 65500 + send_downstream_traceparent: true webserver: listen_addresses: diff --git a/regression-tests.dnsdist/test_ProxyProtocol.py b/regression-tests.dnsdist/test_ProxyProtocol.py index 10988bfb93..d8d7edfade 100644 --- a/regression-tests.dnsdist/test_ProxyProtocol.py +++ b/regression-tests.dnsdist/test_ProxyProtocol.py @@ -1,19 +1,21 @@ #!/usr/bin/env python -import dns import selectors import socket import ssl -import requests import struct import sys import threading import time +import dns +import requests + +from dnsdistdohtests import DNSDistDOHTest from dnsdisttests import DNSDistTest, pickAvailablePort from proxyprotocol import ProxyProtocol -from proxyprotocolutils import ProxyProtocolUDPResponder, ProxyProtocolTCPResponder -from dnsdistdohtests import DNSDistDOHTest +from proxyprotocolutils import (ProxyProtocolTCPResponder, + ProxyProtocolUDPResponder) # Python2/3 compatibility hacks try: @@ -489,7 +491,7 @@ class TestProxyProtocolTraceParent(TestProxyProtocol): return DNSAction.None end - addAction(AllRule(), SetTraceAction(true, {downstreamTraceparentOptionCode=65500}), {name="Enable tracing"}) + addAction(AllRule(), SetTraceAction(true, {sendDownstreamTraceparent=true}), {name="Enable tracing"}) addAction("values-lua.proxy.tests.powerdns.com.", LuaAction(addValues)) addAction("values-action.proxy.tests.powerdns.com.", SetProxyProtocolValuesAction({ ["1"]="dnsdist", ["255"]="proxy-protocol"})) addAction("random-values.proxy.tests.powerdns.com.", LuaAction(addRandomValue))