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<SpanGuard>(
proposalReceiveSpan(...))/validationReceiveSpan(...) constructions
in onMessage(TMProposeSet)/onMessage(TMValidation), drop the
sp = std::move(span) lambda captures, and drop the
#include <xrpld/telemetry/ConsensusReceiveTracing.h>.
- 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>
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 <xrpld/app/consensus/ConsensusSpanNames.h>.
- 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>
- Drop xrpl.node.amendment_blocked / xrpl.node.server_state from telemetry
surface (constants in SpanNames.h, two filters in tempo.yaml). Operators
read the same data via server_info / server_state RPC; OTel SDK 1.18.0
cannot refresh resource attrs at runtime so resource-level emission was
not viable either.
- Namespace all pathfind span attributes under pathfind_* (underscore form
per Phase 1c rule 5). Renames in PathFindSpanNames.h and call sites in
PathRequest.cpp, PathRequestManager.cpp, plus the rule-5 retention
xrpl.pathfind.ledger_index -> pathfind_ledger_index.
- Wire pathfind_source_account / pathfind_dest_account on pathfind.request
in doPathFind / doRipplePathFind handlers (only when present + string).
- Collapse per-asset pathfind.discover / pathfind.rank spans into one
pathfind.discover hoisted around the per-source-asset loop in
PathRequest::findPaths. Span count goes from 2N to 1 per RPC call;
per-asset breakdown traded for bounded storage and cardinality. Trade-off
documented inline.
- Fix pathfind_num_paths semantics: now sums getBestPaths().size() across
the loop (paths actually returned) instead of the maxPaths input cap.
- PathRequestManager::updateAll: move span creation after the locked
requests_ snapshot, early-return when no active subscriptions exist
(avoids empty span on every ledger close), set pathfind_num_requests
= requests.size().
- Update Phase2_taskList.md and 02-design-decisions.md to match.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Per PR #6438 review thread r3250432621: known-command errors
(rpcTOO_BUSY, rpcNO_PERMISSION, etc.) were collapsing into a
single rpc.command.unknown span name, hiding per-command error
rates in dashboards. Same anti-pattern existed for gRPC, where
every method was bucketed under grpc.request with the method
relegated to an attribute.
- RPCHandler.cpp: doCommand error path uses cmdName as the span
suffix; the rpc_span::val::unknownCommand fallback only applies
when the request truly omits both command and method fields.
- GRPCServer.cpp: gRPC span name is now grpc.<MethodName>
(e.g. grpc.GetLedger). Method also retained as an attribute.
- GrpcSpanNames.h: drop the unused op::request constant; update
the span-hierarchy comment.
- RpcSpanNames.h: update the gRPC span diagram to match.
Dashboards on downstream phases will benefit from per-command
breakdowns without needing TraceQL attribute filters.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
