From 5e8fbd7c3f1ed229a78a5901eb2f21c67619c7c0 Mon Sep 17 00:00:00 2001 From: Otto Moerbeek Date: Mon, 21 Jul 2025 15:53:59 +0200 Subject: [PATCH] Better code for setting and getting traceids and spanids from EDNS Signed-off-by: Otto Moerbeek --- pdns/dnsdistdist/dnsdist-actions-factory.cc | 2 +- pdns/protozero-trace.cc | 15 +++-- pdns/protozero-trace.hh | 64 +++++++++++++++++++++ pdns/sdig.cc | 9 +-- 4 files changed, 77 insertions(+), 13 deletions(-) diff --git a/pdns/dnsdistdist/dnsdist-actions-factory.cc b/pdns/dnsdistdist/dnsdist-actions-factory.cc index e8f5d7af20..07e8ea9b19 100644 --- a/pdns/dnsdistdist/dnsdist-actions-factory.cc +++ b/pdns/dnsdistdist/dnsdist-actions-factory.cc @@ -1521,7 +1521,7 @@ public: auto [protocol, httpProtocol] = ProtocolToDNSTap(dnsquestion->getProtocol()); // NOLINTNEXTLINE(cppcoreguidelines-pro-type-reinterpret-cast) - DnstapMessagemessage(std::move(data), !dnsquestion->getHeader()->qr ? DnstapMessage::MessageType::client_query : DnstapMessage::MessageType::client_response, d_identity, &dnsquestion->ids.origRemote, &dnsquestion->ids.origDest, protocol, reinterpret_cast(dnsquestion->getData().data()), dnsquestion->getData().size(), &dnsquestion->getQueryRealTime(), nullptr, boost::none, httpProtocol); + DnstapMessage message(std::move(data), !dnsquestion->getHeader()->qr ? DnstapMessage::MessageType::client_query : DnstapMessage::MessageType::client_response, d_identity, &dnsquestion->ids.origRemote, &dnsquestion->ids.origDest, protocol, reinterpret_cast(dnsquestion->getData().data()), dnsquestion->getData().size(), &dnsquestion->getQueryRealTime(), nullptr, boost::none, httpProtocol); { if (d_alterFunc) { auto lock = g_lua.lock(); diff --git a/pdns/protozero-trace.cc b/pdns/protozero-trace.cc index c4b4a72285..5b1fa712f4 100644 --- a/pdns/protozero-trace.cc +++ b/pdns/protozero-trace.cc @@ -512,18 +512,17 @@ void extractOTraceIDs(const EDNSOptionViewMap& map, pdns::trace::InitialSpanInfo // parent_span_id gets set from edns options (if available and well-formed, otherwise it remains cleared (no parent)) // span_id gets inited randomly bool traceidset = false; - const auto traceIDSize = span.trace_id.size(); if (const auto& option = map.find(EDNSOptionCode::OTTRACEIDS); option != map.end()) { - // 1 byte version, then tracid then optinal spanid - if (option->second.values.size() > 0) { - if (option->second.values.at(0).size >= 1 + traceIDSize) { + const auto& value = option->second.values.at(0); + const EDNSOTTraceRecordView data{reinterpret_cast(value.content), value.size}; // NOLINT(cppcoreguidelines-pro-type-reinterpret-cast) + // 1 byte version, then tracid then optional spanid + uint8_t version{}; + if (data.getVersion(version) && version == 0) { + if (data.getTraceID(span.trace_id)) { traceidset = true; - pdns::trace::fill(span.trace_id, &option->second.values.at(0).content[1], traceIDSize); // NOLINT(cppcoreguidelines-pro-bounds-pointer-arithmetic) it's the API - } - if (option->second.values.at(0).size == 1 + traceIDSize + span.parent_span_id.size()) { - pdns::trace::fill(span.parent_span_id, &option->second.values.at(0).content[traceIDSize + 1], span.parent_span_id.size()); // NOLINT(cppcoreguidelines-pro-bounds-pointer-arithmetic) it's the API } + (void)data.getSpanID(span.parent_span_id); } } if (!traceidset) { diff --git a/pdns/protozero-trace.hh b/pdns/protozero-trace.hh index 1c1250adb6..a66328655e 100644 --- a/pdns/protozero-trace.hh +++ b/pdns/protozero-trace.hh @@ -712,6 +712,70 @@ inline KeyValueList KeyValueList::decode(protozero::pbf_reader& reader) return pdns::trace::decode(reader); } +struct EDNSOTTraceRecord +{ + // 1 byte version 16 bytes traceid, optional 8 bytes spanid + static constexpr size_t fullSize = 1 + 16 + 8; + static constexpr size_t sizeNoSpanID = 1 + 16; + static constexpr size_t traceIDOffset = 1; + static constexpr size_t spanIDOffset = 1 + 16; + + EDNSOTTraceRecord(uint8_t* arg) : + data(arg) {} + // NOLINTBEGIN(cppcoreguidelines-pro-bounds-pointer-arithmetic) + void setVersion(uint8_t version) + { + data[0] = version; + } + void setTraceID(const TraceID& traceid) + { + std::copy(traceid.begin(), traceid.end(), &data[traceIDOffset]); + } + void setSpanID(const SpanID& spanid) + { + std::copy(spanid.begin(), spanid.end(), &data[spanIDOffset]); + } + // NOLINTEND(cppcoreguidelines-pro-bounds-pointer-arithmetic) +private: + uint8_t* data; +}; + +struct EDNSOTTraceRecordView +{ + EDNSOTTraceRecordView(const uint8_t* arg, size_t argsize) : + data(arg), size(argsize) {} + + // NOLINTBEGIN(cppcoreguidelines-pro-bounds-pointer-arithmetic) + [[nodiscard]] bool getVersion(uint8_t& version) const + { + if (size > 0) { + version = data[0]; + return true; + } + return false; + } + [[nodiscard]] bool getTraceID(TraceID& traceid) const + { + if (size >= pdns::trace::EDNSOTTraceRecord::sizeNoSpanID) { + std::copy(&data[EDNSOTTraceRecord::traceIDOffset], &data[EDNSOTTraceRecord::traceIDOffset + traceid.size()], traceid.begin()); + return true; + } + return false; + } + [[nodiscard]] bool getSpanID(SpanID& spanid) const + { + if (size == pdns::trace::EDNSOTTraceRecord::fullSize) { + std::copy(&data[EDNSOTTraceRecord::spanIDOffset], &data[EDNSOTTraceRecord::spanIDOffset + spanid.size()], spanid.begin()); + return true; + } + return false; + } + // NOLINTEND(cppcoreguidelines-pro-bounds-pointer-arithmetic) +private: + const uint8_t* const data; + const size_t size; +}; + void extractOTraceIDs(const EDNSOptionViewMap& map, pdns::trace::InitialSpanInfo& span); } // namespace pdns::trace diff --git a/pdns/sdig.cc b/pdns/sdig.cc index 98cbc91c10..887dafb38e 100644 --- a/pdns/sdig.cc +++ b/pdns/sdig.cc @@ -97,10 +97,11 @@ static void fillPacket(vector& packet, const string& q, const string& t if (otids) { const auto traceid = otids->first; const auto spanid = otids->second; - std::array data{}; - data.at(0) = 0; // version - std::copy(traceid.begin(), traceid.end(), &data.at(1)); - std::copy(spanid.begin(), spanid.end(), &data.at(1 + traceid.size())); + std::array data{}; + pdns::trace::EDNSOTTraceRecord record{data.data()}; + record.setVersion(0); + record.setTraceID(traceid); + record.setSpanID(spanid); opts.emplace_back(EDNSOptionCode::OTTRACEIDS, std::string_view(reinterpret_cast(data.data()), data.size())); // NOLINT } pw.addOpt(bufsize, 0, dnssec ? EDNSOpts::DNSSECOK : 0, opts); -- 2.47.2