Commit c5829df68f added a per-round event to the txset.acquire span but
updated only three of the surfaces that describe that span. Fills the rest.
The operator glossary gains the round-naming behaviour, and loses a claim it
should not have made: it described a tx-set fetch as being on the critical
path of a round. It is not. acquireTxSet returns immediately and the round
continues without the set; a set that arrives after the round has accepted is
discarded. The cost is a round resolving its disputes on less information,
not a round held up. Two later sentences restated the same claim and are
reworded to match.
The Tempo config records that the new event attributes are queryable through
event:name / event.<attr> on the pinned 2.9.4, but that a dedicated parquet
column accepts only span or resource scope, so those filters scan rather than
index and no event-scoped column should be added.
The collector config adds them to its deliberately-not-promoted list, with
the reason that matters: they are per-round and so unbounded, but more
fundamentally spanmetrics derives metrics from spans and not from span
events, so an event attribute cannot be a metric dimension at all. The
Grafana Cloud collector only cross-references that note, so it is unchanged.
No dashboard change: this adds no metric, so there is no panel to add and no
new filter dimension.
Verification: both YAML files parse; pre-commit passes on the changed files.
A tx-set fetch carried no key tying it to the consensus round that needed
the set, so attributing a stalled fetch to a round meant guessing from
timestamps. One fetch is wanted by many rounds -- it is keyed by set hash,
survives the round sweep, and the round never blocks on it -- so a single
parent, link or attribute cannot describe the relationship.
Instead the fetch span records one timestamped event per requesting round,
carrying the round's parent-ledger hash and the ledger it is building. Both
attribute keys already existed in the shared telemetry namespace with
exactly this meaning, and the existing addEvent API is used as-is, so no
new telemetry surface is added and the whole feature compiles out with
telemetry disabled.
The event fires once per round rather than once per peer proposal, keyed on
the round's parent-ledger hash: that distinguishes rounds started on
different forks at the same height, which a ledger-height compare cannot.
A mid-round wrong-ledger recovery re-enters consensus without re-caching the
round identity, so a fetch begun after that switch is attributed to the
pre-switch round; the limitation is documented where the values are cached.
Also fixes the fetch span's end time, which depended on when the C++ object
was destroyed. Three of the four exits that stop pursuing a fetch -- the set
arriving from elsewhere, the round sweep, and shutdown -- ended the span
only via the destructor, so the recorded duration included however long any
reference happened to be held. Each exit now ends the span itself, plus
cancel() and container teardown, and the destructor asserts the span is
already closed rather than closing it: a fallback that can never legitimately
fire should fail loudly instead of hiding a missed exit. An abandoned fetch
also no longer asks peers for a set nobody wants, which previously led to
charging those peers for answering our own request.
Verification: pre-commit and TIDY=1 clang-tidy pass; levelization is
unchanged. NOT compiled -- the branch is blocked by a gcc-15 internal
compiler error in the unrelated xrpl.libxrpl.rdb unity translation unit.
Runtime behaviour is unasserted: xrpl_tests links only xrpl.libxrpl, so
TransactionAcquire is unreachable from GTest; the added tests cover the new
span-name and attribute constants only.
The workload harness gates regressions on histogram_quantile over
job_queued_us / job_running_us, so re-cutting the microsecond ladder changes
what those queries return and the stored baselines no longer describe the
same measurement.
baseline-timings.json's job.acceptLedger.queued.p95 was 96.79us, which is
0.95 / 0.9926 x 100 -- the old 100us bucket edge scaled by the quantile, with
99.3% of samples beneath it. It was never a latency. Keeping it would make the
gate LESS sensitive rather than more: a genuine regression from a real 40us to
90us would still sit under 96.79us + 50% and pass.
Removes the four job.* entries and records why, including their values. The
comparer reports a metric absent from the baseline as "new metric (not in
baseline)" and skips it, so the span baselines stay live and gating continues
for everything unaffected. is_placeholder() still returns False, so this does
not disable the gate wholesale. Recapture the job.* numbers on a node running
the re-cut ladder.
Also corrects _bucket_note in regression-thresholds.json. It described the
spanmetrics ladder as 15 edges starting at 1ms; the collector config has 20,
including five sub-millisecond edges. The note's own reasoning was void too --
it justified the 10ms absolute span bound as "~2 low-end bucket widths", but
the low-end bucket width is 0.01ms, not 5ms. The bound is kept and justified
on the band where span quantiles actually sit, rather than on a derivation
from a ladder that no longer exists.
The microsecond ladder's first edge was 100us, which sat ABOVE the mass of
every instrument using it. Measured on devnet: 99.3% of job_queued_us
samples, 92.5% of job_running_us and 90.4% of getobject_lookup_us fell in
that first bucket. histogram_quantile then interpolated inside bucket 0 and
returned `quantile / fraction_in_bucket_0 x first_edge` -- p75/p95/p99 of
job_queued_us read 75.52/95.66/99.69us against a prediction of
75.53/95.67/99.70. Three-decimal agreement: those panels were reporting
arithmetic on the bucket edge, not latency.
The fix was already half-written. kSubMillisecondBoundaries had been parked
in MetricsRegistry.cpp as [[maybe_unused]] with a comment noting exactly this
problem for nodestore reads. Its edges are now folded into kMicrosecondBuckets
rather than deleted, so the parked intent is carried forward: 1..1000us
resolution where the mass is, upper edges unchanged so multi-second stalls
stay measurable.
Also moves the GetObject count and charge ladders into HistogramBuckets.h, so
all five ladders have one owner and one set of invariant tests (29 now).
Adds check_bucket_parity.py, wired into the existing OTel naming workflow.
The C++ millisecond ladder and the collector's spanmetrics ladder are
specified to agree over their shared range; they were identical when shipped,
then the collector side alone was extended and nothing noticed for eleven
phases. The check asserts containment rather than equality, because jobs
outlive spans -- jobq_updatepaths averages ~60s, which no span approaches, so
demanding equality would force a ceiling that censors it. Verified it rejects
a missing collector edge, a bogus in-range edge, and a return to the 5s
ceiling.
ledger-data-sync's "Job Queue Wait p95 By Type" moves off the beast
jobq_*_q_milliseconds pair onto job_queued_us filtered by job_type. Those
beast metrics are ms-quantised at the source (Event rounds up to a whole
millisecond), so 94-100% of their samples sat in the first bucket and no
ladder change could fix them. Note the label values are camelCase
(job_type="ledgerData"), not the lowercase metric-name fragments.
Both histogram-fed alert thresholds re-validated and left unchanged, with the
measured basis recorded so neither gets tuned against the old artefact: only
0.0022% of job_queued_us samples exceed the 1s threshold, and every edge
bracketing the 1000ms ios_latency threshold survived the ladder change.
Docs: the rpc_size "known issue -- tracked separately" notes in the runbook
and 09-data-collection-reference are now resolved notes, the stale 10-edge
span_duration bucket list is corrected to the collector's real 20, and the
runbook gains a "Reading A Histogram Percentile" section covering both
saturation traps and the expected discontinuity after a ladder change.
This is the change that actually lifts the 5 s ceiling. Until now the
millisecond ladder and the Unit type existed but nothing consumed them.
Telemetry.cpp registered ONE histogram view: instrument name pattern "*",
unit exactly "ms", boundaries {1, 5, ..., 1000, 5000}. Verified against the
installed SDK, "*" matches every name and "ms" matches exactly, so that view
governed every beast::insight Event -- all 54 of them, whatever they measure.
Measured on devnet: 24.9% of rpc_size samples and 100% of jobq_updatepaths
samples fell above 5000. A quantile landing in the `+Inf` bucket reads back
as the second-highest edge, so those p95s reported a flat 5000 rather than a
measurement, and the 1 s to 5 s span was a single four-second-wide bucket
that any quantile inside it had to interpolate across.
Replaces it with one view per unit, keyed on the unit an instrument declares:
- `ms` gets kMillisecondBuckets: every representable edge of the collector's
spanmetrics ladder, plus 60 s and 120 s. The extensions are deliberate --
jobq_updatepaths was measured averaging 59,956 ms, which no span
approaches, so parity alone would still censor it.
- `By` gets kByteBuckets, placed from the measured response distribution
(mean 2131 B, half under 1 kB, tail mean bounded at 7538 B).
OTelEventImpl now derives its declared unit AND its description from unit()
instead of hardcoding "Duration in ms"/"ms", so rpc_size exports as
rpc_size_bytes on the byte ladder. rpc-pathfinding's "RPC Response Size"
panel follows the rename; its unit was already decbytes and is now truthful.
Also corrects Phase7_taskList.md, which still specified the 5000 ladder as
"matching SpanMetrics". That was true when written and became false when the
collector ladder was extended on its own -- implementing the plan as written
reproduced the bug, so the spec is where the defect had come to live. The
edges now have exactly one owner and the plan points at it.
Match the other dashboards: `rgb(15, 122, 102)` instead of the brighter
`rgb(25, 158, 112)`, still clear of the separation floors against the JMeter grey.
Use `rgb(15, 122, 102)` instead of `rgb(25, 158, 112)`: the brighter step drew
too much attention for a background band.
This is the darkest teal that still separates from the JMeter grey by a readable
margin -- normal-vision dE 15.6 against a floor of 15, CVD dE 12.3 against a
target of 8, and at least 3:1 on the dark surface. Dimmer steps fail: rgb(25,
100, 90) lands at dE 9.5, and a grey-derived rgb(25, 70, 70) at dE 5.8, which is
indistinguishable from the JMeter grey even with full colour vision.
Both checks selected on {job="xrpld"}. Loki's OTLP ingestion promotes
service.name to the label `service_name` and keeps a `job` attribute as
structured metadata, which a stream selector cannot match, so the selector
returned zero streams whatever had been ingested. The collector config and
TESTING.md already say to select on `service_name`.
Invert the cross-reference. Picking an arbitrary trace from Tempo and
expecting it in Loki fails even when correlation works, because a log line
carries a trace_id only when emitted inside a sampled span and most spans
log nothing at `warning` level. Start from a logged trace_id instead and
resolve it in Tempo, which is the invariant worth asserting, and try every
id found so one unexported trace does not fail the check.
Bound the log queries in time. Nothing here set start/end, so every query
relied on Loki's one-hour default and returned nothing when re-run later to
investigate a result.
Match the other dashboards: restore `rgb(70, 70, 70)` on `Perf Runs (JMeter)`
so regions that rendered before keep their colour. `Perf Runs (Locust)` stays
aqua.
The driver split changed the existing perf-run regions from grey to violet,
which was not asked for. Restore `rgb(70, 70, 70)` on `Perf Runs (JMeter)` so
every region that rendered before keeps its colour; `Perf Runs (Locust)` stays
aqua, since it is new.
Grey separates from aqua well (dE 22.8 deutan, 25.9 tritan, 26.1 normal), but it
sits at 1.98:1 against the dark-theme surface, below the 3:1 floor, so its region
edges read faint there. Noted in the runbook.
Apply the same two-layer split as the other dashboards to the sync-health board,
which is introduced on this branch: `Perf Runs (JMeter)` and
`Perf Runs (Locust)`, each matching ["perf-iac", "<driver>"] with
matchAny:false, in place of the single generic `perf-iac` layer.
A single "Annotate perf-iac runs" layer matched only `perf-iac`, so a Locust
load window was indistinguishable from a JMeter one. perf-iac now tags every
region with its load driver, so each driver can have its own layer and colour.
- Replace that layer with `Perf Runs (JMeter)` and `Perf Runs (Locust)`, each
matching ["perf-iac", "<driver>"] with matchAny:false, on 12 dashboards.
- job-queue, ledger-data-sync and log-derived-insights had an empty annotations
list and drew no perf regions at all; they now carry the builtIn layer plus
both driver layers.
- Grafana tag matching is a superset AND with no negation, so a generic
`perf-iac` layer also matches every driver region. Keeping one alongside the
driver layers would draw each load window twice, so it is replaced, not kept.
- Document the layers in the telemetry runbook, including two rendering limits:
annotations draw only on timeseries, state-timeline and candlestick panels,
and the shaded fill is 10% opacity so the region edges carry the colour.
- Add `jmeter` to the cspell dictionary; the hook rejects the bare word.
beast::insight instruments are created during ApplicationImp's member-init
list, and opentelemetry-cpp 1.28 never rebinds an already-vended Meter, so an
instrument created before the MeterProvider is published records nothing for
the rest of the process. Observable instruments carry the opposite constraint:
registering one arms the SDK reader thread, and its callbacks run hook handlers
that read services which do not exist that early.
Publish the provider in Telemetry's constructor, ahead of every producer, and
defer only the observables. Collector gains onCollectionReady() and
onCollectionStopping(); OTelCollector arms and disarms its gauges in response.
StatsDCollector starts its polling thread in its own constructor and had the
same hazard, so it uses the pair to gate that thread.
The metrics resource carries service.instance.id and is immutable once built,
so the node public key is resolved in Main.cpp, where a config error can still
be reported, and passed to makeApplication(). getNodeIdentity() remains
authoritative; both paths now share readNodeIdentity(), so telemetry cannot
report a key the node has abandoned.
An explicit ~ApplicationImp stops observing and stops telemetry, covering the
setup() failure paths that never reach run(). Telemetry::stop() is once-only
and no longer clears another instance's global pointer. The histogram view's
meter selector now matches the meter actually in use, so its bucket boundaries
apply for the first time.
Node identity reached the OTel resource only as service.instance.id, which is
config-overridable and carries a deployment-chosen label rather than the node's
own identity. Add xrpl.node.id, set unconditionally from the node public key
(base58, TokenType::NodePublic), so traces and metrics share a stable per-node
key independent of [telemetry] service_instance_id.
Set on the tracer resource via Telemetry::setNodeId(), called from
ApplicationImp::setup() once nodeIdentity_ is known, and on the MetricsRegistry
resource via an added start() parameter. The beast::insight meter provider is
built in TelemetryImpl's constructor, before the wallet DB exists, so its
resource cannot carry the value; that path is left for later and the attribute
is omitted rather than stamped blank.
Also drops the transform/spanidentity collector processor added in
4a361a496d: per-node identity belongs on the resource, not copied onto every
span.
Consensus spans share one deterministic, ledger-derived trace_id, so a
single trace holds spans from every node and the resource-level node id is
not a reliable per-span discriminator in stored traces.
Add transform/spanidentity to both collector configs, copying
service.instance.id onto every span as service_instance_id so TraceQL can
filter per node with the same value the $node dashboard variable already
uses on the metrics side. Wired into the traces pipeline locally and into
traces/store (after tail_sampling) on the Grafana Cloud variant.
The transitions panel used increase(...[$__rate_interval]). $__rate_interval is
defined as max($__interval + scrape, 4 * scrape), i.e. deliberately one scrape
longer than the step so rate() windows overlap and lose no counter increase.
That overlap is harmless for rate(), but this panel reads the value as a count
of discrete events, and the overlap counts each event in more than one bucket.
Measured against a log-derived ground truth of 106 syncing transitions on
devnet-otel-usw2-01 over 2026-08-11T11:05Z..2026-08-12T23:04Z, the old query
reported 111.3 at a 300s step and 133.7 at a 60s step -- the error grew to +26%
as you zoomed in, because the overlap is a larger fraction of a smaller step.
Switch to $__interval so the buckets tile exactly, and wrap in round() because
increase() extrapolates to the window edges and so reports fractional counts for
an integer counter. The same measurement now gives 106 at 300s, 105 at 60s and
107 at 900s. Every state and both nodes land within a few counts of truth at any
zoom, and the legend Total is now a meaningful figure.
Pin Min step to 1m: the real scrape interval is 60s while the datasource
declares 15s, so without a floor $__interval can fall below one sample.
Draw as bars with 0 decimals -- the value is a discrete count per bucket, and a
line implies interpolation between counts that does not exist.
Nine conflicts, resolved as follows.
src/xrpld/app/ledger/detail/InboundLedger.cpp -- kept this branch's version.
phase10 sets the span's outcome/timeouts/peer_count attributes inline at each
exit; this branch replaced that with the idempotent finalizeAcquireSpan(), called
on all four exits (init, done, give-up, destructor). Taking phase10's blocks
would have set the outcome twice against a helper documented as not overwriting
what the real exit recorded. phase10's comment explains why peer_count must not
be read in a destructor; the helper solves that structurally by taking
std::optional<std::size_t> and being passed std::nullopt from there.
src/xrpld/telemetry/MetricsRegistry.cpp -- kept metric::ledgerEconomy over
phase10's "ledger_economy" literal. This branch added the naming check that
requires constants for converted families, so the literal would regress it. Took
phase10's comment cleanup.
src/xrpld/telemetry/MetricsRegistry.h -- kept registerRotationStateGauge(), which
only exists here, and took phase10's removal of the stale task-number comment.
validate_telemetry.py -- combined both. phase10 replaced serial metric polling
with a concurrent fan-out on one shared deadline, because 58 metrics x 45 s of
additive timeout overran the CI budget; that is kept. Its target list filters on
SKIPPED_METRIC_GROUPS rather than the two literals it hardcoded, so the
sync_diagnostics group stays owned by assert_sync_diagnostics_metrics() instead
of being polled and reported twice. Both SYNC_DIAGNOSTICS_GROUP and
METRIC_POLL_CONCURRENCY are needed and both are kept.
check_otel_naming.py -- both sides extend the rule docstring. Took phase10's
fuller Rule E text (doc discovery, allow-dotted markers) and re-appended rules
I/J/K/L, which exist only here.
expected_metrics.json -- the two sides add disjoint sibling groups, so both are
kept: sync_diagnostics alongside node_health_gauges, overlay_reduce_relay,
overlay_overflow, validation_lifetime_counters and not_asserted. Both dashboard
uids are kept, giving 16 asserted uids against 16 dashboards on disk.
expected_spans.json -- kept this branch's span set, a superset that adds the
acquire phase spans, ledger.serve, txset.acquire and peer.dial, and expands
ledger.acquire's required attributes. Took phase10's description, which documents
what the totals mean, and its note on how the RPC wildcard span is created.
total_span_types and total_unique_attributes are recomputed for the union: 48 and
74, since each side's figure counted only its own spans.
Docs: took phase10's more accurate wording on what the dashboard check actually
covers, and corrected the dashboard count from 15 to 16 where the merge made it
stale.
Verified: no conflict markers remain, both JSON contracts parse, both Python
files compile, asserted dashboard uids match the dashboards on disk exactly, and
the OTel naming check reports all layers consistent.
The Operating Mode Transitions panel queried state_accounting_*_transitions
directly. Those are monotonic counters, so the panel drew a slowly rising line
and a few transitions per hour were invisible against a total in the hundreds.
It also fell off a cliff whenever xrpld restarted and the counters reset to 0,
which reads as missing data rather than a restart.
Wrap each target in increase(...[$__rate_interval]) so each point is the number
of transitions in that bucket and the series survives a counter reset. This is
what the sibling panels on the same row (Operating Mode (Time Share), State
Duration Rate) already do.
Verified against devnet-otel-usw2-01/02 over 2026-08-11T11:01Z..2026-08-12T16:23Z:
the fixed expression reports 107 and 123 syncing transitions, matching the
counter deltas, and stays continuous across the 12:07 restart where the raw
counter dropped 630 -> 1.
Brief mode flaps remain invisible on Operating Mode (State Timeline) because a
~2 s dwell cannot be captured by a 60 s scrape; this panel is the place to read
them.
This branch had already made the same corrections independently, and in
richer form, so the resolution keeps this branch's version nearly throughout:
- 09-data-collection-reference.md: this branch already documents the
state-accounting gauges as cumulative **microseconds** with an explanatory
callout, and already names `jobq_job_count` with its `jobq` group. Kept.
- telemetry-runbook.md: already carries `jobq_job_count` in both tables. Kept,
along with this branch's larger additions.
- OpenTelemetryPlan.md: kept this branch's rewritten section 9 blurb, which
describes the inventory without hardcoding counts and so cannot drift.
- consensus-health.json: kept this branch's rewrite. It deliberately removed
the four TraceQL close-time detail panels and renamed the agreement panel;
the incoming side would have resurrected them. Panel count unchanged at 26.
- integration-test.sh: this branch's unprefixed native metric names were kept,
but it still asserted `job_count`, so the `jobq_job_count` correction was
carried over. That check would otherwise always fail.
Conflict resolution kept this branch's evolution and re-applied phase-6's
fixes on top of it, rather than taking either side wholesale:
- consensus-health.json: kept the native `span_calls_total` metric name and
the `interval: 15s` and point styling from this branch; added phase-6's
`close_time_correct` PromQL filter and the NetClock axis labels. The
TraceQL boolean-regex filter stays removed and the `byRegexp` overrides
carry over. Panel count unchanged at 27.
- 09-data-collection-reference.md: kept this branch's headings, its more
detailed consensus attribute table (which already types
`consensus_round_id` as int64) and its section numbering, including the
deliberate removal of the SpanNames inventory. Carried over only the
correction that the state-accounting duration gauges are cumulative
microseconds, not seconds.
- telemetry-runbook.md: kept this branch's native metric names
(`span_calls_total`, `span_duration_milliseconds_bucket`); carried the
`rpc.request` -> `rpc.http_request` span-name fix and the `jobq_` segment
on the job-queue depth metric.
- integration-test.sh: kept this branch's `check_otel_metric` form and
carried the `jobq_job_count` correction.
The integration test asserted `rippled_job_count`, which never reports any
series, so that check always failed. `JobQueue` registers the gauge as
`makeGauge("job_count")`, but `Application.cpp` passes it
`collectorManager_->group("jobq")`, so the emitted StatsD name is
`jobq.job_count` and the exported Prometheus name is
`<prefix>_jobq_job_count`.
Corrected the same name in two runbook tables that also dropped the `jobq`
segment. `09-data-collection-reference.md` already had it right, which is why
the two documents disagreed.
Routed here rather than to the phase-10 PR where it was reported: the wrong
name is present in `integration-test.sh` on every branch from phase 6
onward, and this is the branch that introduces the file.
Left alone deliberately:
- `statsd-node-health.json` still queries the old name, but that dashboard is
deleted at phase 7 in favour of `node-health.json`
- `06-implementation-phases.md` names `job_count`, which is accurate as the
code-level makeGauge argument rather than the exported metric name
The file_storage extension was added to otel-collector-config.yaml, which
every stack mounts. That made the extension mandatory: the collector image
runs as 10001:10001 and ships no writable directory, so any stack without a
prepared volume would fail to start rather than merely lose offsets. The
workload-validation stack mounts this same config and has no such volume.
Offset persistence is only useful where logs outlive a restart. The workload
harness creates a fresh log directory per run, so it has nothing to resume
from. Move the extension, the receiver's storage reference and the extended
service.extensions list into otel-collector-filestorage.yaml, layered as a
second --config by the developer stack alone. The base config keeps
start_at: beginning, which is what actually fixes the reported defect, and
stays self-sufficient for every other stack.
Verified against the pinned collector image: the base config validates and
runs on its own with no volume mounted and still ingests a line written
before startup; base plus overlay validates, preserves the base receiver's
operators through the merge, and re-ingests that line zero times on a second
run against the same volume.
Conflict resolutions:
- docker/telemetry/xrpld-telemetry.cfg: relocation conflict. phase-9 had
already moved [insight] to the end of the file with server=otel, so the
incoming block was dropped rather than inserted. Keeping both would have
produced two [insight] sections, which merge last-wins into a single
effective section, silently reviving the bug this branch just fixed.
phase-9's per-branch service_instance_id=xrpld-devnet is preserved.
- OpenTelemetryPlan/06-implementation-phases.md: kept both corrections.
phase-9's "Tempo" is right (no Jaeger anywhere in the stack) and
phase-8's "active, sampled span" is right: Log.cpp:328 injects only
when spanCtx.IsValid() && spanCtx.IsSampled().
- OpenTelemetryPlan/09-data-collection-reference.md and
docs/telemetry-runbook.md: kept phase-9's structured-metadata LogQL.
The collector's filelog regex_parser already extracts partition,
severity, trace_id and span_id, so phase-8's inline regexp forms are
redundant, and a line filter matches the literal text in a message body.
