Merge branch 'pratik/otel-phase7-native-metrics' into pratik/otel-phase8-log-correlation

Carries the telemetry.md doc refresh (coro-aware SpanGuard model) forward.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

docs/build/telemetry.md

@@ -16,6 +16,10 @@ This document explains how to build xrpld with OpenTelemetry distributed tracing
     - [CMake target not found](#cmake-target-not-found)
   - [Conditional compilation](#conditional-compilation)
   - [Span lifetime and cross-thread handling](#span-lifetime-and-cross-thread-handling)
+    - [`SpanGuard` versus `ScopedSpanGuard`](#spanguard-versus-scopedspanguard)
+    - [Coroutine-aware context storage](#coroutine-aware-context-storage)
+    - [Handing a span to a job](#handing-a-span-to-a-job)
+    - [Why are unrelated spans in my trace?](#why-are-unrelated-spans-in-my-trace)
 
 ## Overview
 
@@ -131,31 +135,96 @@ This two-layer approach ensures zero overhead when telemetry is not wanted.
 
 ## Span lifetime and cross-thread handling
 
-`SpanGuard` is an RAII wrapper: it activates a span on the current thread when
-constructed and ends it when destroyed. A normal (scoped) guard binds an OTel
-`Scope` to the constructing thread's thread-local context stack. Because of
-this, a scoped guard must be used and destroyed on the thread that created it.
+Telemetry exposes two RAII guards with split responsibilities, plus a
+non-owning activation helper. Picking the right one is what keeps a trace's
+parent/child nesting and its per-line log correlation correct.
 
-Two entry points cover the cases where the default scoped behaviour is wrong:
+### `SpanGuard` versus `ScopedSpanGuard`
 
-- **`SpanGuard::rootSpan(cat, prefix, name)`** starts a span as a fresh trace
-  root, ignoring whatever span is already active on the current thread. Use it
-  at an inbound entry point that runs on a shared worker thread (for example a
-  peer message received while unrelated work is still on the stack), so those
-  unrelated spans do not become parents of the new trace.
+- **`SpanGuard`** owns a span and nothing else. It is _thread-free_: it never
+  touches the active-context stack, so it carries no thread affinity and may be
+  moved to and ended on any thread. It is movable and move-assignable. Create
+  one with `SpanGuard::span(cat, prefix, name)`, or with
+  `SpanGuard::freshRoot(...)` to start a fresh trace root that ignores whatever
+  span is currently ambient. Reach for a plain `SpanGuard` whenever the span
+  must leave the context store that created it — for example when it is handed
+  into a job.
 
-- **`std::move(guard).detached()`** returns a new guard that holds the same
-  span with **no** thread-local `Scope`. Call it on the origin thread before
-  handing the guard to a job that runs on another thread. The returned guard
-  can be moved freely across threads and ended anywhere; only its final
-  destruction ends the span.
+- **`ScopedSpanGuard`** owns a `SpanGuard` _plus_ an active OTel scope that
+  pushes the span onto the current context store. While it lives, the span is
+  the ambient parent for child spans created on that store, and log lines
+  emitted under it carry its `trace_id`. It is non-copyable and non-movable —
+  short-lived stack RAII. It offers the same factories (`freshRoot(...)`,
+  `childSpan(...)`). When the span must outlive the scope, convert it with
+  `operator SpanGuard() &&`: that pops the scope on the origin store and yields
+  the bare, thread-free `SpanGuard`.
+
+Rule of thumb: use `ScopedSpanGuard` for same-thread (or same-coroutine)
+nesting and log correlation; use the plain `SpanGuard` whenever the span
+crosses out of the store that created it.
+
+```mermaid
+flowchart TD
+    SG["SpanGuard<br/>(unscoped, thread-free)<br/>owns span only; movable across threads and coroutines"]
+    SSG["ScopedSpanGuard<br/>(scoped, store-bound)<br/>owns a SpanGuard plus an active scope on the current store"]
+    SA["ScopedActivation<br/>(non-owning)<br/>activates a borrowed span; never owns or ends it"]
+
+    SSG -->|"handoff: pop scope, yield bare span"| SG
+    SG -->|"activate / activateIfLive"| SA
+    SA -.->|"borrows span, no ownership"| SG
+
+    classDef box fill:#e8f0fe,stroke:#3b5bdb,color:#111827;
+    class SG,SSG,SA box;
+```
+
+### Coroutine-aware context storage
+
+The active-context stack is not a plain `thread_local`. At telemetry start
+xrpld installs `CoroAwareContextStorage`, which keeps the stack in an
+`xrpl::LocalValue`. Because `JobQueue::Coro::resume()` swaps the coroutine's
+`LocalValue` store in and out with the coroutine, the ambient context _follows
+the coroutine_ across every yield and resume — even when it resumes on a
+different worker thread. A `ScopedSpanGuard` held across a coroutine yield is
+therefore safe: its scope rides the coroutine and pops on the same store it was
+pushed onto, so it never pops the wrong stack. Off a coroutine the `LocalValue`
+transparently gives each thread its own store, so behaviour matches OTel's
+default thread-local storage. This is what lets the RPC entry, process, and
+command spans be scoped — for correct nesting and per-line log-trace
+correlation — even though the RPC path yields.
+
+### Handing a span to a job
+
+The hand-off pattern is: create a thread-free `SpanGuard` at the origin (or
+convert a `ScopedSpanGuard` with `operator SpanGuard() &&`), move it into the
+job closure, and inside the worker body activate it non-destructively with
+`telemetry::activateIfLive(handle)`. That call takes no ownership and returns a
+`ScopedActivation` (a no-op if the handle is empty or the span inactive) which
+makes the span the ambient context so log lines in the worker body carry its
+`trace_id`. The activation neither owns nor ends the span — the owning
+`SpanGuard` still controls its lifetime and ends it when the closure is
+destroyed. Keep the activation confined to a synchronous, non-yielding block.
+There is no detach step: a `SpanGuard` is already thread-free.
 
 ### Why are unrelated spans in my trace?
 
-If a trace shows hundreds of unrelated spans nested under one operation, a
-scoped guard was likely moved to another thread (for example stored in a job
-and destroyed on a worker thread). Destroying a scoped guard off its origin
-thread pops the wrong context stack, leaving the origin thread's active span
-in place so later spans inherit it. Fix it by calling `.detached()` on the
-origin thread before the hand-off, or by starting the operation with
-`rootSpan()` so it never inherits an ambient parent.
+Historically a scoped guard destroyed off its origin thread popped the wrong
+context stack, leaving a stale ambient span in place that later work inherited.
+The current design removes that failure mode in two ways:
+
+- **Coroutine-aware storage** makes a scope held across a coroutine yield pop on
+  the same store it was pushed onto, so a coroutine that resumes on another
+  worker never pops the wrong stack.
+
+- **A same-store assertion** in `ScopedSpanGuard` (and `ScopedActivation`)
+  records the `LocalValue` store its scope was pushed onto and checks, in
+  debug/test/fuzzing builds, that destruction, hand-off, and discard all happen
+  while that same store is active — turning a genuine cross-store misuse into an
+  immediate assertion failure rather than a silently corrupted trace.
+
+If a trace still shows unrelated spans nested under one operation, the usual
+cause is an inbound entry point that inherited an ambient parent it should not
+have. Start such an operation with `freshRoot()` so it begins a clean trace
+root and never adopts whatever span happened to be active. To move a span
+across a store boundary, keep it in a thread-free `SpanGuard` (or convert via
+`operator SpanGuard() &&`) rather than holding a `ScopedSpanGuard` across the
+boundary.