From ad233846faa2bb20a2e0e62cb1362a28eb391915 Mon Sep 17 00:00:00 2001
From: Pratik Mankawde <3397372+pratikmankawde@users.noreply.github.com>
Date: Fri, 17 Apr 2026 17:15:36 +0100
Subject: [PATCH] fix(telemetry): address Phase 1b code review findings

- SpanContext::isValid(): add inline no-op when XRPL_ENABLE_TELEMETRY
  is not defined, preventing a linker error if called in that path
- linkedSpan(): set kIsRootSpanKey on the StartSpanOptions parent
  context so linked spans start a genuinely independent sub-tree
  instead of silently becoming children of the current active span
- Telemetry::instance_: use std::atomic with acquire/release ordering
  to avoid a data race between start()/stop() and factory methods
  called from worker threads

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
---
 include/xrpl/telemetry/SpanGuard.h  |  8 ++++++++
 include/xrpl/telemetry/Telemetry.h  | 10 +++++++---
 src/libxrpl/telemetry/SpanGuard.cpp | 18 +++++++++++++++---
 3 files changed, 30 insertions(+), 6 deletions(-)

diff --git a/include/xrpl/telemetry/SpanGuard.h b/include/xrpl/telemetry/SpanGuard.h
index 4d0ce632c2..b4c03c7730 100644
--- a/include/xrpl/telemetry/SpanGuard.h
+++ b/include/xrpl/telemetry/SpanGuard.h
@@ -139,8 +139,16 @@ public:
     SpanContext() = default;
 
     /** @return true if this context holds a valid trace context. */
+#ifdef XRPL_ENABLE_TELEMETRY
     bool
     isValid() const;
+#else
+    bool
+    isValid() const
+    {
+        return false;
+    }
+#endif
 };
 
 // ---------------------------------------------------------------------------
diff --git a/include/xrpl/telemetry/Telemetry.h b/include/xrpl/telemetry/Telemetry.h
index 6e412fba4e..d3b729815a 100644
--- a/include/xrpl/telemetry/Telemetry.h
+++ b/include/xrpl/telemetry/Telemetry.h
@@ -72,6 +72,7 @@
 #include <xrpl/basics/BasicConfig.h>
 #include <xrpl/beast/utility/Journal.h>
 
+#include <atomic>
 #include <chrono>
 #include <memory>
 #include <string>
@@ -92,9 +93,12 @@ class Telemetry
     /** Global singleton pointer, set by start()/stop() in the active
         implementation. Allows SpanGuard factory methods to access the
         Telemetry instance without callers passing it explicitly.
+
+        Atomic with acquire/release ordering: start()/stop() store on
+        the initialization thread, factory methods load on worker threads.
         @see setInstance(), getInstance()
     */
-    inline static Telemetry* instance_ = nullptr;
+    inline static std::atomic<Telemetry*> instance_{nullptr};
 
 public:
     /** Get the global Telemetry instance.
@@ -103,7 +107,7 @@ public:
     static Telemetry*
     getInstance()
     {
-        return instance_;
+        return instance_.load(std::memory_order_acquire);
     }
 
     /** Set the global Telemetry instance.
@@ -114,7 +118,7 @@ public:
     static void
     setInstance(Telemetry* t)
     {
-        instance_ = t;
+        instance_.store(t, std::memory_order_release);
     }
 
     /** Configuration parsed from the [telemetry] section of xrpld.cfg.
diff --git a/src/libxrpl/telemetry/SpanGuard.cpp b/src/libxrpl/telemetry/SpanGuard.cpp
index bf1cc73d8b..8d1aebdd97 100644
--- a/src/libxrpl/telemetry/SpanGuard.cpp
+++ b/src/libxrpl/telemetry/SpanGuard.cpp
@@ -200,7 +200,13 @@ SpanGuard::linkedSpan(std::string_view name) const
     auto tracer = tel->getTracer("xrpld");
     auto spanCtx = impl_->span->GetContext();
 
+    // Mark as root span so it starts a new trace sub-tree rather than
+    // inheriting the current thread's active span as parent.
     otel_trace::StartSpanOptions opts;
+    opentelemetry::context::Context rootCtx;
+    rootCtx = rootCtx.SetValue(otel_trace::kIsRootSpanKey, true);
+    opts.parent = rootCtx;
+
     return SpanGuard(
         std::make_unique<Impl>(tracer->StartSpan(
             std::string(name), {}, {{spanCtx, {{"xrpl.link.type", "follows_from"}}}}, opts)));
@@ -218,16 +224,22 @@ SpanGuard::linkedSpan(std::string_view name, SpanContext const& linkCtx)
     auto tracer = tel->getTracer("xrpld");
 
     // Extract the span from the captured context to get its SpanContext.
-    auto parentSpan = otel_trace::GetSpan(linkCtx.impl_->ctx);
-    if (!parentSpan || !parentSpan->GetContext().IsValid())
+    auto linkSpan = otel_trace::GetSpan(linkCtx.impl_->ctx);
+    if (!linkSpan || !linkSpan->GetContext().IsValid())
         return {};
 
+    // Mark as root span so it starts a new trace sub-tree rather than
+    // inheriting the current thread's active span as parent.
     otel_trace::StartSpanOptions opts;
+    opentelemetry::context::Context rootCtx;
+    rootCtx = rootCtx.SetValue(otel_trace::kIsRootSpanKey, true);
+    opts.parent = rootCtx;
+
     return SpanGuard(
         std::make_unique<Impl>(tracer->StartSpan(
             std::string(name),
             {},
-            {{parentSpan->GetContext(), {{"xrpl.link.type", "follows_from"}}}},
+            {{linkSpan->GetContext(), {{"xrpl.link.type", "follows_from"}}}},
             opts)));
 }