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>
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 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>
The decision rule for "slow to reach full" keyed on an absolute read-time
threshold and a found-rate threshold that misclassified the very run they
were written to explain: a populated-store run reading 31.8 us at 88.3%
found fell through both cuts and came out as "disk-bound" rather than the
cold-read case it is.
Replace it with two ordered questions -- is the read cost several times a
warm read, and is the write path queueing -- and demote the found rate to a
splitter that only applies once reads are known to be expensive. A high
found rate on its own is the normal state of a populated store, so it can
never be a trigger. The rule now classifies all four reference datasets
correctly, and the runbook shows the rule applied to each so the "confirm
against the reference points" step agrees with the table.
Also in the runbook:
- name the source of the devnet incident figures at the point of use, and
point forward to the caveat from the same paragraph
- state the provenance of the measured columns, and split the incident
figures into their own table marked as not our measurement
- say plainly that the compounding-factor explanation is an unconfirmed
hypothesis
- correct the deferral gate: it fires at the acquisition's own job limit of
5, not at the ledgerData lane cap of 3
- note that no read-max gauge exists, so the tie-break uses max_over_time
of the mean or the read histogram's p99
- split a PromQL block that put two expressions on adjacent lines, which
parses as one invalid expression
In the data-collection reference:
- node_reads_hit counts fetches that found an object, not cache hits
- nudb_bytes is cumulative payload bytes from the same accessor as
node_written_bytes, not on-disk size, so their ratio is a constant 1.0
- write_load and nudb_writers_in_flight are the same atomic on NuDB;
document that and what write_load means on RocksDB
- correct the instrument count to 8 and the view count to 7 after
nodestore_read_us and its view were added
- scope the Phase 9 query examples to one node with the regex form
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The new PromQL used an exact match on service_instance_id. The $node
template variable is multi-value, so an exact match returns nothing as
soon as more than one node is selected. Every one of the 442 filters
across the dashboards uses the regex form; the runbook now agrees.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Two different bottlenecks both present as the ledgerData job lane pinned
at its concurrency cap of 3, so lane occupancy diagnoses neither. One is
write-serialized (NuDB takes one global mutex per insert, so inserts
queue), the other is cold-read-bound on a populated store. Telling them
apart needs the storage-side signals, not the lane.
Adds to docs/telemetry-runbook.md a "Slow to reach full" procedure: a
Mermaid diagram of the two modes, a decision table keyed on whether
acquisitions are completing, the measured reference values from both
runs, and the deferral/timeout pair that fingerprints the disarmed
give-up path. States plainly that node_reads_hit is a found count rather
than a cache-hit rate, which is why a ~100% "hit rate" at 113 us per
read is the cold-read signature and not a contradiction.
Records honestly that the populated-store run was twice as fast despite
slower reads, so cold reads alone do not explain the long incident.
Adds reference rows for the 13 new nodestore_state label values and the
nodestore_read_us histogram to
OpenTelemetryPlan/09-data-collection-reference.md, in the authoritative
Phase 9 OTel SDK section alongside the existing NodeStore I/O table.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Slowness on the peer object-fetch path could be observed but not
attributed. Job duration metrics carry only `job_type`, and both
`RcvGetLedger` and `RcvGetObjByHash` report as `ledgerRequest`, so a
queue-wait spike could not be traced to a handler. Nothing measured
NodeStore cost, request size, or the differential charge.
Latency now decomposes into three additive parts, each separately
measurable:
end-to-end = queue wait + NodeStore lookup + everything else
- `handler` label on job_queued_total/_started_total/_finished_total and
job_queued_us/job_running_us. The value is sanitised: a name passes
through only if non-empty and all ASCII letters, else "other". Two job
names embed a ledger sequence, so a raw label would mint one series
per ledger; the rule bounds the domain at 43 names plus "other".
- getobject_lookup_us, _request_objects, _lookups_total{result},
_rejected_total{reason} and _charge, recorded at their call sites.
All three histograms get explicit bucket views: the SDK default stops
at 10,000, which every one of them exceeds.
- Per-job-type waiting/running/deferred gauges for the 35 non-special
job types. `deferred` is the leading indicator, since addJob never
rejects -- it defers, so backpressure otherwise shows up only as
latency after the fact.
`JobQueue::collect()` snapshots the counters under the queue lock and
publishes gauges after releasing it. Writing them while holding the lock
would invert a lock order against the collector's own lock, which the
collector's flush thread already holds when it calls this hook.
Tests assert exact values, including that the charge is priced on the
requested count rather than the capped iteration count.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Add current_ledger_seq / current_ledger_hash to the tx.process, tx.receive,
and txq.enqueue span-reference rows, correct the txq.enqueue parent note
(parents to tx.process on the submission path via explicit context; a root on
the open-ledger rebuild path), and add a "Correlating a transaction to the
ledger it was worked on" recipe joining the txID-keyed tx/txq spans to the
ledger trace via current_ledger_seq.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
These inbound peer-message entry points (kConsumer) used span(), which
inherits whatever span is active on the peer thread — including a leaked
tx.receive scope — so validations/proposals were wrongly nested under
unrelated transaction traces. rootSpan() starts a fresh trace root.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Replace the TxQ Accept Status piechart on the Transaction Overview
dashboard with a state-timeline showing each node's applied fraction of
TxQ accepts (applied / applied+failed) over time, colored by threshold
(green >=0.9, yellow >=0.7, red below). Remove the now-orphaned
txq_status template variable (the piechart was its only consumer) and
document the panel in the telemetry runbook.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The spanmetrics connector had no namespace, so it emitted traces_span_metrics_*
metric names by default. The span dashboards and docs are renamed to query
span_* names; this is only correct if the connector emits them too, so add
namespace: "span" to the spanmetrics connector. Both sides change together:
renaming the dashboards without the namespace (or vice versa) would break the
pipeline. Matches the phase9 collector config.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The phase7 OTelCollector::formatName lowercases and strips names, emitting
snake_case metrics with no xrpld_ prefix. The native Grafana dashboards and
the telemetry docs still queried the old xrpld_CamelCase names, so they were
broken against their own pipeline. Rename every metric name to match what the
code emits: drop the xrpld_ prefix and lowercase the remainder. The two job
histograms also drop the redundant 'duration' word (job_queued_us,
job_running_us) to match the phase9 forms. Add havetxset to the cspell
dictionary since the lowercased metric name no longer word-splits.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- Replace unused observer_result.h with nostd/shared_ptr.h and
nostd/unique_ptr.h in MetricsRegistry.h (misc-include-cleaner)
- Use auto for cast initializations in MetricsRegistry.cpp and
ValidationTracker test (modernize-use-auto)
- Run rename scripts: rippled -> xrpld in comments and docs
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The collector's log ingestion used a container path named after the old
`rippled` binary and defaulted its host mount to a location that either
needed root or diverged from where the telemetry configs actually write.
Consolidate on one accessible log root:
- container mount target renamed /var/log/rippled -> /var/log/xrpld
- filelog glob -> /var/log/xrpld/*/debug.log
- compose mount source defaults to the repo-relative ./data/logs
(user-owned, no root), overridable via XRPLD_LOG_DIR
- devnet telemetry cfg writes to data/logs/devnet/debug.log so it lands
one subdir below the mount root and matches the glob
- integration-test.sh sets XRPLD_LOG_DIR to its own workdir
- docs/runbook/task-list updated to match
The default xrpld config (cfg/xrpld-example.cfg) is intentionally left
untouched.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- Correct log-timestamp examples to real Logs::format() output
(2024-Jan-15 10:30:45.123456 UTC) in docs, TESTING.md, and reference.
- Update Loki to v3.4.2 and switch the collector exporter to
otlphttp/loki on the native /otlp endpoint (docs + task list).
- Make the collector log-mount path configurable via XRPLD_LOG_DIR.
- Remove implementation-phase references from shipped config/script
comments, keeping the functional descriptions.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Replace the placeholder webhook contact point with two env-driven tiers:
- xrpld-default (Slack) receives warning-severity alerts
- xrpld-critical (Slack + email) receives critical-severity alerts, via a
severity=critical child route in the notification policy
Secrets stay out of git: contactpoints.yaml references ${SLACK_WEBHOOK_URL},
${SLACK_CHANNEL}, and ${ALERT_EMAIL_TO}, which Grafana expands from a
gitignored .env.alerting (loaded via an optional env_file on the grafana
service). .env.alerting.example documents the variables; email additionally
needs GF_SMTP_* configured.
Fold the ALERTING.md runbook content into the Alerting section of
docs/telemetry-runbook.md (operator docs live in one place) and delete
ALERTING.md. Move the .env.* ignore rule into docker/telemetry/.gitignore.
Verified live: Grafana 11.5.2 provisioned both contact points, the
severity-routed policy, and all 6 rules with no errors.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- Add PathFinding Spans section to the runbook Span Reference and
remove the incorrect "not emitted / later-phase" annotations:
pathfind.request/compute/discover/update_all are emitted since
Phase 2 (upstream of Phase 5).
- Runbook label names use the bare spanmetrics dimensions
(command, rpc_status, local, consensus_mode); fix stale docs.
- 05-configuration-reference: note tls_client_cert/key require use_tls=1.
- TelemetryConfig: reject mTLS cert paths when use_tls=0 so telemetry
is not silently sent in plaintext.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- Add an operator-facing Alerting section to the runbook covering the six
provisioned Grafana rules and how to point them at a real receiver.
- Strip the "(Phase 9)" suffix from the MetricsRegistry Gauges heading.
- ALERTING.md: drop the internal dev-phase reference from the intro.
- Phase9_taskList.md: expand Task 9.9 to describe the provisioned alerting
(rules, contact point, policy, ALERTING.md runbook).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Operator-facing cleanup: strip the "(Phase 8)" suffix from the Log-Trace
Correlation section heading in the runbook.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Same operator-facing cleanup as the RPC/transaction/consensus headings: the
runbook documents the running system, not the development phase that added
each span. Strip the "(Phase 6)" suffixes from the Ledger and Peer span
reference headings.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The operator runbook is for people running OpenTelemetry against a node, not
a record of the internal development phases. Strip the "(Phase N)" suffixes
from the span-reference headings and replace the personal-name example
service.name value with a generic one.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
