expected_spans.json: composed both sides. Kept this branch's 16 extra spans,
relationships and the per_ledger join group; took phase-10's allowed_parents
gate for the shared entries and derived allowed_parents for the 16 from their
factories. ledger.store, ledger.validate and ledger.acquire are hashSpan roots
on this branch, so they list ROOT alone rather than consensus.accept.apply.
The accept.apply factory no longer falls back to the round context, so the
harness contract lists consensus.accept alone. Note rewritten to describe the
scoped child as it is now.
The contract's parent field was documentation only, so no check could fail a
span for being parented when it should be a root. A live sweep found three
spans nested one level off while every harness check passed. The field becomes
allowed_parents, a list derived per span from its creation factory and every
call path that reaches it, and is asserted per span.
A parent on another node and a parent absent from the trace are both
inconclusive rather than violations: the receive spans are parented cross-node
by design, and a rotation in flight has not exported its root yet. Spans
reached by two call paths list every lawful parent.
A second gate reads a round as a unit: every required phase child under the
round span on the same node, their start times in protocol order, and no
mode_change recording mode_old == mode_new. It selects traces that already hold
the accept span, since that span always ends after the round span and can land
in a later export batch.
num_cleared leaves txq.batch_clear's required attributes: the code writes it
only after the batch actually clears.
expected_spans.json kept both sides' relationship entries: this branch's
eight nodestore.rotate phase children and the upstream branch's three
cross-node pairs. Both sides appended at the same list position.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The harness checked parentage only within a node. Every declared
relationship named two spans from one process, so a receive span that
lost its cross-node parent still satisfied the contract: it is created
either way, with all its normal attributes, and only its parent differs.
Declare the three pairs that cross a node boundary, so a send side that
stops propagating context fails the run.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The runbook's nodestore_state table kept AppMetricGauges.cpp as the source
column, which is where those gauges are actually registered on every branch
in the chain; the incoming side named MetricsRegistry.cpp, which registers
none of them. Took the incoming side's clearer wording for the sweep row.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Both workload scripts decoded JSON and went straight to .get() or .items().
An array or a scalar decodes fine and then raises AttributeError, which is not a
ValueError, so the handler around the weights parsing could not catch it and the
operator saw a traceback naming neither the command nor what arrived.
Four sites across the two files, all four guarded, and the weights handler now
also catches TypeError for a non-numeric weight value.
Negative weights are left alone: they do not raise, the type is dropped from the
mix silently, and changing that changes accepted input.
The sanity bound required the value to be strictly positive, which rejected
three of the four readings the gauge can legitimately produce: a negative
count once the validator list has expired, the -1 sentinel for "no published
list fetched", and +inf for a config-listed list that never expires.
The negative case is the one that matters. It is the signal that expiry has
already passed, so the gauge deliberately does not clamp at zero, and a gate
that rejects it would fail exactly when an operator most needs the reading.
The -1 sentinel already violated the bound and had simply never been hit,
because the validation cluster always fetches a published list.
The floor is now a century, which still catches a broken clock. Detecting the
unsigned wrap this bound used to hide moves to the MetricsRegistry::daysUntil
unit tests, which are deterministic and do not need a running cluster.
The list mirrors the collector's spanmetrics dimensions, and that dimension is
gone. The reduce-relay and squelch counters keep their own names, which are a
different thing.
tx.receive no longer carries a suppressed attribute: the span is created only
once the node has decided to process the transaction, so there is no dropped
copy for the attribute to describe.
The validator fails a span that is missing a required attribute, so leaving it
listed turns the telemetry-validation leg red.
The previous commit switched the generated node config from [ips] to
[ips_fixed] on the grounds that the variable, the comment and the sibling cfg
template all named ips_fixed, and that ips_fixed is the section whose
documented meaning fits a private cluster. Both of those are still true. The
switch is reverted anyway, because it is a workload change rather than a
naming fix.
Measured on CI, parent commit against this branch's previous tip, one
functional config line apart:
span.consensus.ledger_close.p95 0.57 ms -> 6.43 ms (tripped the gate)
span.consensus.ledger_close.p99 0.94 ms -> 9.50 ms
span.consensus.accept.p50 0.97 ms -> 2.63 ms
span.tx.process.p50 0.36 ms -> 0.18 ms (faster)
job.acceptLedger.running.p95 21157 us -> 10938 us (faster)
Every consensus-path span rose and every transaction-path metric fell, which
is the shape a denser always-connected mesh produces and not the shape of
run-to-run variance. [ips_fixed] holds connections open to all four peers
instead of treating the list as a discovery hint, so each node processes
proposals and validations from the full mesh every round. Nothing else in that
commit touches the consensus path: the emitted config differed in exactly
three lines, of which one is a die message and one expands to an identical
string.
The committed baseline describes the [ips] topology. Adopting [ips_fixed]
therefore needs a refreshed baseline and re-derived bounds, which is the
process baselines/README.md already documents for a workload change. Left as
its own work item rather than smuggled in behind a section rename, and the
reason is now recorded beside the line so it is not repeated.
This also falsified a claim the previous commit had written into
baselines/README.md and regression-thresholds.json: that none of the six
weakly-guarded keys fires on any observed run. Corrected in both, and the
measurement above is cited in place of the absolute.
The harness killed and probed node directories named `node<N>`, but the
directories it creates are `validator-<N>` in run-full-validation.sh and
`bench-node-<N>` in benchmark.sh. Verified with pgrep against processes whose
command lines mimic the real ones: the pattern matched nothing either script
produces. Three consequences, all live:
- `--cleanup` deleted the workdir and left the xrpld processes running. They
are host processes, so the compose teardown does not reach them.
- The pre-run cleanup could not free the previous run's RPC, WS and peer
ports, which surfaces much later as a cluster that never reaches consensus.
- The startup crash fast-fail read a pid path that never exists, so its
`stopped > 0` branch was unreachable and a dead node waited out the full
120-attempt window.
Rather than patch four literals, derive every node path, kill pattern and log
glob from one NODE_PREFIX per script. The directory name is also the node's
identity: the collector's file_log receiver lifts that segment into
service.instance.id, so the directory and the [telemetry] service_instance_id
must agree. Deriving both from one value is what stops them drifting again.
Also in the same files, each confirmed by test rather than inspection:
- The collector readiness probe could never fail. curl -w '%{http_code}'
prints 000 on a refused connection and then exits non-zero, so the
`|| echo 000` inside the substitution appended a second 000 and the
"not ready" comparison never matched. Move the fallback outside.
- The generated config wrote [ips], the starter-list section. A loopback mesh
that must reach quorum is the [ips_fixed] case, which is what the variable,
the comment and the sibling cfg template already said.
- benchmark.sh returned exit 1 for a row it could not measure, though the
exit-code table reserves 1 for "every metric was measured and one breached".
Report 2 there instead.
- Five bc computations fell back to 0, which clears every threshold. The
guards beside them already fall back to the inconclusive token; these now
do too.
- A comment claimed a `|| guard` after a heredoc lands in the heredoc, and
that claim had removed a real guard from the config write. It does not: the
guard runs, and fires when cat fails.
- The EXIT trap was installed 88 lines before stop_workload was defined. If it
fired in that window, errexit aborted the handler on "command not found" and
the cluster reap never ran. Install it below both handlers.
- jq exits 5 on malformed JSON, outside this script's documented codes, so
read_metric now routes that through cannot_measure.
- --nodes and --duration were unvalidated, and --nodes 0 made the pid-count
guard compare 0 with 0 and pass, handing the sampler no pids at all.
- --cleanup now passes -v so the named tempo-data volume goes with it.
Otherwise the next run's Tempo still serves the previous run's traces and a
span assertion can be satisfied by them.
- Five messages reported an attempt count as seconds, though each attempt is
a sleep plus every node's probe.
The baselines README and the two regression JSON files had gone stale when the
baseline was refreshed to a three-run median: they described 20 gated keys and
five exclusions, against an actual 19 and six, and cited the superseded run,
date and commit. Re-derive every affected figure from the committed files. The
detection floors are recomputed (2.00x to 7.41x, so a 10x regression is now
caught on all 19 keys), the newly excluded span.ledger.build.p99 is documented,
and figures that no committed artifact can verify are either replaced with
derivable ones or labelled with their numerator.
No baseline value, threshold bound or derivation entry changes.
Review follow-up on the freshen lock-hold fix:
- Drop host names, dates and one-site figures from the new comments,
harness notes and docs; explain the mechanism in general terms.
- RotationPhase stores its stage and cache labels as owned std::string,
not std::string_view: the ctor still takes views so the label
constants pass without a copy, but a member view would dangle if a
caller ever passed a temporary. freshenCache/recordFreshen take the
cache name by std::string_view (read-only, call-scoped).
- The new DatabaseRotating test called fetchNodeObject through the
derived type, whose private override hides the public base method;
call it through Database& instead. This was the dev-box build break.
- freshenCache reports the exact fetched count when a health abort cuts
it short, and stops labelling the per-partition hold 'getKeys'.
- Remove a [[maybe_unused]] that silenced no warning (the build sets
-Wno-unused-parameter and disables misc-unused-parameters).
The reference doc, span-harness notes and histogram-bucket comments
named the internal AWS dev box and dates while explaining why the
rotation phases are timed. Reword to the general mechanism (a
multi-second freeze at the copy-walk to freshen boundary on a populated
node); the specific hosts, dates and trace ids stay in the task notes.
The online-delete rotation's cache freshen called TaggedCache::getKeys(),
which held the cache mutex while copying every key. On the dev box's 26
million entry tree-node cache that hold lasted 5-6 s, froze every job
that touches the cache, and dropped the RocksDB node out of sync once per
rotation: each "getKeys held the lock" warning was followed within 1-5 s
by "View of consensus changed" (5 of 5 rotations on 2026-09-15).
Copy the keys one map partition at a time instead. TaggedCache gains
forEachKeyPartition(), which holds the mutex only while one partition's
keys are copied and runs the callback with the mutex released, so the
longest hold shrinks by the partition count (8 on the dev box). The
freshen.keys rotation phase no longer exists as one step, so its span,
stage value, harness entries and docs are removed; the per-partition hold
still shows on the cache lock-hold peak gauge.
Measure what the freshen achieves, which no existing signal did.
DatabaseRotating gains duplicateCopyForwardTotal(), counting archive
copies made on duplicate fetches (the rotation's own copy walk and
freshen); copyForwardTotal() deliberately excludes those. The freshen
phase records rotation_freshen_keys_total{cache,outcome} and stamps
key_count, cache and keys_copied on its span; the copy phase stamps
nodes_copied. A warn log line per freshen reports the same numbers, and
the ledger-sync-health dashboard gets a Rotation Freshen Yield panel.
Log the "STATE->" operating-mode change at warn instead of info. It is
the only record of a mode change with an exact timestamp; the
state_changes_total counter is scraped once a minute and cannot order a
flap against a multi-second event.
Tests: five GTests for forEachKeyPartition (every key once, empty cache,
mutex free during the callback, concurrent insert, lock-hold peak), three
for duplicateCopyForwardTotal over two memory backends, one for the new
counter's series, and the new name literals.
Resolutions:
- MetricsRegistry.cpp: keep both <exception> and <limits>; drop
incrementStateChanges(), which this branch removed on purpose (the
labelled state_changes_total call site in NetworkOPsImp::setMode
replaces it, and a compile-time test guards that).
- tests/MetricsRegistry.cpp: constructor-built pipeline wording from
phase-10, this branch's test list and gauge paragraphs kept; the two
lifecycle tests now call startAsyncGauges() and pass kTestOptions.
- tests/MetricMacros.cpp: comments name the recording() gate.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
CI's clang-tidy leg flagged eight include-cleaner errors and three
misc-const-correctness / readability-convert-member-functions-to-static /
modernize-use-designated-initializers issues, all inside WP-B6's own code.
Fixed as follows:
- `MetricsRegistry.h`: `#include <opentelemetry/metrics/observer_result.h>`
for ObserverResult; `observeCacheLockHoldPeaks` is now `static` because it
touches neither instance state nor telemetry members.
- `SHAMapStoreImp.h`: adds direct includes for `<cstddef>`, `<string_view>`
and `<xrpl/telemetry/SpanNames.h>` (the StaticStr provider). `seconds` in
`RotationPhase::~RotationPhase` is `[[maybe_unused]]` so a
`-DXRPL_ENABLE_TELEMETRY=0` build under `-Werror` keeps compiling.
- `SHAMapStoreImp.cpp`: direct includes for `SHAMapStoreSpanNames.h`,
`SpanGuard.h`, `SpanNames.h`; `RotationPhase` locals that never call
`setAttribute` are declared `const`; `RotationOutcome` uses designated
initialisers.
Final-review findings (WP-B6-rotation-stall-tracing.md, "What to check
when reviewing"):
- Panels 74 and 75 on `ledger-sync-health.json` still carried panel 41's
description, axisLabel, Source and Keywords copy; rewritten to describe
rotation phase duration and cache lock hold respectively.
- `consensus_view_change_total` and the `view.change` round-span event
were emitted but not registered with the harness. Added the counter to
`not_asserted.metrics_excluded` (workload-gated) and annotated the
`consensus.round` span note with the event and its two attribute keys.
Not fixed (parked, see progress ledger):
- The reviewer's second Important finding — a plan/code contradiction on
the consensus counter — was based on a misread of the plan; the plan's
"Rejected alternatives" table lists a new `TraceCategory::Nodestore` and
the getKeys() fix, not the consensus counter. No action.
- The Minor note about `sweep()`'s peak including lock-acquire time and
`getKeys()`'s not: `sweep()` acquires and releases the lock via a
`scoped_lock`, so `noteLockHold` still runs after the release and the
numbers are comparable. No action.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds `nodestore.rotate` and its eight phase children to expected_spans.json,
all `optional: true` because the 5-node localhost harness cluster never reaches
`online_delete`. Their parent-child relationships are asserted but skip-marked
so a run without a rotation stays green.
Adds `cache_metrics{metric="treenode_lock_hold_peak_us"|"fullbelow_lock_hold_peak_us"}`
to the asserted sync_diagnostics group -- both are observable and always emit,
even at zero. Puts `rotation_phase_duration_seconds` and `jobq_stall_total` in
`not_asserted.metrics_excluded`; both are workload-gated.
On the Cloud collector, adds an `ottl_condition` policy that keeps any trace
carrying a span whose name matches `^nodestore\.rotate`, so the 0.5% probabilistic
tail sampler cannot drop a rotation trace. Sampler is OR'd across policies.
The committed baseline was captured 2026-08-26, before the account-funding
race was detectable. Phases whose funding silently failed submitted no
transactions, so the capture recorded artificially low ledger and transaction
timings, and job.transaction.queued.p95 and job.transaction.running.p95 could
not be captured at all. Once funding worked, span.ledger.build.p99 read
29.00 ms against a 9.11 ms baseline and turned the gate red on a run whose
200 span and metric checks all passed.
Refresh every value to the median of CI runs 34495527952, 34505215266 and
34507425933, the first three with the fix in place, and re-derive each
absolute bound as hi_next - baseline from that median.
Exclude span.ledger.build.p99. Across those three runs it read 29.00, 7.06
and 8.94 ms, a 4.11x spread whose maximum is 1.16x its 25 ms trip point, so a
healthy run reddens CI. Widening cannot fix it: a bound tolerating 29.00 ms
would reach into the bucket above and restore the single-crossing false
positive the derivation rule removes. span.ledger.build.p95 stays gated at
0.48 of its trip point, so ledger construction keeps coverage.
The other 19 keys sit between 0.17 and 0.76 of their trip points.
span.tx.process.p95 is the tightest and is the first to re-measure if the gate
reddens again.
Repoint one bounds-checker test at span.ledger.build.p95, since it mutated the
p99 override this commit removes.
Account setup submitted the funding Payments, slept a flat 10 seconds, then
read each sequence once. The txq-burst and mixed-peak phases escalate the
open-ledger fee on purpose, so the funding transactions were queued, every
account read Sequence 0, and the phase aborted with "only 0 of 8 created
accounts were funded". The run then reddened on a workload gate rather than on
anything telemetry had done.
Poll the ledger until each account has a sequence, with a deadline, so a late
confirmation is still seen and a healthy cluster pays no waiting cost. Pay a
multiple of the current open-ledger fee, so funding is not queued behind the
load a phase creates deliberately. terQUEUED no longer marks an account funded:
only a ledger read does.
Retry the accounts that never confirmed, once, after re-reading the genesis
sequence from the ledger. consumes_sequence advances the local counter on
terQUEUED, so a dropped funding transaction leaves it ahead of the ledger and
every resubmit would otherwise land on a future sequence.
The funding wait can run twice, so raise the orchestrator's grace above twice
the timeout. A test pins that relationship, since the two constants live in
different files.
Also save each generator's full stdout and stderr beside its JSON report. Only
the last 200 characters of stderr reached the phase error and stdout was
dropped, so none of the per-account funding results appeared in CI.
The collector reads the per-node directory off the log file path and stamps it
as the Loki label service_instance_id, so the directory name has to equal the
node's own [telemetry] service_instance_id or log lines carry a node name that
no trace or metric shares and nothing joins.
Both harness scripts disagreed with themselves: run-full-validation.sh wrote to
node$i while setting validator-${i}, and benchmark.sh wrote to node$i while
setting bench-node-${i}. Rename the directories to match the ids rather than
the reverse, so no existing trace or metric label value moves and no harness
expectation has to be re-checked. Only path references are renamed; the
human-readable "node$i" in log and error messages is left as prose.
The config template is not rendered by any script, so its DATA_DIR
documentation gains a note about the same constraint instead.
Also rename the deprecated otlphttp/filelog collector component names in the
harness scripts and docs.
The workload README contradicted itself on --skip-loki: one bullet said CI always
passes it and so the two log-correlation checks are never exercised, another said
the workflow no longer passes it. The workflow mentions the flag nowhere, so the
first was the stale half.
Other claims checked against the tree and corrected:
- both the README and the plan doc described the push trigger as filtered on
branch names. The workflow has no branches filter, deliberately, because
GitHub ANDs branches with paths
- the plan doc printed 6 of the workflow's 12 paths globs, and claimed the
workflow was 367 lines against an actual 451. The glob block is now generated
from the workflow, and the line count dropped rather than restated
- rpcNOT_SUPPORTED does not exist anywhere in the tree. The symbol is
RpcNotSupported, and the refusal sites are RipplePathFind.cpp:59-60 and
PathFind.cpp:50-51, not :48-49 and :39
- RCLConsensus.cpp:666 and :663 are not log or event lines; the tx.included event
is at :720 and the per-transaction debug log at :715
- LedgerMaster.cpp:463 is fixIndex, not the ledger.store span, which is at :470
- ServerHandler.cpp:705 is inside makeJsonError; processRequest is at :718
- file counts: docker/telemetry/workload/ is 25 files, include/xrpl/telemetry/ 13
- the optional-span bullet named five causes covering 10 of 16 entries, omitting
the txq.* family and the WebSocket handshake
- the /api/v1/series choice was attributed to stale StatsD gauges; this harness
runs no StatsD
A line number in run-full-validation.sh was cited in five places and drifts on
every edit to that file, so those now name the file only. The keygen helper's
header records what production does instead -- validator-keys-tool create_keys
then create_token, keeping the master key off the node -- and why a disposable
cluster does not.
The four external-parity bounds checks each ran a single Prometheus instant query
and failed on an empty result. The metric checks that run earlier poll
/api/v1/series, which returns a series regardless of staleness, but a bounds
check needs the sample value and so cannot use that endpoint. This file's own
docstring records the consequence: a beast::insight gauge that stops changing can
fall out of an instant query while /api/v1/series still returns it, so one
attempt is not enough to call the series absent.
Poll to the same deadline the metric checks use. A Prometheus error is raised
rather than retried, because a rejected query never becomes valid and retrying it
only burns the full timeout.
compare_to_baseline took the unit from the baseline entry and dropped the current
run's, and nothing compared the two, so a us -> ms change was scored as a numeric
delta: four keys rewritten to the same physical durations reported 99.9%
improvements and the gate exited 0. prom_queries.py says the baseline preserves
the unit "so the comparator can sanity-check unit drift"; it never did. A unit
mismatch now fails and names both units.
The workflow's step summary printed total, regressions and improvements. total is
every key in the report -- the union of baseline and current -- so it was neither
the baseline count nor what was gated, and missing_in_current was computed and
never printed. A run that gated 16 of 20 keys read as a full comparison. The
comparator now reports a real "compared" count and the summary prints it beside
the not-captured count, with a warning when any key was missed. The table also
refused nothing on a truncated report; existence is not readability.
check_regression_bounds told the operator to add max_abs_increase while reading
max_abs_increase_ms / _us, so following the message added a key nothing reads and
the gate kept failing with no explanation. The committed thresholds use only the
suffixed spelling, so the message was the defect. Its three JSON inputs were also
unchecked: a top-level null, list or number parsed and then died on the first
.get, and a string "metrics" survived the placeholder test and reported its own
characters as gated keys -- wrong advice rather than a crash.
Four tests cover these; all four fail against the previous checker.
The overhead benchmark generated no workload. Each arm was start_cluster ->
collect_metrics -> stop_cluster, and collect_metrics only ran the sampler, so the
only client traffic was the sampler's own server_info probes at under
1 request/sec. The hottest instrumented paths -- tx.*, txq.*, the transactor
stage spans, every rpc.command.* other than server_info -- were never entered,
which is where per-operation span cost appears. Both arms now drive
rpc_load_generator and tx_submitter at one fixed rate for the whole window, over
a [port_ws] listener present in both arms so the listener is not part of the
delta. A flat rate rather than a workload profile, because both arms must issue
the same work and a profile's phase shaping only adds variance.
The sampler also selected xrpld host-wide. run-full-validation.sh leaves its five
validation nodes running while the benchmark's three start, so both arms averaged
eight processes -- diluting the CPU delta and making memory_rss_mb_peak report a
validation node either way. It now takes an optional pid list, and the benchmark
passes its own nodes' pids and refuses to measure if it cannot collect them all.
consensus_round_mean_ms counted distinct ledger sequences seen by a loop that
sampled every 5 s, so it read back 5000 ms for every close time from 2 s to 5 s
and a 10% regression measured 0%. Sampling at 2 s -- the close-time floor from
ConsensusParms.h:93 -- resolves a 10% regression as at least 9.3%. It also
divided by the requested DURATION rather than the measured ELAPSED, which the
TPS calculation in the same file already used.
Key generation, the workdir setup and the seed read exited 1 under errexit, the
code this script reserves for a measured threshold breach, so an infrastructure
failure was reported as "telemetry is too expensive". They map to cannot_measure
now. No guard is added after the config heredoc: a guard there is read as the
heredoc's first line, lands in the generated config and never runs.
curl probes across the harness had no --max-time, so a server that accepts the
connection and then stops answering blocks forever and the loops' attempt counts
stop bounding anything.
Three ways a run could produce no traffic and still report success:
- tx_submitter logged a funding shortfall and returned an empty stats object;
main() then printed the summary and exited 0, so the failure only surfaced
later as "spans missing", which points nowhere. It now records setup_failed in
the summary and exits 1 after the report is written.
- --weights was checked for valid JSON but not for a positive sum. An all-zero
mapping reached random.choices, which raises ValueError from inside the
dispatch loop where only CancelledError is caught. Rejected at parse time now,
in both generators.
- a profile phase declaring neither rpc nor tx logged a warning and returned no
error. Both error rates short-circuit to 0.0 when nothing was sent, so a
mistyped key produced zero traffic and still passed the exit gate. That phase
is now an error.
The close-time span attributes name their unit and epoch:
close_time_ripple_epoch_s, parent_close_time_ripple_epoch_s and
close_time_self_ripple_epoch_s. The harness inventory still required the
unsuffixed keys, so the attribute checks for consensus.accept.apply and
ledger.build failed on every validation run while the spans themselves
were correct.
Rename the four required_attributes entries to the keys the code emits.
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.
Notes across the workload contract described earlier versions of themselves, or
cited commits that only exist inside this chain. A squash merge publishes none of
it, so each reference resolves nowhere.
Notes that described their own earlier text:
- expected_spans.json: 'this note previously concluded', 'this note previously
said', 'Un-skipped 2026-08-26', 'the reason had simply gone stale for two
weeks' and 'the claim this entry carried' are replaced by the standing reason
each entry holds. The wildcard pairs now say the validator globs the child via
_span_name_matches(), and state the literal-collapse failure as what a
different validator WOULD do rather than as history.
- regression-thresholds.json: 'an earlier version of this note wrongly claimed',
'the earlier version oversold it' and 'an earlier note called that' become the
cautions themselves -- do not reason from 'every ladder step is at least 2x',
do not oversell the backstop, do not read a false fire as a missing override.
- test_check_regression_bounds.py: the docstring gives the reason a literal is
wrong here, not the story of two tests that once hard-coded one.
Baseline-refresh history rewritten as measurement:
- README.md, baselines/README.md, telemetry-runbook.md and regression-metrics.json
no longer attribute threshold moves to 'the 2026-08-26 refresh'. The evidence
is kept as measurement -- span.tx.apply.p50 has read 0.7917 ms and 0.00597 ms
on the same workload, 132x apart; job.acceptLedger.running.p95 has measured a
5.74x floor on one baseline and 16.28x on another -- which is what supports the
claim that a single-run baseline cannot bound these keys.
Two chain-only commit ids removed, d059f21bf3 and 3860c93db2. Neither is
reachable from develop, so both cease to exist on merge; the second is chain
bookkeeping. The facts they were cited for (the validator globs wildcards; the
span ladder's floor is 0.01 ms) are stated directly instead.
Capture provenance is deliberately kept: baseline-timings.json 'captured_at',
the 2026-08-26 baseline heading, and the 2026-08-24 figures cited as data.
Documentation, JSON note strings and one docstring only, no behaviour change.
The harness contract described how path-finding load came to be absent rather
than why it is absent. The load exists on no branch before this one, so a squash
merge publishes no revision that ever issued it: the 2026-08-25 date resolves
nowhere, and 'removing it', 'used to satisfy' and 'has now cleared' compare
against a state a reader cannot reach.
- README: state that DEFAULT_WEIGHTS carries no ripple_path_find entry, and give
the error floor as what WOULD happen if it did, rather than what removing it
fixed. 'Putting it back' becomes 'Enabling it'.
- expected_spans.json: the pathfind.request and pathfind.compute notes, and both
hierarchy skip_reasons, now put the span's presence in the conditional -- the
parent would appear if the RPC were issued, because the ScopedSpanGuard at
RipplePathFind.cpp:35 sits above the rpcNOT_SUPPORTED guard at :48-49.
- expected_metrics.json: the rpc_method_errored_total, pathfind_fast and
pathfind_full notes drop the date and keep both independent reasons the
metrics stay absent.
- regression-thresholds.json: span.ledger.store 'is excluded from' the gated
surface rather than 'was removed from' it.
The reasoning is unchanged: pathfinding is off because Config.cpp:725-726 zeroes
pathSearchMax when [validation_seed] is present, a refused call still exports an
error span, and at a 3% weight that is a ~3% STATUS_CODE_ERROR floor.
Documentation and JSON note strings only, no behaviour change.
Comments across the sync-diagnostic work described earlier revisions of the same
change, or cited identifiers a reader of the merged tree cannot resolve.
Prior-state comparisons rewritten in the present tense:
- MetricsRegistry.cpp carried two adjacent paragraphs prescribing opposite
behaviour for a disabled quorum, one publishing int64 max and one omitting
the series. The code omits it; the superseded paragraph is gone and the
surviving reason SIZE_MAX must not be cast is kept.
- MallocTrim, LedgerMaster, LedgerReplayTask, TransactionAcquire, Application:
say what the signal is the only record of, rather than what was 'previously
trace-only', 'not logged at all here' or 'used to sit inside if (debug())'.
- LedgerMaster.h and SpanGuardScope: without an explicit join each ledger's
spans WOULD be separate traces -- not that they were 'before this'.
- Handshake: the message is forwarded byte for byte, not 'byte-identical to the
previous behaviour', and the helper throws rather than 'throws as before'.
- MetricNames: quorum_disabled is a separate boolean rather than a sentinel,
stated without what the state 'used to be encoded by'.
- LedgerMaster.cpp no longer claims to mirror the unl_quorum gauge; it does not.
That gauge omits the series while this stores int64 max.
- 'Split out of' / 'Split from' become 'Kept separate from' in five places.
Plan-internal identifiers removed:
- All 24 WP-Ax / WP-Bx work-package labels across the telemetry tests, the
collector configs, tempo.yaml and the expected_* inventories. They are defined
in no file in the repo, so they resolve nowhere once merged.
- The two references to OpenTelemetryPlan/, which does not reach develop, now
point at docs/telemetry-glossary.md 'Fresh-node sync diagnostics'.
Comments and JSON note strings only, no behaviour change.
Three harness comments described the behaviour this change replaced, which the
squash merge does not publish.
- run-full-validation.sh: the capture flag means CAPTURE_EXIT is not the only
record of capture health; and a gated capture failure is an infrastructure
error, stated without 'exactly as before'.
- tx_submitter.py: give the reason the first occurrence logs at WARNING (DEBUG
is off in CI) rather than what a failed run 'previously produced'.
- workload_orchestrator.py: a wedged process cannot stall the profile, rather
than 'can no longer'.
Comments only, no behaviour change.
capture_timings.py decides whether a captured timings file may become a
regression baseline. Every way of getting that wrong is silently green: a
capture that asked Prometheus for nothing still writes valid JSON, and once
accepted it is pasted in as a baseline, still reads as a placeholder, and the
regression gate stays off while the workflow reports it as activated.
Covered: an empty surface is not complete (0 of 0 is 100% by arithmetic), the
minimum ratio is inclusive, null values count as declared but not captured, the
threshold is recorded so a rejected capture can be judged later, and the exit
code follows the flag rather than recomputing the ratio. The empty case has its
own error path because the percentage message divides by the declared count.
Neither this file nor test_validate_telemetry.py ran anywhere before: not in
CI, not in run-full-validation.sh, not in pre-commit. They now run in the
naming job, which is fast and fires on nearly every PR, so a broken harness
surfaces in seconds rather than after an xrpld build.
They run as plain scripts. unittest discover would collect nothing from them,
since they hold bare functions rather than TestCase subclasses, and would exit
0 -- which is why each file fails when it collects no tests. The dependency
install is a separate step, placed after every stdlib-only check so those stay
reachable if PyPI is unavailable.
