ARCHIVE/rippled
1
0
Fork 0
mirror of https://github.com/XRPLF/rippled.git synced 2026-07-26 00:20:41 +00:00
Code Issues Packages Projects Releases Wiki Activity

Merge branch 'pratik/otel-phase9-metric-gap-fill' into pratik/otel-phase10-workload-validation

Browse Source 
Brings the coroutine-aware OTel context storage refactor forward to phase-10:
CoroAwareContextStorage + install, ScopedSpanGuard same-store assertion,
non-owning ScopedActivation, scoped rpc.command spans, tx/consensus/ledger
worker-body activation, coro-store-swap tests, and the M1/M2 hardening.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
Pratik Mankawde
2026-07-22 23:16:18 +01:00
parent addd7f8d39 24db0b246c
commit 48a5aad91d
17 changed files with 770 additions and 121 deletions
Download Patch File Download Diff File Expand all files Collapse all files

68
src/libxrpl/telemetry/CoroAwareContextStorage.cpp Normal file
View File

@@ -0,0 +1,68 @@
/**
* Implementation of CoroAwareContextStorage.
*
* Mirrors opentelemetry's ThreadLocalContextStorage stack semantics, but the
* stack lives in an xrpl::LocalValue so it follows a JobQueue::Coro across
* yield/resume. All OpenTelemetry types stay confined to this translation unit.
*
* @see CoroAwareContextStorage (CoroAwareContextStorage.h)
*/
#ifdef XRPL_ENABLE_TELEMETRY
#include <xrpl/telemetry/CoroAwareContextStorage.h>
#include <opentelemetry/context/context.h>
#include <opentelemetry/context/runtime_context.h>
#include <opentelemetry/nostd/unique_ptr.h>
namespace xrpl::telemetry {
opentelemetry::context::Context
CoroAwareContextStorage::GetCurrent() noexcept
{
auto& stack = *stack_;
return stack.empty() ? opentelemetry::context::Context{} : stack.back();
}
opentelemetry::nostd::unique_ptr<opentelemetry::context::Token>
CoroAwareContextStorage::Attach(opentelemetry::context::Context const& context) noexcept
{
stack_->push_back(context);
return CreateToken(context);
}
bool
CoroAwareContextStorage::Detach(opentelemetry::context::Token& token) noexcept
{
auto& stack = *stack_;
// Fast path: the token's frame is on top (the common LIFO case).
if (!stack.empty() && token == stack.back())
{
stack.pop_back();
return true;
}
// Fallback: token not on top — verify it is present, then pop down to and
// including it (mirrors ThreadLocalContextStorage::Detach; also detaches
// any child frames left above it).
bool found = false;
for (auto const& frame : stack)
{
if (token == frame)
{
found = true;
break;
}
}
if (!found)
return false;
while (!stack.empty() && !(token == stack.back()))
stack.pop_back();
if (!stack.empty())
stack.pop_back();
return true;
}
} // namespace xrpl::telemetry
#endif // XRPL_ENABLE_TELEMETRY

22
src/libxrpl/telemetry/DeterministicIdGenerator.cpp
View File

@@ -22,6 +22,7 @@
#include <opentelemetry/trace/trace_id.h>
#include <array>
#include <atomic>
#include <cstdint>
#include <optional>
@@ -42,6 +43,14 @@ thread_local std::optional<std::array<std::uint8_t, 16>> gTlsPendingTraceId;
*/
thread_local bool gTlsPendingConsumed = false;
/**
* Process-wide count of deterministic trace_ids dropped without being consumed.
* Bumped in ~PendingTraceId when the pending id was never taken by a root span.
* Atomic because it is summed across all threads; relaxed ordering is enough
* for a diagnostic tally. Exposed via unconsumedDeterministicIdDrops().
*/
std::atomic<std::uint64_t> gUnconsumedDeterministicIdDrops{0};
} // namespace
DeterministicIdGenerator::DeterministicIdGenerator()
@@ -84,10 +93,23 @@ PendingTraceId::~PendingTraceId() noexcept
XRPL_ASSERT(
gTlsPendingConsumed,
"xrpl::telemetry::PendingTraceId : deterministic trace_id was not consumed");
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.
gUnconsumedDeterministicIdDrops.fetch_add(1, std::memory_order_relaxed);
}
gTlsPendingTraceId.reset(); // never leak, even in release
gTlsPendingConsumed = false;
}
std::uint64_t
unconsumedDeterministicIdDrops() noexcept
{
return gUnconsumedDeterministicIdDrops.load(std::memory_order_relaxed);
}
} // namespace xrpl::telemetry
#endif // XRPL_ENABLE_TELEMETRY

146
src/libxrpl/telemetry/SpanGuard.cpp
View File

@@ -10,10 +10,12 @@
* touches the thread-local context stack, so a SpanGuard is
* thread-free and may be moved to and destroyed on any thread.
* - ScopedSpanGuard::ScopedImpl holds a SpanGuard plus an optional
* Scope. The Scope pushes the span onto the constructing thread's
* context stack; member order (guard first, scope second) ensures
* Scope. The Scope pushes the span onto the constructing context
* store's stack; member order (guard first, scope second) ensures
* the Scope pops BEFORE the span ends on destruction. It is
* thread-bound: construct and destroy on the same thread.
* store-bound: construct and destroy while the same LocalValue
* context store is active (the same thread, or the same JobQueue
* coroutine even if it resumes on another worker).
*
* Static factory methods access the global Telemetry instance via
* Telemetry::getInstance(), check whether the requested TraceCategory
@@ -28,6 +30,7 @@
#include <xrpl/telemetry/SpanGuard.h>
#include <xrpl/basics/LocalValue.h>
#include <xrpl/beast/utility/instrumentation.h>
#include <xrpl/telemetry/DeterministicIdGenerator.h>
#include <xrpl/telemetry/DiscardFlag.h>
@@ -59,7 +62,6 @@
#include <optional>
#include <string>
#include <string_view>
#include <thread>
#include <typeinfo>
#include <utility>
#include <vector>
@@ -588,21 +590,35 @@ struct ScopedSpanGuard::ScopedImpl
SpanGuard guard;
/**
* Active OTel Scope binding the span to this thread's context stack.
* Present while the guard is active; reset() (via operator SpanGuard)
* pops it eagerly on the origin thread. Declared AFTER `guard` so
* destruction order pops the scope before the span ends.
* Active OTel Scope binding the span to the active context store's
* stack. Present while the guard is active; reset() (via operator
* SpanGuard) pops it eagerly under the constructing store. Declared
* AFTER `guard` so destruction order pops the scope before the span
* ends.
*/
std::optional<otel_trace::Scope> scope;
/**
* Thread that constructed this ScopedImpl. The Scope is bound to
* this thread's context stack, so popping it (via ~Scope or reset())
* on a different thread corrupts that thread's stack. Checked in the
* destructor and operator SpanGuard() to turn a silent cross-thread
* corruption into an assertion failure in debug/test/fuzzing builds.
* Identity of the LocalValue store the Scope was pushed onto (the
* coroutine's store on a coro, else the thread's own store). Popping
* the Scope (via ~Scope or reset()) must happen while the SAME store
* is active, else a different stack is corrupted. Coroutine-aware
* storage lets a coro legitimately resume on another worker while
* keeping the same store, so store-identity — not thread-id — is the
* correct invariant. Checked in the destructor and operator
* SpanGuard() to turn a silent cross-store pop into an assertion
* failure in debug/test/fuzzing builds.
*
* Assigned in the constructor body AFTER the Scope push, not as a
* member initializer: the push is the first LocalValue touch on a
* fresh thread for a root or explicit-parent span (the OTel SDK skips
* the ambient-context read in those cases), so it materializes the
* store. Capturing before the push would record nullptr while the
* destructor sees the now-materialized store. For a null guard no
* Scope is pushed and the assertions short-circuit, so this holds
* whatever store is active then.
*/
std::thread::id owner{std::this_thread::get_id()};
void const* owner = nullptr;
/**
* Wrap a SpanGuard and activate its span on this thread. If the
@@ -614,6 +630,9 @@ struct ScopedSpanGuard::ScopedImpl
{
if (guard)
scope.emplace(guard.impl_->span);
// Capture after the push so `owner` names the store the Scope
// actually lives on, even when the push materialized it.
owner = detail::getLocalValues().get();
}
};
@@ -626,13 +645,14 @@ ScopedSpanGuard::ScopedSpanGuard(SpanGuard&& guard)
ScopedSpanGuard::~ScopedSpanGuard()
{
// A live Scope must be popped on the thread that pushed it; destroying
// the guard on any other thread corrupts that thread's context stack.
// A null guard holds no Scope, so the check is skipped.
// A live Scope must be popped while its constructing context store is
// active; destroying the guard under a different store corrupts that
// store's context stack. A null guard holds no Scope, so the check is
// skipped.
XRPL_ASSERT(
!impl_ || !impl_->scope.has_value() || impl_->owner == std::this_thread::get_id(),
"xrpl::telemetry::ScopedSpanGuard::~ScopedSpanGuard : destroyed on "
"constructing thread");
!impl_ || !impl_->scope.has_value() || impl_->owner == detail::getLocalValues().get(),
"xrpl::telemetry::ScopedSpanGuard::~ScopedSpanGuard : destroyed on the "
"constructing context store");
}
// ===== ScopedSpanGuard factory methods =====================================
@@ -677,13 +697,14 @@ ScopedSpanGuard::linkedSpan(std::string_view name, SpanContext const& linkCtx)
ScopedSpanGuard::
operator SpanGuard() &&
{
// The scope must be popped on the thread that pushed it; handing off
// elsewhere would pop the wrong stack.
// The scope must be popped while its constructing context store is
// active; handing off under a different store would pop the wrong stack.
// A null guard holds no Scope, so the check is skipped.
XRPL_ASSERT(
impl_->owner == std::this_thread::get_id(),
"xrpl::telemetry::ScopedSpanGuard::operator SpanGuard : handoff on "
"constructing thread");
impl_->scope.reset(); // eager pop, on the origin thread
!impl_->scope.has_value() || impl_->owner == detail::getLocalValues().get(),
"xrpl::telemetry::ScopedSpanGuard::operator SpanGuard : handoff on the "
"constructing context store");
impl_->scope.reset(); // eager pop, on the origin store
return std::move(impl_->guard);
}
@@ -754,14 +775,15 @@ ScopedSpanGuard::recordException(std::exception const& e)
void
ScopedSpanGuard::discard()
{
// A live Scope must be popped on the thread that pushed it; discarding
// on any other thread corrupts that thread's context stack. A null guard
// holds no Scope, so the check is skipped.
// A live Scope must be popped while its constructing context store is
// active; discarding under a different store corrupts that store's
// context stack. A null guard holds no Scope, so the check is skipped.
XRPL_ASSERT(
!impl_ || !impl_->scope.has_value() || impl_->owner == std::this_thread::get_id(),
"xrpl::telemetry::ScopedSpanGuard::discard : discarded on constructing thread");
// Pop the scope first (on this thread) so no span stays active on the
// stack, then discard the span via the owned guard.
!impl_ || !impl_->scope.has_value() || impl_->owner == detail::getLocalValues().get(),
"xrpl::telemetry::ScopedSpanGuard::discard : discarded on the "
"constructing context store");
// Pop the scope first (under the constructing store) so no span stays
// active on the stack, then discard the span via the owned guard.
impl_->scope.reset();
impl_->guard.discard();
}
@@ -772,6 +794,64 @@ operator bool() const
return impl_ && static_cast<bool>(impl_->guard);
}
// ===== ScopedActivation ====================================================
struct ScopedActivation::Impl
{
/**
* Active OTel Scope binding an externally-owned span to the current
* context store. Popped on destruction; never ends the span.
*/
otel_trace::Scope scope;
/**
* Identity of the LocalValue store the Scope was pushed onto. The Scope
* must pop while the SAME store is active (see
* ScopedSpanGuard::ScopedImpl::owner). Initialized AFTER `scope` in the
* member init list: members initialize in declaration order, so the
* Scope's push (the first LocalValue touch on a fresh worker, which
* materializes the store) runs first, then this captures the
* now-materialized store. Capturing before the push would record
* nullptr while the destructor sees the materialized store.
*/
void const* owner;
/**
* Push an externally-owned span onto the current context store.
* @param span The already-owned span to activate (not owned here).
*/
explicit Impl(opentelemetry::nostd::shared_ptr<otel_trace::Span> const& span)
: scope(span), owner(detail::getLocalValues().get())
{
}
};
ScopedActivation::ScopedActivation() = default;
ScopedActivation::ScopedActivation(std::unique_ptr<Impl> impl) : impl_(std::move(impl))
{
}
ScopedActivation::~ScopedActivation()
{
// A live Scope must be popped while its constructing context store is
// active; destroying the activation under a different store corrupts
// that store's context stack. A null activation holds no Scope, so the
// check is skipped.
XRPL_ASSERT(
!impl_ || impl_->owner == detail::getLocalValues().get(),
"xrpl::telemetry::ScopedActivation::~ScopedActivation : destroyed on the "
"constructing context store");
}
ScopedActivation
SpanGuard::activate() const
{
if (!impl_ || !impl_->span)
return {};
return ScopedActivation(std::make_unique<ScopedActivation::Impl>(impl_->span));
}
} // namespace xrpl::telemetry
#endif // XRPL_ENABLE_TELEMETRY

21
src/libxrpl/telemetry/Telemetry.cpp
View File

@@ -20,11 +20,13 @@
#include <xrpl/basics/Log.h>
#include <xrpl/beast/utility/Journal.h>
#include <xrpl/telemetry/CoroAwareContextStorage.h>
#include <xrpl/telemetry/DeterministicIdGenerator.h>
#include <xrpl/telemetry/DiscardFlag.h>
#include <xrpl/telemetry/SpanNames.h>
#include <opentelemetry/context/context.h>
#include <opentelemetry/context/runtime_context.h>
#include <opentelemetry/exporters/otlp/otlp_http_exporter_factory.h>
#include <opentelemetry/exporters/otlp/otlp_http_exporter_options.h>
#include <opentelemetry/exporters/otlp/otlp_http_metric_exporter_factory.h>
@@ -308,6 +310,13 @@ class TelemetryImpl : public Telemetry
*/
std::shared_ptr<metrics_sdk::MeterProvider> meterProvider_;
/**
* Coroutine-aware runtime-context storage, installed globally so the OTel
* ambient context follows JobQueue coroutines. Held for the process
* lifetime because it must outlive every span (SDK requirement).
*/
opentelemetry::nostd::shared_ptr<opentelemetry::context::RuntimeContextStorage> contextStorage_;
public:
TelemetryImpl(Setup setup, beast::Journal journal) : setup_(std::move(setup)), journal_(journal)
{
@@ -397,6 +406,18 @@ public:
std::move(sampler),
std::make_unique<DeterministicIdGenerator>());
// Install coroutine-aware context storage BEFORE any span is created
// so the OTel ambient context follows JobQueue coroutines across
// yield/resume (fixes wrong-thread scope pop; keeps log-trace
// correlation). Must precede SetTracerProvider and the first span.
// Not reset in stop(): resetting the storage while spans may still
// exist is undefined behaviour (SDK), and by stop() all spans are
// gone, so the storage is simply left installed for process lifetime.
contextStorage_ =
opentelemetry::nostd::shared_ptr<opentelemetry::context::RuntimeContextStorage>(
new CoroAwareContextStorage());
opentelemetry::context::RuntimeContext::SetRuntimeContextStorage(contextStorage_);
// Set as global provider
trace_api::Provider::SetTracerProvider(
opentelemetry::nostd::shared_ptr<trace_api::TracerProvider>(sdkProvider_));

184
src/tests/libxrpl/telemetry/SpanGuardScope.cpp
View File

@@ -6,11 +6,12 @@
// 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.
// - ScopedSpanGuard is scoped and store-bound: it also pushes an OTel Scope
// so the span is the ambient parent on the constructing context store (the
// thread's own store, or a coroutine's store). Its `operator SpanGuard() &&`
// pops that Scope eagerly on the origin store and yields a thread-free
// SpanGuard; destroying it while a different store is active trips an
// owner-store 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.
@@ -21,6 +22,8 @@
#ifdef XRPL_ENABLE_TELEMETRY
#include <xrpl/basics/LocalValue.h>
#include <xrpl/telemetry/CoroAwareContextStorage.h>
#include <xrpl/telemetry/DeterministicIdGenerator.h>
#include <xrpl/telemetry/SpanGuard.h>
#include <xrpl/telemetry/Telemetry.h>
@@ -279,6 +282,17 @@ makeTraceIdBytes()
/**
* Installs a TestTelemetry as the global instance for each test and clears it
* afterwards so the singleton never dangles between cases.
*
* The fixture also installs a CoroAwareContextStorage as the process-global
* OTel runtime-context storage. That storage backs the ambient-context stack
* with an xrpl::LocalValue, which is what makes a ScopedSpanGuard's Scope live
* in the ACTIVE store (thread or coroutine) rather than a plain thread_local.
* The store-swap and activate() tests depend on this to observe the ambient
* context move with the store.
*
* @note SetRuntimeContextStorage is process-global. Re-installing it in every
* SetUp is idempotent (last writer wins) and safe because GTests run serially;
* a fresh storage per test keeps the coro/thread context stacks isolated.
*/
class SpanGuardScopeTest : public ::testing::Test
{
@@ -288,6 +302,13 @@ protected:
{
telemetry_ = std::make_unique<TestTelemetry>();
Telemetry::setInstance(telemetry_.get());
// Back the OTel ambient context with an xrpl::LocalValue store so a
// scope follows the active store, not a bare thread_local. Installed
// before any span is created in the test body (SDK requirement).
storage_ = opentelemetry::nostd::shared_ptr<opentelemetry::context::RuntimeContextStorage>(
new CoroAwareContextStorage());
opentelemetry::context::RuntimeContext::SetRuntimeContextStorage(storage_);
}
void
@@ -310,6 +331,13 @@ protected:
* The in-memory Telemetry installed for the duration of a test.
*/
std::unique_ptr<TestTelemetry> telemetry_;
/**
* The coro-aware runtime-context storage installed for the test. Kept
* alive by the fixture so the process-global storage pointer stays valid
* for the test body; replaced each SetUp (see class note).
*/
opentelemetry::nostd::shared_ptr<opentelemetry::context::RuntimeContextStorage> storage_;
};
// freshRoot() must ignore the ambient active span and start a brand-new trace.
@@ -445,6 +473,115 @@ TEST_F(SpanGuardScopeTest, scopedGuard_conversion_result_ends_span_once)
EXPECT_EQ(countSpans(spans, "ledger.build"), 1u);
}
// A ScopedSpanGuard's scope lives in the ACTIVE LocalValue store, and the
// coro-aware storage makes the ambient context follow that store. This
// simulates a coroutine that yields (its store swapped out for the worker's
// own) then resumes on another worker (store swapped back in): the span must be
// hidden while off-store, visible again on resume, and pop cleanly under the
// same store it was pushed on -- so the owner-store assertion never trips.
//
// Fails without CoroAwareContextStorage: OpenTelemetry's default thread-local
// stack ignores the LocalValue swap, so the span would stay visible off-store
// (the EXPECT_NE below would fail). Passes only because the fixture installs the
// coro-aware storage that binds the ambient stack to the active store.
TEST_F(SpanGuardScopeTest, scopedGuard_survives_localvalue_store_swap)
{
namespace ctx = opentelemetry::context;
namespace trc = opentelemetry::trace;
// Stand-in stores for the coroutine and a worker thread's own store. Both
// are onCoro=true so LocalValues::cleanup() never deletes these stack
// objects. Keeping a stack store active across every GetCurrent() call below
// stops LocalValue from materializing (then leaking) a heap thread-store.
xrpl::detail::LocalValues coroStore;
xrpl::detail::LocalValues workerStore;
// Detach (do NOT delete) the fixture's active store, run on the coro store,
// and remember the original so teardown gets it back.
auto* saved = xrpl::detail::getLocalValues().release();
xrpl::detail::getLocalValues().reset(&coroStore);
trc::SpanContext captured = trc::SpanContext::GetInvalid();
{
ScopedSpanGuard const span(TraceCategory::Rpc, "rpc", "process");
ASSERT_TRUE(static_cast<bool>(span));
auto active = trc::GetSpan(ctx::RuntimeContext::GetCurrent())->GetContext();
ASSERT_TRUE(active.IsValid());
captured = active;
// Yield: swap the coro store OUT, the worker's own store IN.
xrpl::detail::getLocalValues().release();
xrpl::detail::getLocalValues().reset(&workerStore);
auto offCoro = trc::GetSpan(ctx::RuntimeContext::GetCurrent())->GetContext();
EXPECT_FALSE(offCoro.IsValid()); // no ambient span off-coro
EXPECT_NE(offCoro.span_id(), captured.span_id()); // span NOT visible
// Resume on another worker: swap the coro store back IN.
xrpl::detail::getLocalValues().release();
xrpl::detail::getLocalValues().reset(&coroStore);
auto resumed = trc::GetSpan(ctx::RuntimeContext::GetCurrent())->GetContext();
EXPECT_TRUE(resumed.IsValid()); // ambient again
EXPECT_EQ(resumed.span_id(), captured.span_id()); // same span visible
} // ~ScopedSpanGuard pops from coroStore (its owner store active) -- no assert
// The scope popped cleanly: the coro store's stack is empty again.
auto afterPop = trc::GetSpan(ctx::RuntimeContext::GetCurrent());
EXPECT_FALSE(afterPop->GetContext().IsValid());
// Restore (re-own) the fixture's store for teardown before any stack store
// leaves scope, so the thread pointer never dangles.
xrpl::detail::getLocalValues().release();
xrpl::detail::getLocalValues().reset(saved);
// The span ended exactly once, when the scope popped on resume.
EXPECT_EQ(countSpans(spanData()->GetSpans(), "rpc.process"), 1u);
}
// SpanGuard::activate() makes an already-owned span the ambient context for the
// activation's lifetime WITHOUT owning or ending it: before activate() the span
// is not ambient, during it the span is the current context, and after it the
// prior (empty) context is restored. ~ScopedActivation must NOT end the span --
// the owning guard ends it exactly once. Identity is proved by matching the
// ambient span_id seen during activation to the single exported span.
TEST_F(SpanGuardScopeTest, activate_sets_ambient_without_owning)
{
namespace ctx = opentelemetry::context;
namespace trc = opentelemetry::trace;
auto guard = SpanGuard::freshRoot(TraceCategory::Transactions, "tx", "process");
ASSERT_TRUE(static_cast<bool>(guard));
// Unscoped: the guard's span is NOT ambient before activate().
EXPECT_FALSE(trc::GetSpan(ctx::RuntimeContext::GetCurrent())->GetContext().IsValid());
trc::SpanId activeId;
{
auto activation = guard.activate();
auto active = trc::GetSpan(ctx::RuntimeContext::GetCurrent())->GetContext();
EXPECT_TRUE(active.IsValid()); // now ambient
activeId = active.span_id();
} // ~ScopedActivation pops the scope; it does NOT end the span.
// A valid ambient span was observed while the activation was live.
EXPECT_TRUE(activeId.IsValid());
// Prior context restored: no ambient span after the activation drops.
EXPECT_FALSE(trc::GetSpan(ctx::RuntimeContext::GetCurrent())->GetContext().IsValid());
// Non-owning: the activation did NOT end the span, so nothing is exported.
EXPECT_EQ(countSpans(spanData()->GetSpans(), "tx.process"), 0u);
// Ending the OWNING guard exports the span exactly once.
guard = SpanGuard{};
auto spans = spanData()->GetSpans();
EXPECT_EQ(countSpans(spans, "tx.process"), 1u);
// The span made ambient during activation WAS the guard's own span.
auto* txSpan = findSpan(spans, "tx.process");
ASSERT_NE(txSpan, nullptr);
EXPECT_EQ(txSpan->GetSpanId(), activeId);
}
// 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)
@@ -541,15 +678,16 @@ TEST_F(SpanGuardScopeTest, deterministicIdGenerator_ambient_child_ignores_pendin
EXPECT_FALSE(root->GetParentSpanId().IsValid());
}
// Death test guarding the cross-thread scope-leak bug, now on ScopedSpanGuard.
// Death test guarding the cross-store scope-leak bug on ScopedSpanGuard.
//
// 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().
// A ScopedSpanGuard's Scope is bound to the LocalValue context store that was
// active when it was constructed, so it must be destroyed while that same store
// is active. A different thread has a different LocalValue store, so destroying
// the guard on another thread pops the Scope against a foreign context stack --
// ~ScopedSpanGuard's owner-store XRPL_ASSERT turns that silent corruption into a
// loud abort(). 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, whose store differs from this thread's.
//
// 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()
@@ -558,38 +696,40 @@ TEST_F(SpanGuardScopeTest, deterministicIdGenerator_ambient_child_ignores_pendin
// 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".
// between "the" 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, scopedGuard_cross_thread_death_asserts_at_wrong_thread_destroy)
TEST_F(SpanGuardScopeTest, scopedGuard_cross_thread_death_asserts_at_wrong_store_destroy)
{
#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.";
"cross-store scope-leak assertion this test exercises does not fire.";
#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.";
"the cross-store scope-leak assertion this test exercises does not crash.";
#else
EXPECT_DEATH(
{
// Scoped guard constructed on THIS thread; its Scope binds here.
// Scoped guard constructed on THIS thread; its Scope binds to this
// thread's LocalValue store.
auto scoped =
std::make_unique<ScopedSpanGuard>(TraceCategory::Ledger, "ledger", "build");
// 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().
// the worker when the lambda's captures are destroyed; the worker's
// store differs from the constructing store, tripping the owner-store
// 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; assert() stringifies the expression source via the
// preprocessor '#' operator, keeping both quoted literals with the
// "\" \"" gap between "on" and "constructing". Match across that gap.
".*destroyed on.*constructing thread.*");
// "\" \"" gap between "the" and "constructing". Match across that gap.
".*destroyed on.*constructing context store.*");
#endif
}

7
src/xrpld/app/consensus/RCLConsensus.cpp
View File

@@ -558,6 +558,13 @@ RCLConsensus::Adaptor::doAccept(
{
namespace cs = telemetry::consensus::span;
// Make the accept span ambient for the whole accept so doAccept's log lines
// (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{};
prevProposers_ = result.proposers;
prevRoundTime_ = result.roundTime.read();

46
src/xrpld/app/ledger/detail/InboundLedger.cpp
View File

@@ -453,25 +453,37 @@ InboundLedger::done()
touch();
// Finalize the acquire span with the outcome, timeout count, and peer
// count, then end it (reset) so its duration is exported.
if (acquireSpan_ && *acquireSpan_)
// count. Keep it active as the ambient context across the finalize and
// the outcome log below so that log line carries the span's trace_id.
// The activation is non-owning; acquireSpan_ still owns the span. The
// activation pops at the end of this block (restoring the prior context)
// while acquireSpan_ is still alive, then reset() ends the span.
{
using namespace telemetry;
acquireSpan_->setAttribute(
ledger_span::attr::outcome,
failed_ ? std::string_view(ledger_span::val::failed)
: std::string_view(ledger_span::val::complete));
acquireSpan_->setAttribute(ledger_span::attr::timeouts, static_cast<int64_t>(timeouts_));
acquireSpan_->setAttribute(
ledger_span::attr::peerCount, static_cast<int64_t>(getPeerCount()));
}
acquireSpan_.reset();
auto acquireActivation = (acquireSpan_ && *acquireSpan_) ? acquireSpan_->activate()
: telemetry::ScopedActivation{};
if (acquireSpan_ && *acquireSpan_)
{
using namespace telemetry;
acquireSpan_->setAttribute(
ledger_span::attr::outcome,
failed_ ? std::string_view(ledger_span::val::failed)
: std::string_view(ledger_span::val::complete));
acquireSpan_->setAttribute(
ledger_span::attr::timeouts, static_cast<int64_t>(timeouts_));
acquireSpan_->setAttribute(
ledger_span::attr::peerCount, static_cast<int64_t>(getPeerCount()));
}
JLOG(journal_.debug()) << "Acquire " << hash_ << (failed_ ? " fail " : " ")
<< ((timeouts_ == 0)
? std::string()
: (std::string("timeouts:") + std::to_string(timeouts_) + " "))
<< stats_.get();
JLOG(journal_.debug()) << "Acquire " << hash_ << (failed_ ? " fail " : " ")
<< ((timeouts_ == 0) ? std::string()
: (std::string("timeouts:") +
std::to_string(timeouts_) + " "))
<< stats_.get();
// acquireActivation pops here, before the span is ended below.
}
// End the acquire span after its outcome log. Unconditional so the span
// never leaks even when it was inactive.
acquireSpan_.reset();
XRPL_ASSERT(complete_ || failed_, "xrpl::InboundLedger::done : complete or failed");

8
src/xrpld/app/misc/NetworkOPs.cpp
View File

@@ -1644,6 +1644,14 @@ NetworkOPsImp::apply(std::unique_lock<std::mutex>& batchLock)
auto newOL = registry_.get().getOpenLedger().current();
for (TransactionStatus const& e : transactions)
{
// Make this transaction's span ambient for the duration of its
// apply so the per-tx log lines below carry its trace_id.
// 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{};
if (e.span && *e.span)
{
e.span->setAttribute(

4
src/xrpld/overlay/detail/PeerImp.cpp
View File

@@ -1435,6 +1435,10 @@ PeerImp::handleTransaction(
batch,
stx,
sp = std::move(span)]() {
// 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{};
if (auto peer = weak.lock())
peer->checkTransaction(flags, checkSignature, stx, batch);
});

7
src/xrpld/rpc/detail/RPCHandler.cpp
View File

@@ -162,7 +162,10 @@ template <class Object, class Method>
Status
callMethod(JsonContext& context, Method method, std::string const& name, Object& result)
{
auto span = SpanGuard::span(TraceCategory::Rpc, rpc_span::prefix::command, name);
// Scoped so this command nests under rpc.process and becomes the ambient
// parent of any command-internal spans (e.g. pathfind.request). Coro-aware
// storage keeps the scope correct across doRipplePathFind's yield.
auto span = ScopedSpanGuard(TraceCategory::Rpc, rpc_span::prefix::command, name);
span.setAttribute(rpc_span::attr::command, name.c_str());
span.setAttribute(rpc_span::attr::version, static_cast<int64_t>(context.apiVersion));
span.setAttribute(
@@ -257,7 +260,7 @@ doCommand(RPC::JsonContext& context, json::Value& result)
// registered handler names (plus "unknown") — see the helper for why
// raw request input must not reach the telemetry pipeline.
auto const cmdName = resolveCommandSpanName(context);
auto span = SpanGuard::span(TraceCategory::Rpc, rpc_span::prefix::command, cmdName);
auto span = ScopedSpanGuard(TraceCategory::Rpc, rpc_span::prefix::command, cmdName);
span.setAttribute(rpc_span::attr::command, cmdName);
span.setAttribute(rpc_span::attr::rpcStatus, rpc_span::val::error);
span.setError(getErrorInfo(error).token.cStr());

5
src/xrpld/rpc/detail/ServerHandler.cpp
View File

@@ -665,7 +665,10 @@ ServerHandler::processRequest(
std::string_view forwardedFor,
std::string_view user)
{
// Scoped child: nests under the httpRequest span active on this thread.
// Scoped child of rpc.http_request. Safe to hold across the coroutine
// yield in doRipplePathFind: the coro-aware context storage moves this
// scope with the coroutine on resume (it is never stranded on a worker's
// thread-local stack), so nesting and log-trace correlation both hold.
auto span = ScopedSpanGuard(TraceCategory::Rpc, rpc_span::prefix::rpc, rpc_span::op::process);
auto rpcJ = app_.getJournal("RPC");

16
src/xrpld/rpc/handlers/orderbook/RipplePathFind.cpp
View File

@@ -27,16 +27,12 @@ json::Value
doRipplePathFind(RPC::JsonContext& context)
{
using namespace telemetry;
// 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.
// - 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(
// pathfind.request nests under rpc.command. This scope is held across
// context.coro->yield() below; the coro-aware OTel context storage moves it
// with the coroutine on resume (it is never stranded on a worker's
// thread-local stack), so the scope pops on the correct store and the
// span's log lines stay trace-correlated.
auto span = ScopedSpanGuard(
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())