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 f28e65e5c9714ea1a2681ab953d4b4b2cacbc96c..b7232f96bcf9c5882e39eb7a5eb94140536dbda6 100644 (file)
--- 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 fd78ca8ada8a120512bd7602cd96437217558177..ff6d83e6ef9c0471223ae2123b7a46f1cc4ea2fe 100644 (file)
--- a/pdns/dnsdistdist/dnsdist-opentelemetry.hh
+++ b/pdns/dnsdistdist/dnsdist-opentelemetry.hh
@@ -309,6 +309,7 @@ private:
    * @brief Stores all miniSpans.
    */
   LockGuarded<std::vector<miniSpan>> 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<TraceID> d_traceid{};
+
+  /**
+   * @brief A stack of SpanID's that tracks the "stack" of SpanIDs
+   */
+  std::vector<SpanID> 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 bb01c3d871456f94606a87b044624acfa3635723..3382639c34795efcb7fe8a5950c4867fdcaf1236 100644 (file)
--- 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);