diff --git a/src/xrpld/app/consensus/RCLConsensus.cpp b/src/xrpld/app/consensus/RCLConsensus.cpp index f301c4608f..b10e709b67 100644 --- a/src/xrpld/app/consensus/RCLConsensus.cpp +++ b/src/xrpld/app/consensus/RCLConsensus.cpp @@ -230,8 +230,8 @@ RCLConsensus::Adaptor::share(RCLCxTx const& tx) void RCLConsensus::Adaptor::propose(RCLCxPeerPos::Proposal const& proposal) { - // Child of the round span via its captured context (roundSpan_ is detached, - // so it is no longer the thread's ambient parent). + // Child of the round span via its captured context (roundSpan_ is a + // thread-free SpanGuard, so parent explicitly via its context). auto span = telemetry::SpanGuard::childSpan( telemetry::consensus::span::proposalSend, roundSpanContext_); span.setAttribute( @@ -347,8 +347,8 @@ RCLConsensus::Adaptor::onClose( { namespace cs = telemetry::consensus::span; - // Child of the round span via its captured context (roundSpan_ is detached, - // so it is no longer the thread's ambient parent). + // Child of the round span via its captured context (roundSpan_ is a + // thread-free SpanGuard, so parent explicitly via its context). auto span = telemetry::SpanGuard::childSpan(cs::ledgerClose, roundSpanContext_); span.setAttribute(cs::attr::ledgerSeq, static_cast(ledger.ledger->header().seq) + 1); span.setAttribute(cs::attr::mode, toDisplayString(mode).c_str()); @@ -534,14 +534,12 @@ RCLConsensus::Adaptor::makeAcceptSpan(Result const& result) // "validation follows acceptance" causal model). if (*span) { - acceptSpanContext_ = span->captureContext(); - // The accept span is moved into the JtAccept worker (onAccept) and - // ended there. Detach its Scope now, on this thread, AFTER the capture - // above, so it is not popped on the wrong thread. accept.apply parents - // via acceptSpanContext_ (captured above), so no ambient child is - // orphaned on either the sync (onForceAccept) or async (onAccept) path. - // detachInPlace rebuilds the shared_ptr (move-assign is deleted). - span = telemetry::detachInPlace(std::move(span)); + // span is a thread-free SpanGuard handed to the JtAccept worker + // (onAccept), which ends it there. spanContext() captures the guard's + // own span, so accept.apply parents via acceptSpanContext_ regardless + // of which thread runs the sync (onForceAccept) or async (onAccept) + // path -- no scope work is needed. + acceptSpanContext_ = span->spanContext(); } return span; } @@ -584,11 +582,10 @@ RCLConsensus::Adaptor::doAccept( closeTimeCorrect = true; } - // Parent accept.apply via the captured accept context (acceptSpanContext_) - // rather than the accept guard's ambient Scope: the accept span is detached - // (its Scope no longer sits on this thread), 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. + // 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_); @@ -1075,9 +1072,10 @@ RCLConsensus::Adaptor::onModeChange(ConsensusMode before, ConsensusMode after) { namespace cs = telemetry::consensus::span; - // Child of the round span via its captured context (roundSpan_ is detached, - // so it is no longer the thread's ambient parent). A mode change outside a - // round leaves roundSpanContext_ invalid, yielding a null guard (no-op). + // Child of the round span via its captured context (roundSpan_ is a + // thread-free SpanGuard, so parent explicitly via its context). A mode + // change outside a round leaves roundSpanContext_ invalid, yielding a null + // guard (no-op). auto span = telemetry::SpanGuard::childSpan(cs::modeChange, roundSpanContext_); span.setAttribute(cs::attr::modeOld, toDisplayString(before).c_str()); span.setAttribute(cs::attr::modeNew, toDisplayString(after).c_str()); @@ -1323,14 +1321,11 @@ RCLConsensus::Adaptor::startRoundTracing(RCLCxLedger const& prevLgr) roundSpan_->addEvent(cs::event::phaseOpen); - roundSpanContext_ = roundSpan_->captureContext(); - - // roundSpanContext_ (captured above) is the durable handle that child - // spans on other threads link to. The guard itself is reset() on a - // different worker than it was emplaced on, so detach its Scope now -- - // AFTER the context capture -- to avoid a wrong-thread Scope pop. - // detachInPlace re-emplaces it (move-assignment is deleted). - telemetry::detachInPlace(roundSpan_); + // roundSpanContext_ is the durable handle that child spans on other + // threads parent to. roundSpan_ is a thread-free SpanGuard that is + // reset() on a different worker than it was emplaced on, so spanContext() + // captures its own span and no scope work is needed. + roundSpanContext_ = roundSpan_->spanContext(); } std::optional diff --git a/src/xrpld/consensus/Consensus.h b/src/xrpld/consensus/Consensus.h index df81c7702a..86987d9bc0 100644 --- a/src/xrpld/consensus/Consensus.h +++ b/src/xrpld/consensus/Consensus.h @@ -650,24 +650,21 @@ private: /** * Span for the establish phase of consensus. * Created when the ledger closes and we enter phaseEstablish; - * cleared (ended) when consensus is reached. Stored detached (its Scope - * is stripped right after its context is captured) because it is emplaced - * and reset on different job workers. + * cleared (ended) when consensus is reached. A thread-free SpanGuard, + * emplaced and reset() on different job workers. */ std::optional establishSpan_; /** - * Captured context of establishSpan_, snapshotted while it was still - * scoped. Same-thread children (update_positions, check) build from this - * explicit context instead of the (now detached) ambient establish span. + * Captured context of establishSpan_ (its own span context). Children + * (update_positions, check) build from this explicit context. */ xrpl::telemetry::SpanContext establishSpanContext_; /** * Span for the open phase of consensus. * Created in startRoundInternal(); cleared (ended) in closeLedger(). - * Stored detached: emplaced and reset on different job workers; - * detached() prevents wrong-thread Scope pop. + * A thread-free SpanGuard, emplaced and reset() on different job workers. */ std::optional openSpan_; @@ -779,17 +776,14 @@ Consensus::startRoundInternal( // early-returns when establishSpan_ is populated). establishSpan_.reset(); establishSpanContext_ = telemetry::SpanContext{}; - // Child of the round span via its captured context: the round span is - // detached (no longer the thread's ambient parent), so parent phase.open - // explicitly under roundSpanContext_ rather than the ambient stack. An - // invalid round context (round span not yet created) yields a null guard. - // Detached in turn: emplaced here on one job worker and reset() on - // another, so strip the thread-local Scope to avoid a wrong-thread pop. - // No same-thread child span nests under openSpan_. + // Child of the round span via its captured context: parent phase.open + // explicitly under roundSpanContext_. An invalid round context (round span + // not yet created) yields a null guard. openSpan_ is a thread-free + // SpanGuard emplaced here on one job worker and reset() on another; no + // scope to strip. openSpan_.emplace( telemetry::SpanGuard::childSpan( - telemetry::consensus::span::phaseOpen, adaptor_.roundSpanContext()) - .detached()); + telemetry::consensus::span::phaseOpen, adaptor_.roundSpanContext())); // On the Recovered path, fire phase.open here because startRoundTracing // (which fires it for the Initial path) is not called on re-entry. On // the Initial path this is a no-op because the round span hasn't been @@ -1631,8 +1625,8 @@ Consensus::updateOurPositions(std::unique_ptr const& // NOLINTBEGIN(bugprone-unchecked-optional-access) assert above using namespace telemetry; // Child of the establish span via its captured context (establishSpan_ is - // detached, so it is no longer the thread's ambient parent). Null context - // (establish not started) yields a null guard, same as before. + // a thread-free SpanGuard, so parent explicitly via its context). Null + // context (establish not started) yields a null guard, same as before. auto span = SpanGuard::childSpan(consensus::span::updatePositions, establishSpanContext_); span.setAttribute( consensus::span::attr::convergePercent, static_cast(convergePercent_)); @@ -1841,7 +1835,7 @@ Consensus::haveConsensus(std::unique_ptr const& clog // NOLINTBEGIN(bugprone-unchecked-optional-access) assert above using namespace telemetry; // Child of the establish span via its captured context (establishSpan_ is - // detached, so it is no longer the thread's ambient parent). + // a thread-free SpanGuard, so parent explicitly via its context). auto span = SpanGuard::childSpan(consensus::span::check, establishSpanContext_); // CHECKME: should possibly count unacquired TX sets as disagreeing @@ -2101,22 +2095,19 @@ Consensus::startEstablishTracing() { if (establishSpan_) return; - // Child of the round span via its captured context: the round span is - // detached (no longer the thread's ambient parent), so parent establish + // Child of the round span via its captured context: parent establish // explicitly under roundSpanContext_. An invalid round context (round span // not yet created) yields a null guard. establishSpan_.emplace( telemetry::SpanGuard::childSpan( telemetry::consensus::span::establish, adaptor_.roundSpanContext())); - // Capture the establish context while the guard is still scoped, then - // detach. Same-thread children (update_positions, check) link to this - // captured context explicitly instead of relying on establishSpan_ being - // the ambient parent -- the guard is reset() on a different worker than it - // is emplaced on, so it must not keep a thread-local Scope. + // Capture the establish context; children (update_positions, check) parent + // to it explicitly via establishSpanContext_. establishSpan_ is a + // thread-free SpanGuard reset() on a different worker than it is emplaced + // on -- no scope work. if (*establishSpan_) { - establishSpanContext_ = establishSpan_->captureContext(); - telemetry::detachInPlace(establishSpan_); + establishSpanContext_ = establishSpan_->spanContext(); } } diff --git a/src/xrpld/overlay/detail/PeerImp.cpp b/src/xrpld/overlay/detail/PeerImp.cpp index ed64edb30d..699f8651be 100644 --- a/src/xrpld/overlay/detail/PeerImp.cpp +++ b/src/xrpld/overlay/detail/PeerImp.cpp @@ -1849,10 +1849,9 @@ PeerImp::onMessage(std::shared_ptr const& m) // Create a receive span that links to the sender's trace context // (if propagated). shared_ptr keeps it alive across the job boundary. - // Detach the guard's Scope on this peer thread so it is not popped on the - // job worker thread (which would leak this thread's context stack). - auto span = - std::make_shared(telemetry::proposalReceiveSpan(set).detached()); + // The receive span is a thread-free SpanGuard handed to the job worker; + // no scope to strip. + auto span = std::make_shared(telemetry::proposalReceiveSpan(set)); span->setAttribute(telemetry::consensus::span::attr::proposalTrusted, isTrusted); span->setAttribute( telemetry::consensus::span::attr::round, static_cast(set.proposeseq())); @@ -2447,10 +2446,9 @@ PeerImp::onMessage(std::shared_ptr const& m) // Create a receive span that links to the sender's trace context // (if propagated). shared_ptr keeps it alive across the job boundary. - // Detach the guard's Scope on this peer thread so it is not popped on - // the job worker thread (which would leak this thread's context stack). - auto span = - std::make_shared(telemetry::validationReceiveSpan(*m).detached()); + // The receive span is a thread-free SpanGuard handed to the job worker; + // no scope to strip. + auto span = std::make_shared(telemetry::validationReceiveSpan(*m)); span->setAttribute(telemetry::consensus::span::attr::validationTrusted, isTrusted); if (val->isFieldPresent(sfLedgerSequence)) { diff --git a/src/xrpld/telemetry/ConsensusReceiveTracing.h b/src/xrpld/telemetry/ConsensusReceiveTracing.h index 83e6db0774..5b7582e13b 100644 --- a/src/xrpld/telemetry/ConsensusReceiveTracing.h +++ b/src/xrpld/telemetry/ConsensusReceiveTracing.h @@ -19,7 +19,7 @@ * +--- has trace_context? ----+ * | yes | no * v v - * SpanGuard::hashSpan() with SpanGuard::rootSpan() + * SpanGuard::hashSpan() with SpanGuard::freshRoot() * extracted parent context (fresh trace root) * * When XRPL_ENABLE_TELEMETRY is not defined, the functions return @@ -85,7 +85,7 @@ proposalReceiveSpan([[maybe_unused]] protocol::TMProposeSet const& msg) } #endif // No propagated context — start a fresh trace root (not an ambient child). - return SpanGuard::rootSpan( + return SpanGuard::freshRoot( TraceCategory::Consensus, seg::consensus, consensus::span::op::proposalReceive); } @@ -123,7 +123,7 @@ validationReceiveSpan([[maybe_unused]] protocol::TMValidation const& msg) } #endif // No propagated context — start a fresh trace root (not an ambient child). - return SpanGuard::rootSpan( + return SpanGuard::freshRoot( TraceCategory::Consensus, seg::consensus, consensus::span::op::validationReceive); }