Two live-verified dashboard defects found while auditing the devnet nodes.
Sub-millisecond spanmetrics buckets. The ladder started at 1ms, but most
xrpld spans are far below that: tx.preflight means ~0.012ms and tx.preclaim
~0.15ms. On the live nodes the first bucket held 13,761,009 of 13,761,419
tx.preflight samples (99.997%), so histogram_quantile pinned every p95/p99
at a constant 0.95ms and ~10 latency panels reported a boundary artifact
rather than a measurement. Adds 0.01/0.05/0.1/0.25/0.5ms ahead of 1ms in all
three bucket lists, which must stay identical or the connector misbuckets.
consensus_mode filter on spans that never carry the label. Only
consensus.round and consensus.ledger_close are stamped with consensus_mode
(verified against live series), so panels 1, 3, 12 and 14 -- which query
consensus.validation.send, consensus.proposal.send and
consensus.accept.apply -- blanked as soon as an operator picked any value
from the Consensus Mode dropdown. Drops the selector from those four; ids 2
and 11 keep it because their spans do carry it.
Note: existing histogram series retain the old le boundaries, so
span-latency panels show a discontinuity at collector restart.
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.
Conflict in processSession(): this branch added the request-payload-size
attribute on the same lines where the incoming change reverted
processRequest() from bool back to void. Kept both — the payload-size
attribute stays, and the call no longer captures a return value. The
trailing status block takes the incoming side, which leaves the
rpc.http_request span's status unset.
Six related defects in the RPC/gRPC span surface, all cases where a failure
was recorded as success or an attribute was missing on an error path.
GRPCServer: the non-exception branch set the span Ok unconditionally, then
sent a possibly-failed grpc::Status. The handler can return a non-OK status
without throwing, so every failed call traced as successful. Status now
follows result.second, with the error message as the span description.
ServerHandler: eight per-item error branches appended an error reply without
recording that the request failed. Batch responses and ripplerpc < 3.0 always
carry HTTP 200, so those failures were invisible and an entirely failed batch
ended its span as successful. Added an appendItemError() helper next to the
existing httpReplyError() lambda and routed all eight sites through it, so the
flag cannot be forgotten at a new call site.
ServerHandler: the early-return validation paths set the span error but not the
rpc_status attribute. Added it to httpReplyError() so every such path gets it.
RPCHandler: the fillHandler error path set only command and rpc_status, while
callMethod sets command, version and rpc_role. Error spans were therefore not
filterable by API version or role. The error path now mirrors that set.
RPCHandler: resolveCommandSpanName() checked only that command/method were
present, not that they agreed, while fillHandler rejects a mismatch as
rpcUNKNOWN_COMMAND. A request supplying both with different values was labelled
with one of the two names, misattributing the error to a command that never
dispatched. It now mirrors fillHandler's rule and collapses to "unknown".
ServerHandler: processRequest returned bool solely so the caller could set its
span status. Telemetry should read state, not shape the signature of the code it
observes, so the signature returns to void and rpc.process sets its own status
from spanHadError. The enclosing rpc.http_request span now leaves status unset:
the OTel spec has instrumentation leave status unset unless the operation itself
errored, and reserves Ok for an operator asserting verified success.
Conflict in src/libxrpl/telemetry/SpanGuard.cpp: both sides added a different
include in the same alphabetical slot — phase-4 added <initializer_list>, the
incoming std::format change added <format>. Both are required, so both are
kept in sorted order.
SpanGuard::span() and freshRoot() joined the prefix and suffix by hand with
reserve() + three append() calls. std::format expresses the same join in one
line; libstdc++ has shipped <format> since GCC 13 and the project floor is
GCC 15.2 (BUILD.md), so it is available on every supported compiler.
Both callers are noexcept and std::format can throw (std::bad_alloc, or
std::format_error on a malformed spec), so an escaping exception would
terminate the process. The call is wrapped in a joinSpanName() helper that
catches and returns std::nullopt; the caller then returns a null guard, which
is the same degrade-to-no-op path already taken when telemetry is disabled.
Telemetry must never bring the node down.
One helper rather than a try/catch at each site keeps both call sites a single
line and puts the exception-safety requirement in one documented place.
Addresses a review comment on PR #6437.
Conflict in OpenTelemetryPlan/03-implementation-strategy.md §3.9: both
branches independently fixed the same defect (stale hand-maintained line
counts) in different ways.
- 1b (0ff947454c) stripped the Lines Added / Lines Changed columns but kept
the §3.9.1 and §3.9.2 tables.
- 1a (3ad525a48a) removed both sections outright.
Resolved in favour of 1a's deletion. The tables were pre-implementation
estimates with no source of truth, so trimming the columns only defers the
next drift; the file/component lists duplicated §3.1's directory tree, which
1b already keeps current (DiscardFlag.h, FilteringSpanProcessor). Keeping the
upstream resolution also stops the same conflict recurring on 1c..10.
1b-specific content verified intact after the merge: §3.1 retains the
DiscardFlag.h entry, the FilteringSpanProcessor and discard() annotations,
and the TracingInstrumentation removal. No references to the deleted
sections remain in any plan doc.
The §3.9.1 "Files Modified Summary" and §3.9.2 "Detailed File Impact"
tables carried hand-maintained per-component line counts that had drifted
from the plan: the Lines Added column summed to 1,565 while the Total row
claimed ~1,670. Files (34) and Lines Changed (120) reconciled, so only the
Added total was stale — residue from expanding Core Telemetry 5 -> 11 files.
Rather than patch one cell, remove both tables. They were pre-implementation
estimates with no source of truth, so any figure in them drifts again on the
next phase. §3.1's directory tree remains the canonical list of the 11 new
telemetry files, and §3.9.3-3.9.7 (risk, architectural impact, backward
compatibility, rollback) carry the assessment without inventing line counts.
No references to the removed sections exist elsewhere in the plan docs.
Resolves src/tests/libxrpl/CMakeLists.txt test_modules by keeping both
sides: develop's beast/nodestore/protocol additions (nodestore moved into
alphabetical position) and this branch's ledger module.
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>
The lane-attribution change added acquire_ledger_deferrals and
acquire_ledger_timeouts to the gauge but left this test expecting seven
labels, so it failed on every platform of both PRs: nine emitted against
seven expected. The exporters and the counters went in as separate
commits, which is how the test escaped the update.
The two new labels are asserted with values that differ from the all-lane
totals they are a subset of, so a counter wired to the wrong lane, or one
that ignored the flag and counted every lane, lands on the totals instead
and fails.
The round built each thread's batch inside the thread, before arriving at
the latch, so a throw there left the remaining threads waiting on an
arrival that never came -- the test hung instead of failing. A spawn loop
that ended early did the same.
Batches are now built before any thread starts, so nothing between spawn
and arrival can throw, and a guard counts down the shortfall for threads
that were never spawned before joining the ones that were.
The depth accounting having moved to insert entry, depthSamples is now
the denominator of the mean depth, so it gets its own assertions: equal
to insertCount once every thread has been joined, and moving with the
duplicate-key round. The overlap assertions are unchanged.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The counters that attribute deferrals and timeouts to ledger acquisition
compared a string literal in TimeoutCounter against a separate literal in
InboundLedger. Renaming the job would have silently sent both metrics to
zero: no build error, no failing test, and documentation that quietly
became wrong. Both now use one shared constant.
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>
