From: Pieter Lexis Date: Thu, 27 Nov 2025 14:09:13 +0000 (+0100) Subject: feat(opentelemetry): Add infrastructure to support a stack-based tracking of spanids X-Git-Tag: rec-5.4.0-beta1~65^2~9 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=141e9610e3e992e8454e4e5fdaca2aeca625c5a9;p=thirdparty%2Fpdns.git feat(opentelemetry): Add infrastructure to support a stack-based tracking of spanids --- diff --git a/pdns/dnsdistdist/dnsdist-opentelemetry.cc b/pdns/dnsdistdist/dnsdist-opentelemetry.cc index f28e65e5c9..b7232f96bc 100644 --- a/pdns/dnsdistdist/dnsdist-opentelemetry.cc +++ b/pdns/dnsdistdist/dnsdist-opentelemetry.cc @@ -57,12 +57,13 @@ TracesData Tracer::getTracesData() otTrace.resource_spans.at(0).scope_spans.at(0).scope.attributes.push_back(hostnameAttr); auto traceid = getTraceID(); + for (auto const& lockedSpan : *d_spans.read_only_lock()) { otTrace.resource_spans.at(0).scope_spans.at(0).spans.push_back( { .trace_id = traceid, - .span_id = lockedSpan.span_id, - .parent_span_id = lockedSpan.parent_span_id, + .span_id = lockedSpan.span_id == d_oldAndNewRootSpanID.oldID ? d_oldAndNewRootSpanID.newID : lockedSpan.span_id, + .parent_span_id = lockedSpan.parent_span_id == d_oldAndNewRootSpanID.oldID ? d_oldAndNewRootSpanID.newID : lockedSpan.parent_span_id, .name = lockedSpan.name, .kind = pdns::trace::Span::SpanKind::SPAN_KIND_SERVER, .start_time_unix_nano = lockedSpan.start_time_unix_nano, @@ -90,7 +91,7 @@ SpanID Tracer::addSpan([[maybe_unused]] const std::string& name) #ifdef DISABLE_PROTOBUF return 0; #else - return addSpan(name, SpanID{}); + return addSpan(name, getLastSpanID()); #endif } @@ -109,7 +110,7 @@ SpanID Tracer::addSpan([[maybe_unused]] const std::string& name, [[maybe_unused] .attributes = {}, }); - d_lastSpanID = spanID; + d_spanIDStack.push_back(spanID); return spanID; #endif } @@ -124,17 +125,18 @@ void Tracer::setTraceID([[maybe_unused]] const TraceID& traceID) void Tracer::setRootSpanID([[maybe_unused]] const SpanID& spanID) { #ifndef DISABLE_PROTOBUF - // XXX: We just assume that the first Span is the RootSpan SpanID oldRootSpanID; - if (auto lockedSpans = d_spans.lock(); !lockedSpans->empty()) { - oldRootSpanID = lockedSpans->begin()->span_id; - lockedSpans->begin()->span_id = spanID; - for (auto& span : *lockedSpans) { - if (span.parent_span_id == oldRootSpanID) { - span.parent_span_id = spanID; - } + if (auto spans = d_spans.read_only_lock(); !spans->empty()) { + auto iter = std::find_if(spans->cbegin(), spans->cend(), [](const auto& span) { return span.parent_span_id == pdns::trace::s_emptySpanID; }); + if (iter != spans->cend()) { + oldRootSpanID = iter->span_id; } } + if (oldRootSpanID == pdns::trace::s_emptySpanID) { + return; + } + d_oldAndNewRootSpanID.oldID = oldRootSpanID; + d_oldAndNewRootSpanID.newID = spanID; #endif } @@ -160,7 +162,11 @@ void Tracer::closeSpan([[maybe_unused]] const SpanID& spanID) [spanID](const miniSpan& span) { return span.span_id == spanID; }); if (spanIt != lockedSpans->rend() && spanIt->end_time_unix_nano == 0) { spanIt->end_time_unix_nano = pdns::trace::timestamp(); - return; + + // Only closers are allowed, so this can never happen + assert(!d_spanIDStack.empty()); + assert(d_spanIDStack.back() == spanID); + d_spanIDStack.pop_back(); } #endif } @@ -191,13 +197,17 @@ SpanID Tracer::getRootSpanID() #ifdef DISABLE_PROTOBUF return 0; #else + if (d_oldAndNewRootSpanID.newID != pdns::trace::s_emptySpanID) { + return d_oldAndNewRootSpanID.newID; + } + if (auto spans = d_spans.read_only_lock(); !spans->empty()) { auto iter = std::find_if(spans->cbegin(), spans->cend(), [](const auto& span) { return span.parent_span_id == pdns::trace::s_emptySpanID; }); if (iter != spans->cend()) { return iter->span_id; } } - return SpanID{}; + return pdns::trace::s_emptySpanID; #endif } @@ -206,10 +216,13 @@ SpanID Tracer::getLastSpanID() #ifdef DISABLE_PROTOBUF return 0; #else - if (auto lockedSpans = d_spans.read_only_lock(); !lockedSpans->empty()) { - return lockedSpans->back().span_id; + if (d_spanIDStack.empty()) { + return pdns::trace::s_emptySpanID; } - return pdns::trace::s_emptySpanID; + if (d_spanIDStack.size() == 1 && d_spanIDStack.front() == d_oldAndNewRootSpanID.oldID) { + return d_oldAndNewRootSpanID.newID; + } + return d_spanIDStack.back(); #endif } diff --git a/pdns/dnsdistdist/dnsdist-opentelemetry.hh b/pdns/dnsdistdist/dnsdist-opentelemetry.hh index fd78ca8ada..ff6d83e6ef 100644 --- a/pdns/dnsdistdist/dnsdist-opentelemetry.hh +++ b/pdns/dnsdistdist/dnsdist-opentelemetry.hh @@ -309,6 +309,7 @@ private: * @brief Stores all miniSpans. */ LockGuarded> d_spans; + /** * @brief All attributes related to this Trace (added to the ScopeSpan) */ @@ -320,10 +321,21 @@ private: * it is mutable because it is set the first time it is accessed */ mutable LockGuarded d_traceid{}; + + /** + * @brief A stack of SpanID's that tracks the "stack" of SpanIDs + */ + std::vector d_spanIDStack; + /** - * @brief The last SpanID that was added to this Tracer + * Set when setRootSpanID is called, used to replace the + * root span id (and the parent span ids) when the PB is generated */ - SpanID d_lastSpanID{}; + struct + { + SpanID oldID; + SpanID newID; + } d_oldAndNewRootSpanID; #endif }; } // namespace pdns::trace::dnsdist diff --git a/pdns/dnsdistdist/test-dnsdist-opentelemetry_cc.cc b/pdns/dnsdistdist/test-dnsdist-opentelemetry_cc.cc index bb01c3d871..3382639c34 100644 --- a/pdns/dnsdistdist/test-dnsdist-opentelemetry_cc.cc +++ b/pdns/dnsdistdist/test-dnsdist-opentelemetry_cc.cc @@ -57,14 +57,16 @@ BOOST_AUTO_TEST_CASE(getLastSpanID) BOOST_CHECK_EQUAL(lastSpanID, SpanID{}); // Add event before activation - auto spanid = tracer->openSpan("myevent").getSpanID(); + auto closer = tracer->openSpan("myevent"); + auto spanid = closer.getSpanID(); lastSpanID = tracer->getLastSpanID(); BOOST_CHECK_EQUAL(spanid, lastSpanID); for (auto i = 0; i < 4; i++) { - spanid = tracer->openSpan("myevent" + std::to_string(i)).getSpanID(); + auto closer2 = tracer->openSpan("myevent" + std::to_string(i)); + spanid = closer2.getSpanID(); + lastSpanID = tracer->getLastSpanID(); } - lastSpanID = tracer->getLastSpanID(); BOOST_CHECK_EQUAL(spanid, lastSpanID); } @@ -100,8 +102,6 @@ BOOST_AUTO_TEST_CASE(Closer) SpanID openevent2SpanID; { - auto closer = tracer->getCloser(spanid); - auto openEventCloser = tracer->openSpan("openEvent"); openeventSpanID = openEventCloser.getSpanID(); auto openEventCloser2 = tracer->openSpan("openEvent2", openeventSpanID);