fix(telemetry): address code review findings for Phase 4 consensus tracing

Fix quorum attribute to use actual validator quorum instead of proposer
count, add missing ConsensusState::Expired handling in haveConsensus()
span, move ConsensusSpanNames.h to xrpld/consensus/ to resolve
levelization cycle, remove unused constants, enrich proposal receive
span with sequence, and correct stale documentation references.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
Pratik Mankawde
2026-04-28 18:14:00 +01:00
parent d50e0ff48e
commit c01f8ae99c
10 changed files with 20 additions and 22 deletions

0
.github/scripts/levelization/generate.py vendored Normal file → Executable file
View File

View File

@@ -7,9 +7,6 @@ Loop: test.jtx test.unit_test
Loop: xrpl.telemetry xrpld.rpc
xrpld.rpc > xrpl.telemetry
Loop: xrpld.app xrpld.consensus
xrpld.app > xrpld.consensus
Loop: xrpld.app xrpld.overlay
xrpld.app > xrpld.overlay

View File

@@ -236,6 +236,7 @@ xrpl.tx > xrpl.protocol
xrpld.app > test.unit_test
xrpld.app > xrpl.basics
xrpld.app > xrpl.core
xrpld.app > xrpld.consensus
xrpld.app > xrpld.core
xrpld.app > xrpl.json
xrpld.app > xrpl.ledger

View File

@@ -251,8 +251,8 @@ resource::SemanticConventions::SERVICE_INSTANCE_ID = <node_public_key_base58>
"xrpl.consensus.proposers_total" = int64 // Total peer positions
"xrpl.consensus.agree_count" = int64 // Peers that agree (haveConsensus)
"xrpl.consensus.disagree_count" = int64 // Peers that disagree
"xrpl.consensus.threshold_percent" = int64 // Current threshold (50/65/70/95)
"xrpl.consensus.result" = string // "yes", "no", "moved_on"
"xrpl.consensus.threshold_percent" = int64 // Close-time consensus threshold (avCT_CONSENSUS_PCT = 75%)
"xrpl.consensus.result" = string // "yes", "no", "moved_on", "expired"
"xrpl.consensus.mode.old" = string // Previous consensus mode
"xrpl.consensus.mode.new" = string // New consensus mode
```

View File

@@ -181,11 +181,12 @@ SHAMap tracing are not implemented.
| Span Name | Location | Attributes |
| --------------------------- | ---------------------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
| `consensus.proposal.send` | `RCLConsensus.cpp:177` | `xrpl.consensus.round` |
| `consensus.ledger_close` | `RCLConsensus.cpp:282` | `xrpl.consensus.ledger.seq`, `xrpl.consensus.mode` |
| `consensus.accept` | `RCLConsensus.cpp:395` | `xrpl.consensus.proposers`, `xrpl.consensus.round_time_ms` |
| `consensus.accept.apply` | `RCLConsensus.cpp:521` | `xrpl.consensus.close_time`, `close_time_correct`, `close_resolution_ms`, `state`, `proposing`, `round_time_ms`, `ledger.seq`, `parent_close_time`, `close_time_self`, `close_time_vote_bins`, `resolution_direction` |
| `consensus.validation.send` | `RCLConsensus.cpp:753` | `xrpl.consensus.proposing` |
| `consensus.phase.open` | `Consensus.h:707` | _(none)_ |
| `consensus.proposal.send` | `RCLConsensus.cpp:232` | `xrpl.consensus.round` |
| `consensus.ledger_close` | `RCLConsensus.cpp:341` | `xrpl.consensus.ledger.seq`, `xrpl.consensus.mode` |
| `consensus.accept` | `RCLConsensus.cpp:492` | `xrpl.consensus.proposers`, `xrpl.consensus.round_time_ms`, `xrpl.consensus.quorum` |
| `consensus.accept.apply` | `RCLConsensus.cpp:541` | `xrpl.consensus.close_time`, `close_time_correct`, `close_resolution_ms`, `state`, `proposing`, `round_time_ms`, `ledger.seq`, `parent_close_time`, `close_time_self`, `close_time_vote_bins`, `resolution_direction` |
| `consensus.validation.send` | `RCLConsensus.cpp:900` | `xrpl.consensus.ledger.seq`, `xrpl.consensus.proposing` |
### Exit Criteria
@@ -279,7 +280,7 @@ See [Phase4_taskList.md](./Phase4_taskList.md) for full task details.
validations) to enable true distributed tracing between nodes.
**Status**: Design documented, NOT implemented. Protobuf fields (field 1001)
and `TraceContextPropagator` class exist. Wiring deferred until Phase 4a is
and `TraceContextPropagator` free functions exist. Wiring deferred until Phase 4a is
validated in a multi-node environment.
**Prerequisites**: Phase 4a complete and validated.

View File

@@ -903,6 +903,6 @@ share the same trace_id. P2P propagation adds **span-level** linking:
## Prerequisites
- Phase 4a (this task list) — establish phase tracing must be in place
- `TraceContextPropagator` class (already exists in
- `TraceContextPropagator` free functions (already exist in
`include/xrpl/telemetry/TraceContextPropagator.h`)
- Protobuf `TraceContext` message (already exists, field 1001)

View File

@@ -1,6 +1,5 @@
#include <xrpld/app/consensus/RCLConsensus.h>
#include <xrpld/app/consensus/ConsensusSpanNames.h>
#include <xrpld/app/consensus/RCLCensorshipDetector.h>
#include <xrpld/app/consensus/RCLCxLedger.h>
#include <xrpld/app/consensus/RCLCxPeerPos.h>
@@ -19,6 +18,7 @@
#include <xrpld/app/misc/ValidatorKeys.h>
#include <xrpld/app/misc/ValidatorList.h>
#include <xrpld/consensus/Consensus.h>
#include <xrpld/consensus/ConsensusSpanNames.h>
#include <xrpld/consensus/ConsensusTypes.h>
#include <xrpld/overlay/Overlay.h>
#include <xrpld/overlay/predicates.h>
@@ -495,7 +495,8 @@ RCLConsensus::Adaptor::makeAcceptSpan(Result const& result)
span->setAttribute(
telemetry::cons_span::attr::roundTimeMs,
static_cast<int64_t>(result.roundTime.read().count()));
span->setAttribute(telemetry::cons_span::attr::quorum, static_cast<int64_t>(result.proposers));
span->setAttribute(
telemetry::cons_span::attr::quorum, static_cast<int64_t>(app_.getValidators().quorum()));
return span;
}

View File

@@ -1,8 +1,8 @@
#pragma once
#include <xrpld/app/consensus/ConsensusSpanNames.h>
#include <xrpld/consensus/ConsensusParms.h>
#include <xrpld/consensus/ConsensusProposal.h>
#include <xrpld/consensus/ConsensusSpanNames.h>
#include <xrpld/consensus/ConsensusTypes.h>
#include <xrpld/consensus/DisputedTx.h>
@@ -1804,6 +1804,8 @@ Consensus<Adaptor>::haveConsensus(std::unique_ptr<std::stringstream> const& clog
stateStr = "yes";
else if (result_->state == ConsensusState::MovedOn)
stateStr = "moved_on";
else if (result_->state == ConsensusState::Expired)
stateStr = "expired";
span.setAttribute(cons_span::attr::result, stateStr);
CLOG(clog) << "Consensus has been reached. ";

View File

@@ -31,7 +31,7 @@
* +-- consensus.establish [main thread, child]
* | Created: Consensus::startEstablishTracing()
* | Ended: Consensus::phaseEstablish() on accept
* | Attrs: converge_percent, tx_count, disputes_count
* | Attrs: converge_percent, establish_count, proposers
* |
* +-- consensus.update_positions [main thread]
* | Created: Consensus::updateOurPositions()
@@ -166,9 +166,6 @@ inline constexpr auto resolutionDirection = join(xrplConsensus, makeStr("resolut
inline constexpr auto convergePercent = join(xrplConsensus, makeStr("converge_percent"));
/// "xrpl.consensus.establish_count"
inline constexpr auto establishCount = join(xrplConsensus, makeStr("establish_count"));
/// "xrpl.consensus.proposers_agreed"
inline constexpr auto proposersAgreed = join(xrplConsensus, makeStr("proposers_agreed"));
// Avalanche threshold attributes
/// "xrpl.consensus.avalanche_threshold"
inline constexpr auto avalancheThreshold = join(xrplConsensus, makeStr("avalanche_threshold"));
@@ -189,8 +186,6 @@ inline constexpr auto thresholdPercent = join(xrplConsensus, makeStr("threshold_
inline constexpr auto result = join(xrplConsensus, makeStr("result"));
/// "xrpl.consensus.quorum"
inline constexpr auto quorum = join(xrplConsensus, makeStr("quorum"));
/// "xrpl.consensus.validation_count"
inline constexpr auto validationCount = join(xrplConsensus, makeStr("validation_count"));
// Trace strategy attribute
/// "xrpl.consensus.trace_strategy"

View File

@@ -1,6 +1,5 @@
#include <xrpld/overlay/detail/PeerImp.h>
#include <xrpld/app/consensus/ConsensusSpanNames.h>
#include <xrpld/app/consensus/RCLCxPeerPos.h>
#include <xrpld/app/consensus/RCLValidations.h>
#include <xrpld/app/ledger/InboundLedgers.h>
@@ -10,6 +9,7 @@
#include <xrpld/app/misc/Transaction.h>
#include <xrpld/app/misc/TxSpanNames.h>
#include <xrpld/app/misc/ValidatorList.h>
#include <xrpld/consensus/ConsensusSpanNames.h>
#include <xrpld/consensus/Validations.h>
#include <xrpld/overlay/Cluster.h>
#include <xrpld/overlay/ClusterNode.h>
@@ -1963,6 +1963,7 @@ PeerImp::onMessage(std::shared_ptr<protocol::TMProposeSet> const& m)
telemetry::seg::consensus,
telemetry::cons_span::op::proposalReceive));
span->setAttribute(telemetry::cons_span::attr::trusted, isTrusted);
span->setAttribute(telemetry::cons_span::attr::round, static_cast<int64_t>(set.proposeseq()));
std::weak_ptr<PeerImp> const weak = shared_from_this();
app_.getJobQueue().addJob(