feat: add more instrumentation to xrpld - #7875
Draft
pratikmankawde wants to merge 159 commits into
Draft
Conversation
Adds the anchors the sync-diagnostics signals attach to, with no signals emitted yet: - New "Ledger Sync Health" dashboard (uid ledger-sync-health) with the standard template-variable block copied from an existing board, plus empty "Bootstrap (Domain 0)" and "Sync pipeline" rows. - Signal index section in the data-collection reference, an operator-flow stub in the telemetry runbook, and a glossary anchor. - A sync_diagnostics group in expected_metrics.json and a matching assertion helper in validate_telemetry.py so CI fails when a signal regresses to absent. Also registers the new dashboard uid with the harness so the board is covered by validation. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…el-sync-diagnostics
A freshly started node most often stalls before it ever peers or reaches quorum, and that whole chain had no telemetry. Adds the six signals that make it observable: - dns_resolve_total / dns_resolve_latency_ms: configured-peer hostname resolution, emitted from OverlayImpl so libxrpl stays independent. - overlay_connect_total / overlay_dial_latency_ms: outbound dial outcome by terminal reason, plus dial duration. - handshake_negotiation_fail_total: protocol and network-id negotiation rejections, labelled by reason, so a misconfigured network is no longer indistinguishable from unreachable peers. - unl_fetch_total and the unl_quorum gauge: validator-list fetch outcome per site and trusted key count against the required quorum. Without these a bad validators.txt leaves the node syncing forever with no signal. - clock_close_offset_seconds: network close-time offset, which server_info hides below 60s but which stalls consensus participation. Panels land in the Bootstrap row of the Ledger Sync Health dashboard, the metrics are asserted by the workload validator, and both the reference and the runbook flow describe them. Levelization baseline regenerated: overlay now includes MetricMacros.h, so the overlay/telemetry pair is reported one-way instead of bidirectional. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Three defects found reviewing the WP-A1 commit: - Handshake.cpp moved a std::string into std::runtime_error, which has no rvalue constructor. The move never happened and clang-tidy rejects it under performance-move-const-arg, so CI would fail even though the local hook only runs clang-tidy with TIDY=1. Takes the message by const reference instead, and drops the <utility> include that existed only for that move. - ValidatorList disables quorum by returning SIZE_MAX. Casting that to int64_t wrapped it to -1, so the headroom panel computed 0 - (-1) = +1 and coloured yellow on a node that can never validate: the sign inverted in exactly the bootstrap failure these signals exist to catch. Reports the disabled state as int64 max so headroom goes strongly negative instead. - The runbook claimed an expired list loads no keys. Expired counts as accepted, so its keys are loaded and then dropped by the expiry sweep, which calls for a different fix than replacing validators.txt. Pending is likewise a future-dated refresh, not a rejection. Documents both, plus how the quorum-disabled state now reads. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Five signals that explain why a node is not advancing toward full, none of
which were observable before:
- state_changes_total now carries {from,to} mode labels, emitted at
setMode using the existing strOperatingMode helper. A bare count could
not distinguish a healthy climb from a node flapping between tracking
and connected. Removes the now-unused incrementStateChanges wrapper.
- sync_state{initial_full_duration_us}: time to first reach full, which
StateAccounting already computed but exposed only in server_info.
- sync_state{network_ledger_gate}: whether the node is still refusing to
build ledgers because it has no network ledger.
- sync_state{server_stall_seconds} and server_stall_events_total: how
long the main thread has been unresponsive. LoadManager computed this
and only logged it, so a stall was invisible until the fatal threshold.
The episode rule is a pure function so it can be tested without adding
a test-only mutator to LoadManager.
- sync_state{ledgers_behind}: how far our validated sequence trails the
best sequence any peer advertises, read from already-cached peer ranges
so no extra network traffic is added.
Also fixes the naming checker: it derived only the first label of a
multi-label instrument, so a dashboard querying the second label was
wrongly rejected.
Note: the clang-tidy hook cannot run in this worktree (no build
directory); the remaining pre-commit hooks, the naming check, dashboard
schema and harness syntax all pass.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signals that separate a sync that is merely slow from one that will never
finish:
- sync_acquire{missing_state_nodes_max, missing_tx_nodes_max, in_flight,
received_data_depth}: how many SHAMap nodes each in-flight acquire is
still waiting for. getMissingNodes already computed this and the callers
discarded it after a trace log. A count that stays flat means the
acquire is wedged; a shrinking count means it is progressing. Recorded
once per sweep, never inside the per-node walk, and reset when a tree
completes so a finished acquire does not read as stuck forever.
- shamap_cache_hit_rate{treenode}: hit rate of the in-memory tree-node
cache, which sits above the node store, so it is distinct from the
existing NuDB ratio. A cold cache on a fresh node sends every traversal
step to disk.
- sync_acquire_no_progress_total: timer ticks where an acquire made no
progress, previously only logged.
- sync_addnode_total{good,duplicate,invalid}: whether arriving nodes are
useful, duplicated or rejected, so wasted fetch work is visible.
- sync_acquire_source_total{local,network}: whether a ledger was served
from the local store or had to be fetched.
Adds getBad()/getDuplicate() to SHAMapAddNode and an acquireProgress()
accessor on InboundLedgers so the xrpld gauge can read these without
libxrpl depending on telemetry.
ledger_seq is deliberately not a metric label: it is unbounded. Per-ledger
identity stays on the ledger.acquire span; the metrics expose bounded
aggregates instead.
The full-below cache hit rate is not exported: KeyCache updates different
counters than getHitRate() reads, so it would always report zero. That
libxrpl bug is documented rather than papered over.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Sync-critical job types run at very low concurrency limits (ledgerRequest
and ledgerData allow 3 each), so a node can stall simply because those
jobs are held back behind other work. Nothing exposed that until now:
the existing job metrics are rates and quantiles of jobs that already
moved, or a single queue-wide depth.
- jobq_backlog{metric,job_type}: instantaneous waiting, running and
deferred counts per job type. Deferred is the starvation signal and had
no exposure anywhere; it is set when a type is at its concurrency limit.
- jobq_saturation{metric}: running tasks, worker-thread count and total
waiting, so a slowdown spanning several subsystems can be attributed to
worker-pool exhaustion instead of being diagnosed once per victim.
Both read through two new const accessors on JobQueue that take the
existing mutex once and copy integers, so a single reading is internally
consistent and no per-job cost is added. The job_type label reuses the
same JobTypes name helper the existing job counters use, so the two label
sets join.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…-A7)
Whether the network can even serve this node, and whether the node is
about to be shut out of validation, were both invisible:
- peer_ledger_supply: how many connected peers advertise a range covering
the sequence being fetched. Peers each track a range from status
changes, but nothing aggregated them, so "nobody has what I need" looked
identical to "peers are slow".
- peerfinder_slot_census: outbound active against capacity, connection
attempts, inbound, fixed configured against active, and the bootcache
and livecache sizes. All were computed already; only two were exported,
read at unrelated instants, so they could not be compared.
- peer_disconnect_total{reason,direction} and peer_accept_total{outcome}:
every disconnect previously collapsed into one number, so our own
backpressure could not be told from topology or network faults. Reasons
are a fixed set of literals recorded on the peer and emitted once at
close, never data supplied by the remote end.
- serve_refused_total{request,reason}: the other half of the sync
exchange, when this node declines to serve a peer.
- amendment_block: whether an unsupported amendment is expected and how
long until it activates. Amendment-blocked is terminal for validation,
so the countdown is the only leading indicator. The amendment id is
deliberately not a label, since the network can vote an id this build
has never heard of; it is already logged.
- ledger_jump_total: repeated last-closed-ledger switches, which mean the
node is thrashing between chains.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…, WP-A6) Quorum and publish (A5): - ledger_quorum_publish gauge: the trusted-validation tally against the quorum the candidate ledger must reach, plus the gap between the validated and published sequences. The published sequence was never exported, so a publish pipeline falling behind healthy validation was invisible. - ledger_quorum_shortfall_total: counts the pre-accept early return where a node has peers and validators yet still declines to declare a ledger validated. That path was log-only, and it is the difference between accumulating toward quorum and never reaching it. Back-fill and persistence (A6): - nodestore_latency: read and write service time. storeDurationUs_ was declared but never written and had no accessor, so there was no write latency signal at all. This is the direct fingerprint of a node with an existing database syncing slower than a fresh one, where the node cache is cold and every tree step reaches disk. Distinct from the existing NuDB read panels, which show volume and hit ratio rather than service time. - ledger_replay_fallback_total and ledger_replay_outcome_total: the replay path silently falls back to plain acquisition on timeout or failure, so a defeated optimisation looked like ordinary slow back-fill. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The span only recorded an outcome on the normal completion path. An acquire that stalled and was later swept ended with no outcome at all, and its duration stretched to the sweep interval rather than the real fetch time. So the one case these signals exist to catch, a fetch that never finishes, was the one case that could not be traced, and aggregate outcome and timeout rates read low exactly when nodes are stuck. - Adds an abandoned outcome value for the swept-while-fetching case. - Routes every exit through one idempotent finalizer, so a span is finalized exactly once whether it completes, fails, short-circuits on local data, or is destroyed mid-fetch. The destructor path cannot throw. - Adds the ledger hash to the span and backfills the sequence once known, since by-hash acquires start without one and could not otherwise be tied to a specific ledger. - Record layer: outcome stays a span-metrics dimension in both collector configs, which drift apart if only one is edited. The ledger hash is indexed in Tempo for trace search instead, because a per-ledger value as a metric dimension would mint a new series every ledger. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Registers the new gauges, renders them, asserts them and documents them, so each signal reaches an operator rather than stopping at the emit site: - MetricsRegistry: gauge registration for ledger_quorum_publish, nodestore_latency, peer_ledger_supply, peerfinder_slot_census and amendment_block, each guarded by the detached-callbacks check and tolerant of services that are not ready yet. - Ledger Sync Health dashboard: panels for the new signals, filtered by the node template variable like every other board. - Workload validation: the new series are asserted, so a signal that regresses to absent fails CI. Signals the local cluster structurally cannot produce, such as a replay fallback or an amendment block, are noted rather than asserted, which would fail red on a healthy run. - Reference, runbook and glossary entries, including the diagnosis order for a node that has peers and validators but never validates. - Regenerated levelization baseline: three new one-way edges from the telemetry and test modules, no new cycles. Also drops an unused cstddef include from the macro tests, which the include checker rejects. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The quorum and publish gauges were emitted but never surfaced: no panel, no harness assertion, no reference entry. Completes those layers. - Four panels: trusted validations against the quorum target on one axis so a tally climbing toward quorum is visually distinct from one flat below it; publish lag; pre-accept shortfall rate; and time to first validated ledger. - Both signals are asserted by the workload validator. The shortfall counter does fire on a healthy cluster, because this node validates and then immediately re-enters the accept gate before its peers' validations arrive, so the first evaluation of every round tallies short. The panel and note say so, and give the fault signature instead: the shortfall rate outpacing the ledger-close rate while the tally stays flat and nothing ever reaches first-validated. - The quorum target is deliberately drawn as its own line rather than as a headroom stat, so the disabled-quorum sentinel reads as an unreachable target instead of an unreadable negative number. Also removes three reference rows that were appended twice when two agents each documented the same back-fill signals. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Four blind spots in the sync exchange, each now a span:
- txset.acquire: transaction-set acquisition had no span at all, though it
is the sibling of ledger.acquire and runs every consensus round. A round
that falls behind because its tx set never arrived was indistinguishable
from one that deliberated slowly.
- ledger.acquire.{header,astree,txtree}: the acquire span was flat, so the
account-state tree, which dominates a fresh sync, could not be separated
from the transaction tree. These are children, closed before the parent.
- peer.dial: the outbound dial already had outcome counters; the span adds
the per-attempt timeline, so a slow stage is visible rather than only its
terminal reason.
- ledger.serve: serving a peer's ledger request was uninstrumented, so this
node's contribution to someone else's sync was invisible.
Every span finalizes exactly once. Outcomes come from shared compile-time
rules rather than a literal per branch, so no exit can mislabel itself and
an exit added later cannot omit one. Destructor paths are noexcept.
One rule needed care: the timeout path also sets the failure flag, because
that is how the timeout loop stops, so precedence puts timeout ahead of
failure or a timed-out acquire would read as a data fault.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ogram (WP-B3) A slow fresh-sync ledger produced spans scattered across threads with no way to relate them. They now share a trace id derived from the ledger's own hash, the one value every participating site already holds, so nothing new is plumbed across threads. This is the pattern the transaction pipeline already uses for its tx id. Joined: ledger.validate, ledger.store, and a new consensus.validation.accept recorded when a trusted validation arrives. In Tempo, searching one ledger hash returns them together, so an operator can tell whether the ledger was slow to arrive, slow to be accepted, or slow to be stored. They are siblings rather than a chain because the accept gate is entered from three different threads, so no fixed parent order exists. consensus.validation.accept also records why an arriving validation did or did not advance the gate, which makes "validations arrive but are all rejected" visible for the first time. consensus_round_duration_ms turns the existing round-time span attribute into a histogram, so a fleet trend needs a metric query rather than raw trace inspection. An explicit bucket view is required, not optional: the SDK default tops out at ten seconds while consensus abandons a round at two minutes, so slow rounds would all fall in one bucket and every quantile would read exactly ten seconds. Cost is one record per round. Record layer: the histogram is native and needs no collector change. The two new bounded attributes are added as span-metric dimensions to both collector configs. The ledger hash stays out of them, since a per-ledger dimension mints a series per ledger; it is indexed in Tempo as the join key. The ledger.acquire span is not joined yet, because that file was being changed concurrently. It is registered as an optional member of the join group so nothing fails, and switching it is a one-line follow-up. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Completes the join left open when the acquire and consensus work landed in parallel. The acquire span now derives its trace id from the ledger hash it already records, so a fetch shares one trace with that ledger's validation, acceptance and store, and its three phase children inherit the same id. Reading one trace now answers the whole question for a slow ledger: whether the data was slow to arrive, slow to be accepted, or slow to persist. The acquire span stays an optional member of the join group, for the reason its own entry already gives: a healthy cluster agreeing from genesis rarely back-fills, so the span need not appear on every run. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…tes (WP-B4) The board and runbook had grown by append across eight work packages, so they read in the order the work was done rather than the order a node progresses. This is the coherence pass; it adds no new instrumentation. - Dashboard: 52 panels regrouped from two rows into nine that follow the fresh-start sequence — bootstrap, peer supply, sync state, acquire and SHAMap fetch, job queue, quorum and publish, terminal blockers, then back-fill and spans collapsed since they answer conditional questions. Layout only: no title, query or description changed. - Runbook: the flat step list becomes a decision tree branching on the observed symptom, with the amendment-block check first because it is terminal. Each branch names the panels, what healthy and unhealthy look like, and what to conclude. The existing steps are kept as the detail bodies. - Reference table: every signal name re-checked against the code and every named panel against the board; four stale panel references fixed. - Validation: every signal is now either asserted or covered by a note explaining why a five-node local cluster cannot produce it. Also fixes the write-latency signal, which was inert on a real node: the store duration was only recorded on the database-import path, while the two production store implementations did not time themselves, so an ordinary node reported a write count with no latency. Both now time the backend write, which is the disk work this signal exists to expose. Without it the "existing database syncs slower than a fresh one" diagnosis had no primary signal. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… it (WP-A8) Metric names and label keys were bare string literals, repeated across the emit site, the gauge registration, the unit test, the workload manifest, the dashboard queries and the reference table. A rename touched six places and a typo in any one of them failed silently: a metric that never appears, or a label that never joins. The span side already had this right, with names and attribute keys declared once in the *SpanNames.h headers and a CI rule rejecting literals at call sites. That rule only ever covered spans, so the metric side had no equivalent and no suffix convention was enforced by anything. - Adds MetricNames.h declaring every instrument name, label key and bounded label value this story emits, grouped by subsystem, following the existing span-name header layout. - Converts the call sites subsystem by subsystem. The emitted strings are unchanged: 75 names before, the same 75 after, verified by extracting the wire strings from both trees and diffing the sets. - Extends the naming check with three rules: no literal instrument name or label key at an emit site, the duration and counter suffix conventions, and every name in the workload manifest resolving to a constant. The first rule is ratcheted per metric family so the pre-existing families warn rather than block, keeping the remaining work visible instead of forcing one unreviewable change. Constants are character arrays rather than the span headers' StaticStr, because the metrics API takes a string view that will not construct from it. Two things the conversion exposed: a serve-refusal reason that the original inventory missed because it is passed through a ternary, and a label whose constant made it invisible to the checker's literal scan, which would have failed a dashboard rule. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…el-sync-diagnostics Phase-10 independently instrumented the peer object-fetch path while this branch instrumented fresh-node sync, so the two overlapped in three places. Resolved by keeping each side's stronger implementation rather than shipping both. Per-job-type waiting/running/deferred existed twice. Phase-10's version survives: it publishes per-type gauges from JobQueue::collect(), which snapshots under the queue lock and publishes after releasing it, a deliberate lock-order fix against the collector's own lock. This branch's jobq_backlog gauge and the JobQueue::getJobTypeCounts() accessor that fed it are removed, along with their panels, assertions and reference rows. jobq_saturation stays: it reports the whole worker pool, which phase-10 has no equivalent for. The histogram view helper also existed twice with identical bodies under two names; one survives, and the microsecond ladder is now the named array rather than boundaries repeated inline. The job_type label was declared twice, once as a file-local constant invisible to the naming check; both it and handler now come from the constants header. Two things phase-10 adds are complementary, not duplicates, and are kept as they are: the handler label, which separates the two request kinds that both report as the same job type, and getobject_rejected_total, which counts malformed requests where this branch's serve_refused_total counts requests this node declined to serve. Also fixes two naming-check failures that pre-date this merge on phase-10. The check derived label keys only from namespaced constants, so it could not see the per-subsystem headers' flat k-prefixed style and rejected dashboards querying labels the code really emits. It now reads both styles, with the enforcement rules unchanged. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
These are the first failures found by actually building and running the new telemetry tests. All three were defects in the tests, not the metrics. - state_changes_total expected six series from seven transitions, but two edges were traversed twice, so there are five distinct from/to pairs. The repeat is the point of the label: flapping raises the count on one edge rather than minting a new series, so the assertion now says five and explains why. - shamap_cache_hit_rate compared for exact double equality against a value that reaches the gauge through a float hit rate, so 0.9 arrives as 0.89999997. Now compares within a tolerance far tighter than any threshold a dashboard reads, since asserting exact equality was only asserting the float representation. - nodestore_latency shared one provider across four scenarios. The reader reports cumulative temporality, so a mean observed by an earlier scenario was still present in the next collection, which defeated the two assertions that a mean is absent when its denominator is zero. Each scenario now collects from its own provider. 139 of 139 telemetry tests pass. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The write-duration test encoded the behaviour from before the store path was timed. It populated the source database through store(), then asserted that database's write duration was still zero, which was only true while importDatabase was the sole timed path. Both concrete store overrides now time their backend write, so an ordinary store run accumulates a duration and the assertion failed. Turned that stale assertion into a positive one: an ordinary storeBatch must produce a non-zero duration, which is the case a real node actually exercises. The negative half of the test, proving the accumulation is per-database rather than a shared global, now checks that the import leaves the source's store COUNT unchanged, since the source's duration is legitimately non-zero from its own writes. 80552 tests across the six affected suites pass. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
pratikmankawde
added
the
DraftRunCI
pratikmankawde
changed the base branch from
develop
to
pratik/otel-phase10-workload-validation
July 27, 2026 13:13
Found by reviewing what each metric actually measures, with attention to the derived and bucketed ones. All five could report healthy while the node was not, or the reverse. - The nodestore latency panel took rate() of a mean. The gauge already divides duration by count in code, so rating it produced a figure with no unit, and Prometheus discards a gauge's decreases, so a heavy back-fill read as roughly zero microseconds per operation. The cumulative duration totals are now exported alongside the means, and the panel divides the rate of the total by the rate of the count, which is the latency over the panel's own window rather than a since-boot average that flattens with uptime. - The DNS-resolve and outbound-dial histograms had no explicit buckets, so they inherited a ladder that stops at ten seconds while the dial timer is fifteen. Every timed-out dial fell in the overflow bucket and p95 read exactly ten seconds however bad it got. Both now have a ladder reaching thirty seconds with fifteen on its own boundary, so a timeout is distinguishable from merely slow. - The missing-node counts only cleared when a tree completed, so a timed-out or failed acquire left its last count latched. Since the gauge reports the maximum across everything still in the collection, and eviction waits on a grace period plus the sweep interval, a finished node reported as stuck for minutes. That inverts the one signal that separates stuck from slow. Cleared unconditionally on the terminal path instead. - A disabled quorum published a sentinel so large that, on a timeseries axis shared with the trusted-key count, it flattened the key line to the baseline and hid the outage it was meant to mark. The series is now omitted and a quorum_disabled flag carries the state. - Two panel descriptions claimed a one-second export cycle. The reader is configured for ten. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
pratikmankawde
marked this pull request as ready for review
July 27, 2026 14:31
Two suspects from the 3.3.0 slowdown investigation had no signal. Both were already computing the numbers and throwing them away, so this exposes them rather than adding measurement. Per-sweep heap trim. The trim runs after every cache sweep, and its cost scales with resident heap, so it is the leading explanation for a node with a populated database syncing slower than a fresh one. The report already carried duration, fault deltas and reclaimed pages, but the whole measurement sat behind a debug-journal check, so an ordinary node measured nothing, and the call site discarded the result. The measurement now always runs and only the log line stays gated. Records trim duration, minor faults and reclaimed kilobytes. Measured cost of the always-on path is about six microseconds per sweep against a trim costing milliseconds, at a cadence of ten to a hundred and twenty seconds. Honest limit, stated in the runbook: the fault delta spans only the trim call, so it shows the trim itself faulting but not the faults that follow as caches refill. The duration is the signal to correlate against sweep-job queueing. Rotation writes. Rotation copies archive-served reads forward and re-stores nodes missing from both backends, both of which compete with sync I/O and only happen on a populated online_delete database. The copy-forward count existed but was reset by the rotation's own log line, so a metric reading it would drop to zero on every swap; a never-reset total sits beside it now. The re-store count was not measured at all. Rotation duration is deliberately not recorded: the health throttle sleeps at eight points inside the sequence and dominates exactly when the node is unhealthy, so the number would conflate work with waiting. Nothing added for the other two suspects. Get-object serving is already covered by the handler label, the lookup histogram and the deferred and saturation gauges; peer churn by the disconnect-reason counter. Also replaces nine per-file cspell ignores with one ignoreRegExpList entry for the telemetry macro names, and picks up the levelization baseline for the consensus span-name test. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
All eleven were reproduced locally against the same checks before fixing. Six unused includes, left behind when the merge unioned two sets of includes and later edits removed their only users: algorithm, functional, numeric and thread in the job-queue test, and cstdint in the sync-state test and the load manager. Each verified unused by grepping for every symbol the header provides, so none is a still-needed include being dropped. The telemetry registry header included ranges for a std::ranges::all_of call, but that algorithm comes from algorithm, which the header already included. Two consteval handler-name loops became std::ranges::all_of, which reads as the predicate it is, and the two flagged fixtures are const. Also picks up the levelization baseline the check asked for: the consensus span-name test adds one edge from the libxrpl tests to xrpld.consensus, which is the exact line CI's diff requested. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The gcc debug-coverage job failed where clang, macos and windows passed: gcc's -Werror=type-limits rejects comparing an unsigned expression against zero with >=, because it is always true. The assertion was vacuous anyway. Both operands are unsigned, so the check proved nothing, while the comment beside it says the intent was a non-zero microsecond figure. Now asserts strictly positive, which is what it meant. The same job also logs a CMake LTO capability probe failing on a missing compiler-ar and prints the code-generation guard's echo line; neither is related to this branch. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The gcc debug-coverage job rejected an unbraced `if` whose body is a GTest assertion: EXPECT_EQ expands to an if/else, so the outer `if` leaves an else that could bind either way, and -Werror=dangling-else refuses it. clang does not warn, which is why only that one job failed. Braced the span-names case that failed, then swept every test file this branch touches for the same shape and braced the two others found, so the next gcc run does not fail on the next one down the list. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
json::ValueConstIterator and ValueIterator declared difference_type, reference and pointer but not value_type or iterator_category. Under C++23, std::iterator_traits then classifies them as output iterators, so std::all_of over a Value's members (isValidJson2 in RPCCall.cpp) fails to instantiate on GCC 13/14 with: cannot convert 'output_iterator_tag' to 'std::input_iterator_tag' GCC 15 masks this via LWG-3798/P2609, but the perf CI image ships GCC 13, so the source needs the traits regardless. The iterators wrap a std::map iterator (++/-- only), so the category is bidirectional. Add value_type + iterator_category to both iterators, include <iterator>, and add a regression test asserting the traits and that std::all_of / std::count_if compile and run over Value members. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The isValidJson2 call site in RPCCall.cpp already forces instantiation of std::all_of over json::ValueConstIterator, so a regression that removed the iterator traits would fail the real build. A dedicated static_assert test is redundant; remove it. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Compare-then-store is the recurring shape behind these values: read whether an incoming value differs from the last, store it, report only on a change. Document changedTo() as doing all three, and state the two constraints -- it is not synchronized, so it cannot replace an atomic read from another thread, and it requires a copyable T so its copy semantics match across builds. Recording.h declares Mirror beside Stopwatch and Counter, so its overview diagram lists Mirror too and its thread-safety note covers it.
…el-sync-diagnostics
|
All conflicts have been resolved. Assigned reviewers can now start or resume their review. |
CI runs the pre-commit hooks over every file, and black and prettier both rewrote files under .github/scripts/otel-naming/, which fails the job. The changes are cosmetic: two blank lines in the checker, and the pipe padding of one markdown table.
…el-sync-diagnostics
…el-sync-diagnostics
reportOutcome() is the only reader and writer of outcomeReported_, and its body is compiled out with telemetry, so a telemetry-off build sees an unused private field and -Werror rejects it. Marked rather than guarded, so the member set does not depend on the build, matching how the dial timestamp and span handle either side of it are declared.
…el-sync-diagnostics
…el-sync-diagnostics
…el-sync-diagnostics
IPEndpoint.h supplies to_string() for a peer endpoint. The only call to it in this file sets the remote address on the dial span, which is compiled only when telemetry is enabled. With telemetry disabled the header is included but never used directly, and the include-cleaner check rejects that. Guarding the include keeps both configurations clean. Removing the include would instead leave to_string() with no directly included provider when telemetry is enabled.
…el-sync-diagnostics
A unity build compiles this file in the same translation unit as the consensus span-name tests. Their directive imports consensus::span::attr while this file's imports xrpl::telemetry::attr, so a bare `attr::` names both and the compile fails on an ambiguous reference. Qualifies the two shared-side references in full. The sibling consensus test already avoids the mirror image of this by taking an alias instead of a second directive, and records the reason above its aliases. Only reachable with unity enabled, which is why CI never saw it: CI builds one object per file, so the two directives never share a unit.
pratikmankawde
commented
Aug 27, 2026
pratikmankawde
left a comment
pratikmankawde
left a comment
Contributor
Author
There was a problem hiding this comment.
Severity counts: 0 BLOCKING - 0 SHOULD-FIX - 1 NIT.
Review of the sync-diagnostics draft at 0883faf650, against its base pratik/otel-phase10-workload-validation (e219026010). Read entirely from the git refs.
The one substantive defect I found is already covered by the open Copilot thread on node-health.json, so I have not duplicated it here: this PR changes the "Job Queue Saturation (Running vs Limit)" panel description from "1-second export cycle" to "10-second". description is the only key that differs on that panel between the two branches, and 1 second is the correct figure - the jobq_* gauges are beast::insight gauges published by JobQueue::collect through a hook (src/libxrpl/core/detail/JobQueue.cpp:39, :67-107) into the global MeterProvider OTelCollector uses (OTelCollector.cpp:824), whose reader interval is 1000 ms (Telemetry.cpp:545, comment: "matching the beast OTelCollector path"). The 10 s reader is MetricsRegistry's own (MetricsRegistry.cpp:324) and does not publish these gauges. I would rate it SHOULD-FIX / Medium: documentation only, no query breaks, but it misstates staleness by 10x. The sibling panel on ledger-data-sync.json is already correct and I have resolved that thread.
What I checked and found correct:
- No harness-vs-code drift in either direction. All 7 added spans resolve to header constants at this commit (
consensus.validation.accept,ledger.acquire.header/.astree/.txtree,ledger.serve,txset.acquire,peer.dial), everyrequired_attributeskey resolves, and all 30 metric families declared undersync_diagnosticshave a matching string literal insrc/orinclude/. - The two spans marked required really are unconditional where they fire:
consensus.validation.accept's five attributes are all set immediately after the guard atRCLValidations.cpp:193-200, andpeer.dial'sremote_endpointis set atConnectAttempt.cpp:163-165directly after the span is created. - The
SKIPPED_METRIC_GROUPSmerge hazard is genuinely resolved: I ran the real_metric_check_targetson this branch's contract andsync_diagnosticsis excluded, so its 61 metrics are polled once, not twice. (The stale comment about it is the one nit below.) - New signal wired end to end:
peer_disconnect_totalis emitted withreasonanddirectionlabels (PeerImp.cpp:667-674), the two new peer-quality panels query it, and matching$disconnect_reason/$disconnect_directiontemplate variables exist withallValue: ".*".$nodefilters onservice_instance_id, as required. - Dashboard registration is complete: 16 provisioned, 16 uids in
expected_metrics.json, exact match - the newledger-sync-health.jsonis registered. check_otel_naming.pyexits 0 at this commit (run in an isolated clone), warnings only.PeerImp.h:534'schar const* == std::string_viewcomparison is valid C++20 and this PR's ownubuntu-clang-release-amd64leg with-Dwerr=ONpassed at this SHA; I have resolved that thread.
The empty "tooltip": {} object this PR adds to one node-health.json panel's options is not a finding - the file already carries 7 of them at the base, so it matches existing convention.
| # reinstate this exclusion inside _metric_check_targets, or fold | ||
| # assert_sync_diagnostics_metrics into it and drop the separate pass. Either is | ||
| # fine; keeping both walkers without an exclusion is not. | ||
| SKIPPED_METRIC_GROUPS = ("description", "grafana_dashboards", SYNC_DIAGNOSTICS_GROUP) |
Contributor
Author
There was a problem hiding this comment.
[NIT · Low] — the merge-hazard note above this line states something that is no longer true of the function three lines below it.
The comment says phase-10's _metric_check_targets() "selects every group satisfying isinstance(category_data, dict) and has no equivalent of this tuple". At this commit it does have the equivalent — the comprehension reads if isinstance(category_data, dict) and category_key not in SKIPPED_METRIC_GROUPS, and that function's own docstring explains the name-based exclusion at length ("The name-based exclusion is still required for one group, so the isinstance test alone is NOT equivalent to it"). So the two comments contradict each other, and a maintainer following this one will go looking for a missing exclusion that is present.
I confirmed the resolution works rather than reading it: calling the real _metric_check_targets on this branch's expected_metrics.json yields 79 metric targets across 25 groups, and sync_diagnostics is not among them — its 61 metrics are walked once, by assert_sync_diagnostics_metrics, exactly as intended.
Worth removing rather than rewording: phase-10 is this PR's base, so the merge it warns about has already happened here. After a squash merge the note would point at a state that does not exist in history. The information worth keeping is already in the _metric_check_targets docstring.
pratikmankawde
commented
Sep 2, 2026
| "note": "The supply side: this node answering a peer's TMGetLedger request on the JtLedgerReq worker. In practice every node in the harness cluster is listed in the others' [ips], so they exchange ledger and tx-set requests continuously throughout the run -- but optional even so, because the supply side cannot be required while every demand-side span is optional. A TMGetLedger request is constructed in exactly two places, InboundLedger.cpp and TransactionAcquire.cpp, and their ledger.acquire and txset.acquire spans are both optional; requiring this one would rest a mandatory check on an optional cause. Promote it together with them if a future harness step forces a real fetch. A fresh trace root -- the request arrives from the wire on a shared worker, so it must not inherit an unrelated span active there. object_type is header|tx|as|txset and outcome is complete|partial|refused, both derived by shared rules in LedgerSpanNames.h so the eight exits of processLedgerRequest cannot disagree. ledger_seq is present only once getLedger() succeeded, so it is not required." | ||
| }, | ||
| { | ||
| "name": "txset.acquire", |
Contributor
Author
There was a problem hiding this comment.
This PR takes the span inventory from 41 families in 8 groups to 48 in 9, but docker/telemetry/TESTING.md still states the old numbers.
Parsing this file at both refs:
| ref | total | groups | per group |
|---|---|---|---|
pratik/otel-phase10-workload-validation |
41 | 8 | consensus 13, grpc 1, ledger 4, pathfind 4, peer 2, rpc 5, tx 6, txq 6 |
| this branch | 48 | 9 | consensus 14, grpc 1, ledger 8, pathfind 4, peer 3, rpc 5, tx 6, txq 6, txset 1 |
TESTING.md (lines 390-392) still says "all 41 span-name families the code emits, in eight subsystem groups — RPC (5), gRPC (1), Transaction (6), TxQ (6), Consensus (13), Ledger (4), Peer (2), PathFind (4)", and its "Span → How to Trigger" table has no row for any of the seven this PR adds:
consensus.validation.acceptledger.acquire.header,ledger.acquire.astree,ledger.acquire.txtreeledger.servetxset.acquirepeer.dial
txset.acquire is the one that also changes the group count: prefix::txset is deliberately its own root rather than nested under ledger., per the comment in LedgerSpanNames.h.
The numbers were correct through phase-10, so this is the branch that owes the update — TESTING.md is not itself in this PR's diff, hence the comment here. Since that table is the trigger catalogue, each new row also wants its config toggle and its Test column; note most of these are marked optional in this inventory precisely because a fresh local cluster may not force a fetch, so they should not be listed as expected in T1/T2 without that caveat.
pratikmankawde
commented
Sep 2, 2026
| "note": "One attempt to fetch a transaction set a consensus proposal referenced (TransactionAcquire, which had zero telemetry before WP-B2). Optional because it only fires when a node does NOT already hold a proposed set: in the harness cluster every node sees the same relayed transactions and builds the same set locally, so InboundTransactions::getSet finds it in its map and never constructs a TransactionAcquire. It is the sibling of ledger.acquire -- same TimeoutCounter base, same trigger/onTimer/takeNodes shape -- and shares the trace_ledger flag so the two halves of a stuck sync cannot be enabled apart. outcome is complete|failed|timeout|abandoned, stamped on all three exits (done(); abandonAcquireSpan(), when InboundTransactions stops pursuing the fetch -- giveSet, the newRound sweep, stop, or ~InboundTransactionsImp destroying the container; and cancel(), when the TimeoutCounter base marks the task failed without reaching done()); the destructor only asserts one of them already ran. A fetch dropped in the window before init() runs emits NO span at all rather than an abandoned one, so the outcome set describes the spans that exist, not every TransactionAcquire ever constructed. The requesting round(s) are carried by repeated round.request EVENTS (current_ledger_hash + current_ledger_seq), not by a parent or a link: one round starts many fetches and one fetch is wanted by many rounds, so no round owns the span -- which is why there is deliberately no parent_child_relationships entry for it. Events are not asserted by this validator; they are read in Tempo." | ||
| }, | ||
| { | ||
| "name": "peer.dial", |
Contributor
Author
There was a problem hiding this comment.
Related: the two references TESTING.md sends readers to for these spans do not cover them either.
TESTING.md lines 394-398 point at docs/telemetry-runbook.md § Span Reference for "full attribute set and description, per subsystem", and assert "Both are kept in step with the code, so they are the reference to trust."
On this branch § Span Reference (lines 253-469) contains none of the seven families this PR adds. Checking each name inside that line range: consensus.validation.accept 0, ledger.acquire.header 0, ledger.acquire.astree 0, ledger.acquire.txtree 0, ledger.serve 0, txset.acquire 0, peer.dial 0. Two of them — ledger.acquire.astree and ledger.acquire.txtree — appear nowhere in the runbook at all, though their attributes are pinned in expected_spans.json here.
The same section's dashboard inventory has drifted for the same reason: § Grafana Dashboards still opens with "Fifteen dashboards are pre-provisioned", and never names ledger-sync-health, which this PR adds as the 16th (it is documented instead further down, under Troubleshooting). TESTING.md line 515 carries that count too — "(this section previously named 5 of the 15 provisioned)". Both were accurate at phase-10; this branch makes them 16.
So three edits belong with this PR: add the seven families to § Span Reference, add ledger-sync-health to § Grafana Dashboards and bump both "fifteen"/"15" counts, and update the TESTING.md catalogue header and table (previous comment). Otherwise the doc that tells readers it is the trustworthy reference is the one missing this branch's own spans.
Contributor
Author
There was a problem hiding this comment.
Follow-up on the counts, since the runbook now disagrees with itself rather than just with TESTING.md:
docs/telemetry-runbook.md:2140-2141— "Fifteen dashboards are pre-provisioned … Fourteen are Prometheus-backed"docs/telemetry-runbook.md:4670— "currently all 16 provisioned dashboards"docker/telemetry/workload/README.md:219— "covers all 16 dashboards"
So this branch updated the two counts that are derived from expected_metrics.json and left the two prose counts at fifteen/fourteen. With ledger-sync-health being Prometheus-backed, the correct pair at :2140-2141 is sixteen and fifteen.
…sync diagnostics 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.
Mirror::store() and Mirror::changedTo() silenced an unused parameter with a (void) cast in their telemetry-disabled arm. The warning was real: Mirror<uint256> is instantiated in InboundTransactions.cpp. store()'s #else arm held nothing else, so it collapses to a bare #endif; changedTo() keeps its #else for the return. readProcFile() and measuredTrim() are added by this PR and gain [[nodiscard]]. MallocTrim.cpp predates the telemetry work, but neither function exists on develop, so this annotates declarations this branch authored rather than surrounding code. No caller discards either.
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.
Both changes had reached phase-10 and were lost here, because this branch rewrote checkAccept() around makeLedgerTraceSpan() and the merge kept its version of the file. ledger.validate was measuring the flag-ledger upgrade-warning check. That branch runs on one ledger in 256 and reads every trusted validation of the parent, so one span in 256 was a duration outlier for work that is not part of promoting a ledger. The measured region is now an explicit scope ending before it. tryAdvance() stays inside: it sets a flag and posts a job, so it adds no measurable time. Verified safe to scope: base, fees and fee are all consumed before the boundary, and nothing after it uses an unqualified telemetry name, so the using-directive stays at function scope. Also renames the guards to match the spans they hold, storeSpan and validateSpan. valSpan forced a reader back to the declaration to learn which span it was.
…ordJobQueued Both parameters already carry [[maybe_unused]], so the telemetry-disabled arm needed nothing: the casts suppressed a warning that cannot fire. (void)enabled_ never suppressed anything either, because enabled_ is a member that isEnabled() reads outside every #ifdef. recordJobQueued was the only one of the seven record* stubs still carrying an #else arm; it now has the same shape as recordJobStarted and the rest. The file is left with no (void) casts and no #else at all.
NetworkOPs.cpp include block: kept develop's rpc/detail/SyntheticFields.h alongside this branch's MetricMacros.h and the guarded MetricNames.h.
…el-sync-diagnostics
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Consolidated PR: #7770
High Level Overview of Change
Makes fresh-node ledger-sync problems diagnosable. A node that takes a long time to reach
full, or never gets there, previously left an operator with the coarse picture only: the mode it was in, how stale its validated ledger was, and how much peer traffic was flowing. None of that answers the three questions actually asked during an incident — how far behind am I, why is this particular ledger not arriving, and is this a peer problem, a disk problem, or a bootstrap misconfiguration.Adds 41 native metrics and 6 spans across the whole fresh-start path, a "Ledger Sync Health" dashboard, and a runbook that branches on the observed symptom rather than listing signals.
Context of Change
The starting point was an audit of the real startup path, traced from the XRPL server-state, peer-protocol and ledger-history docs plus the protobuf definitions, then reconciled against the code. It produced 18 stages from process launch to
full.The result was that existing telemetry covers the post-peering pipeline reasonably well and is blind to the entire pre-quorum bootstrap chain — which is where a fresh node, as opposed to a restarting one, most often gets stuck. Six files on that path carried no instrumentation at all: DNS resolution, the outbound dial, protocol and network-id negotiation, the validator-list fetch, the trusted-key/quorum bootstrap, and clock skew. The single worst gap: an unreachable validator-list site or a bad
validators.txtyields zero trusted keys, so quorum can never form and the node sits insyncingindefinitely, with nothing in telemetry to say why.Two patterns recurred and are fixed once rather than per signal:
ledger.acquireonly recorded one on the normal completion path, so an acquire that stalled and was later swept ended with no outcome and a duration stretched to the sweep interval. The one case these signals exist to catch was the one case that could not be traced.API Impact
libxrplchange (any change that may affectlibxrplor dependents oflibxrpl)libxrplgains read-only accessors so xrpld telemetry can observe library state without libxrpl depending on telemetry: worker-pool occupancy onJobQueue, a store-duration total and accessor on the node store, missing-node and stash depth on the inbound ledger,getBad()/getDuplicate()onSHAMapAddNode, and a copy-forward total on the rotating database. No behaviour change to any of them.What Changed
Bootstrap, previously uninstrumented — DNS resolve outcome and latency; outbound dial outcome and latency; negotiation failures by reason, so a wrong
network_idis distinguishable from unreachable peers; validator-list fetch outcome per site, and trusted key count against the required quorum; the network close-time offset, whichserver_infohides below 60 seconds.Sync state machine — mode transitions labelled
from/to, so a healthy climb is distinguishable from flapping; time to firstfull; the network-ledger gate; server stall seconds; how many ledgers behind the network.Ledger acquire and SHAMap fetch — outstanding missing tree nodes per acquire, which is what separates "slow" from "will never finish"; tree-node cache hit rate; a no-progress counter; add-node good/duplicate/invalid; whether a ledger came from local storage or the network.
Job queue — whole-pool saturation, complementing the per-type occupancy from the base branch.
Quorum and publish — the trusted-validation tally against quorum at the pre-accept gate, and the gap between validated and published sequences.
Back-fill and persistence — node-store read and write service time, where the write side had no signal at all; fetch-pack no-peer; replay fallback and outcome; the per-sweep heap-trim cost and the
online_deleterotation write amplification, both named in the 3.3.0 slowdown investigation.Peer supply and terminal blockers — how many peers can actually serve the wanted sequence; disconnect reasons and connect outcomes, previously one undifferentiated number; the PeerFinder slot census; serve refusals; the amendment-block countdown; wrong-chain ledger jumps.
Spans —
txset.acquire(transaction-set acquisition had none), the acquire split into header/account-state/transaction-tree phases, the outbound dial, and serving a peer's request. Every span finalises exactly once on every exit, including the destructor.One trace per ledger — the acquire, validation-accept, validate and store spans now derive their trace id from the ledger's own hash, so one Tempo search shows whether a slow ledger was slow to fetch, to be accepted, or to persist. They are siblings rather than a chain because the accept gate is entered from three different threads.
Metric naming — every instrument name, label key and bounded label value is a compile-time constant, and the naming check gained three rules enforcing that plus the duration and counter suffix conventions, which nothing enforced before.
Dashboard and docs —
ledger-sync-health.json, 39 panels in 9 rows ordered the way a node actually progresses; a runbook decision tree branching on symptom; reference and glossary entries for every signal.Before / After
Test Plan
service_instance_idTen signals are documented rather than asserted, each with a note saying why: a healthy five-node localhost cluster cannot produce a peer disconnect, a serve refusal, a wrong-chain jump, a replay fallback, an amendment block, or an
online_deleterotation. Asserting them would fail CI on a healthy run.Future Tasks
[ledger_replay].FullBelowCachereports a hit rate of zero becauseKeyCacheupdates different counters thangetHitRate()reads. Pre-existing in libxrpl; the panel is deliberately not shipped rather than shipping a permanently empty series.