From ea7f585337e9803f3584b1dd93fe026fa82258d1 Mon Sep 17 00:00:00 2001 From: Pratik Mankawde <3397372+pratikmankawde@users.noreply.github.com> Date: Fri, 17 Jul 2026 22:24:52 +0100 Subject: [PATCH] fix(telemetry): root peer.validation.receive and peer.proposal.receive spans MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit These inbound peer-message entry points (kConsumer) used span(), which inherits whatever span is active on the peer thread — including a leaked tx.receive scope — so validations/proposals were wrongly nested under unrelated transaction traces. rootSpan() starts a fresh trace root. Co-Authored-By: Claude Opus 4.8 (1M context) --- OpenTelemetryPlan/09-data-collection-reference.md | 5 +++++ docs/telemetry-runbook.md | 6 ++++++ src/xrpld/overlay/detail/PeerImp.cpp | 8 ++++++-- 3 files changed, 17 insertions(+), 2 deletions(-) diff --git a/OpenTelemetryPlan/09-data-collection-reference.md b/OpenTelemetryPlan/09-data-collection-reference.md index fa5422f115..d911cf88ff 100644 --- a/OpenTelemetryPlan/09-data-collection-reference.md +++ b/OpenTelemetryPlan/09-data-collection-reference.md @@ -211,6 +211,11 @@ Controlled by `trace_peer` in `[telemetry]` config. **Enabled by default** (high | `peer.proposal.receive` | — | PeerImp.cpp | Consensus proposal received from peer | | `peer.validation.receive` | — | PeerImp.cpp | Validation message received from peer | +A `—` parent means the span is a fresh trace root (`kConsumer`): it is started +via `SpanGuard::rootSpan()` at the inbound-message entry point and never +inherits an ambient span left active on the peer thread, so it does not nest +under an unrelated transaction's trace. + **Where to find**: Tempo → TraceQL: `{resource.service.name="xrpld" && name=~"peer.*"}` **Grafana dashboard**: _Peer Network_ (`peer-network`) diff --git a/docs/telemetry-runbook.md b/docs/telemetry-runbook.md index 249ac3cdc8..826ab159a5 100644 --- a/docs/telemetry-runbook.md +++ b/docs/telemetry-runbook.md @@ -175,6 +175,12 @@ RED metrics on the _Transaction Overview_ dashboard. | `peer.proposal.receive` | PeerImp.cpp:1667 | `peer_id`, `proposal_trusted` | Proposal received from peer | | `peer.validation.receive` | PeerImp.cpp:2264 | `peer_id`, `validation_trusted` | Validation received from peer | +Both peer receive spans are `kConsumer` inbound entry points started as fresh +trace roots. They never inherit an ambient span left active on the peer thread, +so they do not nest under an unrelated transaction's trace. The distributed +child span that links back to the sending node is the separate +`consensus.*.receive` / `tx.receive` span (see Cross-Node Trace Propagation). + --- ## Insights and Sample Queries diff --git a/src/xrpld/overlay/detail/PeerImp.cpp b/src/xrpld/overlay/detail/PeerImp.cpp index 6d960b33bd..3f2d3ed3e6 100644 --- a/src/xrpld/overlay/detail/PeerImp.cpp +++ b/src/xrpld/overlay/detail/PeerImp.cpp @@ -1750,7 +1750,9 @@ void PeerImp::onMessage(std::shared_ptr const& m) { using namespace telemetry; - auto span = SpanGuard::span(TraceCategory::Peer, seg::peer, peer_span::op::proposalReceive); + // root: inbound peer message entry point (kConsumer); must not inherit + // any span left active on this peer thread. + auto span = SpanGuard::rootSpan(TraceCategory::Peer, seg::peer, peer_span::op::proposalReceive); span.setAttribute(peer_span::attr::peerId, static_cast(id_)); protocol::TMProposeSet const& set = *m; @@ -2367,8 +2369,10 @@ void PeerImp::onMessage(std::shared_ptr const& m) { using namespace telemetry; + // root: inbound peer message entry point (kConsumer); must not inherit + // any span left active on this peer thread. auto valSpan = - SpanGuard::span(TraceCategory::Peer, seg::peer, peer_span::op::validationReceive); + SpanGuard::rootSpan(TraceCategory::Peer, seg::peer, peer_span::op::validationReceive); valSpan.setAttribute(peer_span::attr::peerId, static_cast(id_)); if (m->validation().size() < 50)