Adds `consensus_view_change_total{consensus_mode}` and a `view.change` event
on the round span, both fired from `RCLConsensus::Adaptor::getPrevLedger`
on the same transition-into-WrongLedger edge that already calls
`consensusViewChange()`. The counter is the exact detector for
"consensus disagreed with this node's view this minute"; the event lands the
disagreement on the same trace that carries the round.
`net_ledger_prefix` (16 hex chars) joins `prev_ledger_prefix` on the event,
so a Tempo view of one flap shows both ledger identities on a single line.
`consensus_mode` labels the mode being left (never WrongLedger itself).
Together with the rotation-phase spans, this closes the proof chain a
rotation-driven `full`->`syncing` flap needs — the runbook's step 5
resolves now that the counter and event exist.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
consensus_trace_strategy was read as a std::string and compared against the
literal "attribute" in startRoundTracing(), while the runbook documented
"deterministic" and "random". The documented value "random" therefore fell
through to the default and did nothing.
Parse the setting once into ConsensusTraceStrategy, so the consensus code
branches on a type. The accepted spellings are now "deterministic" and
"random"; anything else fails at startup instead of silently defaulting.
The behaviour behind the old "attribute" name is unchanged and is now
reached by "random".
Document consensus_trace_strategy in xrpld-example.cfg, stating that
"random" is experimental and not used: it gives each node its own trace id,
so one round arrives as one trace per node.
Also state on the tx.included event that it covers the agreed consensus set
before the ledger is built, so it is a superset of the accepted ledger.
startRoundTracing runs as an argument to Consensus::startRound, so it creates
consensus.round before startRoundInternal applies the new mode. Reading mode_
there recorded the previous round's value, and a validator switching from
observing to proposing got a round span labelled observing that nothing corrected.
The attribute is now written in onModeChange, from the mode being applied. All
three MonitoredMode::set paths funnel through there, so round start, a wrong-ledger
switch and a bow-out all correct the parent span with one statement. Every path
reaches it under RCLConsensus::mutex_ on the thread that created the span.
The stale write is removed rather than kept alongside: neither Consensus::startRound
nor startRoundInternal has an early return before mode_.set, so every round span is
stamped. If a future path ever skipped it the attribute would be absent, which reads
as a gap, instead of confidently wrong. onClose also sets consensus_mode, from the
engine's own parameter, and is correct as it stands.
Also tests addEvent's attribute overload on a live span, reading the exported event
name and each value back off the in-memory exporter. It was previously only ever
called on a null guard, so a dropped attribute exported nothing and failed nothing.
Two different facts were sharing one attribute key. The sync-diagnostics
branch already emits validation_status on consensus.validation.accept,
carrying what the validation store did (ValStatus: current, stale,
bad_seq, multiple, conflicting, unknown) with a mapping function and
tests behind it. This branch then added validation_status on
consensus.validation.receive for which exit the receive path took
(queued, dropped_diverged, dropped_load).
One key with two value domains means any aggregation that does not also
filter on span name mixes them. Merging the two branches also produced a
duplicate constexpr declaration in one header, which is how the compiler
surfaced it.
The established accept-path key keeps its name; this one becomes
validation_receive_status, qualified by the span phase it describes. The
values are unchanged.
Three conflicts, all between this branch's own sync-diagnostics work and
phase-10's older versions. Resolved to this branch in each case, since it
owns the newer content:
- InboundLedger.h keeps the missing-node and receive-depth gauges and the
fuller acquire-span contract.
- MetricsRegistry.cpp keeps the namespaced label:: constants.
- LedgerMaster.cpp keeps makeLedgerTraceSpan(), which joins the store and
validate spans into one per-ledger trace by hash.
LedgerMaster.cpp needed a second pass. The automatic merge had kept both
sides outside the conflict markers, nesting phase-10's older promotion
block inside this branch's `if (!pubLedger_)` — so setValidated,
setFull and setValidLedger would each have run twice. Taking this
branch's file wholesale removes the duplicate; brace balance and a single
"Advancing accepted ledger" confirm it.
That resolution drops two things phase-10 was carrying into this file:
the storeSpan/validateSpan guard names, and the explicit scope that keeps
the one-in-256 flag-ledger check outside the ledger.validate measurement.
Both are re-applied on this branch in the next commit; the scope needs a
variable-lifetime check that does not belong in a merge.
consensus.validation.receive is created before the drop decision, so one
span name covered three exits with unrelated cost profiles: two drop
paths that end in microseconds, and a queued path whose handle is moved
into the job and so covers job wait plus checkValidation.
No existing attribute separated them. validation_trusted=true implies
the queued path, but validation_trusted=false spans both drop paths and
the queued path, so any quantile over the span mixed the populations.
Both drop paths are live in the default config, which sets
relayUntrustedValidations.
Adds validation_status, set once on each exit rather than as a default,
following the tx_status precedent in the same file: dropped_diverged,
queued, dropped_load. On the queued path it is set before the handle is
moved into the job.
Both comments pointed at a state the squash merge does not publish.
- Consensus.h: 'yields a null guard, same as before' had no antecedent in the
round or the function. Say instead that a null guard makes the setAttribute
calls below no-ops, which is what SpanGuard's impl_ guard does.
- ConsensusSpanLabels.h: 'Split from ConsensusSpanNames.h' describes a split
performed entirely within this change; both headers first appear here. The
dependency rationale and the diagram are unchanged.
Comments only, no behaviour change.
The three close-time span attributes hold NetClock readings, which are whole
seconds since the XRP Ledger epoch of 2000-01-01 rather than the Unix epoch.
Neither the unit nor the epoch was recoverable from the key, so a consumer
rendering one as a wall-clock time without first adding the epoch offset lands
roughly thirty years early. The sibling close_resolution_ms already named its
unit, so the header disagreed with itself.
close_time -> close_time_ripple_epoch_s
parent_close_time -> parent_close_time_ripple_epoch_s
close_time_self -> close_time_self_ripple_epoch_s
Emitted values do not change; only the keys do. The public RPC response fields
of the same name are deliberately untouched, as renaming those would break the
ledger API.
Two conflicts.
RCLConsensus.cpp: upstream restructured makeAcceptSpan so the accept span's
attributes sit behind if (*span). This branch's own contribution there is the
consensus round-duration histogram, which is kept -- placed inside the
telemetry guard but OUTSIDE the span-liveness test, because a metric must
still record when the trace category is disabled or the span was not created.
The duplicated attribute lines on this side are dropped; the guarded block
upstream added supersedes them.
MetricsRegistry.cpp: kept this branch's JobQueue.h include, which it uses.
Its Journal.h include was dropped as a duplicate -- the file already includes
that header higher up, with a comment explaining why it is unguarded, and
readability-duplicate-include is fatal under WarningsAsErrors.
updateOurPositions built a 64-character hex string from the transaction
id plus two std::to_string number conversions, then attached them to a
span event. That ran for every dispute that flipped position, on every
establish tick of every consensus round, whether or not anything was
recording.
The three strings and the event have no other consumer; the vote change
itself (mutableSet insert/erase) is untouched.
The block now sits behind if (span). With telemetry compiled out the
stub's operator bool is a literal false, so it is eliminated; with
telemetry on it is also skipped when the span is null because the
establish context was never captured. The guard is inside the function
body, so Consensus<Adaptor>'s adaptor interface is unchanged and the
csf::Peer simulator is unaffected.
Brings phase-10 up to 3836078a78 (74 commits). Seven conflicts, resolved
per-hunk; no side was taken wholesale.
validate_telemetry.py, four hunks. The module docstring keeps both category
lists, renumbered. _log_prometheus_metric_names takes phase-10's version: it
returns the family list that the new reverse-coverage check consumes, and
_log_name_list prints every family sorted one per line, which supersedes the
hand-maintained prefix filter this branch had been extending -- that filter
existed only to keep the log readable and listed strictly less. validate_metrics
takes phase-10's _metric_check_targets call. assert_sync_diagnostics_metrics and
phase-10's _check_metric_label both landed at the same place; both are kept.
That third hunk carried the hazard this branch had flagged in advance.
_metric_check_targets selects every group satisfying isinstance(dict) and had no
equivalent of SKIPPED_METRIC_GROUPS, because on phase-10 there was no group that
needed excluding. Merged as-is it would have walked sync_diagnostics while
assert_sync_diagnostics_metrics also walks it, polling and reporting all 61
metrics twice. The exclusion is reinstated inside that function, and its
docstring claim that the isinstance test "selects exactly the same groups the
previous name-based exclusion list did" is corrected -- true on phase-10, false
here, and the reason is ownership, which no structural test can express.
expected_metrics.json: both sides appended to metrics_excluded, so both sets are
kept, 29 entries. expected_spans.json: phase-10's fuller pathfind.compute
skip_reason replaces this branch's, and this branch's ledger.acquire ->
ledger.acquire.astree relationship is kept.
Both docs carried stale counts, and the two sides disagreed with each other --
15 dashboards against 16, and both claiming 41 span types when the contract holds
48. Rather than pick a stale side, every figure is recomputed from the resolved
contract: 48 span types as 28 required and 20 optional, 145 metric checks across
26 asserting categories as 140 names plus 5 required_labels, of which 61 are the
sync_diagnostics names, and 16 dashboards, which matches both the uid list and
the files on disk. The runbook keeps phase-10's table, which adds the reverse-
coverage row.
ConsensusSpanNames.h: both sides added different constants to namespace val;
both kept. Confirmed no identifier is redefined -- the merged file's duplicate
set is identical to this branch's, and those duplicates are distinct namespaces
(op::round against the enclosing span::round), not redefinitions.
ConsensusSpanNames.cpp was an add/add: both branches wrote this file
independently, 9 tests here and 8 on phase-10, with no name in common. All 17
are kept. The guard is dropped rather than applied to the union: SpanNames.h
documents that its constants are deliberately NOT guarded by
XRPL_ENABLE_TELEMETRY, ConsensusSpanNames.h has no guard, and the tests this
branch contributed reference ValStatus only in comments while calling
validationStatusValue with plain ints. So they compile without telemetry, and
unguarding them gains coverage in a -Dtelemetry=OFF build rather than losing it.
One defect belongs to the merge itself, appearing on neither parent. phase-10
added a job_queue_per_type_gauges group holding six jobq_<type>_running/_waiting
names; this branch declared jobq_saturation in MetricNames.h. Rule K checks a
name only when its family is owned, so declaring that constant made jobq_ owned
and turned phase-10's six entries into violations. They are beast::insight gauges
created per job type by JobTypeData's constructor, so no constant can exist for
them -- one triple per job type, minted at runtime. The group joins
NON_OTEL_METRIC_GROUPS alongside statsd_gauges for the same reason.
Verification: no conflict markers repo-wide and no unmerged index entries; both
JSON contracts parse; validate_telemetry.py compiles; every count written into
the docs re-derived from the resolved files and matching; span counters still 48
and 74; check_otel_naming.py exits 0, and Rule K proven still able to fail by
injecting a bogus name in an owned family; levelization baseline clean after
regeneration, with 17 incoming include changes; doxygen style clean across all
tracked C++ at CI scope; pre-commit --all-files clean except cargo-fmt, which
reports "Executable `cargo` not found" and touches none of the 0 Rust files here.
NOT compiled -- no approval to build, so the incoming C++ is unverified by a
compiler on this branch.
Splitting the consensus span labels into their own header moved both
constexpr std::string_view helpers out of ConsensusSpanNames.h, but left
the include they needed behind. The header names string_view nowhere now,
and SpanNames.h already provides the type for the conversion operator.
clang-tidy misc-include-cleaner reports this as an error under
-warnings-as-errors, which fails the clang-tidy job for the whole chain.
Review of the preceding commits found a clang-tidy failure and a convention
break, both rooted in the same place: the enum-to-label helpers were put in
ConsensusSpanNames.h, which pulled two domain headers into it.
misc-include-cleaner rejected the new test: it used xrpl::LedgerCloseReason
without directly including ConsensusTypes.h, relying on the transitive
include. misc-* is enabled and this path is not in IgnoreHeaders, so it would
have failed CI.
ConsensusSpanNames.h had also become the only one of the eight *SpanNames.h
headers to include anything beyond SpanNames.h. That cost is paid by every
consumer: PeerImp.cpp, ConsensusReceiveTracing.h and RCLConsensus.cpp want
only name and key constants, but were newly compiling ConsensusTypes.h and
DisputedTx.h through it.
Move both helpers to a new ConsensusSpanLabels.h, which owns the domain
includes. ConsensusSpanNames.h is dependency-free again like its siblings, and
the labels reach their only production caller, Consensus.h, directly.
Also from the review:
- phaseOpen() had grown to 81 lines, over the 80-line limit. Extract
annotateOpenStart() and annotateOpenClose(), which also removes the repeated
span guards. phaseOpen is 72 lines; startRoundInternal drops 103 to 93,
still over the limit but it was 99 before this work began.
- Note at the CLOG why the log text keeps the shouldCloseLedger name: existing
consumers match on it.
- whyCloseLedger's doc claimed "both log identically", implying the wrapper
logs too. It delegates, so the logging happens once either way.
- Cross-reference proposers_validated and proposers_finished, which sit eight
lines apart and count different things: validators of the previous ledger
versus those already past it.
- The two static_asserts no longer sit inside TEST bodies with SUCCEED(); they
fire at compile time regardless. Also "consteval-safe" was wrong; they are
constexpr.
- SpanGuardFactory.cpp claimed a libxrpl test cannot include the consensus
span-name header. The new test in the same directory does exactly that, so
the claim is corrected to name the real constraint: the rpc_* constants it
needs live in an xrpld-level header.
Three defects found in review of the two preceding commits.
Drop disputes_count_initial. It claimed to be the dispute count carried in
from the positions held at close, but startEstablishTracing() runs a full
timer tick after closeLedger(): timerEntry() dispatches
`if (phase_ == Open) phaseOpen(); else if (phase_ == Establish)
phaseEstablish();`, and phase_ was Open on the closing tick, so the else-if
cannot run. With ledgerGRANULARITY at 1s the value absorbed up to a second of
dispute growth from peer proposals and arriving tx sets. Making it honest
needs either a member captured at close or moving span creation into
closeLedger(), so it is removed rather than shipped mislabelled.
Record close_time_avalanche_state on recovered rounds. startRoundInternal()
reset establishSpan_ inline, discarding the span before the attribute was
written, so the value was present only on rounds that reached Accepted --
survivor bias in exactly the rounds worth investigating. It now calls
endEstablishTracing(). The comment claiming this avoided "reporting a stale
regime" was wrong: closeTimeAvalancheState_ is not reset until 39 lines
later, so the value was still that span's terminal regime.
Rename avalanche_state to close_time_avalanche_state. DisputedTx carries a
second, per-transaction avalanche tracker; the bare name invited reading a
close-time-only value as the transaction one, which is the tracker that
actually escalates in a stuck round.
Also: both label helpers now fall through to "unknown" instead of a
plausible-looking regime, matching to_string(ConsensusPhase); and the header
now records that the end-of-open attributes are absent on recovered and
simulated rounds, and that tx_sets_acquired can skew either way because
handleWrongLedger clears currPeerPositions_ but not acquired_.
Tests: the minimum-open-time assertion used prevRoundTime=10s, where
openTime=1s trips the too-fast branch as well, so deleting the ledgerMinClose
check entirely left it green. Replaced with prevRoundTime=2s, which isolates
the branch. Added the others-closed boundary, which is strict and was
untested in either direction, its integer truncation for odd prevProposers,
and its precedence over the no-transactions and minimum-open branches.
The open phase ended for one of four distinct reasons, but
shouldCloseLedger() collapsed them into a bool, so a trace could say when a
phase ended and never why. "The network closed without us" and "nothing was
waiting" are the same span today.
Add whyCloseLedger(), which holds the decision and returns
LedgerCloseReason. shouldCloseLedger() keeps its exact signature and becomes
a one-line delegation, so its callers and unit tests are untouched and the
branch logic is not duplicated. phaseOpen() calls whyCloseLedger() directly;
both emit the same journal and CLOG output, so only one is called.
New attributes on consensus.phase.open, both set once on the closing tick:
close_reason anomaly | others_closed | idle | normal
proposers_validated trusted peers that had already validated the prior
ledger, reusing the value the decision was made on
Absent on the simulate() close path, which bypasses the decision rather than
having a reason invented for it.
Skipped has_open_transactions: hasOpenTransactions() is
!getOpenLedger().empty(), which is false on a quiet network for most of a
round, and close_reason=idle already implies it. The sibling
consensus.ledger_close span carries tx_count_open, which is the same fact
with a count instead of a boolean.
shouldCloseLedger() now has no production caller; it stays exported so the
public API and its tests are unchanged.
Tests pin every input vector from should_close_ledger to its literal reason,
including that the anomaly check outranks others-closed, and cover the
inclusive idle boundary either side by one millisecond.
consensus.phase.open and consensus.establish carried almost no state of
their own. Span attributes are not inherited, so the ledger context on the
parent consensus.round span does not describe either child, and the few
attributes the establish span did carry are rewritten on every iteration
and therefore only ever report the final value.
Add seven attributes that are read from state already in scope, are
written exactly once, and are not duplicates of the parent round span:
consensus.phase.open (start)
start_reason initial, or recovered on a handleWrongLedger
re-entry, which emplaces a SECOND phase.open
span under the same round
previous_close_agree feeds the sinceClose branch in phaseOpen()
peer_positions_at_open positions in hand after playbackProposals(),
the head start the round began with
early_close_triggered the round skipped the timer because enough
peers had already closed
consensus.phase.open (end)
tx_sets_acquired candidate tx sets held at close, read before
our own position is added; a low count against
a high peer_positions_at_close means tx-set
fetches did not land, not disagreement
consensus.establish (start)
disputes_count_initial disputes carried in from the positions held at
close, as opposed to disputes_count, which is
overwritten each iteration
consensus.establish (end)
avalanche_state terminal close-time convergence regime; the
derived avalanche_threshold is a weight and
cannot be inverted back to the state
The avalanche label is mapped by a new constexpr avalancheStateLabel() in
ConsensusSpanNames.h rather than an inline switch, so the four labels stay
under the naming check's L1 ownership and are unit-testable.
Deliberately not added: ledger_seq and consensus_mode, which would only
copy the parent round span's values down; tx set size and position hash,
which the TxSet concept does not expose portably across RCLTxSet and the
csf simulator; and the peer-unchanged and dead-node counters, whose
underlying state is reset mid-round and so would report a misleading value.
Behaviour is unchanged. The early-close condition is hoisted into a named
local so the annotation happens before timerEntry(), which can reach
closeLedger() and end the open-phase span.
Tests pin the wire strings for every new key and value and cover all four
enumerators of the avalanche mapping. They need no telemetry runtime: the
csf simulator returns an invalid round span context and a null Telemetry,
so consensus spans there are null guards and attribute writes are no-ops.
* upstream/release/3.3.x: (41 commits)
chore: Bump version to 3.3.0
chore: Bump version to 3.3.0-rc7
fix: Increase manifest protocol message size cap and fix manifests relay
fix: Cap untrusted manifests per message and drop oversized ones
chore: Bump version to 3.2.1
chore: Bump version to 3.2.1-rc1
fix: Cap untrusted manifests per message and drop oversized ones
fix: Reject oversized validator manifest before decoding
fix: Reduce untrusted manifest cache cap to 100
fix: Bound untrusted manifest cache
chore: Bump version to 3.3.0-rc6
feat: Package validator-keys inside rippled
chore: Bump version to 3.3.0-rc5
fix: Switch SponsorshipSet to use a delta for sfFeeAmount
fix: Re-revert "fix: Set request size limits and differential pricing for get-object-by-hash calls"
chore: Bump version to 3.3.0-rc4
fix: Revert "fix: Set request size limits and differential pricing for get-object-by-hash calls"
chore: Bump version to 3.3.0-rc3
fix: Reduce untrusted manifest cache cap to 100
fix: Revert "fix: Reject oversized SHAMap nodes in gotStaleData and fetch-pack path"
...
Phase-10 brought in the upstream nodestore/peerfinder/consensus reorganisation
along with its own write-path telemetry, which collided with the sync-diagnostic
signals on this branch. Twelve files conflicted; every resolution keeps both
intents rather than picking a side.
The nodestore write timing was implemented twice, independently. Both sides
added getStoreDurationUs()/getFetchDurationUs() to Database and both timed the
backend call in each concrete store(). Keeping both would have added twice to
storeDurationUs_ per store while storeStats() still counted one, so the mean
write latency would have read double on every dashboard -- silently, since no
test on either side asserts an exact microsecond figure. Resolved to one
accumulator API: recordStoreDuration(), which takes a duration, clamps a
sub-microsecond sample to zero and uses a relaxed atomic add. Phase-10's
storeDurationStats() is gone and its two call sites now use the survivor, so
all three store paths -- both store() overrides and importInternal() -- add
exactly once.
SlotCensus and its pure virtual moved from src/xrpld/peerfinder/ to
include/xrpl/peerfinder/PeerfinderManager.h, following the Manager interface
upstream relocated. The xrpld header is now phase-10's makeConfig shim, and
Overlay.h, MetricMacros.cpp and the getSlotCensus() override chain point at the
new location. ConsensusSpanNames.h and peerfinder Slot.h/Config.h include paths
followed their headers into libxrpl the same way.
InboundLedger gained phase-10's AcquireStats counters next to this branch's
span activations in both the destructor abort path and done(); neither
displaces the other. nodestore_state keeps the constant-based name this branch
requires of it and phase-10's fuller description.
Upstream #7292 deleted src/test/nodestore/Database_test.cpp, which held this
branch's testDurationAccessors. Phase-10 restored the per-store half of that
coverage in DatabaseConfig_test, but nothing covered importInternal -- it writes
through storeBatch() and never through store(), so it is a third store path that
has to time itself. That half is ported to a GTest in
src/tests/libxrpl/nodestore/Database.cpp, keeping the exact zero-before and
accumulate-after assertions and the per-instance negative check.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The consensus headers moved into the isolated xrpl/consensus module and
took a dependency on xrpl/telemetry for the tracing span constants, but
two things were left behind:
- Four includes still pointed at the old src/xrpld/consensus/ location,
which no longer exists, so the build failed with
"fatal error: 'xrpld/consensus/ConsensusParms.h' file not found".
- xrpl.libxrpl.consensus never linked xrpl.libxrpl.telemetry. add_module
isolates each module's headers, so xrpl/telemetry/SpanNames.h was not
on the include path even once the include was repointed.
Repoint the stale includes at xrpl/consensus/, and declare the telemetry
module before consensus so consensus can link it. Regenerate ordering.txt
for the resulting edge.
