mirror of
https://github.com/XRPLF/rippled.git
synced 2026-07-24 07:30:30 +00:00
fix(telemetry): detach consensus member/accept spans; reparent children via captured context
roundSpan_, establishSpan_, openSpan_ and the accept span are emplaced on one job worker and reset()/moved to another, leaking their thread-local OTel Scope. Each is now detached AFTER its context is captured. Spans that previously nested under them via the ambient thread context are re-pointed to the captured SpanContext explicitly: - round children (proposal.send, ledger_close, mode_change) -> roundSpanContext_ - establish children (update_positions, check) -> establishSpanContext_ - accept.apply -> acceptSpanContext_ onForceAccept (synchronous) keeps the accept span's behaviour; only onAccept (moved into the JtAccept job) needs the detach. Member move-assignment is deleted, so re-assignment uses emplace(). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -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<int64_t>(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<int64_t>(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<telemetry::SpanGuard>(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<int64_t>(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<telemetry::SpanGuard>
|
||||
|
||||
@@ -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<telemetry::SpanGuard> roundSpan_;
|
||||
|
||||
|
||||
@@ -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<xrpl::telemetry::SpanGuard> 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<xrpl::telemetry::SpanGuard> openSpan_;
|
||||
|
||||
@@ -741,11 +751,16 @@ Consensus<Adaptor>::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<Adaptor>::updateOurPositions(std::unique_ptr<std::stringstream> 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<int64_t>(convergePercent_));
|
||||
span.setAttribute(
|
||||
@@ -1792,8 +1809,9 @@ Consensus<Adaptor>::haveConsensus(std::unique_ptr<std::stringstream> 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<Adaptor>::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 <class Adaptor>
|
||||
@@ -2082,6 +2110,7 @@ void
|
||||
Consensus<Adaptor>::endEstablishTracing()
|
||||
{
|
||||
establishSpan_.reset();
|
||||
establishSpanContext_ = telemetry::SpanContext{};
|
||||
}
|
||||
|
||||
} // namespace xrpl
|
||||
|
||||
Reference in New Issue
Block a user