Grafana renders `unit: "ops"` as the literal string "ops/s" and `cps`
as "counts/s", so seven rate panels on this dashboard read as
"operations per second" regardless of what they counted -- nodes added
to a SHAMap, dial attempts, ledger-data requests served.
Replace them with custom-suffix units naming the quantity, following
the `suffix:/hr` and `si:drops` precedent already in this repo. Each
noun was checked against the emitting code rather than the panel title:
sync_addnode_total counts SHAMap nodes, peer.dial spans are connect
attempts, ledger.serve spans are peer requests.
Also spell out "Ops / Sec" as "Operations / Sec" on the NodeStore panel,
since abbreviating the noun defeats the point of naming it.
Queries are unchanged; the values were already correct.
The two key-job latency panels on node-health declared milliseconds
while querying `job_running_us` / `job_queued_us`, which record
microseconds. Every reading was a thousand times too large: the p95 for
acceptLedger, 241495us, rendered as "241 sec" instead of 241ms.
job-queue.json already read the same metrics as microseconds, so the
two dashboards disagreed by 1000x on identical data.
Also replace the generic `ops`, `cps` and `cpm` units, which Grafana
renders as the literal "ops/s", "counts/s" and "counts/min", with
custom-suffix units naming what each panel counts -- jobs, ledgers,
validations, lookups. The per-minute panels already scale correctly in
their queries; only the noun was missing.
Normalise the micro sign to U+00B5 throughout. Four axis labels used
the visually identical Greek mu, U+03BC, while every unit field used
the micro sign.
Ledger Close Rate plots ledgers closed beside ledger fetches, which one
unit cannot describe; give the fetches series its own unit on a
right-hand axis. Drop two field overrides on NodeStore Read Latency
that restated the panel unit as a custom suffix -- a suffix is appended
verbatim, so it would have suppressed magnitude scaling and left large
values unreadable, the same defect fixed above.
Queries are unchanged apart from the transaction path piechart, which
moves from rate() to increase(): a per-slice "per second" reading is
not a share of a total.
Alongside, widen the Complete Ledger Ranges table to full width, hoist
the stat panels above the fold, and bring the touched panels up to the
tooltip and null-spanning guidelines.
The four job-latency panels on node-health declared milliseconds while
querying `job_running_us` / `job_queued_us`, which record microseconds
(MetricsRegistry records the raw value, and the instrument description
says microseconds). Every reading was therefore a thousand times too
large: the p95 for acceptLedger, 241495us, rendered as "241 sec"
instead of 241ms. job-queue.json already read these same metrics as
microseconds, so the two dashboards disagreed by 1000x on identical
data. Switch node-health to microseconds to match.
Also replace the generic `ops` and `cps` units, which Grafana renders
as the literal "ops/s" and "counts/s", with custom-suffix units naming
what each panel counts -- messages, fetches, calls, mismatches.
Two panels plot more than one quantity on a single axis, which no
single unit can describe. Give each series its own unit through field
overrides: reads per second beside two queue depths on NuDB Read
Pressure, and ledgers beside fetches on Ledger Close Rate, the latter
on a right-hand axis.
State Duration Rate plots a seconds-per-second time share, which can
exceed 1.0 and so is not a percentage; label it as the ratio it is.
The normalised share already exists as its own panel.
Queries are unchanged; the values were already correct.
Alongside, bring the touched panels up to the dashboard guidelines and
hoist the stat panels above the fold.
Grafana renders `unit: "ops"` as the literal string "ops/s", so every
rate panel read as "operations per second" regardless of what it
actually counted. `Ledger Build Rate` showed "0.258 ops/s" where the
value is one ledger every 3.9s -- the number was right, the unit was
meaningless.
Replace the generic units with Grafana custom-suffix units naming the
quantity, following the existing `suffix:/hr` and `si:drops` precedent
in this repo. Nine of these are `stat` panels with no axis, so the unit
string was the only text a reader ever saw.
Also switch the two trusted/untrusted piecharts and the transaction
path piechart from rate() to increase(): a per-slice "per second"
reading is not a share of a total, counts in the window are.
Queries are unchanged apart from those three; the values were already
correct.
Alongside, bring the touched panels up to the dashboard guidelines:
tooltip mode/sort/max-height, 30-minute null spanning, and axis labels
in title case. Hoist the stat panels above the fold on
ledger-operations and rpc-performance.
Panels that a later branch in this chain removes are deliberately left
alone -- fixing them would only add merge conflicts.
Someone filling in .env.grafanacloud-alloy has no reason to know that
deployment.environment and xrpl.network.type are not env vars: they are
literals inside the tier transform, because OTTL statements do not expand
sys.env(). Left alone they ship as prod/mainnet and every dashboard
environment and network filter reads the wrong tier. The example now says
so, and notes service_instance_id needs nothing here.
Alloy declared 24 spanmetrics dimensions where both collector configs
declare 28. The four missing ones -- timed_out, object_type,
validation_status and accept_gated -- are emitted by the code and queried
by ledger-sync-health panels "Ledger Serve Rate by Object Type" and
"Trusted Validation Accept Rate by Status", so on an Alloy deployment
those panels collapsed to one undifferentiated series.
deploy-run-otel-xrpld.md already requires the dimension sets to stay in
sync; the config had drifted from it. Alloy's other differences from the
collectors (no filelog, no tail_sampling) are deliberate -- it is
cloud-only by design.
Two gaps meant the last failure produced no evidence of its cause.
The node-log upload was gated on `if: failure()`, but the validation step
sets continue-on-error, so the job is not failing at that point and the
condition never fired. Every failed run silently skipped the one artifact
that records why a node did not reach consensus. It now keys on the
validation step's own outcome, and also collects the harness logs.
Transaction failures were logged at DEBUG, which CI does not enable, so a
run where all 3052 submissions failed on a refused connection reported
nothing about it. The first occurrence of each distinct failure kind is now
a warning and repeats stay at DEBUG, so one refused connection says so once
instead of 3052 times.
Three template variables (xrpl_work_item, xrpl_branch, xrpl_node_role)
select their datasource via ${DS_PROMETHEUS}, but the variable itself was
absent from templating.list, so those label_values() lookups resolved to
no datasource and the filters stayed empty.
Add the datasource variable as the first templating entry, matching the
other dashboards in this folder.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The dashboard's template variables and panel targets select their
datasource via ${DS_PROMETHEUS}, but the variable itself was absent from
templating.list. An unresolvable datasource variable leaves those queries
without a datasource, so the panels render empty.
Add the datasource variable as the first templating entry, matching the
other dashboards in this folder.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Three measurement fixes landed with no doc or dashboard change, leaving text
that is now false and one fix unusable from a dashboard.
Deferrals and timeouts are recorded in TimeoutCounter, a base shared by five
subclasses, so the all-lane pair could show the documented livelock
fingerprint while ledger acquisition was healthy. The runbook procedure and
the reference doc now name acquire_ledger_deferrals and
acquire_ledger_timeouts and say why the all-lane pair misleads; a new panel
plots the ledger-scoped pair as rates on one axis, since the divergence is
the signal. The existing panel is retitled All Lanes and points at it.
Writer mean depth is depthSum over depthSamples, not over insertCount, and
the measured 1.60 came from the biased estimator, so it and the 37% queueing
share derived from it are lower bounds rather than values. The reference
table now marks them as such, and the decision rule is shown to survive the
correction rather than depending on the exact figures.
Completions were never counted for acquisitions satisfied from the local
store, so the run that read zero across 510 seconds had in fact reached
full. Every place that treated a zero as a symptom now says it only means
something on a build that has the fix.
Also corrects the sync-diagnosis label-value count from 13 to 15 and a stale
source line range; the instrument count stays 35, because both new values
multiplex onto the existing nodestore_state gauge.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Four findings from a review pass over the PR.
The "Spans & traces" row was empty. Moving the row header down to clear
the back-fill panels was only half the change -- the seven span-derived
panels stayed at their old y, one unit below the native panels, so every
pair overlapped and Grafana parented all fifteen to "Back-fill &
persistence". The panels now sit below the row header, which restores
the split the runbook already describes: eight native panels answer "how
much", seven span-derived ones answer "which". Both rows stay expanded,
so the docs no longer call them collapsed.
metric_constants() excises each namespaced block before the flat
prefix pass. The flat pass classifies by identifier prefix and is meant
for headers that name the role in the identifier because they have no
`namespace metric`/`label`/`lval`; it was running over the whole header,
so a `kLabel`-prefixed constant written inside `namespace metric` landed
in both buckets and an instrument name became a valid label key for Rule
D. Nothing in the tree does that today, which is why it went unnoticed,
and why the guard is a test rather than a fix for an observed failure.
The `site` label now keeps a non-default port and drops userinfo residue
from the host. Omitting the port unconditionally merged two local sites
that differ only by port; printing it unconditionally would have renamed
the existing `https://vl.ripple.com` series. Comparing against the
scheme default distinguishes a configured port from the one the Resource
constructor fills in. parseUrl's host group also permits '@', so a
malformed URI with two of them leaves part of the userinfo in `domain`.
Adversarial validation of the previous commit found one of its two code fixes
was diagnosed wrongly and the other incomplete. Both are corrected here, along
with the layers the first pass missed.
1. The new dial outcome was named for the wrong condition. It was added as
`duplicate` on the belief that PeerFinder had already granted a slot for the
address. It has not: `Logic::onConnected` contains exactly ONE false-returning
path and it is the self-connect check, which logs "Logic dropping as self
connect" (include/xrpl/peerfinder/detail/Logic.h). The duplicate check lives
in `newOutboundSlot`, evaluated before a ConnectAttempt exists, so a real
duplicate can never reach this branch.
That mattered beyond the name: the previous commit told operators the outcome
was benign churn to ignore, when it actually reports a local misconfiguration
-- this node has its own address in [ips_fixed] or behind its advertised
endpoint, and every dial to it is wasted. Renamed to `self_connection`,
reusing the slug `handshake_negotiation_fail_total` already publishes for the
same fault so it reads identically on both signals, and every description
corrected to say so. The fail() string now reads "Self connection" too.
The first pass also missed three enforcement and contract sites: the
ConnectAttempt.h Doxygen state machine (which still mapped the slot branch
onto tls_fail), the LedgerSpanNames unit test (which pinned exactly five
values over a std::array<..., 5> and so left the new member untested), and the
span-derived twin panel plus two reference docs that still published the old
five-value domain.
2. The credential-free site label was incomplete twice over.
- It appended the port, and `Resource::Resource` DEFAULTS that to 443/https
and 80/http when the config omits one. The label would have become
`https://vl.ripple.com:443/` where Grafana Cloud currently holds
`https://vl.ripple.com`, silently renaming the series for every deployment
already scraping this metric. Verified against live label values before and
after; the port is now omitted.
- parseUrl's path group is `(/.*)?`, greedy to end of string, so a query or
fragment lands inside `path`. A list URL authenticated by `?token=...` would
have leaked exactly as userinfo did. The path is now truncated at the first
'?' or '#'.
Also updated the MetricNames.h usage example, which still taught the raw-URI
pattern to the next author, and the 09-doc row that described the label as the
configured URI.
3. Rule J hardening from the same review: `classify_instrument_kind` returns an
`other` sentinel for a non-factory macro, and storing it in the kind set could
render a future conflict as "created as counter and other". The sentinel is
now skipped, keeping it doing what it already did -- matching no shape rule.
Added a second regression test whose input the pre-fix code reported as CLEAN
(gauge-then-histogram on a `_us` name), so the guard is proven by a 0-vs-1
difference and not only by a changed message. Both new tests were run against
a reconstructed last-wins implementation and both fail against it.
Documented the conflict class in the Rule J rows of the checker README and
CONTRIBUTING, which previously described only the suffix conventions.
Verified: naming checker exits 0 with Rule J passing all 40 real names; 140
checker tests pass; 15 dashboards validate; both workload JSON files parse;
clang-tidy over the full compile database reports no finding on any changed line
of ConnectAttempt.cpp or ValidatorSite.cpp; pre-commit passes.
Not verified: not compiled. The label change adds string truncation and the
outcome rename touches a constexpr used across three translation units, so CI's
build remains the first real check on both.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Four defects from the automated review on PR #7875, each verified against the
current tree before fixing (one further comment, the row-63 dashboard overlap,
was already fixed by an earlier commit and needed nothing).
1. Rule J could not detect an instrument-kind mismatch. instrument_kinds() wrote
`kinds[wire] = ...`, so a wire name created through two different factories
kept only the kind visited last and whichever emit site the file walk reached
last silently decided the verdict. It now collects a set per name and reports
the conflict itself -- one name exporting two instruments is the defect, and
no suffix can be correct for both. Added a regression test that builds a name
as both a counter and an observable gauge and asserts the message names both.
2. A duplicate connection was reported as `tls_fail`. The TLS handshake had in
fact succeeded; PeerFinder simply already held a slot for that address, which
is ordinary churn on a healthy node. Conflating the two made a rising
`tls_fail` unreadable -- it could mean unreachable peers or merely a busy
PeerFinder, and those need opposite responses. Added a distinct `duplicate`
outcome and carried the widened vocabulary through every place that
enumerates it: the panel description, both filter descriptions, the runbook
branch table, the runbook outcome list and the expected_spans note. The
`dial_outcome` template variable is a label_values() query, so it picks the
new value up on its own.
3. ConnectAttempt::onShutdown had no `operation_aborted` guard, unlike the five
other handlers in the same file. A clean teardown was therefore counted as
`upgrade_fail`, inflating that outcome on any node shutting down with dials in
flight.
4. ValidatorSite used the raw configured URI as a Prometheus label.
[validator_list_sites] accepts credentials in the URI and ParsedUrl keeps them
in username/password, so a configured `https://user:pass@host` would have
copied the secret into a metric label and on into the collector, Prometheus
and every dashboard. The label is now rebuilt from scheme, host, port and
path -- everything needed to tell one site apart, and nothing more.
Verified: naming checker exits 0 with Rule J still passing all 40 real
instrument names; its unit tests now number 139 and all pass; 15 dashboards
validate; both workload JSON files parse; clang-tidy over the full compile
database reports no finding on either changed .cpp; pre-commit passes.
Not verified: not compiled. Item 4 introduces string concatenation and item 2 a
new constexpr, so CI's build is the first real check on both.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Brings in the phase-10 revert of the nodestore read-latency histogram plus the
nudb_bytes -> stored_object_bytes rename.
Conflicts in MetricsRegistry.{h,cpp} resolved keeping both intents:
- MetricsRegistry.cpp: dropped everything that existed only to serve the
reverted nodestore_read_us histogram -- the addSubMillisecondHistogramView()
helper, its call site, the kSubMillisecondBoundaries array and the
NodeStoreMetricNames.h include. Kept every view this branch registers
(consensus round duration, sweep_malloc_trim_us, dns_resolve_latency_ms,
overlay_dial_latency_ms) and the shared addHistogramView() base helper.
Took the rename at the storage_detail observe() call site.
- MetricsRegistry.h: took phase-10's move of the four nodestore_state observe
helpers and their ObserveFn sink from private to public, while keeping this
branch's enriched Doxygen on observeNodeStoreTotals().
Also corrected the registered-view count in the 09 reference doc: neither side's
arithmetic survives the merge, since this branch adds four views phase-10 never
saw and the revert removes one. Ten views are registered now, not six or seven.
Drops nodestore_read_us and everything added to reach it. read_mean_us already
carries microsecond precision and separated the two sync failure modes cleanly
in live testing -- 8.8 us on a clean store against a 223 us cold-store peak --
so the distribution added no signal that changed a diagnosis.
The cost of getting it was disproportionate. NodeStoreScheduler had no path to
the metrics registry, so its production constructor grew a ServiceRegistry
parameter: a metric addition changing a production signature. That in turn
forced an edit to a pre-existing test, src/test/app/SHAMapStore_test.cpp, whose
only stake in this is that it constructs a scheduler. Worse, the scheduler is
built in Application's member initializer list, long before metricsRegistry_
exists, so the registry could not be captured once and had to be re-resolved on
every fetch -- a lookup on a path that runs millions of times per sync.
The constructor returns to taking JobQueue& alone and SHAMapStore_test.cpp
returns to the single-argument call, leaving that file differing from its
pre-change form only by the NodeStore:: to node_store:: rename it picked up from
develop.
FetchReport::elapsed stays microseconds and onFetch keeps its explicit
duration_cast to milliseconds for addLoadEvents, which takes milliseconds. That
widening was a separate fix and is what makes read latency measurable at all.
kSubMillisecondBoundaries loses its only consumer and regains [[maybe_unused]],
which is the state the commit that introduced it left it in; without the
attribute an unused constant is an error under wextra with werr.
Also removes the ledger-data-sync panel that charted the histogram and the
fetch_type and found template variables, which filtered on labels no metric
emits any more, plus the runbook and reference-doc sections and the two
instrument and view counts that named it.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The Sync State panel only ever showed Connected and Full. Two causes:
1. state_tracking{metric="state_value"} is emitted with values 0-6, where
5 is FULL+validating and 6 is FULL+proposing, but the panel declared
max: 4 with value mappings for 0-4 only. Values above 4 were pinned to
the axis ceiling and rendered unmapped. Most nodes sit at 6, so the
majority of series were clipped.
2. The gauge samples the instantaneous mode on a 10s export tick, so
states shorter than one tick fall between samples. A real sync showed
SYNCING for a single scrape and skipped TRACKING entirely. This is the
sampling hazard already noted on StateAccounting in NetworkOPs.h.
Extend the panel domain to 0-6 with Validating and Proposing mappings and
matching threshold steps, and add a colour-coded state-timeline panel where
each band's width is the time spent in that state, so brief states appear
as thin slivers rather than disappearing. The timeline reads
server_info{metric="server_state"} (raw OperatingMode 0-4) rather than
state_value, which folds 5 and 6 onto FULL and would split one Full band
into three colours.
Panel layout below the insertion point shifts down by 6 rows.
Note this makes short states legible, not lossless: exporting
StateAccounting's per-state duration accumulators is the sampling-immune
fix and is left as follow-up.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The nudb_bytes label value on the storage_detail gauge named something the
code never measured. It observes Database::getStoreSize(), which returns the
storeSz_ accumulator: the cumulative payload bytes of objects this process
handed to the NodeStore. That is not a NuDB file size. It excludes NuDB's
keys, bucket padding and log, and it resets with the process while the files
on disk do not.
The name caused two concrete errors. It invited sizing the store on disk from
a number that cannot do it, and it invited a write-amplification ratio against
node_written_bytes -- which reads the same accessor at MetricsRegistry.cpp:836,
so that ratio is a constant 1.0 and measures nothing.
The nudb_ prefix was wrong too. storeSz_ is written only by
Database::storeStats(), called from DatabaseNodeImp, DatabaseRotatingImp and
Database itself. No backend code touches it, so the value reads the same on
RocksDB. That distinguishes it from the real nudb_* family
(nudb_writers_in_flight and friends), which come from getWriteStats() and are
absent entirely on a non-NuDB backend.
stored_object_bytes says what the value is and claims nothing about the
filesystem. Docs already described the value correctly; they keep that
explanation and now also record the old name, so a query pinned to it can be
traced. Neither Backend nor Database exposes an on-disk size accessor and none
was added -- no metric reports the store's on-disk size today.
Updates the node-health panel title, description and PromQL, and the four docs
that name the label value.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Eleven local phase-10 commits, mostly documentation and dashboard-description
work, plus one real include fix. No conflicts (git merge-tree reported 0).
Reviewed for interaction with this branch:
- b50aa17aee adds <xrpl/consensus/ConsensusSpanNames.h> to RCLConsensus.cpp and
PeerImp.cpp. This is the same misc-include-cleaner finding my local clang-tidy
run reported on those two files, so the merge resolves two of the 74
pre-existing findings rather than adding any.
- 8ade2b43d9 / 7da5ac5992 / d18f713f91 rename the node-health found-ratio and
stored-bytes panels and correct the nudb_bytes and NuDB found-ratio
descriptions, then repoint the runbook at the new names. These are the same
correction phase-10 made earlier to node_reads_hit -- 'found an object', not a
cache hit -- carried through to the panels that render it.
- ledger-sync-health.json is untouched by phase-10, so none of this branch's
panel work is affected.
Verified after merging: naming checker exits 0, its 138 unit tests pass, all 15
dashboards validate, levelization produces no diff, and this branch's own work is
intact (4 touch() call sites, the ledgers_behind guard, 54 panels / 18 bargauges).
Brings forward the node_reads_hit and nudb_bytes label corrections from phases 7
and 9.
Conflicts resolved keeping both sides:
- 06-implementation-phases.md: kept phase-10's unprefixed `storage_detail` gauge
name with phase-7's corrected getStoreSize() description.
- node-health.json: kept phase-10's rewritten panel structure and re-applied the
panel renames (NodeStore Read Found Ratio, NuDB Stored Bytes), legends and axis
labels on top of it.
node_reads_hit is a found count, not a cache hit. fetchHitCount_ increments
whenever a fetch returned an object, whatever served it, and a node with
online_delete uses DatabaseRotatingImp which has no NodeObject cache at all. The
ratio therefore reads near 100% while every fetch goes to disk, which made the
cold-read failure mode look impossible on the board.
- Phase9_taskList: node_reads_hit is a found count, not cache-served reads.
- node-health: "NodeStore Cache Hit Ratio" -> "NodeStore Read Found Ratio",
legends "Cache Hit Ratio" -> "Found Ratio" and "Reads Hit (cache)" ->
"Reads Found"; corrected the ratio panel's axis label, which read
"Operations / Sec" on a percentunit panel.
- node-health: "NuDB Storage Size" -> "NuDB Stored Bytes". nudb_bytes observes
getStoreSize(), the accumulator node_written_bytes also reads, so it is
cumulative object-payload bytes and not a filesystem measurement.
- ledger-data-sync: point the read-latency discriminator at the renamed panel.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
nudb_bytes was documented as a NuDB file size, one place even claiming a
filesystem stat. It observes Database::getStoreSize(), which sums the object
payloads this process has written. It excludes NuDB's keys, bucket padding and
log, and resets with the process. node_written_bytes calls the same accessor, so
the two series are equal by construction and a write-amplification ratio built
from them is a constant 1.0. Neither Backend nor Database exposes a file-size
accessor, so nothing reports on-disk size today.
The Ledger Data & Sync panel plotting node_reads_hit / node_reads_total was
titled "NuDB Cache Hit Ratio" and described as reads served from cache.
fetchHitCount_ increments whenever a fetch returned an object, whatever served
it, so the ratio is a found rate. It reads near 100% while every fetch goes to
disk, which made the cold-read failure mode look impossible. Renamed to
"NuDB Read Found Ratio" and rewrote the guidance to pair it with read latency.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Two metric-level defects the sync analysis identified, fixed at the source rather
than worked around on the dashboard.
1. InboundLedgers::sweep() destroys any acquire idle for more than a minute, and
that destruction is what telemetry reports as outcome=abandoned. But
lastAction_ was only refreshed by the constructor, update() and done() --
never by the receive path. With JtLedgerData capped at 3 concurrent jobs and
33 acquires in flight, an acquire whose peers were answering normally could
wait past the cutoff for its turn to apply data and be deleted for looking
idle. Measured on a fresh mainnet sync: 490 abandoned acquires against ZERO
expired retry budgets, so every one was a sweep, not a give-up.
gotData() now calls touch(). The sweeper's idle test measures real inactivity
instead of queue wait.
lastAction_ had to become atomic to allow this. It was a plain
clock_type::time_point written by the acquiring thread and read by sweep() on
the timer thread; adding a third writer on peer threads would have been a data
race. It is now std::atomic<clock_type::duration::rep> with relaxed ordering on
both sides -- the sweeper compares against a 60-second threshold, so a value
one tick stale cannot change its decision.
2. getLedgersBehindNetwork() returned the entire ledger sequence space on a fresh
node. The existing floor only guarded being ahead of every peer; it did not
guard having validated nothing at all, so validated=0 against a live tip gave
105,892,534 -- an accurate subtraction of a meaningless quantity. It
auto-scaled every consumer's axis and would trip any threshold. Distance to
tip is undefined before the first validated ledger, so it now reports 0 until
there is one, and the sync-state signals carry the initial-acquire progress.
The clamp_max(1e6) added to the Ledgers Behind Network panel as a stopgap is
removed: the metric is correct now, and leaving the clamp would hide a real
large backlog.
Verified: clang-tidy over the full compile database reports no finding on any
changed line in the three files (the pre-existing misc-include-cleaner and
misc-const-correctness findings elsewhere in InboundLedger.cpp are untouched by
this change). pre-commit passes including clang-format and the Doxygen style
check; validate_dashboards passes.
Not verified: not compiled -- per instructions.md the build needs approval, so CI
is the first real compile of the atomic change.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Two conflicts, both additive-vs-additive; each resolution keeps both sides.
check_otel_naming.py -- phase-10 taught the L6 label extractor to match the
label MAP first and to resolve a key hoisted into a `k...Label` constant,
scanning headers as well as sources. Our side had added the two-regex
first/subsequent literal scan and the `metric_constants(root)[1]` union that
covers the `namespace label` header style.
Kept phase-10's mechanism whole: METRIC_LABEL_MAP + the `(?:^|\{)` key regex
already subsumes what METRIC_LABEL_NEXT did, since matching inside the map body
makes every pair after the first open with a single `{`. So METRIC_LABEL_NEXT is
dropped as genuinely redundant rather than kept as a duplicate scan, and the
reason it existed is folded into METRIC_LABEL's comment. Re-added our
`metric_constants(root)[1]` union on top: LABEL_CONST_DEF only matches
`k`-prefixed identifiers, so it cannot see MetricNames.h's `label::jobType`
style, and without that union Rule D would reject dashboards querying labels
Rule I forced into constants. The two derivations are complementary and both
are now documented as such.
MetricsRegistry.cpp -- both sides added a new sibling view-registration helper
next to addMicrosecondHistogramView, and both added a registration call in
initExporterAndProvider(). Kept all four helpers
(addHistogramView/Microsecond/RoundDuration/SubMillisecond) and every
registration: phase-10's addSubMillisecondHistogramView + kNodeStoreReadUs
alongside our addRoundDurationHistogramView, sweepMallocTrimUs and the two
millisecond dial/resolve ladders.
phase-10's nodestore_read_us histogram does not duplicate our work. The
nodestore_latency gauge that would have overlapped it was retired in c4e434d520
before this merge, and the surviving nodestore_state gauge is complementary
rather than duplicative: both read the same fetch measurement, but the gauge
publishes only a since-boot mean via scaledMean() and cannot yield a
percentile -- the consequence observeNodeStoreTotals' own docs state plainly --
while the histogram buckets each fetch and can. The histogram also splits by
fetch_type and found, which the gauge cannot. phase-10 registered its
explicit-bucket View, so it does not inherit the SDK default ladder.
Each file keeps its own existing naming style: phase-10's k-prefixed constants
are left as-is, ours stay namespaced.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Second pass on the 54-panel audit. The first commit handled the count panels and
the description drift; these are the query and threshold defects.
Panels 4 and 6 (DNS Resolve / Outbound Dial p95) read NaN for the whole run.
Two faults compounded: both hard-coded [5m] instead of $__rate_interval, so they
ignored the dashboard time range entirely, and both wrapped the histogram buckets
in rate() even though DNS resolution and outbound dialling only happen during
startup -- a windowed rate over a series that stopped moving is 0/0. Dropping
rate() and reading the cumulative buckets gives the real distribution: p95 900 ms
for DNS, 1750 ms for dialling. Panel 52 kept its rate() (consensus rounds are
ongoing) but its hard-coded [5m] became $__rate_interval.
Panel 15 plotted 105,892,534 "ledgers behind" during the flagship window. The
underlying cause is in NetworkOPs.cpp -- getLedgersBehindNetwork() subtracts the
validated sequence from a networkTarget of 0 before any peer has reported -- and
that still needs a code fix. Meanwhile one sentinel spike flattened the real
0-20 backlog for the rest of the window, so the query now clamps at 1e6, far
above any true backlog. Reads 4 where it read 105 million.
Panel 17's sum by (from, to) dropped node identity, so with All nodes selected
every node's transitions summed into one bar. It now carries service_instance_id,
xrpl_branch and xrpl_work_item like every other panel.
Panel 38 divided by clamp_min(op_rate, 1), which turns "no operations in this
interval" into "one operation", reporting the whole duration total as if a single
op had consumed it. Replaced with a `> 0` gate so an idle interval draws a gap
instead of a fabricated latency.
Panels 13 and 45 had inverted thresholds: green began at 1 second, so every
sub-second time-to-full and time-to-first-validated rendered red -- the healthy
case was the alarming colour. Now green by default, yellow past 10 minutes, red
past 30.
Panel 48's p95 had no outcome filter, mixing abandoned and timed-out spans (which
sit at the retry ceiling by construction) into what reads as completion latency.
Restricted to outcome="complete".
Verified against Grafana Cloud: 66 queries, 0 parse errors, 56 with data, 9
legitimately empty (fault-only counters plus the write-timing series that
pre-dates both recorded nodes). The single remaining NaN is on panel 47 and is an
artifact of my verification harness forcing a hard 5m window; the panel's own
$__rate_interval returns 492 ms, so it needs no change.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A 54-panel audit across 11 dimensions, each finding adversarially re-verified
against live data, returned 38 confirmed defects. This fixes them. Most were
introduced by the recent rate-to-count conversion itself.
1. increase() was the wrong function for these counters. A counter that only
moves at startup is born at its final value inside the window and never
rises, so increase() reports 0. Measured: dns_resolve_total reads 4 but
round(increase(...[$__range])) returned 0 -- the panel lost the signal
entirely. increase() also drops whatever accrued before the window opened,
which under-reported the rest (overlay_connect_total 110 against a true 130,
unl_fetch_total 10 against 14, peer_disconnect_total 7 against 8).
All 18 count panels now use last_over_time(...[$__range]), which on a
fresh-node board is the cumulative total since the process started -- exactly
what "count" means here.
EXCEPT panel 49. span_calls_total comes from the collector's spanmetrics
connector, which is collector-side state and does NOT reset when xrpld
restarts, so last_over_time would report the collector's lifetime across every
run: it read 2624 header completions where run C actually had 297. That panel
keeps round(increase(...)) and now reports 297/278/212/7, matching the
analysis. The distinction is process-level counter vs collector-side counter,
and it decides which function is correct.
2. Descriptions still described rates after the conversion, over three passes of
wording (Reading it / Healthy range / Watch for blocks, "Rate of", "per
second", "/s", "a rising rate"). 11 panels corrected; the two surviving uses
of "rate" are legitimate (a cache hit-rate reference, and panel 49 explaining
why a count reads better than a rate).
3. Ten descriptions pointed at panel titles that no longer exist, because the
conversion renamed the panels they cross-referenced. Two others named panels
that never existed on this board at all: "Total Jobs Queued" (now Worker Pool
Capacity & Total Backlog, panel 27) and "Fetch-Pack Peer Starvation" (now
Peers Able to Serve Needed Sequence, panel 28).
4. Panels 18 and 23 applied $acquire_metric on top of a hard-coded metric
selector, so the two ANDed: any selection outside the panel's own values gave
an empty graph and All was the only usable state. The redundant template
selector is gone; the panel's metric pair is its identity.
5. Panel 23 drew two series with different ranges (received_data_depth 0-20,
in_flight 13-49) under one yellow threshold at 16, so in_flight was
permanently yellow. The threshold is now scoped to received_data_depth.
6. The "Spans & traces" row sat at y=248, the same y as panels 38/39, so Grafana
folded the Back-fill panels into the wrong row. Moved to y=296, below the last
back-fill panel. Rows are now strictly ascending with no collision.
Verified against Grafana Cloud Prometheus: all 66 panel queries parse, 0 errors,
57 returning data (up from 56 -- panel 3 was one of the ones increase() had
silenced). validate_dashboards and check_otel_naming both pass.
Not verified: no PNG renders this round; the local Grafana and Prometheus are
down, so every check ran against Cloud data via the datasource proxy.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A stalled ledger sync has two distinct causes that look identical from the
job queue: in both, the ledgerData lane sits at its concurrency limit of 3
with work waiting. Lane occupancy therefore discriminates nothing, and the
dashboard had no panel that separated the two.
Adds a Sync Bottleneck Discrimination row to Ledger Data & Sync with seven
panels built on the nodestore_state write-path and acquire counters:
- NodeStore Read Latency, lifetime and windowed, on a log axis with
threshold lines at 10 and 100 us. This is the discriminator: fast reads
mean the cost is on the write side, slow reads with a high hit ratio
mean every access is paying disk latency.
- NuDB Writer Queue Depth, the confirming half. NuDB takes one global
mutex per insert, so depth above 1 is queueing at that mutex.
- NuDB Insert Time, mean against true max, which quantifies how much of
each insert is wait rather than work.
- Acquire Deferrals vs Timeouts on one panel, because the livelock
fingerprint is deferrals rising while timeouts stay flat, and neither
counter shows that alone.
- Acquisition Progress and Discarded Acquire Work, the outcome side: no
completions while the lane is busy means acquisitions resolve neither
way.
- NodeStore Read Latency Distribution split by fetch_type and found, from
the nodestore_read_us histogram, since a mean cannot tell a uniformly
warm store from a mostly-warm one with a cold tail.
Every expression carries the existing service_instance_id and tier filters.
The histogram introduces two new label dimensions, so fetch_type and found
template variables are added to match.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
nodestore_latency published six values that nodestore_state already
publishes from the same Database accessors, so the two gauges were
duplicate readings of the same atomics:
write_count -> node_writes getStoreCount()
read_count -> node_reads_total getFetchTotalCount()
write_duration_us -> node_writes_duration_us getStoreDurationUs()
read_duration_us -> node_reads_duration_us getFetchDurationUs()
write_mean_us -> write_mean_us store duration / count
read_mean_us -> read_mean_us fetch duration / count
nodestore_state is kept because its means go through scaledMean(), which
saturates at INT64_MAX instead of wrapping and omits a mean when the
denominator is zero rather than reporting a misleading 0 us.
Removes registerNodeStoreLatencyGauge, its instrument member, the
metric::nodestoreLatency constant and the lval::nodestore_latency label
namespace. The gauge-over-histogram rationale and the "p99 is not
obtainable" consequence are folded into observeNodeStoreTotals' docs.
Retargets the gauge-contract test onto nodestore_state rather than
deleting it: the scaledMean arithmetic is covered by the static_asserts
in tests/libxrpl/telemetry/MetricsRegistry.cpp, but nothing else asserts
that these named series multiplex onto one instrument keyed by `metric`.
The test now calls the production scaledMean instead of a copy of the
division, and its sub-microsecond case asserts scaledMean's actual
behaviour (a genuine mean of 0 on a zero numerator with a non-zero
count), which differs from the retired gauge's extra numerator guard.
Rewrites both ledger-sync-health copies' panel 38/39 queries and drops
the obsolete claim that the write numerator was never written: all three
concrete store paths call recordStoreDuration, so write_mean_us is live
on an ordinary node. The same stale [import_db] caveat is removed from
the runbook, the 09 reference row and the workload validator's note.
Two panels were left out of the previous sweep because a blanket rate-to-count
rewrite would have been wrong for them. Handled per target here.
Panel 65 (Sweep Heap-Trim) carries two cumulative counters, minor faults and
reclaimed KB. Both are count-shaped, so it becomes a bargauge over
round(increase(...[$__range])) like the rest, and the legends drop their
"/ Sec" suffix now that the values are totals rather than rates.
Panel 66 (Online-Delete Rotation) stays a timeseries. Its two targets are not
the same kind of thing: target A reads rotation_state{in_flight}, a 0/1 flag
whose whole value is seeing when it is high and for how long, and target B
rates rotation_state{copy_forward}, a cumulative write total. A count bargauge
would destroy the flag's time dimension. Instead the shared "cps" unit -- wrong
for a flag -- is replaced by per-target overrides: the flag pinned to a 0..1
left axis, the write rate on a right axis in cps. This matches how the metric
is documented to be read (MetricNames.h:648): copy_forward climbing while
in_flight is 1 is expected, climbing while it is 0 means the flag leaked.
Verified against Grafana Cloud Prometheus: 0 parse errors on both panels.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Seventeen panels on ledger-sync-health drew low-frequency discrete counters as
timeseries with an ops/s unit. The rate is arithmetically right but unreadable:
"0.1394 ops/s" is 278 abandoned tree phases over 33 minutes, and no reader can
recover the 278. rate() also extrapolates, so whole events rendered as
fractions -- peer disconnects showed 8.008 for 8 actual disconnects.
Measured every candidate against live Prometheus over a 33-minute mainnet sync
before converting; none exceeded 0.48 events/s, so all are count-shaped:
serve_refused_total 955 events 0.4785/s
ledger_quorum_shortfall_total 670 0.3354/s
sync_acquire_source_total 511 0.2559/s
overlay_connect_total 130 0.0652/s
sync_acquire_no_progress_total 109 0.0545/s
unl_fetch_total 12 0.0061/s
peer_disconnect_total 8 0.0040/s
Each becomes a bargauge over round(increase(...[$__range])) with unit short and
decimals 0, matching the existing Mode Transitions panel. Legends follow
instructions.md OTel rule 7 -- "MetricName [labels]" -- and keep node identity
via xrpl_ident, so a nine-node view no longer collapses to one bar. Panel 49
also gained node identity in its aggregation, which it was missing entirely.
Label values stay as emitted (upgrade_fail, not "Upgrade Fail"). They are wire
identifiers: rewriting them in the legend would hide what the metric reports and
break silently when a new value appears. The Title Case sits in the metric name.
The five zero-valued panels were checked rather than assumed dead --
ledger_replay_*, sweep_malloc_trim_* and rotation_copy_node_restore_total all
have real emit sites and are feature-gated off in this configuration.
Verified against Grafana Cloud Prometheus: 0 parse errors across all panel
queries, whole-number results throughout.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Three defects, all found by rendering the panels and running their queries
against a live mainnet node.
1. Ledger Acquire Phase Outcomes had an invalid PromQL escape.
The saved JSON held "ledger\\.acquire\\.(.*)", which decodes to
`ledger\.acquire\.(.*)`. In a PromQL double-quoted string `\.` is not a
legal escape, so Prometheus rejected the whole query:
parse error: unknown escape sequence U+002E '.'
The panel therefore rendered an error badge and "No data". A PromQL string
needs two characters, so the JSON must carry four backslashes.
2. The same panel never grouped by outcome, despite its title.
`sum by (span_name, timed_out, ...)` omitted `outcome`, so complete and
abandoned collapsed into one line. Measured at 16:45 UTC that hid a 29x
difference: astree complete=2085 against abandoned=71, all drawn as a
single indistinguishable series -- and every phase then showed the same
0.2596/s value, which is what made the panel look meaningless.
Now grouped by outcome, giving four real series (verified live):
header complete 0.1439/s, header abandoned 0.0772/s,
astree abandoned 0.1404/s, txtree abandoned 0.1404/s
The selector moves from timed_out (always "false" here, so it carried no
information and its filter var was redundant) to the declared
$span_outcome. Legend becomes "<phase> <outcome>"; axis label reads
"Phases / sec" to match the ops unit.
3. Seven stat panels and one heatmap dumped the raw label set as the legend.
With no fieldConfig.defaults.displayName but textMode "value_and_name",
Grafana has no name to show and falls back to printing every label:
{deployment_environment="local", exported_instance="xrpld-mainnet",
exported_job="xrpld", instance="otel-collector:8889", ...}
Rendered PNGs of panels 10 and 26 confirmed it. Fixed on ids 10, 12, 13,
14, 26, 36, 45 and 52 with the board convention already used by 24 sibling
stat panels: "${__field.labels.series} ${__field.labels.xrpl_ident}".
Verified afterwards by executing all 66 panel queries on this board against
live Prometheus: 0 parse errors, 56 returning data. The 10 empty ones are
counters a healthy node never increments plus panel 29, which is gated to stay
blank until a ledger is validated.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The Peer Ledger Supply Window panel drew supply_min_seq, supply_max_seq and
nothing else on one linear axis. Measured on a mainnet node, those sit around
105,890,000 and roughly 300,000 apart, so the 588-ledger tip movement that
shows whether sync is progressing was 0.0006% of the axis and read as a flat
line. unit "none" also printed the sequences unabbreviated and clipped the
legend.
The panel's own "Watch for" text asked the reader to compare supply_min_seq
against this node's validated sequence, but that line was not on the panel at
all, so the comparison meant switching dashboards.
Plot the two distances instead, which is what the panel was always asking
about:
History Headroom = validated_ledger_seq - supply_min_seq
Tip Gap = supply_max_seq - validated_ledger_seq
Zero is now the boundary in both directions: negative headroom is exactly the
"every peer pruned what I still need" case the description warns about, and it
becomes a zero crossing rather than a line-order comparison. Tip Gap gets the
right-hand axis because the two ranges differ by orders of magnitude
(measured: 299999..300001 against -1..1).
Both operands are gated `> 0`. Ungated, differencing the documented
"unknown window" sentinel of 0 yields the whole sequence space: measured
-105854935 for headroom and 105890295 for tip gap during the first ticks,
which destroys the axis for the rest of the window. Gated, the panel stays
blank until the node has a validated ledger and a peer has advertised a
range, which is the honest reading for that state.
Both queries verified against a live mainnet node through the full template
substitution: refId A = 300001 legend "History Headroom [xrpld-mainnet]",
refId B = -1 legend "Tip Gap [xrpld-mainnet]".
Runbook branch-C table, step 11 walkthrough and the 09 reference row follow
the rename and the new reading.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
