From d6b101069ea01a7e46d03f1cab244a6bb9bb9120 Mon Sep 17 00:00:00 2001 From: Pratik Mankawde <3397372+pratikmankawde@users.noreply.github.com> Date: Thu, 28 May 2026 17:01:46 +0100 Subject: [PATCH] refactor(telemetry): remove consensus tracing from phase-3 Phase-3 (PR #6425) is scoped to transaction tracing only; consensus tracing belongs to phase-4 (PR #6426). The previous commit on this branch removed the namespace/attribute scaffolding c6c019ed8b leaked into phase-3, but phase-3 still carried the consensus span construction and trace-context propagation introduced in earlier commits (61cb1faf8f, 93bed03d8d). Move that out too so phase-3 creates and propagates no consensus spans of any kind. Removed: - src/xrpld/telemetry/ConsensusReceiveTracing.h (deleted; phase-4 owns it). - PeerImp.cpp: remove the std::make_shared( proposalReceiveSpan(...))/validationReceiveSpan(...) constructions in onMessage(TMProposeSet)/onMessage(TMValidation), drop the sp = std::move(span) lambda captures, and drop the #include . - RCLConsensus.cpp: drop the two telemetry::injectToProtobuf() blocks that injected the active trace context into TMProposeSet (in Adaptor::propose, after addSuppression) and TMValidation (in Adaptor::validate, around the broadcast call). Drop the now-unused #include of TraceContextPropagator.h and the XRPL_ENABLE_TELEMETRY-gated include of opentelemetry/context/runtime_context.h. - TraceContextPropagator.h: update file-level @see comment to drop the ConsensusReceiveTracing.h reference and to scope the "wired into the P2P message flow via PropagationHelpers.h" sentence to TMTransaction only. - PropagationHelpers.h: replace the @see ConsensusReceiveTracing.h cross-reference with @see TxTracing.h. Inert consensus metadata (TraceCategory::Consensus enum value, seg::consensus constant, isCategoryEnabled/categoryToSpanKind switch arms, the SpanGuard.h doc-comment example) is intentionally preserved on phase-3: nothing references it after this commit, but phase-4 needs it and removing it would widen the phase-3 -> phase-4 merge surface for no benefit. Verified via git grep: no remaining phase-3 references to proposalReceiveSpan, validationReceiveSpan, ConsensusReceiveTracing, consensus_span::, consensus.proposal, or consensus.validation. Signed-off-by: Pratik Mankawde <3397372+pratikmankawde@users.noreply.github.com> --- .../xrpl/telemetry/TraceContextPropagator.h | 5 +- src/xrpld/app/consensus/RCLConsensus.cpp | 23 ---- src/xrpld/overlay/detail/PeerImp.cpp | 26 +--- src/xrpld/telemetry/ConsensusReceiveTracing.h | 125 ------------------ src/xrpld/telemetry/PropagationHelpers.h | 2 +- 5 files changed, 5 insertions(+), 176 deletions(-) delete mode 100644 src/xrpld/telemetry/ConsensusReceiveTracing.h diff --git a/include/xrpl/telemetry/TraceContextPropagator.h b/include/xrpl/telemetry/TraceContextPropagator.h index d0fb7d576d..d699272810 100644 --- a/include/xrpl/telemetry/TraceContextPropagator.h +++ b/include/xrpl/telemetry/TraceContextPropagator.h @@ -5,13 +5,12 @@ Provides serialization/deserialization of OTel trace context to/from Protocol Buffer TraceContext messages (P2P cross-node propagation). Wired into the P2P message flow via PropagationHelpers.h for - TMTransaction, TMProposeSet, and TMValidation messages. + TMTransaction messages. Only compiled when XRPL_ENABLE_TELEMETRY is defined. @see PropagationHelpers.h (high-level inject helpers), - TxTracing.h (transaction receive-side extraction), - ConsensusReceiveTracing.h (proposal/validation receive-side). + TxTracing.h (transaction receive-side extraction). */ #ifdef XRPL_ENABLE_TELEMETRY diff --git a/src/xrpld/app/consensus/RCLConsensus.cpp b/src/xrpld/app/consensus/RCLConsensus.cpp index eb040f231f..6d99c2ee15 100644 --- a/src/xrpld/app/consensus/RCLConsensus.cpp +++ b/src/xrpld/app/consensus/RCLConsensus.cpp @@ -62,14 +62,9 @@ #include #include #include -#include // IWYU pragma: keep #include -#ifdef XRPL_ENABLE_TELEMETRY -#include -#endif - #include #include @@ -266,16 +261,6 @@ RCLConsensus::Adaptor::propose(RCLCxPeerPos::Proposal const& proposal) app_.getHashRouter().addSuppression(suppression); - // Inject the current thread's active span context (e.g. the - // consensus round span from Phase 4) so receiving peers can link - // their proposal.receive span as a child of this trace. -#ifdef XRPL_ENABLE_TELEMETRY - { - auto ctx = opentelemetry::context::RuntimeContext::GetCurrent(); - telemetry::injectToProtobuf(ctx, *prop.mutable_trace_context()); - } -#endif - app_.getOverlay().broadcast(prop); } @@ -896,14 +881,6 @@ RCLConsensus::Adaptor::validate(RCLCxLedger const& ledger, RCLTxSet const& txns, // Broadcast to all our peers: protocol::TMValidation val; val.set_validation(serialized.data(), serialized.size()); - // Inject the current thread's active span context so receiving - // peers can link their validation.receive span as a child. -#ifdef XRPL_ENABLE_TELEMETRY - { - auto ctx = opentelemetry::context::RuntimeContext::GetCurrent(); - telemetry::injectToProtobuf(ctx, *val.mutable_trace_context()); - } -#endif app_.getOverlay().broadcast(val); // Publish to all our subscribers: diff --git a/src/xrpld/overlay/detail/PeerImp.cpp b/src/xrpld/overlay/detail/PeerImp.cpp index 155b1b57b9..7e725b3893 100644 --- a/src/xrpld/overlay/detail/PeerImp.cpp +++ b/src/xrpld/overlay/detail/PeerImp.cpp @@ -22,7 +22,6 @@ #include #include #include -#include #include #include @@ -1975,17 +1974,9 @@ PeerImp::onMessage(std::shared_ptr const& m) app_.getTimeKeeper().closeTime(), calcNodeID(app_.getValidatorManifests().getMasterKey(publicKey))}); - // Create a receive span that links to the sender's trace context - // (if propagated). shared_ptr keeps it alive across the job boundary. - auto span = std::make_shared(telemetry::proposalReceiveSpan(set)); - span->setAttribute("xrpl.consensus.trusted", isTrusted); - span->setAttribute("xrpl.consensus.round", static_cast(set.proposeseq())); - std::weak_ptr const weak = shared_from_this(); app_.getJobQueue().addJob( - isTrusted ? jtPROPOSAL_t : jtPROPOSAL_ut, - "checkPropose", - [weak, isTrusted, m, proposal, sp = std::move(span)]() { + isTrusted ? jtPROPOSAL_t : jtPROPOSAL_ut, "checkPropose", [weak, isTrusted, m, proposal]() { if (auto peer = weak.lock()) peer->checkPropose(isTrusted, m, proposal); }); @@ -2560,17 +2551,6 @@ PeerImp::onMessage(std::shared_ptr const& m) return; } - // Create a receive span that links to the sender's trace context - // (if propagated). shared_ptr keeps it alive across the job boundary. - auto span = std::make_shared(telemetry::validationReceiveSpan(*m)); - span->setAttribute("xrpl.consensus.trusted", isTrusted); - if (val->isFieldPresent(sfLedgerSequence)) - { - span->setAttribute( - "xrpl.consensus.ledger.seq", - static_cast(val->getFieldU32(sfLedgerSequence))); - } - if (!isTrusted && (tracking_.load() == Tracking::diverged)) { JLOG(p_journal_.debug()) << "Dropping untrusted validation from diverged peer"; @@ -2581,9 +2561,7 @@ PeerImp::onMessage(std::shared_ptr const& m) std::weak_ptr const weak = shared_from_this(); app_.getJobQueue().addJob( - isTrusted ? jtVALIDATION_t : jtVALIDATION_ut, - name, - [weak, val, m, key, sp = std::move(span)]() { + isTrusted ? jtVALIDATION_t : jtVALIDATION_ut, name, [weak, val, m, key]() { if (auto peer = weak.lock()) peer->checkValidation(val, key, m); }); diff --git a/src/xrpld/telemetry/ConsensusReceiveTracing.h b/src/xrpld/telemetry/ConsensusReceiveTracing.h deleted file mode 100644 index 6da9900fab..0000000000 --- a/src/xrpld/telemetry/ConsensusReceiveTracing.h +++ /dev/null @@ -1,125 +0,0 @@ -#pragma once - -/** Helper functions for creating consensus receive trace spans. - * - * Encapsulates the logic for creating SpanGuard instances for incoming - * proposal and validation messages with optional protobuf parent - * extraction. When the incoming message carries a TraceContext with a - * valid span_id, the receive span is created as a child of the - * sender's span, enabling cross-node trace correlation. - * - * Dependency diagram: - * - * protocol::TMProposeSet / TMValidation - * | - * v - * proposalReceiveSpan() / validationReceiveSpan() - * | - * +--- has trace_context? ----+ - * | yes | no - * v v - * SpanGuard::span() with SpanGuard::span() - * extracted parent context (standalone span) - * - * When XRPL_ENABLE_TELEMETRY is not defined, the functions return - * no-op SpanGuard instances (zero overhead, zero dependencies). - * - * Usage: - * @code - * // In PeerImp::onMessage(TMProposeSet): - * auto span = telemetry::proposalReceiveSpan(*m); - * span.setAttribute(...); - * @endcode - * - * @note These span names use inline string_view literals. When - * ConsensusSpanNames.h (from Phase 4) is available, callers should - * migrate to using the constexpr constants defined there. - */ - -#include -#include - -namespace xrpl::telemetry { - -// Inline span name constants for consensus receive spans. -// Phase 4 will provide these via ConsensusSpanNames.h; these are -// temporary definitions for the propagation infrastructure. -namespace detail { -inline constexpr std::string_view proposalReceiveName = "consensus.proposal.receive"; -inline constexpr std::string_view validationReceiveName = "consensus.validation.receive"; -} // namespace detail - -/** Create a "consensus.proposal.receive" span for an incoming proposal. - * - * If the message carries a TraceContext with a valid span_id, the - * receive span is created with the sender's context as parent. - * Otherwise a standalone span is created. - * - * @param msg The incoming TMProposeSet protobuf message. - * @return An active SpanGuard, or a null guard if tracing is disabled. - */ -inline SpanGuard -proposalReceiveSpan([[maybe_unused]] protocol::TMProposeSet const& msg) -{ -#ifdef XRPL_ENABLE_TELEMETRY - if (msg.has_trace_context()) - { - auto const& tc = msg.trace_context(); - if (tc.has_span_id() && tc.span_id().size() == 8 && tc.has_trace_id() && - tc.trace_id().size() == 16) - { - // Create a child span using the sender's trace_id and - // span_id as parent. Use hashSpan with the sender's - // trace_id so the receiving span shares the same trace. - return SpanGuard::hashSpan( - TraceCategory::Consensus, - detail::proposalReceiveName, - reinterpret_cast(tc.trace_id().data()), - tc.trace_id().size(), - reinterpret_cast(tc.span_id().data()), - tc.span_id().size(), - tc.has_trace_flags() ? static_cast(tc.trace_flags()) - : std::uint8_t{0}); - } - } -#endif - // No propagated context — create a standalone span. - return SpanGuard::span(TraceCategory::Consensus, "consensus", "proposal.receive"); -} - -/** Create a "consensus.validation.receive" span for an incoming validation. - * - * If the message carries a TraceContext with a valid span_id, the - * receive span is created with the sender's context as parent. - * Otherwise a standalone span is created. - * - * @param msg The incoming TMValidation protobuf message. - * @return An active SpanGuard, or a null guard if tracing is disabled. - */ -inline SpanGuard -validationReceiveSpan([[maybe_unused]] protocol::TMValidation const& msg) -{ -#ifdef XRPL_ENABLE_TELEMETRY - if (msg.has_trace_context()) - { - auto const& tc = msg.trace_context(); - if (tc.has_span_id() && tc.span_id().size() == 8 && tc.has_trace_id() && - tc.trace_id().size() == 16) - { - return SpanGuard::hashSpan( - TraceCategory::Consensus, - detail::validationReceiveName, - reinterpret_cast(tc.trace_id().data()), - tc.trace_id().size(), - reinterpret_cast(tc.span_id().data()), - tc.span_id().size(), - tc.has_trace_flags() ? static_cast(tc.trace_flags()) - : std::uint8_t{0}); - } - } -#endif - // No propagated context — create a standalone span. - return SpanGuard::span(TraceCategory::Consensus, "consensus", "validation.receive"); -} - -} // namespace xrpl::telemetry diff --git a/src/xrpld/telemetry/PropagationHelpers.h b/src/xrpld/telemetry/PropagationHelpers.h index 44959ae039..8cb9954624 100644 --- a/src/xrpld/telemetry/PropagationHelpers.h +++ b/src/xrpld/telemetry/PropagationHelpers.h @@ -25,7 +25,7 @@ * overlay.relay(txID, tx, toSkip); * @endcode * - * @see ConsensusReceiveTracing.h for receive-side extraction helpers. + * @see TxTracing.h for receive-side extraction helpers. * @see TraceContextPropagator.h for low-level OTel context serialization. */