From f1b7104bb82fa32dd7bb13ed3ae1281b96454bfd Mon Sep 17 00:00:00 2001 From: Pratik Mankawde <3397372+pratikmankawde@users.noreply.github.com> Date: Wed, 22 Jul 2026 16:56:16 +0100 Subject: [PATCH 1/5] feat(telemetry): coroutine-aware OTel context storage Backs the OTel active-context stack with xrpl::LocalValue so the ambient context follows a JobQueue::Coro across yield/resume. Not yet installed. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../scripts/levelization/results/ordering.txt | 1 + .../xrpl/telemetry/CoroAwareContextStorage.h | 121 ++++++++++++++++++ .../telemetry/CoroAwareContextStorage.cpp | 68 ++++++++++ 3 files changed, 190 insertions(+) create mode 100644 include/xrpl/telemetry/CoroAwareContextStorage.h create mode 100644 src/libxrpl/telemetry/CoroAwareContextStorage.cpp diff --git a/.github/scripts/levelization/results/ordering.txt b/.github/scripts/levelization/results/ordering.txt index b38c125c7c..9538b32802 100644 --- a/.github/scripts/levelization/results/ordering.txt +++ b/.github/scripts/levelization/results/ordering.txt @@ -243,6 +243,7 @@ xrpl.server > xrpl.resource xrpl.shamap > xrpl.basics xrpl.shamap > xrpl.nodestore xrpl.shamap > xrpl.protocol +xrpl.telemetry > xrpl.basics xrpl.telemetry > xrpl.config xrpl.tx > xrpl.basics xrpl.tx > xrpl.core diff --git a/include/xrpl/telemetry/CoroAwareContextStorage.h b/include/xrpl/telemetry/CoroAwareContextStorage.h new file mode 100644 index 0000000000..ee982ace6c --- /dev/null +++ b/include/xrpl/telemetry/CoroAwareContextStorage.h @@ -0,0 +1,121 @@ +#pragma once + +/** + * A coroutine-aware OpenTelemetry runtime-context storage. + * + * OpenTelemetry's default ThreadLocalContextStorage keeps the active-context + * stack in a plain static thread_local, so the ambient span does NOT follow a + * JobQueue::Coro across yield/resume — a scope pushed on one worker is stranded + * when the coroutine resumes on another. This storage instead keeps the stack + * in an xrpl::LocalValue, which JobQueue::Coro::resume() swaps in and out with + * the coroutine (see Coro.ipp). The active context therefore rides the + * coroutine: scopes held across yield are safe, and per-line log-trace + * correlation (Log.cpp reads RuntimeContext::GetCurrent()) is retained. + * + * Off a coroutine, LocalValue transparently provides a per-thread store, so + * behaviour is identical to the default thread-local storage. + * + * +-------------------------------------------------+ + * | CoroAwareContextStorage | + * | (opentelemetry RuntimeContextStorage) | + * +-------------------------------------------------+ + * | - stack_ : LocalValue> | + * +-------------------------------------------------+ + * | + GetCurrent() : Context | + * | + Attach(ctx) : unique_ptr | + * | + Detach(token): bool | + * +-------------------------------------------------+ + * | backed by (coro-aware) + * +------------------------+ + * | xrpl::LocalValue store | + * | (swapped by Coro) | + * +------------------------+ + * + * Install once at telemetry start via + * opentelemetry::context::RuntimeContext::SetRuntimeContextStorage(), BEFORE + * any span is created (SDK requirement). + * + * @note Thread-safety: each thread/coroutine sees its own LocalValue store, so + * the stack is never shared across threads — no locking needed. The storage + * object itself is stateless apart from the LocalValue handle. + * @note Known limitation: install once, before the first span; resetting the + * storage while spans exist is undefined behaviour (SDK). A span created on a + * raw worker thread and later moved onto a coroutine does not retro-attach to + * the coroutine's store — not a pattern here (spans are created inside their + * own coro/job body). + * + * Example 1 — install at telemetry start (primary use): + * @code + * using opentelemetry::context::RuntimeContext; + * RuntimeContext::SetRuntimeContextStorage( + * opentelemetry::nostd::shared_ptr< + * opentelemetry::context::RuntimeContextStorage>( + * new xrpl::telemetry::CoroAwareContextStorage())); + * @endcode + * + * Example 2 — a scope held across a coroutine yield stays correct: + * @code + * // Inside a JobQueue::Coro body: + * auto span = ScopedSpanGuard(TraceCategory::Rpc, "rpc", "process"); + * context.coro->yield(); // may resume on another worker + * // span is still the ambient context here — the storage rode the coro. + * @endcode + * + * Example 3 — edge case: off a coroutine, behaves like thread-local storage: + * @code + * // On a plain worker thread (no coro): each thread has its own stack, + * // identical to opentelemetry's default ThreadLocalContextStorage. + * auto span = ScopedSpanGuard(TraceCategory::Ledger, "ledger", "build"); + * @endcode + */ + +#ifdef XRPL_ENABLE_TELEMETRY + +#include + +#include +#include +#include + +#include + +namespace xrpl::telemetry { + +class CoroAwareContextStorage : public opentelemetry::context::RuntimeContextStorage +{ +public: + CoroAwareContextStorage() = default; + + /** + * @return the current (top-of-stack) context for this coro/thread. + */ + opentelemetry::context::Context + GetCurrent() noexcept override; + + /** + * Push a context frame onto this coro/thread's stack. + * @param context the context to make current. + * @return a token that Detach() uses to pop back to the prior frame. + */ + opentelemetry::nostd::unique_ptr + Attach(opentelemetry::context::Context const& context) noexcept override; + + /** + * Pop the stack back through the frame the token refers to. + * @param token a token returned by Attach(). + * @return true if the frame was found and detached. + */ + bool + Detach(opentelemetry::context::Token& token) noexcept override; + +private: + /** + * The active-context stack, stored coro-locally. LocalValue hands back the + * stack for whichever store (coro or thread) is currently installed. + */ + LocalValue> stack_; +}; + +} // namespace xrpl::telemetry + +#endif // XRPL_ENABLE_TELEMETRY diff --git a/src/libxrpl/telemetry/CoroAwareContextStorage.cpp b/src/libxrpl/telemetry/CoroAwareContextStorage.cpp new file mode 100644 index 0000000000..acd0dc77c3 --- /dev/null +++ b/src/libxrpl/telemetry/CoroAwareContextStorage.cpp @@ -0,0 +1,68 @@ +/** + * Implementation of CoroAwareContextStorage. + * + * Mirrors opentelemetry's ThreadLocalContextStorage stack semantics, but the + * stack lives in an xrpl::LocalValue so it follows a JobQueue::Coro across + * yield/resume. All OpenTelemetry types stay confined to this translation unit. + * + * @see CoroAwareContextStorage (CoroAwareContextStorage.h) + */ + +#ifdef XRPL_ENABLE_TELEMETRY + +#include + +#include +#include +#include + +namespace xrpl::telemetry { + +opentelemetry::context::Context +CoroAwareContextStorage::GetCurrent() noexcept +{ + auto& stack = *stack_; + return stack.empty() ? opentelemetry::context::Context{} : stack.back(); +} + +opentelemetry::nostd::unique_ptr +CoroAwareContextStorage::Attach(opentelemetry::context::Context const& context) noexcept +{ + stack_->push_back(context); + return CreateToken(context); +} + +bool +CoroAwareContextStorage::Detach(opentelemetry::context::Token& token) noexcept +{ + auto& stack = *stack_; + // Fast path: the token's frame is on top (the common LIFO case). + if (!stack.empty() && token == stack.back()) + { + stack.pop_back(); + return true; + } + // Fallback: token not on top — verify it is present, then pop down to and + // including it (mirrors ThreadLocalContextStorage::Detach; also detaches + // any child frames left above it). + bool found = false; + for (auto const& frame : stack) + { + if (token == frame) + { + found = true; + break; + } + } + if (!found) + return false; + while (!stack.empty() && !(token == stack.back())) + stack.pop_back(); + if (!stack.empty()) + stack.pop_back(); + return true; +} + +} // namespace xrpl::telemetry + +#endif // XRPL_ENABLE_TELEMETRY From 9fa58067fa34c66a100556f4b3c4a6077ff35653 Mon Sep 17 00:00:00 2001 From: Pratik Mankawde <3397372+pratikmankawde@users.noreply.github.com> Date: Wed, 22 Jul 2026 17:12:11 +0100 Subject: [PATCH 2/5] feat(telemetry): install coroutine-aware context storage at start Install CoroAwareContextStorage as the process-wide OTel runtime context storage in TelemetryImpl::start(), before SetTracerProvider and before any span is created, so the ambient context follows JobQueue coroutines across yield/resume. Hold it in a member for the process lifetime; not reset in stop() because resetting while spans may exist is SDK undefined behaviour. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/libxrpl/telemetry/Telemetry.cpp | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/src/libxrpl/telemetry/Telemetry.cpp b/src/libxrpl/telemetry/Telemetry.cpp index a7ba3a4a11..8076146a9c 100644 --- a/src/libxrpl/telemetry/Telemetry.cpp +++ b/src/libxrpl/telemetry/Telemetry.cpp @@ -20,10 +20,12 @@ #include #include +#include #include #include #include +#include #include #include #include @@ -263,6 +265,13 @@ class TelemetryImpl : public Telemetry */ std::shared_ptr sdkProvider_; + /** + * Coroutine-aware runtime-context storage, installed globally so the OTel + * ambient context follows JobQueue coroutines. Held for the process + * lifetime because it must outlive every span (SDK requirement). + */ + opentelemetry::nostd::shared_ptr contextStorage_; + public: TelemetryImpl(Setup setup, beast::Journal journal) : setup_(std::move(setup)), journal_(journal) { @@ -332,6 +341,18 @@ public: std::move(sampler), std::make_unique()); + // Install coroutine-aware context storage BEFORE any span is created + // so the OTel ambient context follows JobQueue coroutines across + // yield/resume (fixes wrong-thread scope pop; keeps log-trace + // correlation). Must precede SetTracerProvider and the first span. + // Not reset in stop(): resetting the storage while spans may still + // exist is undefined behaviour (SDK), and by stop() all spans are + // gone, so the storage is simply left installed for process lifetime. + contextStorage_ = + opentelemetry::nostd::shared_ptr( + new CoroAwareContextStorage()); + opentelemetry::context::RuntimeContext::SetRuntimeContextStorage(contextStorage_); + // Set as global provider trace_api::Provider::SetTracerProvider( opentelemetry::nostd::shared_ptr(sdkProvider_)); From 7dbbf95c72cb56eebd5f00f69b0246363d3a0bbd Mon Sep 17 00:00:00 2001 From: Pratik Mankawde <3397372+pratikmankawde@users.noreply.github.com> Date: Wed, 22 Jul 2026 18:52:48 +0100 Subject: [PATCH 3/5] refactor(telemetry): ScopedSpanGuard asserts same context store, not thread Coroutine-aware storage lets a scope resume on another worker within the same coroutine store; store-identity is the correct pop-safety invariant. Same-store equals same-thread for synchronous code, so no safety is lost. Co-Authored-By: Claude Opus 4.8 (1M context) --- include/xrpl/telemetry/SpanGuard.h | 47 ++++++++------- src/libxrpl/telemetry/SpanGuard.cpp | 88 ++++++++++++++++++----------- 2 files changed, 82 insertions(+), 53 deletions(-) diff --git a/include/xrpl/telemetry/SpanGuard.h b/include/xrpl/telemetry/SpanGuard.h index 341dc9fe71..d0abaa3769 100644 --- a/include/xrpl/telemetry/SpanGuard.h +++ b/include/xrpl/telemetry/SpanGuard.h @@ -11,9 +11,11 @@ * to and destroyed on any thread. Movable AND * move-assignable. * - ScopedSpanGuard — owns a SpanGuard plus an active OTel Scope that - * pushes the span onto the constructing thread's - * context stack. Thread-bound: it must be created and - * destroyed on the same thread. Non-copyable and + * pushes the span onto the constructing context store's + * stack. Store-bound: it must be created and destroyed + * while the same LocalValue context store is active (the + * same thread, or the same JobQueue coroutine even if it + * resumes on another worker). Non-copyable and * non-movable. * * Both wrap all OpenTelemetry types behind the pimpl idiom so no @@ -446,26 +448,26 @@ public: }; /** - * RAII guard that activates a span on the current thread (scoped). + * RAII guard that activates a span on the current context store (scoped). * * Wraps a SpanGuard (which owns the span) plus an OTel Scope that - * pushes the span onto this thread's thread-local context stack for - * the guard's lifetime, so child spans created on the thread inherit + * pushes the span onto the active context store's stack for the + * guard's lifetime, so child spans created under that store inherit * it as parent. On destruction the Scope pops BEFORE the span ends * (member order: guard first, scope second). Non-copyable and * non-movable — factories return unnamed temporaries, so guaranteed * copy elision (C++17) covers `auto s = ScopedSpanGuard::freshRoot(...)`. * - * When the span must outlive the current thread (e.g. handed to a job + * When the span must outlive the current store (e.g. handed to a job * queue), convert to a plain SpanGuard with `operator SpanGuard() &&`: - * that pops the Scope eagerly on the origin thread and yields a + * that pops the Scope eagerly under the constructing store and yields a * thread-free guard. * * ScopedSpanGuard dependency diagram: * * +--------------------------------------------+ * | ScopedSpanGuard | - * | (scoped, thread-bound) | + * | (scoped, store-bound) | * +--------------------------------------------+ * | - impl_ : unique_ptr (pimpl) | * +--------------------------------------------+ @@ -483,7 +485,7 @@ public: * | | * +----------+ +-----------------------------+ * | SpanGuard| | optional | - * | (span) | | (OTel, thread-bound; | + * | (span) | | (OTel, store-bound; | * | | | present : span active) | * +----------+ +-----------------------------+ * @@ -513,11 +515,14 @@ public: * @endcode * * @note Thread safety: A ScopedSpanGuard must be constructed AND - * destroyed on the same thread — its Scope binds to that thread's - * context stack. `operator SpanGuard() &&` and the destructor must - * both run on the owning thread; both check this with an XRPL_ASSERT - * in debug/test/fuzzing builds. `operator SpanGuard() &&` pops the - * scope eagerly, so the resulting SpanGuard is thread-free. + * destroyed while the same LocalValue context store is active — its + * Scope binds to that store's context stack. This means the same + * thread, or the same JobQueue coroutine even if it resumes on another + * worker (coroutine-aware storage keeps the same store across the + * resume). `operator SpanGuard() &&` and the destructor must both run + * under the constructing store; both check this with an XRPL_ASSERT in + * debug/test/fuzzing builds. `operator SpanGuard() &&` pops the scope + * eagerly, so the resulting SpanGuard is thread-free. * * @note Known limitations: * - Non-movable: cannot be stored in a container that relocates, and @@ -609,9 +614,10 @@ public: // --- Handoff bridge ------------------------------------------------- /** - * Pop the active Scope on the origin thread and yield the span as a - * thread-free SpanGuard. Use to move a span into a job or onto - * another thread. Must be called on the constructing thread. + * Pop the active Scope under the constructing context store and yield + * the span as a thread-free SpanGuard. Use to move a span into a job or + * onto another thread. Must be called while the constructing context + * store is active. * @return The unscoped SpanGuard that now owns the span. */ operator SpanGuard() &&; @@ -692,8 +698,9 @@ public: /** * Mark this span for discard and end it immediately. discard() pops the - * thread-local scope right away (on the owning thread) and discards the - * span. After discard() the guard is inert and its destructor is a no-op. + * scope right away (under the constructing context store) and discards + * the span. After discard() the guard is inert and its destructor is a + * no-op. */ void discard(); diff --git a/src/libxrpl/telemetry/SpanGuard.cpp b/src/libxrpl/telemetry/SpanGuard.cpp index 8c71e55046..744f93a35c 100644 --- a/src/libxrpl/telemetry/SpanGuard.cpp +++ b/src/libxrpl/telemetry/SpanGuard.cpp @@ -10,10 +10,12 @@ * touches the thread-local context stack, so a SpanGuard is * thread-free and may be moved to and destroyed on any thread. * - ScopedSpanGuard::ScopedImpl holds a SpanGuard plus an optional - * Scope. The Scope pushes the span onto the constructing thread's - * context stack; member order (guard first, scope second) ensures + * Scope. The Scope pushes the span onto the constructing context + * store's stack; member order (guard first, scope second) ensures * the Scope pops BEFORE the span ends on destruction. It is - * thread-bound: construct and destroy on the same thread. + * store-bound: construct and destroy while the same LocalValue + * context store is active (the same thread, or the same JobQueue + * coroutine even if it resumes on another worker). * * Static factory methods access the global Telemetry instance via * Telemetry::getInstance(), check whether the requested TraceCategory @@ -28,6 +30,7 @@ #include +#include #include #include #include @@ -48,7 +51,6 @@ #include #include #include -#include #include #include @@ -428,21 +430,35 @@ struct ScopedSpanGuard::ScopedImpl SpanGuard guard; /** - * Active OTel Scope binding the span to this thread's context stack. - * Present while the guard is active; reset() (via operator SpanGuard) - * pops it eagerly on the origin thread. Declared AFTER `guard` so - * destruction order pops the scope before the span ends. + * Active OTel Scope binding the span to the active context store's + * stack. Present while the guard is active; reset() (via operator + * SpanGuard) pops it eagerly under the constructing store. Declared + * AFTER `guard` so destruction order pops the scope before the span + * ends. */ std::optional scope; /** - * Thread that constructed this ScopedImpl. The Scope is bound to - * this thread's context stack, so popping it (via ~Scope or reset()) - * on a different thread corrupts that thread's stack. Checked in the - * destructor and operator SpanGuard() to turn a silent cross-thread - * corruption into an assertion failure in debug/test/fuzzing builds. + * Identity of the LocalValue store the Scope was pushed onto (the + * coroutine's store on a coro, else the thread's own store). Popping + * the Scope (via ~Scope or reset()) must happen while the SAME store + * is active, else a different stack is corrupted. Coroutine-aware + * storage lets a coro legitimately resume on another worker while + * keeping the same store, so store-identity — not thread-id — is the + * correct invariant. Checked in the destructor and operator + * SpanGuard() to turn a silent cross-store pop into an assertion + * failure in debug/test/fuzzing builds. + * + * Assigned in the constructor body AFTER the Scope push, not as a + * member initializer: the push is the first LocalValue touch on a + * fresh thread for a root or explicit-parent span (the OTel SDK skips + * the ambient-context read in those cases), so it materializes the + * store. Capturing before the push would record nullptr while the + * destructor sees the now-materialized store. For a null guard no + * Scope is pushed and the assertions short-circuit, so this holds + * whatever store is active then. */ - std::thread::id owner{std::this_thread::get_id()}; + void const* owner = nullptr; /** * Wrap a SpanGuard and activate its span on this thread. If the @@ -454,6 +470,9 @@ struct ScopedSpanGuard::ScopedImpl { if (guard) scope.emplace(guard.impl_->span); + // Capture after the push so `owner` names the store the Scope + // actually lives on, even when the push materialized it. + owner = detail::getLocalValues().get(); } }; @@ -466,13 +485,14 @@ ScopedSpanGuard::ScopedSpanGuard(SpanGuard&& guard) ScopedSpanGuard::~ScopedSpanGuard() { - // A live Scope must be popped on the thread that pushed it; destroying - // the guard on any other thread corrupts that thread's context stack. - // A null guard holds no Scope, so the check is skipped. + // A live Scope must be popped while its constructing context store is + // active; destroying the guard under a different store corrupts that + // store's context stack. A null guard holds no Scope, so the check is + // skipped. XRPL_ASSERT( - !impl_ || !impl_->scope.has_value() || impl_->owner == std::this_thread::get_id(), - "xrpl::telemetry::ScopedSpanGuard::~ScopedSpanGuard : destroyed on " - "constructing thread"); + !impl_ || !impl_->scope.has_value() || impl_->owner == detail::getLocalValues().get(), + "xrpl::telemetry::ScopedSpanGuard::~ScopedSpanGuard : destroyed on the " + "constructing context store"); } // ===== ScopedSpanGuard factory methods ===================================== @@ -517,13 +537,14 @@ ScopedSpanGuard::linkedSpan(std::string_view name, SpanContext const& linkCtx) ScopedSpanGuard:: operator SpanGuard() && { - // The scope must be popped on the thread that pushed it; handing off - // elsewhere would pop the wrong stack. + // The scope must be popped while its constructing context store is + // active; handing off under a different store would pop the wrong stack. + // A null guard holds no Scope, so the check is skipped. XRPL_ASSERT( - impl_->owner == std::this_thread::get_id(), - "xrpl::telemetry::ScopedSpanGuard::operator SpanGuard : handoff on " - "constructing thread"); - impl_->scope.reset(); // eager pop, on the origin thread + !impl_->scope.has_value() || impl_->owner == detail::getLocalValues().get(), + "xrpl::telemetry::ScopedSpanGuard::operator SpanGuard : handoff on the " + "constructing context store"); + impl_->scope.reset(); // eager pop, on the origin store return std::move(impl_->guard); } @@ -594,14 +615,15 @@ ScopedSpanGuard::recordException(std::exception const& e) void ScopedSpanGuard::discard() { - // A live Scope must be popped on the thread that pushed it; discarding - // on any other thread corrupts that thread's context stack. A null guard - // holds no Scope, so the check is skipped. + // A live Scope must be popped while its constructing context store is + // active; discarding under a different store corrupts that store's + // context stack. A null guard holds no Scope, so the check is skipped. XRPL_ASSERT( - !impl_ || !impl_->scope.has_value() || impl_->owner == std::this_thread::get_id(), - "xrpl::telemetry::ScopedSpanGuard::discard : discarded on constructing thread"); - // Pop the scope first (on this thread) so no span stays active on the - // stack, then discard the span via the owned guard. + !impl_ || !impl_->scope.has_value() || impl_->owner == detail::getLocalValues().get(), + "xrpl::telemetry::ScopedSpanGuard::discard : discarded on the " + "constructing context store"); + // Pop the scope first (under the constructing store) so no span stays + // active on the stack, then discard the span via the owned guard. impl_->scope.reset(); impl_->guard.discard(); } From e3c724423f34a603aee12dc89eb2f99decfb3888 Mon Sep 17 00:00:00 2001 From: Pratik Mankawde <3397372+pratikmankawde@users.noreply.github.com> Date: Wed, 22 Jul 2026 19:32:18 +0100 Subject: [PATCH 4/5] feat(telemetry): add ScopedActivation for non-owning span activation Add SpanGuard::activate() returning a ScopedActivation RAII helper that activates an already-owned span (from a thread-free SpanGuard) as the current context WITHOUT taking ownership. The activation pushes the span onto the current LocalValue context store on construction and pops it on destruction; it never ends the span (its owning SpanGuard does). This lets a job-handoff span be made ambient for the duration of a synchronous, non-yielding worker body so log lines there carry the span's trace_id. Non-copyable and non-movable, mirroring ScopedSpanGuard. owner is captured after the Scope push via declaration-order member initialization (scope declared before owner), matching the A3 capture-after-materialization invariant. A #else no-op stub keeps the API zero-overhead when telemetry is compiled out. Co-Authored-By: Claude Opus 4.8 (1M context) --- include/xrpl/telemetry/SpanGuard.h | 120 ++++++++++++++++++++++++++++ src/libxrpl/telemetry/SpanGuard.cpp | 58 ++++++++++++++ 2 files changed, 178 insertions(+) diff --git a/include/xrpl/telemetry/SpanGuard.h b/include/xrpl/telemetry/SpanGuard.h index d0abaa3769..d4543e145b 100644 --- a/include/xrpl/telemetry/SpanGuard.h +++ b/include/xrpl/telemetry/SpanGuard.h @@ -42,6 +42,7 @@ * | + addEvent(name) | * | + recordException(e) | * | + discard() | + * | + activate() : ScopedActivation | * | + operator bool() | * +--------------------------------------------+ * | hides (pimpl) @@ -233,6 +234,11 @@ public: // --------------------------------------------------------------------------- #ifdef XRPL_ENABLE_TELEMETRY +// SpanGuard::activate() returns a ScopedActivation by value; the full class is +// defined after ScopedSpanGuard below. Forward-declared here so the return type +// is named before its definition. +class ScopedActivation; + /** * RAII owner of an OTel span (unscoped, thread-free). * @@ -440,6 +446,17 @@ public: void discard(); + // --- Activation (non-owning) --------------------------------------- + + /** + * Activate this guard's span as the current context for the returned + * activation's lifetime, without transferring ownership. Returns a no-op + * activation if this guard is null. See ScopedActivation. + * @return an RAII activation; drop it to restore the prior context. + */ + [[nodiscard]] ScopedActivation + activate() const; + /** * @return true if this guard holds an active span. */ @@ -712,11 +729,106 @@ public: operator bool() const; }; +/** + * RAII activation of an already-owned span as the current context, WITHOUT + * taking ownership. Use to give a thread-free SpanGuard (e.g. one handed into + * a JobQueue job) ambient status for the duration of a synchronous, non-yielding + * worker body, so log lines emitted there carry the span's trace_id. The span + * is neither owned nor ended here — its owning SpanGuard still controls its + * lifetime. Non-copyable, non-movable; must be created and destroyed while the + * same context store is active (see ScopedSpanGuard). + * + * Dependency diagram: + * + * +---------------------------------------------+ + * | ScopedActivation | + * | (non-owning RAII span activation) | + * +---------------------------------------------+ + * | - impl_ : unique_ptr | + * | Impl { scope : otel::Scope; owner } | + * +---------------------------------------------+ + * | + (ctor) pushes span onto current store | + * | + (dtor) pops; does NOT end the span | + * +---------------------------------------------+ + * | activates (borrows, no ownership) + * +-----------+ ends the span + * | SpanGuard |----------------------------> (owner) + * +-----------+ + * + * @note Thread-safety: like ScopedSpanGuard, it pushes/pops the current + * LocalValue context store; construct and destroy it while the same store is + * active (asserted in debug). Must NOT outlive the SpanGuard whose span it + * activates. Intended as a short-lived stack local inside a worker body that + * runs to completion on one thread (no coro yield inside the activation). + * + * Example 1 — correlate a handed-off span's worker logs (primary use): + * @code + * // span is std::shared_ptr handed into this job body: + * auto activation = (span && *span) ? span->activate() : ScopedActivation{}; + * JLOG(j.info()) << "applied"; // carries span's trace_id + * // span is still owned/ended elsewhere; activation only pops here. + * @endcode + * + * Example 2 — edge case: a null guard yields a no-op activation: + * @code + * SpanGuard g; // null (telemetry off, or disabled category) + * auto a = g.activate(); // no-op; GetCurrent() unchanged + * @endcode + */ +class ScopedActivation +{ + struct Impl; + std::unique_ptr impl_; + friend class SpanGuard; + explicit ScopedActivation(std::unique_ptr impl); + +public: + /** + * Construct a null / no-op activation. Its destructor does nothing and + * leaves the current context unchanged. Returned by SpanGuard::activate() + * for a null guard. + */ + ScopedActivation(); + + /** + * Pop the activated span off the current context store, restoring the + * prior context. Does NOT end the span (the owning SpanGuard does that). + * A null activation is a no-op. + */ + ~ScopedActivation(); + + ScopedActivation(ScopedActivation&&) = delete; + ScopedActivation& + operator=(ScopedActivation&&) = delete; + ScopedActivation(ScopedActivation const&) = delete; + ScopedActivation& + operator=(ScopedActivation const&) = delete; +}; + // --------------------------------------------------------------------------- // No-op stub (all inline, zero overhead, no OTel dependency) // --------------------------------------------------------------------------- #else // XRPL_ENABLE_TELEMETRY not defined +/** + * No-op activation used when telemetry is disabled at compile time. Holds no + * state and does nothing on construction or destruction. Declared before + * SpanGuard so its inline activate() can return one by value. Non-copyable and + * non-movable to match the real ScopedActivation. + */ +class ScopedActivation +{ +public: + ScopedActivation() = default; + ~ScopedActivation() = default; + ScopedActivation(ScopedActivation&&) = delete; + ScopedActivation& + operator=(ScopedActivation&&) = delete; + ScopedActivation(ScopedActivation const&) = delete; + ScopedActivation& + operator=(ScopedActivation const&) = delete; +}; + class SpanGuard { public: @@ -818,6 +930,14 @@ public: { } + // NOLINTBEGIN(readability-convert-member-functions-to-static) + [[nodiscard]] ScopedActivation + activate() const + { + return {}; + } + // NOLINTEND(readability-convert-member-functions-to-static) + explicit operator bool() const { diff --git a/src/libxrpl/telemetry/SpanGuard.cpp b/src/libxrpl/telemetry/SpanGuard.cpp index 744f93a35c..c7b021c357 100644 --- a/src/libxrpl/telemetry/SpanGuard.cpp +++ b/src/libxrpl/telemetry/SpanGuard.cpp @@ -634,6 +634,64 @@ operator bool() const return impl_ && static_cast(impl_->guard); } +// ===== ScopedActivation ==================================================== + +struct ScopedActivation::Impl +{ + /** + * Active OTel Scope binding an externally-owned span to the current + * context store. Popped on destruction; never ends the span. + */ + otel_trace::Scope scope; + + /** + * Identity of the LocalValue store the Scope was pushed onto. The Scope + * must pop while the SAME store is active (see + * ScopedSpanGuard::ScopedImpl::owner). Initialized AFTER `scope` in the + * member init list: members initialize in declaration order, so the + * Scope's push (the first LocalValue touch on a fresh worker, which + * materializes the store) runs first, then this captures the + * now-materialized store. Capturing before the push would record + * nullptr while the destructor sees the materialized store. + */ + void const* owner; + + /** + * Push an externally-owned span onto the current context store. + * @param span The already-owned span to activate (not owned here). + */ + explicit Impl(opentelemetry::nostd::shared_ptr const& span) + : scope(span), owner(detail::getLocalValues().get()) + { + } +}; + +ScopedActivation::ScopedActivation() = default; + +ScopedActivation::ScopedActivation(std::unique_ptr impl) : impl_(std::move(impl)) +{ +} + +ScopedActivation::~ScopedActivation() +{ + // A live Scope must be popped while its constructing context store is + // active; destroying the activation under a different store corrupts + // that store's context stack. A null activation holds no Scope, so the + // check is skipped. + XRPL_ASSERT( + !impl_ || impl_->owner == detail::getLocalValues().get(), + "xrpl::telemetry::ScopedActivation::~ScopedActivation : destroyed on the " + "constructing context store"); +} + +ScopedActivation +SpanGuard::activate() const +{ + if (!impl_ || !impl_->span) + return {}; + return ScopedActivation(std::make_unique(impl_->span)); +} + } // namespace xrpl::telemetry #endif // XRPL_ENABLE_TELEMETRY From 47a29187fd0e4045b29fdff114600e52c57ece0e Mon Sep 17 00:00:00 2001 From: Pratik Mankawde <3397372+pratikmankawde@users.noreply.github.com> Date: Wed, 22 Jul 2026 19:57:27 +0100 Subject: [PATCH 5/5] refactor(rpc): scope rpc.command spans; coro-aware storage makes RPC scopes yield-safe rpc.command.* becomes a scoped child so it nests under rpc.process, correlates its log lines, and parents pathfind.request. No RPC::Context plumbing needed. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/xrpld/rpc/detail/RPCHandler.cpp | 7 +++++-- src/xrpld/rpc/detail/ServerHandler.cpp | 5 ++++- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/src/xrpld/rpc/detail/RPCHandler.cpp b/src/xrpld/rpc/detail/RPCHandler.cpp index 7a9d85748a..27dacdc560 100644 --- a/src/xrpld/rpc/detail/RPCHandler.cpp +++ b/src/xrpld/rpc/detail/RPCHandler.cpp @@ -162,7 +162,10 @@ template Status callMethod(JsonContext& context, Method method, std::string const& name, Object& result) { - auto span = SpanGuard::span(TraceCategory::Rpc, rpc_span::prefix::command, name); + // Scoped so this command nests under rpc.process and becomes the ambient + // parent of any command-internal spans (e.g. pathfind.request). Coro-aware + // storage keeps the scope correct across doRipplePathFind's yield. + auto span = ScopedSpanGuard(TraceCategory::Rpc, rpc_span::prefix::command, name); span.setAttribute(rpc_span::attr::command, name.c_str()); span.setAttribute(rpc_span::attr::version, static_cast(context.apiVersion)); span.setAttribute( @@ -256,7 +259,7 @@ doCommand(RPC::JsonContext& context, json::Value& result) // registered handler names (plus "unknown") — see the helper for why // raw request input must not reach the telemetry pipeline. auto const cmdName = resolveCommandSpanName(context); - auto span = SpanGuard::span(TraceCategory::Rpc, rpc_span::prefix::command, cmdName); + auto span = ScopedSpanGuard(TraceCategory::Rpc, rpc_span::prefix::command, cmdName); span.setAttribute(rpc_span::attr::command, cmdName); span.setAttribute(rpc_span::attr::rpcStatus, rpc_span::val::error); span.setError(getErrorInfo(error).token.cStr()); diff --git a/src/xrpld/rpc/detail/ServerHandler.cpp b/src/xrpld/rpc/detail/ServerHandler.cpp index 9f221a3823..4abee4c79f 100644 --- a/src/xrpld/rpc/detail/ServerHandler.cpp +++ b/src/xrpld/rpc/detail/ServerHandler.cpp @@ -652,7 +652,10 @@ ServerHandler::processRequest( std::string_view forwardedFor, std::string_view user) { - // Scoped child: nests under the httpRequest span active on this thread. + // Scoped child of rpc.http_request. Safe to hold across the coroutine + // yield in doRipplePathFind: the coro-aware context storage moves this + // scope with the coroutine on resume (it is never stranded on a worker's + // thread-local stack), so nesting and log-trace correlation both hold. auto span = ScopedSpanGuard(TraceCategory::Rpc, rpc_span::prefix::rpc, rpc_span::op::process); auto rpcJ = app_.getJournal("RPC");