diff --git a/include/xrpl/consensus/ConsensusSpanNames.h b/include/xrpl/consensus/ConsensusSpanNames.h index e9d073b5e9..46935294e8 100644 --- a/include/xrpl/consensus/ConsensusSpanNames.h +++ b/include/xrpl/consensus/ConsensusSpanNames.h @@ -59,7 +59,9 @@ * | Attrs: proposers, round_time_ms, quorum * | | * | +-- consensus.accept.apply [jtACCEPT thread, child of accept] - * | Created: Adaptor::doAccept() + * | Created: Adaptor::doAccept(), scoped: the txq spans doAccept + * | goes on to create nest under it; the tx apply-stage + * | spans are hash-derived roots and do not * | Attrs: ledger_seq, close_time_ripple_epoch_s, close_time_correct, * | close_resolution_ms, consensus_state, proposing, round_time_ms, * | parent_close_time_ripple_epoch_s, close_time_self_ripple_epoch_s, diff --git a/src/tests/libxrpl/telemetry/SpanGuardScope.cpp b/src/tests/libxrpl/telemetry/SpanGuardScope.cpp index cb90b8e5a9..135a85c093 100644 --- a/src/tests/libxrpl/telemetry/SpanGuardScope.cpp +++ b/src/tests/libxrpl/telemetry/SpanGuardScope.cpp @@ -57,6 +57,7 @@ #include #include +#include #include #include #include @@ -688,6 +689,58 @@ TEST_F(SpanGuardScopeTest, scopedGuard_addEvent_records_name_and_attribute_value EXPECT_EQ(eventAttribute(events.front(), kTxIdKey), std::string(kTxId)); } +// A scoped child of a captured context is the ambient parent of the spans +// created after it on the same thread. A hash-derived root created inside that +// scope stays a root. consensus.accept.apply relies on both. +TEST_F(SpanGuardScopeTest, scopedChildOfCapturedContextIsAmbientForLaterSpans) +{ + namespace cs = consensus::span; + + auto const h = makeTraceIdBytes(); + { + // consensus.accept: unscoped, thread-free, context captured. + auto accept = + SpanGuard::freshRoot(TraceCategory::Consensus, seg::consensus, cs::op::accept); + ASSERT_TRUE(static_cast(accept)); + auto const acceptCtx = accept.spanContext(); + + // consensus.accept.apply: scoped child of that context. + ScopedSpanGuard const apply = ScopedSpanGuard::childSpan(cs::acceptApply, acceptCtx); + ASSERT_TRUE(static_cast(apply)); + + // ledger.build: a plain ambient scoped guard. + { + ScopedSpanGuard const build(TraceCategory::Ledger, seg::ledger, "build"); + ASSERT_TRUE(static_cast(build)); + } + + // ledger.store: hash-derived, so a deterministic root. + { + auto store = + SpanGuard::hashSpan(TraceCategory::Ledger, "ledger.store", h.data(), h.size()); + ASSERT_TRUE(static_cast(store)); + } + } + + auto spans = spanData()->GetSpans(); + auto* accept = findSpan(spans, cs::accept); + auto* apply = findSpan(spans, cs::acceptApply); + auto* build = findSpan(spans, "ledger.build"); + auto* store = findSpan(spans, "ledger.store"); + ASSERT_NE(accept, nullptr); + ASSERT_NE(apply, nullptr); + ASSERT_NE(build, nullptr); + ASSERT_NE(store, nullptr); + + EXPECT_EQ(apply->GetParentSpanId(), accept->GetSpanId()); + // build nests under apply, not beside it. + EXPECT_EQ(build->GetParentSpanId(), apply->GetSpanId()); + EXPECT_EQ(build->GetTraceId(), apply->GetTraceId()); + // The hash-derived span is a root on its own pinned trace id. + EXPECT_FALSE(store->GetParentSpanId().IsValid()); + EXPECT_TRUE(std::ranges::equal(store->GetTraceId().Id(), h)); +} + // 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) diff --git a/src/xrpld/app/consensus/RCLConsensus.cpp b/src/xrpld/app/consensus/RCLConsensus.cpp index 3763fc9316..c08839c6a0 100644 --- a/src/xrpld/app/consensus/RCLConsensus.cpp +++ b/src/xrpld/app/consensus/RCLConsensus.cpp @@ -595,10 +595,9 @@ 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. + // Make the accept span ambient until accept.apply opens below. Non-owning: + // acceptSpan still owns and ends the span. doAccept runs to completion on + // one thread, so the scope pops on the thread that pushed it. auto acceptActivation = telemetry::activateIfLive(acceptSpan); prevProposers_ = result.proposers; @@ -627,13 +626,10 @@ RCLConsensus::Adaptor::doAccept( closeTimeCorrect = true; } - // Parent accept.apply via the captured accept context (acceptSpanContext_): - // the accept span is a thread-free SpanGuard, so an explicit context is - // used for both the sync (onForceAccept) and async (onAccept) paths. Falls - // back to the round context if the accept span was null. - auto doAcceptSpan = acceptSpanContext_.isValid() - ? telemetry::SpanGuard::childSpan(cs::acceptApply, acceptSpanContext_) - : telemetry::SpanGuard::childSpan(cs::acceptApply, roundSpanContext_); + // Scoped: accept.apply is the ambient parent of every span doAccept creates + // from here on. Parented through acceptSpanContext_ because the accept span + // is a thread-free SpanGuard; the context is valid whenever that span is live. + auto doAcceptSpan = telemetry::ScopedSpanGuard::childSpan(cs::acceptApply, acceptSpanContext_); doAcceptSpan.setAttribute(cs::attr::ledgerSeq, static_cast(prevLedger.seq()) + 1); doAcceptSpan.setAttribute( cs::attr::closeTimeRippleEpochS,