From c9521b97fed502484e06229dfc19554f9b8fb223 Mon Sep 17 00:00:00 2001 From: Pratik Mankawde <3397372+pratikmankawde@users.noreply.github.com> Date: Thu, 28 May 2026 16:51:23 +0100 Subject: [PATCH] refactor(telemetry): pull consensus-tracing scope-leak out of phase-3 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Commit c6c019ed8b ("addressed code review comments") bundled tx-tracing review fixes with consensus-tracing scaffolding that belongs on pratik/otel-phase4-consensus-tracing (PR #6426). This commit lifts the consensus-only parts back out of phase-3 so PR #6425 stays scoped to transaction tracing. Phase-4 already carries the same content (via prior phase-3 -> phase-4 merges) plus its own evolution on top, so nothing is moved across — only removed here. Removed: - src/xrpld/app/consensus/ConsensusSpanNames.h (deleted; phase-4 owns it). - PeerImp.cpp: revert onMessage(TMProposeSet)/onMessage(TMValidation) consensus-attr setAttribute calls to the hardcoded "xrpl.consensus.{trusted,round,ledger.seq}" strings used before c6c019ed8b. Drop the now-unused #include . - RCLConsensus::Adaptor::validate(): remove the TODO(observability/secure-OTel) block on validation trace_context. - TraceContextPropagator.h: remove the TODO(observability/secure-OTel) block on injectToProtobuf(). Tx-tracing parts of c6c019ed8b are intentionally untouched. No phase-3 caller of telemetry::consensus_span:: remains; verified via git grep. No test on phase-3 references the removed header. Signed-off-by: Pratik Mankawde <3397372+pratikmankawde@users.noreply.github.com> --- .../xrpl/telemetry/TraceContextPropagator.h | 8 --- src/xrpld/app/consensus/ConsensusSpanNames.h | 67 ------------------- src/xrpld/app/consensus/RCLConsensus.cpp | 9 --- src/xrpld/overlay/detail/PeerImp.cpp | 10 ++- 4 files changed, 4 insertions(+), 90 deletions(-) delete mode 100644 src/xrpld/app/consensus/ConsensusSpanNames.h diff --git a/include/xrpl/telemetry/TraceContextPropagator.h b/include/xrpl/telemetry/TraceContextPropagator.h index 3f400860fb..d0fb7d576d 100644 --- a/include/xrpl/telemetry/TraceContextPropagator.h +++ b/include/xrpl/telemetry/TraceContextPropagator.h @@ -91,14 +91,6 @@ injectToProtobuf(opentelemetry::context::Context const& ctx, protocol::TraceCont // Serialize flags proto.set_trace_flags(spanCtx.trace_flags().flags()); - - // TODO(observability/secure-OTel): the protobuf TraceContext message - // also carries `trace_state` (field 4), which is currently neither - // populated here nor read by extractFromProtobuf above. The field is - // reserved for the secure tracing pipeline outlined in - // OpenTelemetryPlan/secure-OTel.md, where an authenticated token in - // tracestate will let receivers reject spoofed/poisoned trace context. - // Wire trace_state through inject/extract once the consumer lands. } } // namespace telemetry diff --git a/src/xrpld/app/consensus/ConsensusSpanNames.h b/src/xrpld/app/consensus/ConsensusSpanNames.h deleted file mode 100644 index f400adc7f6..0000000000 --- a/src/xrpld/app/consensus/ConsensusSpanNames.h +++ /dev/null @@ -1,67 +0,0 @@ -#pragma once - -/** Compile-time span name and attribute constants for consensus tracing. - * - * Used by PeerImp (overlay) and RCLConsensus (consensus) for proposal - * and validation lifecycle spans. Built on StaticStr/join() from - * SpanNames.h and follows the rule-5 underscore form for shared - * cross-span attributes (e.g. `consensus_round`, `ledger_seq`). - * - * Phase 3 introduces the receive-side surface used by PeerImp. - * Phase 4 will extend this with the proposer/validator-side spans - * (`consensus.proposal.send`, `consensus.validation.send`, round - * bookkeeping, etc.). - * - * Span hierarchy (cross-node propagation): - * - * Node A (sender) Node B (receiver) - * +----------------------------+ +-------------------------------+ - * | consensus.proposal/...send | proto | consensus.proposal/...receive | - * | inject trace context | -----> | proposalReceiveSpan() / | - * | (RCLConsensus broadcast) | t_ctx | validationReceiveSpan() | - * +----------------------------+ +-------------------------------+ - */ - -#include - -namespace xrpl::telemetry::consensus_span { - -// ===== Span prefixes ======================================================= - -namespace prefix { -/// "consensus" — root prefix for consensus lifecycle spans. -inline constexpr auto consensus = seg::consensus; -/// "consensus.proposal" — proposal sub-tree. -inline constexpr auto proposal = join(consensus, makeStr("proposal")); -/// "consensus.validation" — validation sub-tree. -inline constexpr auto validation = join(consensus, makeStr("validation")); -} // namespace prefix - -// ===== Span operation suffixes ============================================= - -namespace op { -inline constexpr auto receive = makeStr("receive"); -inline constexpr auto send = makeStr("send"); -} // namespace op - -// ===== Full span names ===================================================== - -inline constexpr auto proposalReceive = join(prefix::proposal, op::receive); -inline constexpr auto validationReceive = join(prefix::validation, op::receive); - -// ===== Attribute keys ====================================================== - -namespace attr { -/// Canonical shared constants (defined in SpanNames.h). -using ::xrpl::telemetry::attr::ledgerSeq; - -/// "trusted" — bare field; whether the proposing/validating node is -/// in our UNL. Used only on consensus spans, no cross-domain collision. -inline constexpr auto trusted = makeStr("trusted"); - -/// "consensus_round" — propose-sequence within a consensus round -/// (rule-5 underscore form, shared across consensus spans). -inline constexpr auto round = makeStr("consensus_round"); -} // namespace attr - -} // namespace xrpl::telemetry::consensus_span diff --git a/src/xrpld/app/consensus/RCLConsensus.cpp b/src/xrpld/app/consensus/RCLConsensus.cpp index 4f34953423..eb040f231f 100644 --- a/src/xrpld/app/consensus/RCLConsensus.cpp +++ b/src/xrpld/app/consensus/RCLConsensus.cpp @@ -898,15 +898,6 @@ RCLConsensus::Adaptor::validate(RCLCxLedger const& ledger, RCLTxSet const& txns, 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. - // - // 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. #ifdef XRPL_ENABLE_TELEMETRY { auto ctx = opentelemetry::context::RuntimeContext::GetCurrent(); diff --git a/src/xrpld/overlay/detail/PeerImp.cpp b/src/xrpld/overlay/detail/PeerImp.cpp index eababb8b5a..155b1b57b9 100644 --- a/src/xrpld/overlay/detail/PeerImp.cpp +++ b/src/xrpld/overlay/detail/PeerImp.cpp @@ -1,6 +1,5 @@ #include -#include #include #include #include @@ -1979,9 +1978,8 @@ 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. auto span = std::make_shared(telemetry::proposalReceiveSpan(set)); - span->setAttribute(telemetry::consensus_span::attr::trusted, isTrusted); - span->setAttribute( - telemetry::consensus_span::attr::round, static_cast(set.proposeseq())); + 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( @@ -2565,11 +2563,11 @@ 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. auto span = std::make_shared(telemetry::validationReceiveSpan(*m)); - span->setAttribute(telemetry::consensus_span::attr::trusted, isTrusted); + span->setAttribute("xrpl.consensus.trusted", isTrusted); if (val->isFieldPresent(sfLedgerSequence)) { span->setAttribute( - telemetry::consensus_span::attr::ledgerSeq, + "xrpl.consensus.ledger.seq", static_cast(val->getFieldU32(sfLedgerSequence))); }