diff --git a/include/xrpl/telemetry/SpanGuard.h b/include/xrpl/telemetry/SpanGuard.h index 3e53b2b3cc..78031187e0 100644 --- a/include/xrpl/telemetry/SpanGuard.h +++ b/include/xrpl/telemetry/SpanGuard.h @@ -170,6 +170,10 @@ #include #include +namespace protocol { +class TraceContext; +} // namespace protocol + namespace xrpl::telemetry { /** @@ -436,6 +440,20 @@ public: [[nodiscard]] TraceBytes getTraceBytes() const; + /** + * Inject the calling thread's currently-active OTel context into a + * protobuf TraceContext message for cross-node propagation. + * + * Encapsulates `RuntimeContext::GetCurrent()` + `injectToProtobuf` + * so callers in app-layer code (e.g. RCLConsensus broadcasting + * TMProposeSet / TMValidation) don't depend on any OTel headers. + * No-op if telemetry is disabled or no span is active. + * + * @param proto The protobuf TraceContext to populate. + */ + static void + injectCurrentContextToProtobuf(protocol::TraceContext& proto); + // --- Attribute setters (explicit overloads, no OTel types) --------- /** @@ -613,6 +631,11 @@ public: { return {}; } + + static void + injectCurrentContextToProtobuf(protocol::TraceContext&) + { + } // NOLINTEND(readability-convert-member-functions-to-static) void diff --git a/src/test/csf/Peer.h b/src/test/csf/Peer.h index 74ff076016..dba528886c 100644 --- a/src/test/csf/Peer.h +++ b/src/test/csf/Peer.h @@ -28,6 +28,7 @@ #include #include #include +#include #include #include @@ -704,13 +705,23 @@ struct Peer { } + // The generic engine parents its phase spans under the round span via + // this context; the simulator runs without telemetry, so return an + // invalid context and no phase span is created. + static telemetry::SpanContext + roundSpanContext() + { + return {}; + } + #ifdef XRPL_ENABLE_TELEMETRY - /** Provide telemetry access for the Consensus template. + /** + * Provide telemetry access for the Consensus template. * - * The test Peer adaptor uses a static disabled NullTelemetry instance - * so that all shouldTrace*() checks return false and no spans are - * created during simulation tests. It is static because the shared - * disabled instance does not depend on any per-peer state. + * The test Peer adaptor uses a static disabled NullTelemetry instance + * so that all shouldTrace*() checks return false and no spans are + * created during simulation tests. It is static because the shared + * disabled instance does not depend on any per-peer state. */ static telemetry::Telemetry& getTelemetry() diff --git a/src/tests/libxrpl/telemetry/SpanGuardScope.cpp b/src/tests/libxrpl/telemetry/SpanGuardScope.cpp index 8aac770262..db3796e09c 100644 --- a/src/tests/libxrpl/telemetry/SpanGuardScope.cpp +++ b/src/tests/libxrpl/telemetry/SpanGuardScope.cpp @@ -135,6 +135,17 @@ public: return true; } + /** + * @return A fixed strategy label; the scope tests do not exercise + * deterministic trace-id correlation, so any stable value works. + */ + [[nodiscard]] std::string const& + getConsensusTraceStrategy() const override + { + static std::string const kStrategy{"none"}; + return kStrategy; + } + opentelemetry::nostd::shared_ptr getTracer(std::string_view name) override { diff --git a/src/xrpld/app/consensus/RCLConsensus.cpp b/src/xrpld/app/consensus/RCLConsensus.cpp index c277899316..1eb2c9fd71 100644 --- a/src/xrpld/app/consensus/RCLConsensus.cpp +++ b/src/xrpld/app/consensus/RCLConsensus.cpp @@ -233,7 +233,7 @@ 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). auto span = telemetry::SpanGuard::childSpan( - telemetry::consensus::span::op::proposalSend, roundSpanContext_); + telemetry::consensus::span::proposalSend, roundSpanContext_); span.setAttribute( telemetry::consensus::span::attr::round, static_cast(proposal.proposeSeq())); span.setAttribute(telemetry::consensus::span::attr::isBowOut, proposal.isBowOut()); @@ -349,7 +349,7 @@ RCLConsensus::Adaptor::onClose( // 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_); + 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()); span.setAttribute( @@ -1059,14 +1059,10 @@ RCLConsensus::Adaptor::validate(RCLCxLedger const& ledger, RCLTxSet const& txns, // Inject the current thread's active span context so receiving // peers can link their validation.receive span as a child. // - // TODO(observability/secure-OTel): the trace_context appended below is - // outside the cryptographic signature on `serialized` and is therefore - // unauthenticated. Receivers cannot prove it was not tampered with by - // a relay. A signed trace context (either folded into the validation - // payload or carried by an authenticated trace_state token) is tracked - // as a follow-up — see PR #6425 discussion r3317273388 and - // OpenTelemetryPlan/secure-OTel.md. Until then, downstream consumers - // must treat the validation trace_context as advisory only. + // The trace_context appended below is outside the signature on + // `serialized`, so it is not covered by validation authenticity. + // Downstream consumers treat it as advisory only. A signature-covered + // trace context is a possible future enhancement. telemetry::SpanGuard::injectCurrentContextToProtobuf(*val.mutable_trace_context()); app_.getOverlay().broadcast(val); @@ -1082,7 +1078,7 @@ RCLConsensus::Adaptor::onModeChange(ConsensusMode before, ConsensusMode after) // 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_); + 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()); diff --git a/src/xrpld/app/consensus/RCLConsensus.h b/src/xrpld/app/consensus/RCLConsensus.h index 9bf86bdf4e..39a584194c 100644 --- a/src/xrpld/app/consensus/RCLConsensus.h +++ b/src/xrpld/app/consensus/RCLConsensus.h @@ -285,6 +285,24 @@ class RCLConsensus void onOutcomeEvent(std::string_view eventName); + /** + * Captured context of the current round span. + * + * The generic engine uses this to parent the phase spans it owns + * (consensus.phase.open, consensus.establish) under the round span + * without touching roundSpan_ across threads. roundSpan_ is detached + * after this context is captured, so it is no longer the thread's + * ambient parent; child spans must link via this context. Returns an + * invalid context before the round span is created. + * + * @return The round span's captured context, or an invalid context. + */ + telemetry::SpanContext + roundSpanContext() const + { + return roundSpanContext_; + } + private: //--------------------------------------------------------------------- // The following members implement the generic Consensus requirements diff --git a/src/xrpld/consensus/Consensus.h b/src/xrpld/consensus/Consensus.h index 82898c3950..917bf84463 100644 --- a/src/xrpld/consensus/Consensus.h +++ b/src/xrpld/consensus/Consensus.h @@ -779,14 +779,16 @@ Consensus::startRoundInternal( // 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. + // 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_. openSpan_.emplace( - telemetry::SpanGuard::span( - telemetry::TraceCategory::Consensus, - telemetry::seg::consensus, - telemetry::consensus::span::op::phaseOpen) + telemetry::SpanGuard::childSpan( + telemetry::consensus::span::phaseOpen, adaptor_.roundSpanContext()) .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 @@ -1631,7 +1633,7 @@ Consensus::updateOurPositions(std::unique_ptr const& // 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_); + auto span = SpanGuard::childSpan(consensus::span::updatePositions, establishSpanContext_); span.setAttribute( consensus::span::attr::convergePercent, static_cast(convergePercent_)); span.setAttribute( @@ -1840,7 +1842,7 @@ Consensus::haveConsensus(std::unique_ptr const& clog 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). - auto span = SpanGuard::childSpan(consensus::span::op::check, establishSpanContext_); + auto span = SpanGuard::childSpan(consensus::span::check, establishSpanContext_); // CHECKME: should possibly count unacquired TX sets as disagreeing int agree = 0, disagree = 0; @@ -2099,11 +2101,13 @@ 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 + // explicitly under roundSpanContext_. An invalid round context (round span + // not yet created) yields a null guard. establishSpan_.emplace( - telemetry::SpanGuard::span( - telemetry::TraceCategory::Consensus, - telemetry::seg::consensus, - telemetry::consensus::span::op::establish)); + 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