diff --git a/include/xrpl/telemetry/DeterministicIdGenerator.h b/include/xrpl/telemetry/DeterministicIdGenerator.h new file mode 100644 index 0000000000..d284a2d4b4 --- /dev/null +++ b/include/xrpl/telemetry/DeterministicIdGenerator.h @@ -0,0 +1,170 @@ +#pragma once + +#ifdef XRPL_ENABLE_TELEMETRY + +#include + +#include +#include +#include + +namespace xrpl::telemetry { + +/** + * OTel IdGenerator that can mint a deterministic (hash-derived) trace_id. + * + * By default the OTel SDK generates random trace_ids, so a span derived from + * a stable hash (e.g. a transaction id) cannot become a real trace root: the + * SDK either inherits the active span as parent or invents a random root id. + * This generator lets a caller pin the trace_id of the next forced-root span + * to a chosen 16-byte value, so hash-derived spans line up across nodes into + * one trace. When no value is pinned it behaves exactly like the default + * random generator. + * + * The pinned value lives in a thread-local slot set by PendingTraceId (below). + * Only GenerateTraceId() consults that slot, and only on the SDK's no-parent + * (root) branch; span_ids are always random. is_random_ is false so the W3C + * random-trace-id flag is not set on these deterministic ids. + * + * Dependency / data-flow diagram: + * + * +-----------------------------------------------------------+ + * | DeterministicIdGenerator | + * | (IdGenerator) | + * +-----------------------------------------------------------+ + * | - random_ : unique_ptr (random delegate) | + * +-----------------------------------------------------------+ + * | GenerateTraceId(): | + * | thread-local pending id set? --yes--> return that id | + * | --no --> random_ ... | + * | GenerateSpanId(): always ---------------> random_ ... | + * +-----------------------------------------------------------+ + * ^ | + * sets/clears| delegates| + * | v + * +----------------+ +--------------------+ + * | PendingTraceId | | RandomIdGenerator | + * | (RAII guard) | | (delegate) | + * +----------------+ +--------------------+ + * + * @note Thread safety: the pending trace_id is a file-local thread_local, so + * each thread sees only its own pinned value and no synchronization is needed. + * The pending id is consumed ONLY by GenerateTraceId(), which the SDK calls + * solely when a span has no valid parent; GenerateSpanId() never reads it and + * is always random. + * @note Limitation: minting a deterministic root only works when the caller + * forces the SDK's root branch (e.g. by starting the span with a + * Context{kIsRootSpanKey, true}); otherwise the SDK reuses the parent's + * trace_id and never calls GenerateTraceId(). The primary such caller is + * SpanGuard::hashSpan(), which forces the root branch to mint deterministic + * per-object trace roots. + * + * Example 1 - primary use, inside a forced-root hash span (shown as + * pseudocode): + * @code + * // std::array id = deriveTraceIdFromHash(txHash); + * // PendingTraceId const pending{id}; // pin id for this thread + * // auto root = Context{kIsRootSpanKey, true}; // force the no-parent branch + * // auto guard = telemetry.startSpan("tx.process", root); + * // // GenerateTraceId() returns `id`; the guard's trace_id == id. + * @endcode + * + * Example 2 - edge case, a normal child span never consults the pending id: + * @code + * // With an active parent span, startSpan() inherits the parent's trace_id + * // and the SDK does NOT call GenerateTraceId(), so no PendingTraceId is used. + * // auto child = parentGuard.childSpan("subtask"); // random/parent trace_id + * @endcode + */ +class DeterministicIdGenerator final : public opentelemetry::sdk::trace::IdGenerator +{ + /** + * Random generator the deterministic path falls back to. Used for every + * span_id and for any trace_id when no PendingTraceId is active. + */ + std::unique_ptr random_; + +public: + /** + * Build a generator with is_random_ = false and a random delegate. + */ + DeterministicIdGenerator(); + + /** + * @return the thread's pending trace_id if a PendingTraceId is active, + * otherwise a fresh random trace_id from the delegate. Consuming the + * pending id clears it so it applies to exactly one root span. + */ + opentelemetry::trace::TraceId + GenerateTraceId() noexcept override; + + /** + * @return a fresh random span_id. Never uses the pending trace_id. + */ + opentelemetry::trace::SpanId + GenerateSpanId() noexcept override; +}; + +/** + * RAII guard that pins a deterministic trace_id for the next forced-root span + * started on this thread. + * + * Mirrors DiscardScope: it sets a thread-local pending trace_id on + * construction and clears it on destruction, so the pinned id stays confined + * to the guard's scope and cannot leak onto a later span. On destruction it + * asserts that the id was actually consumed by GenerateTraceId() — if it was + * not, the SDK took a branch other than the root branch the caller intended, + * which is a bug worth catching in debug/test builds. + * + * Wrap ONLY the single forced-root startSpan() call that must receive the + * deterministic id. Non-copyable and non-movable: its sole purpose is the + * scoped lifetime of the pending id. + * + * @note Thread safety: the pending id is thread-local, so a guard on one + * thread never affects another. Construct and destroy the guard on the same + * thread as the startSpan() call it wraps. + * @note Limitation: the consumed-assert only holds when the wrapped span is + * started on the SDK root branch (Context{kIsRootSpanKey, true}); wrapping a + * child span would leave the id unconsumed and trip the assert. Nesting two + * guards on one thread is unsupported: the inner guard consumes/clears first, + * so the outer id is silently dropped and its destructor trips the assert. + * + * Example 1 - primary use, wrapping a forced-root span start (pseudocode): + * @code + * // { + * // PendingTraceId const pending{id}; // pin for this scope + * // auto root = Context{kIsRootSpanKey, true}; + * // auto guard = telemetry.startSpan(name, root);// consumes the pinned id + * // } // ~PendingTraceId asserts the id was consumed, then clears it + * @endcode + * + * Example 2 - edge case, misuse detection: wrapping a non-root span leaves the + * id unconsumed, so ~PendingTraceId trips XRPL_ASSERT in debug/test builds. + */ +class PendingTraceId +{ +public: + /** + * Pin @p id as the pending trace_id for this thread. + * @param id The 16-byte trace_id the next forced-root span should adopt. + */ + explicit PendingTraceId(std::array const& id) noexcept; + + /** + * Assert the pinned id was consumed by a root span, then clear it so it + * never leaks onto the next span (even in release, where the assert is a + * no-op). + */ + ~PendingTraceId() noexcept; + + PendingTraceId(PendingTraceId const&) = delete; + PendingTraceId& + operator=(PendingTraceId const&) = delete; + PendingTraceId(PendingTraceId&&) = delete; + PendingTraceId& + operator=(PendingTraceId&&) = delete; +}; + +} // namespace xrpl::telemetry + +#endif // XRPL_ENABLE_TELEMETRY diff --git a/include/xrpl/telemetry/SpanGuard.h b/include/xrpl/telemetry/SpanGuard.h index 11ace20a49..d235f57ece 100644 --- a/include/xrpl/telemetry/SpanGuard.h +++ b/include/xrpl/telemetry/SpanGuard.h @@ -1,25 +1,38 @@ #pragma once /** - * RAII guard for OpenTelemetry trace spans. + * RAII guards for OpenTelemetry trace spans. * - * Wraps an OTel Span and Scope behind the pimpl idiom so that no - * opentelemetry headers are exposed in this public header. When - * XRPL_ENABLE_TELEMETRY is not defined, SpanGuard is an empty class - * with all-inline no-op methods — zero overhead, zero dependencies. + * This header declares two complementary span guards, split by + * responsibility: * - * Dependency diagram: + * - SpanGuard — owns a span only. Thread-free: it never touches the + * OTel thread-local context stack, so it may be moved + * 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 + * non-movable. + * + * Both wrap all OpenTelemetry types behind the pimpl idiom so no + * opentelemetry headers are exposed here. When XRPL_ENABLE_TELEMETRY + * is not defined, both are empty classes with all-inline no-op methods + * — zero overhead, zero dependencies. + * + * SpanGuard dependency diagram: * * +--------------------------------------------+ * | SpanGuard | + * | (unscoped, thread-free) | * +--------------------------------------------+ * | - impl_ : unique_ptr (pimpl) | * +--------------------------------------------+ * | + span(cat, prefix, name) [static] | - * | + rootSpan(cat, prefix, name) [static] | + * | + freshRoot(cat, prefix, name) [static] | * | + childSpan(name) : SpanGuard | * | + linkedSpan(name) : SpanGuard | - * | + detached() : SpanGuard | * | + captureContext() : SpanContext | * | + setAttribute(key, value) | * | + setOk() / setError(desc) | @@ -29,14 +42,10 @@ * | + operator bool() | * +--------------------------------------------+ * | hides (pimpl) - * +-------+-------------+ - * | | - * +--------+ +---------------------------+ - * | Span | | optional | - * | (OTel) | | (OTel, non-movable) | - * | | | present : scoped guard | - * | | | nullopt : detached guard | - * +--------+ +---------------------------+ + * +--------+ + * | Span | + * | (OTel) | + * +--------+ * * Static factory methods access the global Telemetry instance * internally (via Telemetry::getInstance()), check whether tracing @@ -60,7 +69,7 @@ * TraceCategory::Rpc, rpc_span::prefix::command, commandName); * span.setAttribute(rpc_span::attr::command, commandName); * span.setAttribute(rpc_span::attr::rpcStatus, rpc_span::val::success); - * // span ended automatically on scope exit + * // span ended automatically when the guard is destroyed * @endcode * * 2. Error recording: @@ -110,7 +119,7 @@ * } * @endcode * - * 6. Fresh trace root at an inbound entry point (primary rootSpan use): + * 6. Fresh trace root at an inbound entry point (primary freshRoot use): * @code * #include * using namespace xrpl::telemetry; @@ -119,50 +128,45 @@ * // already have unrelated spans active — start a clean root so * // those do not become parents of this trace. Names come from a * // *SpanNames.h header, never raw literals. - * auto span = SpanGuard::rootSpan( + * auto span = SpanGuard::freshRoot( * TraceCategory::Peer, seg::peer, peer_span::op::validationReceive); * span.setAttribute(peer_span::attr::ledgerHash, hashStr); * @endcode * - * 7. Hand a span to a job on another thread (edge case, detached): + * 7. Hand a span to a job on another thread (thread-free — just move): * @code * #include * using namespace xrpl::telemetry; * - * // Build the guard on THIS thread, then strip its thread-local - * // Scope so it can be safely moved into a job and ended there. + * // SpanGuard owns no thread-local Scope, so it is safe to move + * // into a job and end on the worker thread. No detach step is + * // needed — just move the guard in. * auto span = SpanGuard::span( * TraceCategory::Ledger, seg::ledger, ledger_span::op::build); * jobQueue.addJob( - * [g = std::move(span).detached()]() mutable { + * [g = std::move(span)]() mutable { * doWork(); * // g's span ends when the job's lambda is destroyed, - * // on the worker thread — no origin-stack corruption. + * // on the worker thread. * }); * @endcode * - * @note Thread safety: A SCOPED guard (from span(), rootSpan(), - * childSpan(), linkedSpan()) must only be used on the thread where it - * was constructed — its internal Scope binds to that thread's - * thread-local context stack, and destroying it elsewhere would pop - * the wrong stack. A guard returned by detached() holds no Scope, so - * it may be moved to and destroyed on another thread; detached() - * itself must be called on the origin (constructing) thread. Use - * captureContext() to propagate the trace context to other threads. - * Violating this rule is enforced (not just documented): a scoped - * guard destroyed on a foreign thread, or detached() called from one, - * trips an XRPL_ASSERT in debug/test/fuzzing builds instead of - * silently corrupting the other thread's context stack. + * @note Thread safety: SpanGuard is thread-free. It holds only the + * span (no Scope), so it never binds to a thread-local context stack + * and may be moved to and destroyed on any thread. To make a span the + * ambient parent for child spans on a thread, wrap it in a + * ScopedSpanGuard. Use captureContext() to propagate the trace + * context across threads explicitly. * - * @note Move semantics: Move construction transfers ownership of - * the pimpl pointer — no double-Scope issues. Move assignment is - * deleted to prevent re-scoping mid-flight. + * @note Move semantics: Move construction and move assignment both + * transfer ownership of the pimpl pointer. There is no Scope to + * re-bind, so move assignment is safe and enabled. Copy is deleted. * * @note Known limitations: * - Attributes cannot be removed per the OTel spec; use * setAttribute with an empty value as a convention. - * - SpanGuard::span() (raw Span access) is intentionally not - * exposed — all interaction goes through the public methods. + * - The raw OTel Span is intentionally not exposed — all interaction + * goes through the public methods. */ #include @@ -171,7 +175,6 @@ #include #include #include -#include #include #include @@ -256,9 +259,16 @@ public: #ifdef XRPL_ENABLE_TELEMETRY /** - * RAII wrapper that activates a span on construction and ends it on - * destruction. All OTel types are hidden behind the Impl pointer. - * Non-copyable, move-constructible. + * RAII owner of an OTel span (unscoped, thread-free). + * + * Holds only the span behind the Impl pointer — it never pushes the + * span onto the thread-local context stack, so it carries no + * thread-affinity and may be moved to and destroyed on any thread. + * The span is ended when the guard is destroyed (unless discard() + * ended it earlier). Non-copyable; movable and move-assignable. + * + * To activate a span as the ambient parent for child spans on a + * thread, wrap the guard in a ScopedSpanGuard. */ class SpanGuard { @@ -267,6 +277,10 @@ class SpanGuard explicit SpanGuard(std::unique_ptr impl); + // ScopedSpanGuard wraps a SpanGuard and needs the raw span to build + // its Scope; it reads impl_ directly so no OTel type leaks here. + friend class ScopedSpanGuard; + public: /** * Construct a null (no-op) guard. All methods are safe to call. @@ -276,7 +290,7 @@ public: SpanGuard(SpanGuard&& other) noexcept; SpanGuard& - operator=(SpanGuard&&) = delete; + operator=(SpanGuard&&) noexcept; SpanGuard(SpanGuard const&) = delete; SpanGuard& operator=(SpanGuard const&) = delete; @@ -308,16 +322,17 @@ public: * @param prefix Span name prefix (e.g. "peer"). * @param name Span name suffix (e.g. "validation.receive"). * @return An active root-span guard, or a null guard if disabled. - * @note Must be called on the thread that will own the span, like - * span(); the returned guard is scoped to that thread. */ [[nodiscard]] static SpanGuard - rootSpan(TraceCategory cat, std::string_view prefix, std::string_view name); + freshRoot(TraceCategory cat, std::string_view prefix, std::string_view name); // --- Child / linked span creation ---------------------------------- /** - * Create a child span parented to this guard's active context. + * Create a child span parented to the current thread's active + * context. This is meaningful when this guard's span is active on + * the thread (e.g. wrapped in a ScopedSpanGuard); otherwise the + * child inherits whatever context is currently on the stack. * @param name Span name for the child. * @return A new guard, or null if this guard is inactive. */ @@ -352,30 +367,6 @@ public: [[nodiscard]] static SpanGuard linkedSpan(std::string_view name, SpanContext const& linkCtx); - /** - * Detach this guard's span from the current thread's context stack. - * - * A scoped guard holds an OTel Scope bound to the constructing - * thread's context stack. Moving such a guard to another thread - * (e.g. into a job queue) and destroying it there would pop the - * wrong stack, leaving the origin thread's stack corrupted so - * later spans inherit a stale parent. detached() pops the Scope - * now, on the origin thread, and returns a new guard that holds - * the same span with no thread-local binding. - * - * Consumes this guard (rvalue-qualified): after the call this - * guard is null and the returned guard owns the span. - * - * @return A scope-less guard safe to move to and destroy on - * another thread, or a null guard if this guard was null. - * @note Must be called on the origin (constructing) thread; this is - * checked by an XRPL_ASSERT in debug/test/fuzzing builds. The - * returned guard may be freely moved across threads; only - * its final destruction ends the span. - */ - [[nodiscard]] SpanGuard - detached() &&; - // --- Hash-derived span (category-gated) ----------------------------- /** @@ -547,42 +538,260 @@ public: operator bool() const; }; -// --- Detach-in-place helpers ----------------------------------------------- - /** - * Detach an active `std::optional` member in place. + * RAII guard that activates a span on the current thread (scoped). * - * Equivalent to `guard.emplace(std::move(*guard).detached())`, which is the - * required idiom for detaching a live guard stored in an `optional` (move - * assignment is deleted because the underlying Scope cannot be re-scoped in - * place). No-op if `guard` is empty or already null. + * 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 + * 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(...)`. * - * @param guard The optional guard to detach. Must be called on the thread - * that constructed the active guard inside it (same rule as - * `SpanGuard::detached()`). - * @note Must run BEFORE any `captureContext()` snapshot is taken if the - * caller also needs the pre-detach context — capture first, then call - * this helper (same ordering rule `detached()` itself has). + * When the span must outlive the current thread (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 + * thread-free guard. + * + * ScopedSpanGuard dependency diagram: + * + * +--------------------------------------------+ + * | ScopedSpanGuard | + * | (scoped, thread-bound) | + * +--------------------------------------------+ + * | - impl_ : unique_ptr (pimpl) | + * +--------------------------------------------+ + * | + (cat, prefix, name) [ctor] | + * | + freshRoot(cat, prefix, name) [static] | + * | + childSpan(name) : ScopedSpanGuard | + * | + linkedSpan(name) : ScopedSpanGuard | + * | + operator SpanGuard() && (handoff) | + * | + setAttribute / setOk / setError / ... | + * | + captureContext() / discard() | + * | + operator bool() | + * +--------------------------------------------+ + * | hides (pimpl) + * +------+--------------------+ + * | | + * +----------+ +-----------------------------+ + * | SpanGuard| | optional | + * | (span) | | (OTel, thread-bound; | + * | | | present : span active) | + * +----------+ +-----------------------------+ + * + * Usage examples: + * + * 1. Same-thread scoped tracing (child inherits parent automatically): + * @code + * using namespace xrpl::telemetry; + * + * ScopedSpanGuard span( + * TraceCategory::Rpc, rpc_span::prefix::command, commandName); + * span.setAttribute(rpc_span::attr::command, commandName); + * // childSpan parents to `span` because it is active on this thread + * auto child = span.childSpan(rpc_span::op::dispatch); + * @endcode + * + * 2. Capture on this thread, hand off to another (edge case): + * @code + * using namespace xrpl::telemetry; + * + * auto scoped = ScopedSpanGuard::freshRoot( + * TraceCategory::Ledger, seg::ledger, ledger_span::op::build); + * // Pop the scope now, on this thread, and move a thread-free + * // SpanGuard into the job. + * SpanGuard span = std::move(scoped); + * jobQueue.addJob([g = std::move(span)]() mutable { doWork(); }); + * @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. + * + * @note Known limitations: + * - Non-movable: cannot be stored in a container that relocates, and + * cannot be returned then move-assigned into an existing variable. + * Store the thread-free SpanGuard (via `operator SpanGuard() &&`) + * instead when a movable handle is needed. + * - Same attribute-removal limitation as SpanGuard. */ -void -detachInPlace(std::optional& guard); +class ScopedSpanGuard +{ + struct ScopedImpl; + std::unique_ptr impl_; -/** - * Detach an active `std::shared_ptr`, returning the detached - * guard as a new `shared_ptr`. - * - * Equivalent to - * `guard = std::make_shared(std::move(*guard).detached())`. - * No-op (returns the input unchanged) if `guard` is null or points at a - * null guard. - * - * @param guard The guard to detach, taken by value (the caller's pointer - * is consumed; assign the return value back). - * @return A `shared_ptr` to the detached guard (new allocation), or the - * input pointer unchanged if it was null/inactive. - */ -[[nodiscard]] std::shared_ptr -detachInPlace(std::shared_ptr guard); + explicit ScopedSpanGuard(SpanGuard&& guard); + +public: + /** + * Create a scoped span guarded by a TraceCategory flag and activate + * it on this thread. Equivalent to SpanGuard::span() wrapped in an + * active Scope. The span name is built as "prefix.name". + * @param cat Trace subsystem category. + * @param prefix Span name prefix (e.g. "rpc.command"). + * @param name Span name suffix (e.g. "submit"). + */ + ScopedSpanGuard(TraceCategory cat, std::string_view prefix, std::string_view name); + + ~ScopedSpanGuard(); + + ScopedSpanGuard(ScopedSpanGuard&&) = delete; + ScopedSpanGuard& + operator=(ScopedSpanGuard&&) = delete; + ScopedSpanGuard(ScopedSpanGuard const&) = delete; + ScopedSpanGuard& + operator=(ScopedSpanGuard const&) = delete; + + // --- Static factory methods ---------------------------------------- + + /** + * Create a scoped span that always starts a fresh trace root and + * activate it on this thread. See SpanGuard::freshRoot(). + * @param cat Trace subsystem category. + * @param prefix Span name prefix. + * @param name Span name suffix. + * @return An active scoped root-span guard, or a null one if disabled. + */ + [[nodiscard]] static ScopedSpanGuard + freshRoot(TraceCategory cat, std::string_view prefix, std::string_view name); + + // --- Child / linked span creation ---------------------------------- + + /** + * Create a scoped child span parented to this guard's active span + * and activate it on this thread. + * @param name Span name for the child. + * @return A new scoped guard, or a null one if this guard is inactive. + */ + [[nodiscard]] ScopedSpanGuard + childSpan(std::string_view name) const; + + /** + * Create a scoped child span parented to an explicit captured + * context and activate it on this thread. + * @param name Span name for the child. + * @param parentCtx Context captured via captureContext(). + * @return A new scoped guard, or a null one if parentCtx is invalid. + */ + [[nodiscard]] static ScopedSpanGuard + childSpan(std::string_view name, SpanContext const& parentCtx); + + /** + * Create a scoped span linked (follows-from) to this guard's span + * and activate it on this thread. + * @param name Span name for the linked span. + * @return A new scoped guard, or a null one if this guard is inactive. + */ + [[nodiscard]] ScopedSpanGuard + linkedSpan(std::string_view name) const; + + /** + * Create a scoped span linked to an explicit captured context and + * activate it on this thread. + * @param name Span name for the linked span. + * @param linkCtx Context to link from. + * @return A new scoped guard, or a null one if linkCtx is invalid. + */ + [[nodiscard]] static ScopedSpanGuard + linkedSpan(std::string_view name, SpanContext const& linkCtx); + + // --- 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. + * @return The unscoped SpanGuard that now owns the span. + */ + operator SpanGuard() &&; + + // --- Context capture ----------------------------------------------- + + /** + * Snapshot the current thread's OTel context for cross-thread use. + * @return An opaque SpanContext, or an invalid one if null guard. + */ + [[nodiscard]] SpanContext + captureContext() const; + + // --- Forwarding methods (delegate to the owned SpanGuard) ---------- + + /** + * Set a string attribute. No-op on a null guard. + */ + void + setAttribute(std::string_view key, std::string_view value); + + /** + * Set a string attribute (C-string overload). No-op on a null guard. + */ + void + setAttribute(std::string_view key, char const* value); + + /** + * Set an integer attribute. No-op on a null guard. + */ + void + setAttribute(std::string_view key, std::int64_t value); + + /** + * Set a floating-point attribute. No-op on a null guard. + */ + void + setAttribute(std::string_view key, double value); + + /** + * Set a boolean attribute. No-op on a null guard. + */ + void + setAttribute(std::string_view key, bool value); + + /** + * Mark the span status as OK. No-op on a null guard. + */ + void + setOk(); + + /** + * Mark the span status as error. No-op on a null guard. + * @param description Optional human-readable error description. + */ + void + setError(std::string_view description = ""); + + /** + * Add a named event to the span's timeline. No-op on a null guard. + * @param name Event name. + */ + void + addEvent(std::string_view name); + + /** + * Record an exception as a span event and mark status as error. + * No-op on a null guard. + * @param e The exception to record. + */ + void + recordException(std::exception const& e); + + /** + * 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. + */ + void + discard(); + + /** + * @return true if this guard holds an active span. + */ + explicit + operator bool() const; +}; // --------------------------------------------------------------------------- // No-op stub (all inline, zero overhead, no OTel dependency) @@ -596,7 +805,7 @@ public: ~SpanGuard() = default; SpanGuard(SpanGuard&&) noexcept = default; SpanGuard& - operator=(SpanGuard&&) = delete; + operator=(SpanGuard&&) noexcept = default; SpanGuard(SpanGuard const&) = delete; SpanGuard& operator=(SpanGuard const&) = delete; @@ -608,7 +817,7 @@ public: } [[nodiscard]] static SpanGuard - rootSpan(TraceCategory, std::string_view, std::string_view) + freshRoot(TraceCategory, std::string_view, std::string_view) { return {}; } @@ -635,12 +844,6 @@ public: return {}; } - [[nodiscard]] SpanGuard - detached() && - { - return {}; - } - [[nodiscard]] static SpanGuard hashSpan( TraceCategory, @@ -734,18 +937,114 @@ public: } }; -// --- Detach-in-place helpers (no-op stubs) --------------------------------- - -inline void -detachInPlace(std::optional&) +class ScopedSpanGuard { -} + // Private default ctor lets the inline factories/child methods build + // a no-op instance without exposing a public default constructor + // (mirrors the real class, which has no public default ctor). + ScopedSpanGuard() = default; -[[nodiscard]] inline std::shared_ptr -detachInPlace(std::shared_ptr guard) -{ - return guard; -} +public: + ScopedSpanGuard(TraceCategory, std::string_view, std::string_view) + { + } + ~ScopedSpanGuard() = default; + + ScopedSpanGuard(ScopedSpanGuard&&) = delete; + ScopedSpanGuard& + operator=(ScopedSpanGuard&&) = delete; + ScopedSpanGuard(ScopedSpanGuard const&) = delete; + ScopedSpanGuard& + operator=(ScopedSpanGuard const&) = delete; + + [[nodiscard]] static ScopedSpanGuard + freshRoot(TraceCategory, std::string_view, std::string_view) + { + return {}; + } + + // NOLINTBEGIN(readability-convert-member-functions-to-static) + [[nodiscard]] ScopedSpanGuard + childSpan(std::string_view) const + { + return {}; + } + [[nodiscard]] static ScopedSpanGuard + childSpan(std::string_view, SpanContext const&) + { + return {}; + } + [[nodiscard]] ScopedSpanGuard + linkedSpan(std::string_view) const + { + return {}; + } + [[nodiscard]] static ScopedSpanGuard + linkedSpan(std::string_view, SpanContext const&) + { + return {}; + } + + [[nodiscard]] SpanContext + captureContext() const + { + return {}; + } + // NOLINTEND(readability-convert-member-functions-to-static) + + operator SpanGuard() && + { + return {}; + } + + void + setAttribute(std::string_view, std::string_view) + { + } + void + setAttribute(std::string_view, char const*) + { + } + void + setAttribute(std::string_view, std::int64_t) + { + } + void + setAttribute(std::string_view, double) + { + } + void + setAttribute(std::string_view, bool) + { + } + + void + setOk() + { + } + void + setError(std::string_view = "") + { + } + void + addEvent(std::string_view) + { + } + void + recordException(std::exception const&) + { + } + void + discard() + { + } + + explicit + operator bool() const + { + return false; + } +}; #endif // XRPL_ENABLE_TELEMETRY diff --git a/src/libxrpl/telemetry/DeterministicIdGenerator.cpp b/src/libxrpl/telemetry/DeterministicIdGenerator.cpp new file mode 100644 index 0000000000..2a2c8c65ec --- /dev/null +++ b/src/libxrpl/telemetry/DeterministicIdGenerator.cpp @@ -0,0 +1,92 @@ +/** + * Implementation of DeterministicIdGenerator and PendingTraceId. + * + * The pending trace_id lives in file-local thread_locals shared between the + * generator and the RAII guard. GenerateTraceId() consumes the pending id on + * the SDK's no-parent branch; PendingTraceId sets it and asserts consumption. + * All OpenTelemetry SDK types stay confined to telemetry translation units. + * + * @see DeterministicIdGenerator, PendingTraceId (DeterministicIdGenerator.h) + */ + +#ifdef XRPL_ENABLE_TELEMETRY + +#include + +#include + +#include +#include +#include +#include + +#include +#include +#include + +namespace xrpl::telemetry { + +namespace { + +/** + * Trace_id pinned by PendingTraceId, consumed by GenerateTraceId(). File-local + * and thread-local, so it is set and read on the same thread with no locking. + */ +thread_local std::optional> tlsPendingTraceId; + +/** + * True once GenerateTraceId() has consumed the pending id. ~PendingTraceId + * asserts on it to catch a forced-root span that never reached the SDK root + * branch. + */ +thread_local bool tlsPendingConsumed = false; + +} // namespace + +DeterministicIdGenerator::DeterministicIdGenerator() + : opentelemetry::sdk::trace::IdGenerator(/*is_random=*/false) + , random_(opentelemetry::sdk::trace::RandomIdGeneratorFactory::Create()) +{ +} + +opentelemetry::trace::TraceId +DeterministicIdGenerator::GenerateTraceId() noexcept +{ + if (tlsPendingTraceId) + { + auto const id = *tlsPendingTraceId; + tlsPendingTraceId.reset(); + tlsPendingConsumed = true; + return opentelemetry::trace::TraceId( + opentelemetry::nostd::span(id.data(), 16)); + } + return random_->GenerateTraceId(); +} + +opentelemetry::trace::SpanId +DeterministicIdGenerator::GenerateSpanId() noexcept +{ + return random_->GenerateSpanId(); // ALWAYS random — never uses the pending id +} + +PendingTraceId::PendingTraceId(std::array const& id) noexcept +{ + tlsPendingTraceId = id; + tlsPendingConsumed = false; +} + +PendingTraceId::~PendingTraceId() noexcept +{ + // The forced no-parent (root) span-start MUST have consumed the pending id + // via GenerateTraceId(). If not, the SDK took a different branch than the + // caller forced — a bug — so fail loudly in debug/test builds. + XRPL_ASSERT( + tlsPendingConsumed, + "xrpl::telemetry::PendingTraceId : deterministic trace_id was not consumed"); + tlsPendingTraceId.reset(); // never leak, even in release + tlsPendingConsumed = false; +} + +} // namespace xrpl::telemetry + +#endif // XRPL_ENABLE_TELEMETRY diff --git a/src/libxrpl/telemetry/SpanGuard.cpp b/src/libxrpl/telemetry/SpanGuard.cpp index f9baac4df8..fe427cb8ad 100644 --- a/src/libxrpl/telemetry/SpanGuard.cpp +++ b/src/libxrpl/telemetry/SpanGuard.cpp @@ -1,23 +1,26 @@ /** - * Pimpl implementation for SpanGuard and SpanContext. + * Pimpl implementation for SpanGuard, ScopedSpanGuard and SpanContext. * * All OpenTelemetry SDK types are confined to this translation unit. * The public SpanGuard.h header contains only standard-library types - * and forward-declares the Impl struct. + * and forward-declares the Impl / ScopedImpl structs. + * + * Two guards with split responsibilities live here: + * - SpanGuard::Impl holds ONLY the OTel Span (shared_ptr). It never + * 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 + * the Scope pops BEFORE the span ends on destruction. It is + * thread-bound: construct and destroy on the same thread. * * Static factory methods access the global Telemetry instance via * Telemetry::getInstance(), check whether the requested TraceCategory - * is enabled, and return either an active guard with a real Span+Scope - * or a null guard whose methods are all no-ops. + * is enabled, and return either an active guard with a real Span or a + * null guard whose methods are all no-ops. * - * The Impl struct holds the OTel Span (shared_ptr) and an optional - * Scope. Scope is non-movable, but since Impl lives behind a - * unique_ptr, SpanGuard's move constructor simply transfers the - * pointer — no double-Scope issues. A scoped guard holds the Scope; - * detached() produces an Impl with no Scope (nullopt) so the guard - * carries no thread-local binding and is safe to move across threads. - * - * @see SpanGuard (SpanGuard.h), Telemetry (Telemetry.h), + * @see SpanGuard, ScopedSpanGuard (SpanGuard.h), Telemetry (Telemetry.h), * FilteringSpanProcessor (Telemetry.cpp) */ @@ -25,8 +28,8 @@ #include -#include #include +#include #include #include #include @@ -47,6 +50,7 @@ #include #include +#include #include #include #include @@ -90,65 +94,22 @@ SpanContext::isValid() const struct SpanGuard::Impl { /** - * The OTel span being guarded. Set to nullptr after discard() or - * once detached() moves it into a new guard, so ~Impl skips End(). + * The OTel span being guarded. Set to nullptr after discard() so + * ~Impl skips End(). */ opentelemetry::nostd::shared_ptr span; /** - * Scope that activates span on the current thread's context stack. - * nullopt for a detached guard, which holds the span with no - * thread-local binding and is therefore safe to move across threads. - */ - std::optional scope; - - /** - * Thread that constructed this Impl. Meaningful only when `scope` - * holds a value: a Scope is bound to the thread-local context stack - * of the thread that pushed it, so popping it (via ~Scope, when - * ~Impl runs) on a different thread corrupts that thread's stack. - * Checked in ~Impl() and detached() to turn a silent cross-thread - * corruption into an assertion failure in debug/test/fuzzing builds. - */ - std::thread::id owner{std::this_thread::get_id()}; - - /** - * Construct a scoped guard: the span is pushed onto this thread's - * active-context stack for the lifetime of the guard. + * Construct an unscoped guard: the span is owned but NOT pushed + * onto any thread's context stack. The guard is thread-free. * @param s The span to guard. */ explicit Impl(opentelemetry::nostd::shared_ptr s) : span(std::move(s)) - { - scope.emplace(span); - } - - /** - * Tag type selecting the scope-less (detached) constructor. - */ - struct Detached - { - }; - - /** - * Construct a detached guard: the span is held with NO thread-local - * Scope, so the guard carries no context-stack binding and is safe - * to move to and destroy on another thread. - * @param s The span to guard. - */ - Impl(opentelemetry::nostd::shared_ptr s, Detached) : span(std::move(s)) { } ~Impl() { - // A live Scope (scope engaged) must be popped on the thread that - // pushed it; ending on any other thread corrupts that thread's - // context stack. A detached guard (scope == nullopt) carries no - // binding and may be destroyed anywhere, so the check is skipped. - XRPL_ASSERT( - !scope.has_value() || owner == std::this_thread::get_id(), - "xrpl::telemetry::SpanGuard::Impl::~Impl : scoped guard destroyed on " - "constructing thread"); if (span) span->End(); } @@ -166,6 +127,8 @@ struct SpanGuard::Impl SpanGuard::SpanGuard() = default; SpanGuard::~SpanGuard() = default; SpanGuard::SpanGuard(SpanGuard&&) noexcept = default; +SpanGuard& +SpanGuard::operator=(SpanGuard&&) noexcept = default; SpanGuard::SpanGuard(std::unique_ptr impl) : impl_(std::move(impl)) { @@ -248,7 +211,7 @@ SpanGuard::span(TraceCategory cat, std::string_view prefix, std::string_view nam } SpanGuard -SpanGuard::rootSpan(TraceCategory cat, std::string_view prefix, std::string_view name) +SpanGuard::freshRoot(TraceCategory cat, std::string_view prefix, std::string_view name) { auto* tel = Telemetry::getInstance(); if ((tel == nullptr) || !tel->isEnabled() || !isCategoryEnabled(*tel, cat)) @@ -342,45 +305,9 @@ SpanGuard::linkedSpan(std::string_view name, SpanContext const& linkCtx) // LCOV_EXCL_STOP } -SpanGuard -SpanGuard::detached() && -{ - if (!impl_) - return {}; - // Popping the Scope (below, via impl_.reset()) must happen on the thread - // that pushed it; calling detached() elsewhere would pop the wrong stack. - XRPL_ASSERT( - !impl_->scope.has_value() || impl_->owner == std::this_thread::get_id(), - "xrpl::telemetry::SpanGuard::detached : called on constructing thread"); - // Take the span out; the old Impl.span is now null so ~Impl won't End(). - auto s = std::move(impl_->span); - // Resetting the old Impl destroys its Scope HERE, on the origin thread, - // popping this thread's context stack correctly. The returned guard holds - // the span with no Scope, so it is safe to move to another thread. - impl_.reset(); - return SpanGuard(std::make_unique(std::move(s), Impl::Detached{})); -} - -// ===== Detach-in-place helpers ============================================= - -void -detachInPlace(std::optional& guard) -{ - if (!guard || !*guard) - return; - guard.emplace(std::move(*guard).detached()); -} - -std::shared_ptr -detachInPlace(std::shared_ptr guard) -{ - if (!guard || !*guard) - return guard; - return std::make_shared(std::move(*guard).detached()); -} - // ===== Hash-derived span (category-gated) ================================== +// Standalone hash-derived span: a TRUE root whose trace_id == hashData[0:16]. SpanGuard SpanGuard::hashSpan( TraceCategory const cat, @@ -395,23 +322,19 @@ SpanGuard::hashSpan( if ((tel == nullptr) || !tel->isEnabled() || !isCategoryEnabled(*tel, cat)) return {}; - otel_trace::TraceId const traceId( - opentelemetry::nostd::span(hashData, 16)); - - auto const rval = defaultPrng()(); - std::uint8_t spanIdBytes[8]; - std::memcpy(spanIdBytes, &rval, sizeof(spanIdBytes)); - otel_trace::SpanId const spanId( - opentelemetry::nostd::span(spanIdBytes, 8)); - - otel_trace::SpanContext const syntheticCtx( - traceId, spanId, otel_trace::TraceFlags(1), /* remote = */ false); - - auto parentCtx = opentelemetry::context::Context{}.SetValue( - otel_trace::kSpanKey, - opentelemetry::nostd::shared_ptr( - new otel_trace::DefaultSpan(syntheticCtx))); + // Force the deterministic trace_id via the DeterministicIdGenerator, on the + // SDK root branch, so the span is a TRUE root (empty parent_span_id) whose + // trace_id == hashData[0:16]. The kIsRootSpanKey context forces the SDK's + // no-valid-parent branch, where GenerateTraceId() is called and returns the + // pending id. The PendingTraceId guard scopes that id to this one startSpan. + std::array tid{}; + std::memcpy(tid.data(), hashData, 16); + auto rootCtx = opentelemetry::context::Context{otel_trace::kIsRootSpanKey, true}; + PendingTraceId const pending{tid}; + // When a prior context is supplied, attach it as a follows-from LINK (not a + // parent) so consecutive roots (e.g. consensus rounds) stay navigable while + // this span remains a true root under its deterministic trace_id. if (followsFrom != nullptr && followsFrom->isValid()) { auto linkSpan = otel_trace::GetSpan(followsFrom->impl_->ctx); @@ -419,7 +342,7 @@ SpanGuard::hashSpan( { auto tracer = tel->getTracer("xrpld"); otel_trace::StartSpanOptions opts; - opts.parent = parentCtx; + opts.parent = rootCtx; // root marker → forces GenerateTraceId() opts.kind = categoryToSpanKind(cat); return SpanGuard( std::make_unique(tracer->StartSpan( @@ -430,9 +353,13 @@ SpanGuard::hashSpan( } } - return SpanGuard(std::make_unique(tel->startSpan(std::string(name), parentCtx))); + return SpanGuard( + std::make_unique( + tel->startSpan(std::string(name), rootCtx, categoryToSpanKind(cat)))); } +// Cross-node hash-derived span: a CHILD of the sender's real remote span +// (remote=true), not a root — so it must NOT use PendingTraceId / rootCtx. SpanGuard SpanGuard::hashSpan( TraceCategory const cat, @@ -633,6 +560,201 @@ SpanGuard::discard() } } +// ===== ScopedSpanGuard::ScopedImpl ========================================= + +struct ScopedSpanGuard::ScopedImpl +{ + /** + * The unscoped guard that owns the span. Declared FIRST so it is + * destroyed AFTER `scope`: the Scope must pop before the span ends. + */ + 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. + */ + 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. + */ + std::thread::id owner{std::this_thread::get_id()}; + + /** + * Wrap a SpanGuard and activate its span on this thread. If the + * guard is active, push its span onto the thread-local context + * stack; a null guard leaves `scope` empty. + * @param g The span-owning guard to activate. + */ + explicit ScopedImpl(SpanGuard g) : guard(std::move(g)) + { + if (guard) + scope.emplace(guard.impl_->span); + } +}; + +// ===== ScopedSpanGuard core lifecycle ====================================== + +ScopedSpanGuard::ScopedSpanGuard(SpanGuard&& guard) + : impl_(std::make_unique(std::move(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. + XRPL_ASSERT( + !impl_ || !impl_->scope.has_value() || impl_->owner == std::this_thread::get_id(), + "xrpl::telemetry::ScopedSpanGuard::~ScopedSpanGuard : destroyed on " + "constructing thread"); +} + +// ===== ScopedSpanGuard factory methods ===================================== + +ScopedSpanGuard::ScopedSpanGuard(TraceCategory cat, std::string_view prefix, std::string_view name) + : ScopedSpanGuard(SpanGuard::span(cat, prefix, name)) +{ +} + +ScopedSpanGuard +ScopedSpanGuard::freshRoot(TraceCategory cat, std::string_view prefix, std::string_view name) +{ + return ScopedSpanGuard(SpanGuard::freshRoot(cat, prefix, name)); +} + +ScopedSpanGuard +ScopedSpanGuard::childSpan(std::string_view name) const +{ + return ScopedSpanGuard(impl_->guard.childSpan(name)); +} + +ScopedSpanGuard +ScopedSpanGuard::childSpan(std::string_view name, SpanContext const& parentCtx) +{ + return ScopedSpanGuard(SpanGuard::childSpan(name, parentCtx)); +} + +ScopedSpanGuard +ScopedSpanGuard::linkedSpan(std::string_view name) const +{ + return ScopedSpanGuard(impl_->guard.linkedSpan(name)); +} + +ScopedSpanGuard +ScopedSpanGuard::linkedSpan(std::string_view name, SpanContext const& linkCtx) +{ + return ScopedSpanGuard(SpanGuard::linkedSpan(name, linkCtx)); +} + +// ===== ScopedSpanGuard handoff bridge ====================================== + +ScopedSpanGuard:: +operator SpanGuard() && +{ + // The scope must be popped on the thread that pushed it; handing off + // elsewhere would pop the wrong stack. + 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 + return std::move(impl_->guard); +} + +// ===== ScopedSpanGuard context capture ===================================== + +SpanContext +ScopedSpanGuard::captureContext() const +{ + return impl_->guard.captureContext(); +} + +// ===== ScopedSpanGuard forwarding methods ================================== + +void +ScopedSpanGuard::setAttribute(std::string_view key, std::string_view value) +{ + impl_->guard.setAttribute(key, value); +} + +void +ScopedSpanGuard::setAttribute(std::string_view key, char const* value) +{ + impl_->guard.setAttribute(key, value); +} + +void +ScopedSpanGuard::setAttribute(std::string_view key, std::int64_t value) +{ + impl_->guard.setAttribute(key, value); +} + +void +ScopedSpanGuard::setAttribute(std::string_view key, double value) +{ + impl_->guard.setAttribute(key, value); +} + +void +ScopedSpanGuard::setAttribute(std::string_view key, bool value) +{ + impl_->guard.setAttribute(key, value); +} + +void +ScopedSpanGuard::setOk() +{ + impl_->guard.setOk(); +} + +void +ScopedSpanGuard::setError(std::string_view description) +{ + impl_->guard.setError(description); +} + +void +ScopedSpanGuard::addEvent(std::string_view name) +{ + impl_->guard.addEvent(name); +} + +void +ScopedSpanGuard::recordException(std::exception const& e) +{ + impl_->guard.recordException(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. + 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_->scope.reset(); + impl_->guard.discard(); +} + +ScopedSpanGuard:: +operator bool() const +{ + return impl_ && static_cast(impl_->guard); +} + } // namespace xrpl::telemetry #endif // XRPL_ENABLE_TELEMETRY diff --git a/src/libxrpl/telemetry/Telemetry.cpp b/src/libxrpl/telemetry/Telemetry.cpp index 711efb9733..7e97416a83 100644 --- a/src/libxrpl/telemetry/Telemetry.cpp +++ b/src/libxrpl/telemetry/Telemetry.cpp @@ -20,6 +20,7 @@ #include #include +#include #include #include @@ -329,9 +330,15 @@ public: std::make_shared(setup_.samplingRatio); auto sampler = trace_sdk::ParentBasedSamplerFactory::Create(std::move(rootSampler)); - // Create TracerProvider + // Create TracerProvider with a DeterministicIdGenerator. It returns a + // deterministic trace_id when a PendingTraceId is active on the thread, + // else a random one — letting hash-derived roots (introduced on a later + // branch) become true trace roots. Dormant until such a caller exists. sdkProvider_ = trace_sdk::TracerProviderFactory::Create( - std::move(processor), resourceAttrs, std::move(sampler)); + std::move(processor), + resourceAttrs, + std::move(sampler), + std::make_unique()); // Set as global provider trace_api::Provider::SetTracerProvider( diff --git a/src/tests/libxrpl/telemetry/SpanGuardScope.cpp b/src/tests/libxrpl/telemetry/SpanGuardScope.cpp index 21a69841a9..99bab8c480 100644 --- a/src/tests/libxrpl/telemetry/SpanGuardScope.cpp +++ b/src/tests/libxrpl/telemetry/SpanGuardScope.cpp @@ -1,10 +1,19 @@ -// Tests for SpanGuard::rootSpan() and SpanGuard::detached(). +// Tests for SpanGuard, ScopedSpanGuard and DeterministicIdGenerator. // -// These verify the cross-thread scope-leak fix at the trace level using an -// in-memory span exporter: rootSpan() must start a fresh trace root (ignoring -// the ambient active span), and detached() must strip the thread-local Scope -// on the origin thread so the guard can be moved to and ended on another -// thread without corrupting the origin thread's context stack. +// These verify the span-guard split and the deterministic-root fix at the +// trace level using an in-memory span exporter: +// - SpanGuard is unscoped and thread-free: it owns only the span, never the +// OTel thread-local context stack, so it may be moved to and ended on any +// thread. SpanGuard::freshRoot() starts a brand-new trace root, ignoring +// the ambient active span. +// - ScopedSpanGuard is scoped and thread-bound: it also pushes an OTel Scope +// so the span is the ambient parent on the constructing thread. Its +// `operator SpanGuard() &&` pops that Scope eagerly on the origin thread and +// yields a thread-free SpanGuard; destroying it on another thread trips an +// owner-thread assertion. +// - DeterministicIdGenerator (installed by the test TracerProvider) mints a +// caller-pinned trace_id for a forced-root span. PendingTraceId pins the id +// for one root span; an ambient child under a live parent never adopts it. // // The whole file is telemetry-only: when XRPL_ENABLE_TELEMETRY is not defined // SpanGuard is a no-op stub and the OpenTelemetry SDK headers are unavailable, @@ -12,11 +21,13 @@ #ifdef XRPL_ENABLE_TELEMETRY +#include #include #include #include #include +#include #include #include #include @@ -26,16 +37,20 @@ #include #include #include +#include #include +#include #include #include #include #include #include +#include #include +#include +#include #include -#include #include #include #include @@ -54,7 +69,9 @@ namespace otel_memory = opentelemetry::exporter::memory; * Reports every trace category as enabled and creates spans through an SDK * TracerProvider whose SimpleSpanProcessor forwards ended spans to an * InMemorySpanExporter, so a test can read the exact exported SpanData - * (trace id, span id, parent id, name). + * (trace id, span id, parent id, name). The provider is built with a + * DeterministicIdGenerator so PendingTraceId can pin the trace_id of a + * forced-root span. * * Inheritance: * @@ -81,16 +98,19 @@ public: // Factory populates spanData_ with the exporter's shared buffer. auto exporter = otel_memory::InMemorySpanExporterFactory::Create(spanData_); auto processor = otel_sdk_trace::SimpleSpanProcessorFactory::Create(std::move(exporter)); + // Install the DeterministicIdGenerator (4-arg overload) so a + // PendingTraceId can pin a forced-root span's trace_id in tests. provider_ = otel_sdk_trace::TracerProviderFactory::Create( std::move(processor), opentelemetry::sdk::resource::Resource::Create({}), - otel_sdk_trace::AlwaysOnSamplerFactory::Create()); + otel_sdk_trace::AlwaysOnSamplerFactory::Create(), + std::make_unique()); } /** * @return The exporter's span buffer (drained by GetSpans()). */ - std::shared_ptr + [[nodiscard]] std::shared_ptr spanData() const { return spanData_; @@ -226,6 +246,20 @@ countSpans( return count; } +/** + * Build the 16-byte deterministic trace_id used by the generator tests + * (bytes 1..16). Kept out of line so every generator test pins the same id. + * @return The fixed 16-byte trace_id {1, 2, ..., 16}. + */ +std::array +makeTraceIdBytes() +{ + std::array h{}; + for (int i = 0; i < 16; ++i) + h[i] = static_cast(i + 1); + return h; +} + /** * Installs a TestTelemetry as the global instance for each test and clears it * afterwards so the singleton never dangles between cases. @@ -250,7 +284,7 @@ protected: /** * @return The exporter's span buffer for the active TestTelemetry. */ - std::shared_ptr + [[nodiscard]] std::shared_ptr spanData() const { return telemetry_->spanData(); @@ -262,18 +296,18 @@ protected: std::unique_ptr telemetry_; }; -// rootSpan() must ignore the ambient active span and start a brand-new trace. -TEST_F(SpanGuardScopeTest, rootSpan_ignores_ambient_parent) +// freshRoot() must ignore the ambient active span and start a brand-new trace. +TEST_F(SpanGuardScopeTest, spanGuard_freshRoot_is_true_root_ignoring_ambient) { { // Ambient span becomes the active span on this thread. - auto ambient = SpanGuard::span(TraceCategory::Rpc, "rpc", "command"); + ScopedSpanGuard const ambient(TraceCategory::Rpc, "rpc", "command"); ASSERT_TRUE(static_cast(ambient)); - // rootSpan must NOT inherit the ambient span as its parent. - auto root = SpanGuard::rootSpan(TraceCategory::Peer, "peer", "validation.receive"); - ASSERT_TRUE(static_cast(root)); - } // root ends first, then ambient -- both on this thread. + // freshRoot must NOT inherit the ambient span as its parent. + auto r = SpanGuard::freshRoot(TraceCategory::Peer, "peer", "validation.receive"); + ASSERT_TRUE(static_cast(r)); + } // r ends first, then ambient's scope pops and ambient span ends. auto spans = spanData()->GetSpans(); auto* ambient = findSpan(spans, "rpc.command"); @@ -285,34 +319,76 @@ TEST_F(SpanGuardScopeTest, rootSpan_ignores_ambient_parent) EXPECT_FALSE(ambient->GetParentSpanId().IsValid()); EXPECT_TRUE(ambient->GetTraceId().IsValid()); - // The rootSpan has NO parent and lives in a DIFFERENT trace than ambient. + // The freshRoot span has NO parent and lives in a DIFFERENT trace. EXPECT_FALSE(root->GetParentSpanId().IsValid()); EXPECT_TRUE(root->GetTraceId().IsValid()); EXPECT_NE(root->GetTraceId(), ambient->GetTraceId()); } -// detached() pops the Scope on the origin thread, so ending the guard on -// another thread leaves the origin thread's context stack clean. -TEST_F(SpanGuardScopeTest, detached_does_not_corrupt_origin_stack) +// A ScopedSpanGuard is the ambient active span on its thread the moment it is +// constructed: a child created while it is alive parents to its span. +TEST_F(SpanGuardScopeTest, scopedGuard_is_ambient_on_construct) { - // Scoped guard is active on this (origin) thread. - auto guard = SpanGuard::span(TraceCategory::Ledger, "ledger", "build"); - ASSERT_TRUE(static_cast(guard)); - - // Strip the thread-local Scope here on the origin thread. - auto detached = std::move(guard).detached(); - ASSERT_TRUE(static_cast(detached)); - - // End the detached span on a worker thread. - std::thread worker([d = std::move(detached)]() mutable {}); - worker.join(); - - // Back on the origin thread: a new span must be a fresh root, proving the - // origin stack top was restored by detached() (not left holding - // ledger.build). + opentelemetry::trace::SpanId activeId; { - auto after = SpanGuard::span(TraceCategory::Rpc, "rpc", "command"); - ASSERT_TRUE(static_cast(after)); + ScopedSpanGuard const s(TraceCategory::Rpc, "rpc", "process"); + ASSERT_TRUE(static_cast(s)); + + // While s is alive it is the active span on this thread's context. + auto active = + opentelemetry::trace::GetSpan(opentelemetry::context::RuntimeContext::GetCurrent()) + ->GetContext(); + ASSERT_TRUE(active.IsValid()); + activeId = active.span_id(); + + // A child created here must parent to s's span, proving s is ambient. + auto child = s.childSpan("rpc.dispatch"); + ASSERT_TRUE(static_cast(child)); + } // child ends first, then s pops its scope and ends. + + auto spans = spanData()->GetSpans(); + auto* parent = findSpan(spans, "rpc.process"); + auto* child = findSpan(spans, "rpc.dispatch"); + ASSERT_NE(parent, nullptr); + ASSERT_NE(child, nullptr); + + // The active span observed while s was alive WAS s's span. + EXPECT_TRUE(activeId.IsValid()); + EXPECT_EQ(parent->GetSpanId(), activeId); + + // The child nested under s: same trace, parent = s's span. + EXPECT_EQ(child->GetParentSpanId(), parent->GetSpanId()); + EXPECT_EQ(child->GetTraceId(), parent->GetTraceId()); +} + +// operator SpanGuard() && pops the Scope eagerly on the origin thread, so the +// span is no longer ambient here and the resulting thread-free guard can be +// ended on a worker thread without corrupting this thread's context stack. +TEST_F(SpanGuardScopeTest, scopedGuard_conversion_pops_scope_on_this_thread) +{ + { + ScopedSpanGuard s(TraceCategory::Ledger, "ledger", "build"); + ASSERT_TRUE(static_cast(s)); + + // Convert to a bare SpanGuard: the scope is popped here, on this thread. + SpanGuard bare = std::move(s); + ASSERT_TRUE(static_cast(bare)); + + // The scope is gone: no span is active on this thread now. + auto active = + opentelemetry::trace::GetSpan(opentelemetry::context::RuntimeContext::GetCurrent()) + ->GetContext(); + EXPECT_FALSE(active.IsValid()); + + // A new ambient span here is a fresh root, NOT nested under build. + { + ScopedSpanGuard const after(TraceCategory::Rpc, "rpc", "command"); + ASSERT_TRUE(static_cast(after)); + } + + // End the thread-free guard on a worker thread -- no crash. + std::thread worker([g = std::move(bare)]() mutable {}); + worker.join(); } auto spans = spanData()->GetSpans(); @@ -321,243 +397,186 @@ TEST_F(SpanGuardScopeTest, detached_does_not_corrupt_origin_stack) ASSERT_NE(build, nullptr); ASSERT_NE(after, nullptr); - // 'after' is a new root: zero parent and a different trace than - // ledger.build. + // The span was exported exactly once, by the worker thread. + EXPECT_EQ(countSpans(spans, "ledger.build"), 1u); + + // 'after' did NOT nest under build: fresh root, different trace. EXPECT_FALSE(after->GetParentSpanId().IsValid()); EXPECT_NE(after->GetTraceId(), build->GetTraceId()); } -// A detached span is ended exactly once, by the worker thread that owns it. -TEST_F(SpanGuardScopeTest, detached_span_ends_once) -{ - auto guard = SpanGuard::span(TraceCategory::Ledger, "ledger", "build"); - auto detached = std::move(guard).detached(); - ASSERT_TRUE(static_cast(detached)); - - // Before the worker ends it, the span is still open: nothing exported yet. - auto before = spanData()->GetSpans(); - EXPECT_EQ(countSpans(before, "ledger.build"), 0u); - - // Ending happens once, on the worker thread, when the guard is destroyed. - { - std::thread worker([d = std::move(detached)]() mutable {}); - worker.join(); - } - - auto after = spanData()->GetSpans(); - EXPECT_EQ(countSpans(after, "ledger.build"), 1u); -} - -// rootSpan().detached() is the combination used at RPC coroutine entry points -// (e.g. RipplePathFind) that hold a span across a coro yield: the span must be -// a fresh root AND must not leave a Scope on the worker's context stack. -TEST_F(SpanGuardScopeTest, root_then_detached_is_root_and_leaves_stack_clean) +// The SpanGuard produced by the conversion ends the span exactly once: the +// moved-from ScopedSpanGuard must not re-end it on destruction. +TEST_F(SpanGuardScopeTest, scopedGuard_conversion_result_ends_span_once) { { - // Ambient span active on this (origin) thread. - auto ambient = SpanGuard::span(TraceCategory::Rpc, "rpc", "command"); - ASSERT_TRUE(static_cast(ambient)); + ScopedSpanGuard scoped(TraceCategory::Ledger, "ledger", "build"); + ASSERT_TRUE(static_cast(scoped)); - // Fresh root, then detach the Scope on the origin thread. - auto req = SpanGuard::rootSpan(TraceCategory::Rpc, "pathfind", "request").detached(); - ASSERT_TRUE(static_cast(req)); + SpanGuard const bare = std::move(scoped); + ASSERT_TRUE(static_cast(bare)); - // End the detached root span on a worker thread (mimics coro resume). - std::thread worker([r = std::move(req)]() mutable {}); - worker.join(); + // Nothing exported yet: the span is still open. + EXPECT_EQ(countSpans(spanData()->GetSpans(), "ledger.build"), 0u); - // Origin stack must be clean: a new span here is still parented by the - // ambient span (proving the detached root did not linger on the stack). - { - auto child = SpanGuard::span(TraceCategory::Rpc, "rpc", "sub"); - ASSERT_TRUE(static_cast(child)); - } + // 'bare' ends the span here on destruction; the moved-from 'scoped' + // guard is destroyed too but must NOT end it a second time. } auto spans = spanData()->GetSpans(); - auto* ambient = findSpan(spans, "rpc.command"); - auto* req = findSpan(spans, "pathfind.request"); - auto* child = findSpan(spans, "rpc.sub"); - ASSERT_NE(ambient, nullptr); - ASSERT_NE(req, nullptr); - ASSERT_NE(child, nullptr); - - // The rooted-then-detached span has NO parent and a DIFFERENT trace. - EXPECT_FALSE(req->GetParentSpanId().IsValid()); - EXPECT_NE(req->GetTraceId(), ambient->GetTraceId()); - - // The detached root did not corrupt the origin stack: 'child' still nests - // under the ambient span (same trace, parent = ambient), NOT under req. - EXPECT_EQ(child->GetParentSpanId(), ambient->GetSpanId()); - EXPECT_EQ(child->GetTraceId(), ambient->GetTraceId()); + // Exactly one export: not zero (the guard still owns the span) and not two + // (the moved-from scoped guard does not re-end it). + EXPECT_EQ(countSpans(spans, "ledger.build"), 1u); } -// Death test guarding the cross-thread scope-leak bug. +// A forced-root span started while a PendingTraceId is active adopts that +// pinned 16-byte trace_id and remains a true root (no parent). +TEST_F(SpanGuardScopeTest, deterministicIdGenerator_forced_root_gets_pending_trace_id) +{ + auto const h = makeTraceIdBytes(); + { + PendingTraceId const pending{h}; + auto rootCtx = opentelemetry::context::Context{opentelemetry::trace::kIsRootSpanKey, true}; + auto span = + telemetry_->startSpan("tx.receive", rootCtx, opentelemetry::trace::SpanKind::kInternal); + span->End(); + } // ~PendingTraceId asserts the id was consumed. + + auto spans = spanData()->GetSpans(); + ASSERT_EQ(spans.size(), 1u); + // trace_id == the pinned hash. + EXPECT_EQ(std::memcmp(spans[0]->GetTraceId().Id().data(), h.data(), 16), 0); + // TRUE ROOT: no parent. + EXPECT_FALSE(spans[0]->GetParentSpanId().IsValid()); +} + +// A forced-root span with NO PendingTraceId gets a random (non-zero) trace_id, +// never the deterministic hash -- the safety property when no id is pinned. +TEST_F(SpanGuardScopeTest, deterministicIdGenerator_no_pending_gives_random_root) +{ + auto const h = makeTraceIdBytes(); + { + auto rootCtx = opentelemetry::context::Context{opentelemetry::trace::kIsRootSpanKey, true}; + auto span = + telemetry_->startSpan("tx.receive", rootCtx, opentelemetry::trace::SpanKind::kInternal); + span->End(); + } + + auto spans = spanData()->GetSpans(); + ASSERT_EQ(spans.size(), 1u); + // Random root: valid (non-zero) trace_id... + EXPECT_TRUE(spans[0]->GetTraceId().IsValid()); + // ...and NOT the deterministic hash (no pending id leaked in). + EXPECT_NE(std::memcmp(spans[0]->GetTraceId().Id().data(), h.data(), 16), 0); + EXPECT_FALSE(spans[0]->GetParentSpanId().IsValid()); +} + +// SAFETY: an ambient child under a live parent never adopts a pending id. The +// SDK inherits the parent's trace_id and never calls GenerateTraceId() for the +// child, so the pinned id stays available -- proven here by a trailing +// forced-root span that DOES adopt it (which also consumes the id so +// ~PendingTraceId's consumed-assert holds; see the report for this choice). +TEST_F(SpanGuardScopeTest, deterministicIdGenerator_ambient_child_ignores_pending) +{ + auto const h = makeTraceIdBytes(); + { + // Ambient parent (a random-id root) active on this thread FIRST, before + // any id is pinned, so the parent itself does not consume it. + ScopedSpanGuard const parent(TraceCategory::Rpc, "rpc", "process"); + ASSERT_TRUE(static_cast(parent)); + + // Pin the id, then start an ambient child under the live parent. + PendingTraceId const pending{h}; + + // The child has a valid parent, so the SDK inherits the parent's + // trace_id and never calls GenerateTraceId(): the pending id is ignored. + { + auto child = parent.childSpan("rpc.dispatch"); + ASSERT_TRUE(static_cast(child)); + } + + // Consume the pinned id with a real forced-root span (its intended use), + // so ~PendingTraceId sees the id as consumed. Its trace_id == h. + { + auto rootCtx = + opentelemetry::context::Context{opentelemetry::trace::kIsRootSpanKey, true}; + auto root = telemetry_->startSpan( + "tx.receive", rootCtx, opentelemetry::trace::SpanKind::kInternal); + root->End(); + } + } // ~PendingTraceId: consumed == true, assert holds; then parent ends. + + auto spans = spanData()->GetSpans(); + auto* parent = findSpan(spans, "rpc.process"); + auto* child = findSpan(spans, "rpc.dispatch"); + auto* root = findSpan(spans, "tx.receive"); + ASSERT_NE(parent, nullptr); + ASSERT_NE(child, nullptr); + ASSERT_NE(root, nullptr); + + // The ambient child inherited the parent's trace and did NOT adopt h. + EXPECT_EQ(child->GetTraceId(), parent->GetTraceId()); + EXPECT_EQ(child->GetParentSpanId(), parent->GetSpanId()); + EXPECT_NE(std::memcmp(child->GetTraceId().Id().data(), h.data(), 16), 0); + + // The forced-root span DID adopt the pinned id: proof the id was available + // the whole time -- the ambient child simply never requested it. + EXPECT_EQ(std::memcmp(root->GetTraceId().Id().data(), h.data(), 16), 0); + EXPECT_FALSE(root->GetParentSpanId().IsValid()); +} + +// Death test guarding the cross-thread scope-leak bug, now on ScopedSpanGuard. // -// This used to be a "negative control": a scoped guard was NOT detached, moved -// to and destroyed on a worker thread (popping the WRONG thread's stack), and -// the test then asserted that a later span on the origin thread had SILENTLY -// inherited the stale span -- proving the corruption happened undetected. +// A ScopedSpanGuard's Scope is bound to the thread that constructed it, so it +// must be destroyed on that same thread. ScopedSpanGuard is non-movable, so it +// cannot be moved into a worker directly; instead we own it through a +// unique_ptr and move only that pointer to a worker thread. Destroying the +// pointer there runs ~ScopedSpanGuard on the WRONG thread, which pops the Scope +// on a foreign context stack -- ~ScopedSpanGuard's owner-thread XRPL_ASSERT +// turns that silent corruption into a loud abort(). // -// SpanGuard::Impl::~Impl now enforces thread affinity with an XRPL_ASSERT: a -// live scope must be destroyed on the thread that created it. In debug/test -// builds (NDEBUG unset) XRPL_ASSERT expands to assert(), so the exact sequence -// above now aborts the process inside the worker thread's destructor instead of -// corrupting the origin stack. The bug therefore moved from "silently wrong" to -// "loudly crashes early" -- the intended tradeoff, since a crash is far easier -// to catch than a corrupted trace hierarchy. -// -// The death happens on the WORKER thread (the moved-in guard is destroyed when -// the worker lambda returns, before worker.join() completes). A failed assert() -// calls abort(), which raises SIGABRT process-wide regardless of which thread -// hit it, so EXPECT_DEATH -- which runs the statement in a forked child and -// checks it dies -- observes the crash. The regex matches the assert message -// substring rather than the whole line, because the file:line/function prefix -// glibc prints around it is platform and compiler dependent. +// The death happens on the WORKER thread (the moved-in unique_ptr is destroyed +// when the worker lambda's captures are torn down, before worker.join() +// completes). A failed assert() calls abort(), which raises SIGABRT +// process-wide regardless of thread, so EXPECT_DEATH -- which runs the +// statement in a forked child and checks it dies -- observes the crash. The +// regex matches the assert message substring; the assert message is written as +// two adjacent string literals in SpanGuard.cpp, so the ".*" bridges the gap +// between "on" and "constructing". // // The test is skipped where the assertion cannot fire: under NDEBUG (Release // builds) XRPL_ASSERT is a no-op, and under ENABLE_VOIDSTAR a failed assert // continues instead of aborting -- in both cases the worker would not crash and // EXPECT_DEATH would report a spurious failure. -TEST_F(SpanGuardScopeTest, non_detached_cross_thread_death_asserts_at_wrong_thread_destroy) +TEST_F(SpanGuardScopeTest, scopedGuard_cross_thread_death_asserts_at_wrong_thread_destroy) { - // XRPL_ASSERT only aborts when assertions are live. Under NDEBUG (Release - // builds) it expands to assert(), which the C standard strips to a no-op, - // so the worker thread destroys the guard without crashing. Under - // ENABLE_VOIDSTAR (Antithesis fuzzing) a failed XRPL_ASSERT continues - // execution instead of aborting. In either case the process does not die - // and EXPECT_DEATH would fail, so skip this test there. The two macros are - // mutually exclusive (instrumentation.h #errors on ENABLE_VOIDSTAR+NDEBUG), - // but both break the death check, so guard against each. #ifdef NDEBUG GTEST_SKIP() << "XRPL_ASSERT compiles to a no-op under NDEBUG (Release builds), so the " "cross-thread scope-leak assertion this test exercises does not fire."; -#elif defined(ENABLE_VOIDSTAR) +#elifdef ENABLE_VOIDSTAR GTEST_SKIP() << "ENABLE_VOIDSTAR continues past a failed XRPL_ASSERT instead of aborting, so " "the cross-thread scope-leak assertion this test exercises does not crash."; #else EXPECT_DEATH( { - // Scoped guard is active on this (origin) thread; its Scope is - // bound to this thread. - auto guard = SpanGuard::span(TraceCategory::Ledger, "ledger", "build"); + // Scoped guard constructed on THIS thread; its Scope binds here. + auto scoped = + std::make_unique(TraceCategory::Ledger, "ledger", "build"); - // Move the still-scoped guard to a worker thread WITHOUT detaching. - // ~SpanGuard::Impl runs on the worker when the lambda returns and - // trips the thread-affinity assertion -> abort(). - std::thread worker([d = std::move(guard)]() mutable {}); + // Move only the owning pointer to a worker. ~ScopedSpanGuard runs on + // the worker when the lambda's captures are destroyed and trips the + // owner-thread assertion -> abort(). + std::thread worker([s = std::move(scoped)]() mutable {}); worker.join(); }, // The assert message is written as two adjacent string literals in - // SpanGuard.cpp; glibc's assert() stringifies the expression source via - // the preprocessor '#' operator, keeping both quoted literals with the + // SpanGuard.cpp; assert() stringifies the expression source via the + // preprocessor '#' operator, keeping both quoted literals with the // "\" \"" gap between "on" and "constructing". Match across that gap. - ".*scoped guard destroyed on.*constructing thread.*"); + ".*destroyed on.*constructing thread.*"); #endif } -// detachInPlace(optional&) must detach the live guard the same way the -// hand-rolled emplace(std::move(*opt).detached()) idiom does: after the call -// the guard can be moved to and ended on a worker thread without leaving the -// origin thread's context stack holding it. -TEST_F(SpanGuardScopeTest, detach_in_place_optional_detaches_active_guard) -{ - // Scoped optional guard is active on this (origin) thread. - std::optional opt = SpanGuard::span(TraceCategory::Ledger, "ledger", "build"); - ASSERT_TRUE(opt.has_value()); - ASSERT_TRUE(static_cast(*opt)); - - // Strip the thread-local Scope here on the origin thread via the helper. - detachInPlace(opt); - ASSERT_TRUE(opt.has_value()); - ASSERT_TRUE(static_cast(*opt)); - - // End the detached span on a worker thread. - std::thread worker([d = std::move(*opt)]() mutable {}); - worker.join(); - - // Back on the origin thread: a new span must be a fresh root, proving the - // origin stack top was restored by detachInPlace() (not left holding - // ledger.build). - { - auto after = SpanGuard::span(TraceCategory::Rpc, "rpc", "command"); - ASSERT_TRUE(static_cast(after)); - } - - auto spans = spanData()->GetSpans(); - auto* build = findSpan(spans, "ledger.build"); - auto* after = findSpan(spans, "rpc.command"); - ASSERT_NE(build, nullptr); - ASSERT_NE(after, nullptr); - - // 'after' is a new root: zero parent and a different trace than - // ledger.build. - EXPECT_FALSE(after->GetParentSpanId().IsValid()); - EXPECT_NE(after->GetTraceId(), build->GetTraceId()); -} - -// detachInPlace(optional&) on an empty optional is a no-op: it must not crash, -// must leave the optional empty, and must export nothing. -TEST_F(SpanGuardScopeTest, detach_in_place_optional_noop_on_empty) -{ - std::optional opt; - ASSERT_FALSE(opt.has_value()); - - detachInPlace(opt); - - // Still empty, no span was created or exported. - EXPECT_FALSE(opt.has_value()); - EXPECT_TRUE(spanData()->GetSpans().empty()); -} - -// detachInPlace(shared_ptr) must detach the live guard and return it as a new -// shared_ptr, so the returned guard can be moved to and ended on a worker -// thread without corrupting the origin thread's context stack. -TEST_F(SpanGuardScopeTest, detach_in_place_shared_detaches_active_guard) -{ - // Scoped shared guard is active on this (origin) thread. - auto span = - std::make_shared(SpanGuard::span(TraceCategory::Ledger, "ledger", "build")); - ASSERT_NE(span, nullptr); - ASSERT_TRUE(static_cast(*span)); - - // Strip the thread-local Scope here on the origin thread via the helper. - span = detachInPlace(std::move(span)); - ASSERT_NE(span, nullptr); - ASSERT_TRUE(static_cast(*span)); - - // End the detached span on a worker thread. - std::thread worker([d = std::move(span)]() mutable {}); - worker.join(); - - // Back on the origin thread: a new span must be a fresh root, proving the - // origin stack top was restored by detachInPlace() (not left holding - // ledger.build). - { - auto after = SpanGuard::span(TraceCategory::Rpc, "rpc", "command"); - ASSERT_TRUE(static_cast(after)); - } - - auto spans = spanData()->GetSpans(); - auto* build = findSpan(spans, "ledger.build"); - auto* after = findSpan(spans, "rpc.command"); - ASSERT_NE(build, nullptr); - ASSERT_NE(after, nullptr); - - // 'after' is a new root: zero parent and a different trace than - // ledger.build. - EXPECT_FALSE(after->GetParentSpanId().IsValid()); - EXPECT_NE(after->GetTraceId(), build->GetTraceId()); -} - -// detachInPlace(shared_ptr) on a null pointer is a no-op: it must not crash and -// must return null unchanged. -TEST_F(SpanGuardScopeTest, detach_in_place_shared_noop_on_null) -{ - auto result = detachInPlace(std::shared_ptr{}); - EXPECT_EQ(result, nullptr); -} - } // namespace } // namespace xrpl::telemetry diff --git a/src/xrpld/app/misc/NetworkOPs.cpp b/src/xrpld/app/misc/NetworkOPs.cpp index bffe8c5c9f..aa5a483119 100644 --- a/src/xrpld/app/misc/NetworkOPs.cpp +++ b/src/xrpld/app/misc/NetworkOPs.cpp @@ -1384,11 +1384,10 @@ NetworkOPsImp::processTransaction( FailHard failType) { using namespace telemetry; - // Detached: this span is stored in TransactionStatus and applied on a - // batch worker thread, so it must not leave its Scope bound to this - // thread's context stack (that leak would adopt later work into this - // transaction's trace). - auto span = std::make_shared(txProcessSpan(transaction->getID()).detached()); + // SpanGuard is thread-free (holds no Scope), so it is safe to store here + // and end on the batch worker thread that later applies this transaction — + // no detach step is needed. + auto span = std::make_shared(txProcessSpan(transaction->getID())); span->setAttribute(tx_span::attr::txHash, to_string(transaction->getID()).c_str()); span->setAttribute(tx_span::attr::local, bLocal); if (auto const& stx = transaction->getSTransaction()) diff --git a/src/xrpld/app/misc/detail/TxQ.cpp b/src/xrpld/app/misc/detail/TxQ.cpp index d3a6b3290d..e0c988a418 100644 --- a/src/xrpld/app/misc/detail/TxQ.cpp +++ b/src/xrpld/app/misc/detail/TxQ.cpp @@ -545,7 +545,7 @@ TxQ::tryClearAccountQueueUpThruTx( beast::Journal j) { using namespace telemetry; - [[maybe_unused]] auto span = SpanGuard::span( + [[maybe_unused]] ScopedSpanGuard span( TraceCategory::Transactions, txq_span::prefix::txq, txq_span::op::batchClear); SeqProxy const tSeqProx{tx.getSeqProxy()}; @@ -752,8 +752,7 @@ TxQ::apply( beast::Journal j) { using namespace telemetry; - auto span = - SpanGuard::span(TraceCategory::Transactions, txq_span::prefix::txq, txq_span::op::enqueue); + ScopedSpanGuard span(TraceCategory::Transactions, txq_span::prefix::txq, txq_span::op::enqueue); span.setAttribute(txq_span::attr::txHash, to_string(tx->getTransactionID()).c_str()); if (auto const* fmt = TxFormats::getInstance().findByType(tx->getTxnType())) span.setAttribute(txq_span::attr::txType, fmt->getName().c_str()); @@ -1374,8 +1373,7 @@ void TxQ::processClosedLedger(Application& app, ReadView const& view, bool timeLeap) { using namespace telemetry; - auto span = - SpanGuard::span(TraceCategory::Transactions, txq_span::prefix::txq, txq_span::op::cleanup); + ScopedSpanGuard span(TraceCategory::Transactions, txq_span::prefix::txq, txq_span::op::cleanup); span.setAttribute(txq_span::attr::ledgerSeq, static_cast(view.header().seq)); std::scoped_lock const lock(mutex_); @@ -1466,8 +1464,7 @@ TxQ::accept(Application& app, OpenView& view) // Create the span and read byFee_.size() only after taking the lock, since // byFee_ is guarded by mutex_. - auto span = - SpanGuard::span(TraceCategory::Transactions, txq_span::prefix::txq, txq_span::op::accept); + ScopedSpanGuard span(TraceCategory::Transactions, txq_span::prefix::txq, txq_span::op::accept); span.setAttribute(txq_span::attr::queueSize, static_cast(byFee_.size())); auto const metricsSnapshot = feeMetrics_.getSnapshot(); @@ -1497,7 +1494,7 @@ TxQ::accept(Application& app, OpenView& view) JLOG(j_.trace()) << "Applying queued transaction " << candidateIter->txID << " to open ledger."; - auto txSpan = SpanGuard::span( + ScopedSpanGuard txSpan( TraceCategory::Transactions, txq_span::prefix::txq, txq_span::op::acceptTx); txSpan.setAttribute(txq_span::attr::txHash, to_string(candidateIter->txID).c_str()); txSpan.setAttribute( @@ -1718,7 +1715,7 @@ TxQ::tryDirectApply( beast::Journal j) { using namespace telemetry; - [[maybe_unused]] auto span = SpanGuard::span( + [[maybe_unused]] ScopedSpanGuard const span( TraceCategory::Transactions, txq_span::prefix::txq, txq_span::op::applyDirect); auto const account = (*tx)[sfAccount]; diff --git a/src/xrpld/overlay/detail/PeerImp.cpp b/src/xrpld/overlay/detail/PeerImp.cpp index 3ea6347e41..ed64edb30d 100644 --- a/src/xrpld/overlay/detail/PeerImp.cpp +++ b/src/xrpld/overlay/detail/PeerImp.cpp @@ -1313,10 +1313,9 @@ PeerImp::handleTransaction( uint256 const txID = stx->getTransactionID(); using namespace telemetry; - // Detached: this span is handed to a job-queue worker and must not - // leave its Scope bound to this peer thread's context stack (that - // leak would adopt later peer messages into this transaction's trace). - auto span = std::make_shared(txReceiveSpan(txID, *m).detached()); + // SpanGuard is thread-free (holds no Scope), so it is safe to hand to + // a job-queue worker and end on that thread — no detach step is needed. + auto span = std::make_shared(txReceiveSpan(txID, *m)); span->setAttribute(tx_span::attr::txHash, to_string(txID).c_str()); span->setAttribute(tx_span::attr::peerId, static_cast(id_)); if (auto const* fmt = TxFormats::getInstance().findByType(stx->getTxnType())) diff --git a/src/xrpld/rpc/detail/ServerHandler.cpp b/src/xrpld/rpc/detail/ServerHandler.cpp index 48d5760724..7107566dfd 100644 --- a/src/xrpld/rpc/detail/ServerHandler.cpp +++ b/src/xrpld/rpc/detail/ServerHandler.cpp @@ -229,8 +229,10 @@ ServerHandler::onHandoff( if (!isWs) return statusRequestResponse(request, http::status::unauthorized); - auto span = - SpanGuard::span(TraceCategory::Rpc, rpc_span::prefix::rpc, rpc_span::op::wsUpgrade); + // Fresh root so each WS upgrade is its own trace, not nested under a + // leaked ambient span on a reused coro worker. + auto span = ScopedSpanGuard::freshRoot( + TraceCategory::Rpc, rpc_span::prefix::rpc, rpc_span::op::wsUpgrade); std::shared_ptr ws; try { @@ -348,8 +350,9 @@ ServerHandler::onWSMessage( auto const size = boost::asio::buffer_size(buffers); if (size > RPC::Tuning::kMaxRequestSize || !json::Reader{}.parse(jv, buffers) || !jv.isObject()) { - auto span = - SpanGuard::span(TraceCategory::Rpc, rpc_span::prefix::rpc, rpc_span::op::wsMessage); + // Fresh root so each WS message is its own trace. + auto span = ScopedSpanGuard::freshRoot( + TraceCategory::Rpc, rpc_span::prefix::rpc, rpc_span::op::wsMessage); span.setError(rpc_span::val::invalidJson); json::Value jvResult(json::ValueType::Object); @@ -428,7 +431,9 @@ ServerHandler::processSession( std::shared_ptr const& coro, json::Value const& jv) { - auto span = SpanGuard::span(TraceCategory::Rpc, rpc_span::prefix::rpc, rpc_span::op::wsMessage); + // Fresh root so each WS message is its own trace. + auto span = ScopedSpanGuard::freshRoot( + TraceCategory::Rpc, rpc_span::prefix::rpc, rpc_span::op::wsMessage); if (jv.isMember(jss::command) && jv[jss::command].isString()) { span.setAttribute(rpc_span::attr::command, jv[jss::command].asString().c_str()); @@ -593,8 +598,10 @@ ServerHandler::processSession( std::shared_ptr const& session, std::shared_ptr coro) { - auto span = - SpanGuard::span(TraceCategory::Rpc, rpc_span::prefix::rpc, rpc_span::op::httpRequest); + // Fresh root so each HTTP request is its own trace, not nested under a + // leaked ambient span on a reused coro worker. + auto span = ScopedSpanGuard::freshRoot( + TraceCategory::Rpc, rpc_span::prefix::rpc, rpc_span::op::httpRequest); auto const requestBody = ::xrpl::buffersToString(session->request().body().data()); span.setAttribute(rpc_span::attr::requestPayloadSize, static_cast(requestBody.size())); @@ -658,7 +665,8 @@ ServerHandler::processRequest( std::string_view forwardedFor, std::string_view user) { - auto span = SpanGuard::span(TraceCategory::Rpc, rpc_span::prefix::rpc, rpc_span::op::process); + // Scoped child: nests under the httpRequest span active on this thread. + auto span = ScopedSpanGuard(TraceCategory::Rpc, rpc_span::prefix::rpc, rpc_span::op::process); auto rpcJ = app_.getJournal("RPC"); // Tracks whether any failure occurred. Set on every error path (early diff --git a/src/xrpld/rpc/handlers/orderbook/RipplePathFind.cpp b/src/xrpld/rpc/handlers/orderbook/RipplePathFind.cpp index 24373a677f..b1e2bc64b2 100644 --- a/src/xrpld/rpc/handlers/orderbook/RipplePathFind.cpp +++ b/src/xrpld/rpc/handlers/orderbook/RipplePathFind.cpp @@ -27,19 +27,17 @@ json::Value doRipplePathFind(RPC::JsonContext& context) { using namespace telemetry; - // This RPC span is held live across context.coro->yield() below, so it is - // both rooted and detached: - // - rootSpan: on resume the coroutine may run on a different JobQueue + // This RPC span is held live across context.coro->yield() below, so it must + // be both a fresh root and thread-free: + // - freshRoot: on resume the coroutine may run on a different JobQueue // worker whose thread-local context stack holds unrelated spans; a fresh // root avoids inheriting a stale ambient parent at creation. - // - detached(): strips the thread-local Scope so the guard does not sit on - // the worker's context stack across the yield (which would adopt other - // spans run on that thread) and is not popped on the wrong thread when - // the coroutine resumes elsewhere. The span still ends when this frame - // unwinds after resume. - auto span = SpanGuard::rootSpan( - TraceCategory::Rpc, pathfind_span::prefix::pathfind, pathfind_span::op::request) - .detached(); + // - SpanGuard is thread-free: it holds no thread-local Scope, so it never + // sits on the worker's context stack across the yield and is never popped + // on the wrong thread when the coroutine resumes elsewhere. No scope to + // strip. The span still ends when this frame unwinds after resume. + auto span = SpanGuard::freshRoot( + TraceCategory::Rpc, pathfind_span::prefix::pathfind, pathfind_span::op::request); // Addresses are hashed before emission for privacy. if (auto const& src = context.params[jss::source_account]; src.isString()) span.setAttribute(pathfind_span::attr::sourceAccount, redactAccount(src.asString()));