From ba85aecf008fa66c26dfe15c368e784da8f169ad Mon Sep 17 00:00:00 2001
From: Pratik Mankawde <3397372+pratikmankawde@users.noreply.github.com>
Date: Mon, 15 Jun 2026 17:21:35 +0100
Subject: [PATCH] hide gTlDiscardCurrentSpan better

Signed-off-by: Pratik Mankawde <3397372+pratikmankawde@users.noreply.github.com>
---
 include/xrpl/telemetry/DiscardFlag.h | 53 +++++++++++++++-------------
 src/libxrpl/telemetry/Telemetry.cpp  |  4 +--
 2 files changed, 30 insertions(+), 27 deletions(-)

diff --git a/include/xrpl/telemetry/DiscardFlag.h b/include/xrpl/telemetry/DiscardFlag.h
index 05ba047b9c..b8e7bc4ea7 100644
--- a/include/xrpl/telemetry/DiscardFlag.h
+++ b/include/xrpl/telemetry/DiscardFlag.h
@@ -11,12 +11,13 @@
     This side-channel avoids inspecting the Recordable's internals (which vary
     by exporter type — SpanData vs OtlpRecordable).
 
-    The raw flag lives in `detail` and is mutated only through DiscardScope, a
-    RAII guard that sets it on construction and clears it on destruction. This
-    keeps the set/clear lifetime bound to a scope (so the flag cannot leak onto
-    the next span even if End() were to throw) and prevents any class which includes this header
-    from flipping the flag directly. FilteringSpanProcessor reads it through
-    isDiscardingCurrentSpan().
+    The flag is a *private* thread-local member of DiscardScope, mutated only by
+    its constructor and destructor. This gives real access control rather than a
+    naming convention: no code that includes this header can flip the flag
+    directly — it can only enter a DiscardScope (which sets and clears the flag
+    over its own lifetime) and observe the state via DiscardScope::isActive().
+    Binding set/clear to a scope also means the flag cannot leak onto the next
+    span even if End() were to throw.
 
     Kept in a separate header to avoid transitive include bloat: SpanGuard.h
     only needs this signaling, not the full Telemetry.h with BasicConfig/Journal.
@@ -28,6 +29,10 @@
             DiscardScope discardScope;  // flag set for this scope only
             span->End();                // OnEnd() runs synchronously, sees flag
         }                               // flag cleared here, unconditionally
+
+        // In FilteringSpanProcessor::OnEnd():
+        if (DiscardScope::isActive())
+            return;  // drop the span
     @endcode
 
     @note Thread safety: the flag is thread-local, so each thread observes only
@@ -38,19 +43,12 @@
 
 namespace xrpl::telemetry {
 
-namespace detail {
-
-/** Internal thread-local discard flag. Mutate only via DiscardScope; read
-    only via isDiscardingCurrentSpan(). Not intended for direct use. */
-inline thread_local bool gTlDiscardCurrentSpan = false;
-
-}  // namespace detail
-
 /** RAII guard that marks the current thread's span for discard.
 
     Sets the thread-local discard flag on construction and clears it on
     destruction, so a span ended within the guard's scope is dropped by
     FilteringSpanProcessor::OnEnd() while the flag stays confined to that scope.
+    The flag is private and mutated only here, so no other code can set it.
     Non-copyable and non-movable — its sole purpose is the scoped flag lifetime.
 */
 class DiscardScope
@@ -58,12 +56,12 @@ class DiscardScope
 public:
     DiscardScope() noexcept
     {
-        detail::gTlDiscardCurrentSpan = true;
+        discarding = true;
     }
 
     ~DiscardScope()
     {
-        detail::gTlDiscardCurrentSpan = false;
+        discarding = false;
     }
 
     DiscardScope(DiscardScope const&) = delete;
@@ -72,15 +70,20 @@ public:
     DiscardScope(DiscardScope&&) = delete;
     DiscardScope&
     operator=(DiscardScope&&) = delete;
+
+    /** @return true if the current thread is inside a DiscardScope, i.e. the
+        span ending now should be dropped rather than exported. Read by
+        FilteringSpanProcessor::OnEnd(). */
+    [[nodiscard]] static bool
+    isActive() noexcept
+    {
+        return discarding;
+    }
+
+private:
+    /** Thread-local discard flag. Private, so only this class's ctor/dtor can
+        mutate it; observers use isActive(). One instance per thread. */
+    inline static thread_local bool discarding = false;
 };
 
-/** @return true if the current thread is inside a DiscardScope, i.e. the span
-    ending now should be dropped rather than exported. Read by
-    FilteringSpanProcessor::OnEnd(). */
-[[nodiscard]] inline bool
-isDiscardingCurrentSpan() noexcept
-{
-    return detail::gTlDiscardCurrentSpan;
-}
-
 }  // namespace xrpl::telemetry
diff --git a/src/libxrpl/telemetry/Telemetry.cpp b/src/libxrpl/telemetry/Telemetry.cpp
index 259e0febfc..23643e9a7e 100644
--- a/src/libxrpl/telemetry/Telemetry.cpp
+++ b/src/libxrpl/telemetry/Telemetry.cpp
@@ -62,7 +62,7 @@ namespace resource = opentelemetry::sdk::resource;
 /** SpanProcessor decorator that drops discarded spans.
 
     Wraps a delegate processor (typically BatchSpanProcessor). In OnEnd(),
-    calls isDiscardingCurrentSpan(). If the calling thread is inside a
+    calls DiscardScope::isActive(). If the calling thread is inside a
     DiscardScope (entered by SpanGuard::discard()), the span is silently
     dropped — never entering the batch queue, never sent over the network,
     never stored.
@@ -119,7 +119,7 @@ public:
     void
     OnEnd(std::unique_ptr<trace_sdk::Recordable>&& span) noexcept override
     {
-        if (isDiscardingCurrentSpan())
+        if (DiscardScope::isActive())
         {
             // SpanGuard::discard() is inside a DiscardScope on this thread,
             // which it entered just before calling Span::End() — and End()