diff --git a/include/xrpl/telemetry/CoroAwareContextStorage.h b/include/xrpl/telemetry/CoroAwareContextStorage.h index ee982ace6c..d7c047e752 100644 --- a/include/xrpl/telemetry/CoroAwareContextStorage.h +++ b/include/xrpl/telemetry/CoroAwareContextStorage.h @@ -86,6 +86,17 @@ class CoroAwareContextStorage : public opentelemetry::context::RuntimeContextSto public: CoroAwareContextStorage() = default; + // Non-copyable and non-movable: the LocalValue store is keyed by the + // address of stack_, so copying or moving would mis-key the store and + // strand its entries. Only ever new-ed once and installed on the SDK, so + // these are for intent/safety rather than a live bug. + CoroAwareContextStorage(CoroAwareContextStorage const&) = delete; + CoroAwareContextStorage& + operator=(CoroAwareContextStorage const&) = delete; + CoroAwareContextStorage(CoroAwareContextStorage&&) = delete; + CoroAwareContextStorage& + operator=(CoroAwareContextStorage&&) = delete; + /** * @return the current (top-of-stack) context for this coro/thread. */ diff --git a/include/xrpl/telemetry/SpanGuard.h b/include/xrpl/telemetry/SpanGuard.h index 9777f03384..fa96753bd8 100644 --- a/include/xrpl/telemetry/SpanGuard.h +++ b/include/xrpl/telemetry/SpanGuard.h @@ -929,6 +929,24 @@ public: operator=(ScopedActivation const&) = delete; }; +/** + * Activate a span guard as the ambient context if it is live, else return a + * no-op activation. Accepts any handle (shared_ptr / optional) that is + * contextually convertible to bool and dereferences to a SpanGuard. Non-owning: + * the handle still owns/ends the span. Returns a null ScopedActivation when the + * handle is empty or the span is inactive (e.g. telemetry disabled). + * @param guard A shared_ptr or optional. + * @return An RAII activation; drop it to restore the prior context. + */ +template +[[nodiscard]] ScopedActivation +activateIfLive(SpanGuardHandle const& guard) +{ + if (guard && *guard) + return guard->activate(); + return ScopedActivation{}; +} + // --------------------------------------------------------------------------- // No-op stub (all inline, zero overhead, no OTel dependency) // --------------------------------------------------------------------------- @@ -1215,6 +1233,22 @@ public: } }; +/** + * No-op counterpart to the telemetry-enabled activateIfLive(). Same signature + * and handle contract (shared_ptr / optional of SpanGuard); always yields a + * null ScopedActivation because a stub guard is never live. + * @param guard A shared_ptr or optional. + * @return A null (no-op) ScopedActivation. + */ +template +[[nodiscard]] ScopedActivation +activateIfLive(SpanGuardHandle const& guard) +{ + if (guard && *guard) + return guard->activate(); + return ScopedActivation{}; +} + #endif // XRPL_ENABLE_TELEMETRY } // namespace xrpl::telemetry diff --git a/src/libxrpl/telemetry/CoroAwareContextStorage.cpp b/src/libxrpl/telemetry/CoroAwareContextStorage.cpp index acd0dc77c3..f3d13c6489 100644 --- a/src/libxrpl/telemetry/CoroAwareContextStorage.cpp +++ b/src/libxrpl/telemetry/CoroAwareContextStorage.cpp @@ -12,6 +12,8 @@ #include +#include + #include #include #include @@ -21,6 +23,20 @@ namespace xrpl::telemetry { opentelemetry::context::Context CoroAwareContextStorage::GetCurrent() noexcept { + // Hot path: Log.cpp reads the current context on EVERY log line on EVERY + // thread. Peek the raw per-thread/coro store WITHOUT materializing it: + // getLocalValues().get() returns nullptr when this thread has no store yet + // and never allocates. Dereferencing stack_ (a LocalValue) would instead + // heap-allocate the store + a map entry + the vector on first touch, so we + // avoid that on the no-span path. + // + // Behaviour is identical to touching *stack_: a thread with no store has no + // active span, and a store whose stack was never pushed yields an empty + // vector — both cases map to an empty Context, matching stock + // ThreadLocalContextStorage (Top() returns Context() when the stack is + // empty). Only when a store already exists do we read the top frame. + if (detail::getLocalValues().get() == nullptr) + return opentelemetry::context::Context{}; auto& stack = *stack_; return stack.empty() ? opentelemetry::context::Context{} : stack.back(); } diff --git a/src/libxrpl/telemetry/DeterministicIdGenerator.cpp b/src/libxrpl/telemetry/DeterministicIdGenerator.cpp index c21bdcee5e..90c5c744b3 100644 --- a/src/libxrpl/telemetry/DeterministicIdGenerator.cpp +++ b/src/libxrpl/telemetry/DeterministicIdGenerator.cpp @@ -96,8 +96,10 @@ PendingTraceId::~PendingTraceId() noexcept if (!gTlsPendingConsumed) { // The assert above is stripped in release, so bump a counter too: a - // dropped deterministic trace root stays observable via - // unconsumedDeterministicIdDrops(). Relaxed: this is a diagnostic tally. + // process-wide tally of dropped deterministic trace roots. It is + // exposed via unconsumedDeterministicIdDrops() as an extension point + // for a future metric or diagnostic to read; nothing wires it to a log + // or metric yet. Relaxed: this is a plain diagnostic tally. gUnconsumedDeterministicIdDrops.fetch_add(1, std::memory_order_relaxed); } gTlsPendingTraceId.reset(); // never leak, even in release diff --git a/src/xrpld/app/consensus/RCLConsensus.cpp b/src/xrpld/app/consensus/RCLConsensus.cpp index 30b3e4db32..4da0e5bc01 100644 --- a/src/xrpld/app/consensus/RCLConsensus.cpp +++ b/src/xrpld/app/consensus/RCLConsensus.cpp @@ -560,8 +560,7 @@ RCLConsensus::Adaptor::doAccept( // (and any spans created here) correlate to it. Non-owning: acceptSpan still // owns/ends the span. doAccept runs to completion on the JtAccept worker // (no coroutine yield), so this scope is thread-local and safe. - auto acceptActivation = - (acceptSpan && *acceptSpan) ? acceptSpan->activate() : telemetry::ScopedActivation{}; + auto acceptActivation = telemetry::activateIfLive(acceptSpan); prevProposers_ = result.proposers; prevRoundTime_ = result.roundTime.read(); diff --git a/src/xrpld/app/misc/NetworkOPs.cpp b/src/xrpld/app/misc/NetworkOPs.cpp index f14e9c51a7..54733cf3e0 100644 --- a/src/xrpld/app/misc/NetworkOPs.cpp +++ b/src/xrpld/app/misc/NetworkOPs.cpp @@ -1624,8 +1624,7 @@ NetworkOPsImp::apply(std::unique_lock& batchLock) // Non-owning: the span is still owned/ended by e.span. Scoped to // one loop iteration, so each tx's span is ambient only for its // own processing. No yield in this loop. - auto txActivation = - (e.span && *e.span) ? e.span->activate() : telemetry::ScopedActivation{}; + auto txActivation = telemetry::activateIfLive(e.span); if (e.span && *e.span) { diff --git a/src/xrpld/overlay/detail/PeerImp.cpp b/src/xrpld/overlay/detail/PeerImp.cpp index 83a172c4df..05de8d557a 100644 --- a/src/xrpld/overlay/detail/PeerImp.cpp +++ b/src/xrpld/overlay/detail/PeerImp.cpp @@ -1438,7 +1438,7 @@ PeerImp::handleTransaction( // Activate the tx.receive span so checkTransaction's log // lines carry its trace_id. Non-owning; sp still owns/ends // the span. This job body runs to completion on one worker. - auto activation = (sp && *sp) ? sp->activate() : telemetry::ScopedActivation{}; + auto activation = telemetry::activateIfLive(sp); if (auto peer = weak.lock()) peer->checkTransaction(flags, checkSignature, stx, batch); });