diff --git a/src/xrpld/app/consensus/RCLConsensus.cpp b/src/xrpld/app/consensus/RCLConsensus.cpp index 25abfb6d51..02563ce8c5 100644 --- a/src/xrpld/app/consensus/RCLConsensus.cpp +++ b/src/xrpld/app/consensus/RCLConsensus.cpp @@ -230,10 +230,10 @@ RCLConsensus::Adaptor::share(RCLCxTx const& tx) void RCLConsensus::Adaptor::propose(RCLCxPeerPos::Proposal const& proposal) { - auto span = telemetry::SpanGuard::span( - telemetry::TraceCategory::Consensus, - telemetry::seg::consensus, - telemetry::consensus::span::op::proposalSend); + // Child of the round span via its captured context (roundSpan_ is detached, + // so it is no longer the thread's ambient parent). + auto span = telemetry::SpanGuard::childSpan( + telemetry::consensus::span::op::proposalSend, roundSpanContext_); span.setAttribute( telemetry::consensus::span::attr::round, static_cast(proposal.proposeSeq())); span.setAttribute(telemetry::consensus::span::attr::isBowOut, proposal.isBowOut()); @@ -347,8 +347,9 @@ RCLConsensus::Adaptor::onClose( { namespace cs = telemetry::consensus::span; - auto span = telemetry::SpanGuard::span( - telemetry::TraceCategory::Consensus, telemetry::seg::consensus, cs::op::ledgerClose); + // Child of the round span via its captured context (roundSpan_ is detached, + // so it is no longer the thread's ambient parent). + auto span = telemetry::SpanGuard::childSpan(cs::op::ledgerClose, roundSpanContext_); span.setAttribute(cs::attr::ledgerSeq, static_cast(ledger.ledger->header().seq) + 1); span.setAttribute(cs::attr::mode, toDisplayString(mode).c_str()); span.setAttribute( @@ -534,6 +535,13 @@ RCLConsensus::Adaptor::makeAcceptSpan(Result const& result) 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. + // Rebuild the shared_ptr from the detached rvalue (move-assign deleted). + span = std::make_shared(std::move(*span).detached()); } return span; } @@ -576,8 +584,13 @@ RCLConsensus::Adaptor::doAccept( closeTimeCorrect = true; } - auto doAcceptSpan = acceptSpan - ? acceptSpan->childSpan(cs::acceptApply) + // 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. + auto doAcceptSpan = acceptSpanContext_.isValid() + ? telemetry::SpanGuard::childSpan(cs::acceptApply, acceptSpanContext_) : telemetry::SpanGuard::childSpan(cs::acceptApply, roundSpanContext_); doAcceptSpan.setAttribute(cs::attr::ledgerSeq, static_cast(prevLedger.seq()) + 1); doAcceptSpan.setAttribute( @@ -1066,8 +1079,10 @@ RCLConsensus::Adaptor::onModeChange(ConsensusMode before, ConsensusMode after) { namespace cs = telemetry::consensus::span; - auto span = telemetry::SpanGuard::span( - telemetry::TraceCategory::Consensus, telemetry::seg::consensus, cs::op::modeChange); + // 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). + auto span = telemetry::SpanGuard::childSpan(cs::op::modeChange, roundSpanContext_); span.setAttribute(cs::attr::modeOld, toDisplayString(before).c_str()); span.setAttribute(cs::attr::modeNew, toDisplayString(after).c_str()); @@ -1311,6 +1326,13 @@ 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. Re-emplace + // the detached guard in place (SpanGuard move-assignment is deleted). + roundSpan_.emplace(std::move(*roundSpan_).detached()); } std::optional diff --git a/src/xrpld/app/consensus/RCLConsensus.h b/src/xrpld/app/consensus/RCLConsensus.h index 37b8724abc..3cc94405b9 100644 --- a/src/xrpld/app/consensus/RCLConsensus.h +++ b/src/xrpld/app/consensus/RCLConsensus.h @@ -92,6 +92,10 @@ class RCLConsensus * round begins. When consensusTraceStrategy is "deterministic", * the trace_id is derived from previousLedger.id() so that all * validators in the same round share the same trace_id. + * + * Stored detached: its Scope is stripped right after roundSpanContext_ + * is captured, because it is emplaced and reset on different job + * workers. Child spans link via roundSpanContext_, not the ambient span. */ std::optional roundSpan_; diff --git a/src/xrpld/consensus/Consensus.h b/src/xrpld/consensus/Consensus.h index c4fbc9d559..1632b04637 100644 --- a/src/xrpld/consensus/Consensus.h +++ b/src/xrpld/consensus/Consensus.h @@ -629,12 +629,22 @@ private: /** Span for the establish phase of consensus. * Created when the ledger closes and we enter phaseEstablish; - * cleared (ended) when consensus is reached. + * 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. */ 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. + */ + 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. */ std::optional openSpan_; @@ -741,11 +751,16 @@ Consensus::startRoundInternal( // leak the prior round's span into the new one (startEstablishTracing // early-returns when establishSpan_ is populated). establishSpan_.reset(); + establishSpanContext_ = telemetry::SpanContext{}; + // Detached: emplaced here on one job worker and reset() on another, so + // strip the thread-local Scope to avoid a wrong-thread Scope pop. No + // same-thread child span nests under openSpan_, so detaching is safe. openSpan_.emplace( telemetry::SpanGuard::span( telemetry::TraceCategory::Consensus, telemetry::seg::consensus, - telemetry::consensus::span::op::phaseOpen)); + telemetry::consensus::span::op::phaseOpen) + .detached()); // 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 @@ -1584,8 +1599,10 @@ Consensus::updateOurPositions(std::unique_ptr const& XRPL_ASSERT(result_, "xrpl::Consensus::updateOurPositions : result is set"); // NOLINTBEGIN(bugprone-unchecked-optional-access) assert above using namespace telemetry; - auto span = SpanGuard::span( - TraceCategory::Consensus, seg::consensus, consensus::span::op::updatePositions); + // 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. + auto span = SpanGuard::childSpan(consensus::span::op::updatePositions, establishSpanContext_); span.setAttribute( consensus::span::attr::convergePercent, static_cast(convergePercent_)); span.setAttribute( @@ -1792,8 +1809,9 @@ Consensus::haveConsensus(std::unique_ptr const& clog XRPL_ASSERT(result_, "xrpl::Consensus::haveConsensus : has result"); // NOLINTBEGIN(bugprone-unchecked-optional-access) assert above using namespace telemetry; - auto span = - SpanGuard::span(TraceCategory::Consensus, seg::consensus, consensus::span::op::check); + // Child of the establish span via its captured context (establishSpan_ is + // detached, so it is no longer the thread's ambient parent). + auto span = SpanGuard::childSpan(consensus::span::op::check, establishSpanContext_); // CHECKME: should possibly count unacquired TX sets as disagreeing int agree = 0, disagree = 0; @@ -2057,6 +2075,16 @@ Consensus::startEstablishTracing() telemetry::TraceCategory::Consensus, telemetry::seg::consensus, telemetry::consensus::span::op::establish)); + // 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. + if (*establishSpan_) + { + establishSpanContext_ = establishSpan_->captureContext(); + establishSpan_.emplace(std::move(*establishSpan_).detached()); + } } template @@ -2082,6 +2110,7 @@ void Consensus::endEstablishTracing() { establishSpan_.reset(); + establishSpanContext_ = telemetry::SpanContext{}; } } // namespace xrpl