Commit Graph

16590 Commits

Author SHA1 Message Date
Pratik Mankawde
bca2b35bf0 docs(telemetry): stop showing inert [insight] prefix on the OTel path
ff8629bb11 dropped prefix=xrpld as inert and misleading, but four OTel-path
sites still set it, so the branch contradicted itself.

OTelCollector routes every instrument name through a static formatName()
that only lowercases and maps '.'/space to '_'; the sole read of prefix_ is
the startup log line at OTelCollector.cpp:810. All four instrument factories
funnel through formatName(), so no prefix can reach an exported name.
StatsDCollector does prepend it (StatsDCollector.cpp:551/592/640/715), so the
StatsD example legitimately keeps it.

Removed from the 09 reference's OTel config block and from both
quick-reference setups, and from the cfg integration-test.sh generates. The
StatsD example is unchanged and now states why it keeps the key.

Also corrected run-full-validation.sh: [insight] endpoint was described as
"already matches the built-in default", implying it would matter if it
differed. CollectorManager reads it and hands it to OTelCollector, which also
only logs it; the exporter URL is built in Telemetry::initMetrics() from
[telemetry] endpoint. It is as inert as prefix was.
2026-08-24 21:45:35 +01:00
Pratik Mankawde
4c33ffb9ca docs(telemetry): retire the rpc_size instrument-mismatch warning 2026-08-24 21:08:43 +01:00
Pratik Mankawde
98ba282854 fix(telemetry): make the integration test correlate by construction, record the baseline log-level coupling
Three related follow-ups to running the workload at info.

integration-test.sh has its own log-trace correlation check that the workload
validator knows nothing about: check_log_correlation() greps each node's
debug.log for "trace_id=<hex> span_id=<hex>" and fails when it finds none, then
cross-checks a sample id against Tempo. At warning it had no guaranteed source.
The only warn-or-worse statement inside the activated accept scope is
RCLConsensus.cpp:671, which fires solely when a transaction throws, so the
check was passing incidentally -- helped by scanning whole files with no time
window. Raising it to info gives it the same guarantee the workload now has:
the consensus accept pair, one branch of which fires every accepted round.
Safe here because this script captures no latency baseline, so there is nothing
for the extra log I/O to contaminate.

baselines/README.md now records that the committed baseline is only valid at
the log level the harness generates. Logging is synchronous and several gated
spans contain log statements -- ledger.build has BuildLedger.cpp:81, and
consensus.accept has RCLConsensus.cpp:655/663/686 with :663 logging once per
transaction -- so the configured level is part of the measurement. Moving it
inflates or deflates the quantiles the gate reads without ever reporting a
regression, because the baseline moves with it. Changing the level therefore
requires re-capturing the baseline.

benchmark.sh keeps warning and keeps prefix=xrpld, and now says why. It
measures telemetry overhead as a delta between a telemetry-off and a
telemetry-on arm, so extra synchronous log I/O would inflate both arms and the
thresholds gate the result. The comment exists to stop a future reader
"aligning" it with the workload harness and quietly degrading the measurement.
2026-08-24 20:50:34 +01:00
Pratik Mankawde
ee1f2ce913 Merge branch 'pratik/otel-phase9-metric-gap-fill' into pratik/otel-phase10-workload-validation 2026-08-24 20:50:34 +01:00
Pratik Mankawde
ea4282f6e0 docs(telemetry): correct the stale metric-gate counts in the runbook
The "What Gets Validated" table claimed "58 metrics in 23 categories". Both
numbers were stale: generalising required_labels and adding the asserted
io_latency group changed the gate to 66 checks across 24 asserting categories
(61 metric names plus 5 required_labels checks).

Derived from the inventory rather than counted by hand -- ran the validator's
own _metric_check_targets() against expected_metrics.json:
  metric names = 61 | label checks = 5 | TOTAL = 66
  asserting groups = 24
  label checks per group = {'spanmetrics': 4, 'job_queue': 1}

Also records that labels are gated at all. They were declared in the contract
but nothing read the key until the check was generalised, so four spanmetrics
labels were documented as required while going unverified -- the table
described existence checks only and gave no hint that a label regression was
now catchable.

The other rows in the same table were re-derived and are still accurate:
spans 41 total / 26 required / 15 optional, parity 6 span attributes + 4 value
bounds = 10, dashboards 15 uids against 15 provisioned files, logs 2.
2026-08-24 20:50:34 +01:00
Pratik Mankawde
ff8629bb11 fix(telemetry): drop the inert insight prefix and its false comment
Every generated node cfg carried prefix=xrpld under a comment claiming it
"matches the OTel resource service name and the metric names the dashboards
query". Both halves are false.

Verified inert before removing: CollectorManager.cpp reads the key on the OTel
path and passes it to OTelCollector::New, but the only use of prefix_ anywhere
in OTelCollector.cpp is the startup log line. formatName() -- the single funnel
for every instrument name -- only lowercases the name and turns dots and
spaces into underscores; it never reads prefix_. So exported names carry no
prefix at all. expected_metrics.json's own description records this ("Metric
names have no prefix (the xrpld_ prefix was removed)") and 488 live metric
names confirmed it: jobq_job_count, rpc_requests_total, total_bytes_in.

A reader trusting the comment would look for xrpld_jobq_job_count and find
nothing.

The replacement comment states what is true and checkable: the collector
declares no statsd receiver (its metrics pipeline is [otlp, spanmetrics],
confirmed in otel-collector-config.yaml), so beast::insight must export over
OTLP for system metrics to reach Prometheus at all; server=otel is the only
load-bearing key; exported names carry no prefix.

Metric names, series and dashboards are unchanged. The one observable
difference is the OTelCollector startup log line, which now prints an empty
prefix.

Also updated workload/README.md, which repeated the same prefix=xrpld claim
and would have been left describing a cfg key that no longer exists, and made
the template header state the sync obligation explicitly -- nothing reads that
file, so nothing catches it drifting from the cfg the runner generates.
2026-08-24 20:50:21 +01:00
Pratik Mankawde
5506575d5c docs(telemetry): document the open-phase span attributes in the runbook
The row listed only the two end-of-phase attributes. Add the four set at span
creation and the three set at the close decision.

Records what the previous wording implied but did not state: the end-of-phase
attributes are absent when the round is recovered by handleWrongLedger or
driven by simulate(), because neither reaches closeLedger(). Any average over
open_duration_ms silently excludes those rounds.

Edited here rather than on phase 5, where this cell is empty and would conflict
on the way up.
2026-08-24 20:49:53 +01:00
Pratik Mankawde
2097293e8f fix(telemetry): run the workload at info so log-trace correlation is testable
The two log.trace_id_* checks have failed on every run -- they were the only
failures in the 2026-08-20 run (158/160). The workload never satisfied their
precondition, because warning suppressed the one line that is correlated by
construction.

trace_id is injected in Log.cpp from RuntimeContext::GetCurrent(). Severity
does not affect injection, but JLOG filters on severity before format() runs,
so what matters is which severity emits a line while a span is current.

A span becomes current in either of two ways: as a ScopedSpanGuard, or by
activating a plain SpanGuard via activate() / activateIfLive(). activate()
returns a ScopedActivation holding an otel_trace::Scope built from the span,
which pushes onto the same RuntimeContext store Log.cpp reads. A plain
SpanGuard that is never activated makes no span current.

The guaranteed correlated line at info is the consensus accept pair at
RCLConsensus.cpp:736/740 -- an if/else, so exactly one fires on every accepted
round. doAccept activates the accept span as ambient over its whole body at
:565 via activateIfLive(acceptSpan), and that activation lives to the end of
the function, so both branches are inside it. At roughly one round every 4 s
this gives dozens of correlated lines per run, well inside the validator's 4 h
window. LOG_QUERY_WINDOW_SECONDS stays at 4 h deliberately -- a wider window
would let the check pass on logs from a previous run.

info is the minimum that works, which is what the task asked for. debug would
correlate strictly more, additionally covering BuildLedger.cpp:81 and
RPCHandler.cpp:188, but it is the wrong default: it puts synchronous log I/O
inside ledger.build, consensus.accept (RCLConsensus.cpp:663 logs per
transaction) and tx.apply, which are exactly the spans whose latency
regression-metrics.json gates. The next run reprints the voided baseline, so
capturing at debug would bake log I/O into the latency numbers permanently --
the same class of defect this plan exists to remove. The runbook records how to
get the broader coverage per partition, after a baseline exists.
2026-08-24 20:46:53 +01:00
Pratik Mankawde
dfda1b2eda Merge branch 'pratik/otel-phase8-log-correlation' into pratik/otel-phase9-metric-gap-fill 2026-08-24 20:45:37 +01:00
Pratik Mankawde
113a7a9a5a Merge branch 'pratik/otel-phase7-native-metrics' into pratik/otel-phase8-log-correlation 2026-08-24 20:45:37 +01:00
Pratik Mankawde
f8845b0e00 Merge branch 'pratik/otel-phase6-statsd' into pratik/otel-phase7-native-metrics 2026-08-24 20:45:37 +01:00
Pratik Mankawde
b5e3414f63 Merge branch 'pratik/otel-phase5-docs-deployment' into pratik/otel-phase6-statsd 2026-08-24 20:45:37 +01:00
Pratik Mankawde
162351cfd4 Merge branch 'pratik/otel-phase4-consensus-tracing' into pratik/otel-phase5-docs-deployment 2026-08-24 20:45:37 +01:00
Pratik Mankawde
258adf491e refactor(telemetry): split consensus span labels into their own header
Review of the preceding commits found a clang-tidy failure and a convention
break, both rooted in the same place: the enum-to-label helpers were put in
ConsensusSpanNames.h, which pulled two domain headers into it.

misc-include-cleaner rejected the new test: it used xrpl::LedgerCloseReason
without directly including ConsensusTypes.h, relying on the transitive
include. misc-* is enabled and this path is not in IgnoreHeaders, so it would
have failed CI.

ConsensusSpanNames.h had also become the only one of the eight *SpanNames.h
headers to include anything beyond SpanNames.h. That cost is paid by every
consumer: PeerImp.cpp, ConsensusReceiveTracing.h and RCLConsensus.cpp want
only name and key constants, but were newly compiling ConsensusTypes.h and
DisputedTx.h through it.

Move both helpers to a new ConsensusSpanLabels.h, which owns the domain
includes. ConsensusSpanNames.h is dependency-free again like its siblings, and
the labels reach their only production caller, Consensus.h, directly.

Also from the review:

- phaseOpen() had grown to 81 lines, over the 80-line limit. Extract
  annotateOpenStart() and annotateOpenClose(), which also removes the repeated
  span guards. phaseOpen is 72 lines; startRoundInternal drops 103 to 93,
  still over the limit but it was 99 before this work began.
- Note at the CLOG why the log text keeps the shouldCloseLedger name: existing
  consumers match on it.
- whyCloseLedger's doc claimed "both log identically", implying the wrapper
  logs too. It delegates, so the logging happens once either way.
- Cross-reference proposers_validated and proposers_finished, which sit eight
  lines apart and count different things: validators of the previous ledger
  versus those already past it.
- The two static_asserts no longer sit inside TEST bodies with SUCCEED(); they
  fire at compile time regardless. Also "consteval-safe" was wrong; they are
  constexpr.
- SpanGuardFactory.cpp claimed a libxrpl test cannot include the consensus
  span-name header. The new test in the same directory does exactly that, so
  the claim is corrected to name the real constraint: the rpc_* constants it
  needs live in an xrpld-level header.
2026-08-24 20:45:24 +01:00
Pratik Mankawde
9801b1e297 Merge branch 'pratik/otel-phase8-log-correlation' into pratik/otel-phase9-metric-gap-fill 2026-08-24 20:42:01 +01:00
Pratik Mankawde
1b62ab096f Merge branch 'pratik/otel-phase7-native-metrics' into pratik/otel-phase8-log-correlation 2026-08-24 20:42:01 +01:00
Pratik Mankawde
1515d7fe46 Merge branch 'pratik/otel-phase6-statsd' into pratik/otel-phase7-native-metrics
# Conflicts:
#	src/libxrpl/telemetry/TelemetryConfig.cpp
2026-08-24 20:41:43 +01:00
Pratik Mankawde
ccd209b388 Merge branch 'pratik/otel-phase5-docs-deployment' into pratik/otel-phase6-statsd 2026-08-24 20:40:18 +01:00
Pratik Mankawde
26ba0c3698 Merge branch 'pratik/otel-phase4-consensus-tracing' into pratik/otel-phase5-docs-deployment 2026-08-24 20:40:18 +01:00
Pratik Mankawde
7ccef62219 Merge branch 'pratik/otel-phase8-log-correlation' into pratik/otel-phase9-metric-gap-fill 2026-08-24 20:40:18 +01:00
Pratik Mankawde
9542f87aed Merge branch 'pratik/otel-phase7-native-metrics' into pratik/otel-phase8-log-correlation 2026-08-24 20:40:18 +01:00
Pratik Mankawde
f4718cee08 fix(telemetry): correct mistimed and ambiguous consensus phase attributes
Three defects found in review of the two preceding commits.

Drop disputes_count_initial. It claimed to be the dispute count carried in
from the positions held at close, but startEstablishTracing() runs a full
timer tick after closeLedger(): timerEntry() dispatches
`if (phase_ == Open) phaseOpen(); else if (phase_ == Establish)
phaseEstablish();`, and phase_ was Open on the closing tick, so the else-if
cannot run. With ledgerGRANULARITY at 1s the value absorbed up to a second of
dispute growth from peer proposals and arriving tx sets. Making it honest
needs either a member captured at close or moving span creation into
closeLedger(), so it is removed rather than shipped mislabelled.

Record close_time_avalanche_state on recovered rounds. startRoundInternal()
reset establishSpan_ inline, discarding the span before the attribute was
written, so the value was present only on rounds that reached Accepted --
survivor bias in exactly the rounds worth investigating. It now calls
endEstablishTracing(). The comment claiming this avoided "reporting a stale
regime" was wrong: closeTimeAvalancheState_ is not reset until 39 lines
later, so the value was still that span's terminal regime.

Rename avalanche_state to close_time_avalanche_state. DisputedTx carries a
second, per-transaction avalanche tracker; the bare name invited reading a
close-time-only value as the transaction one, which is the tracker that
actually escalates in a stuck round.

Also: both label helpers now fall through to "unknown" instead of a
plausible-looking regime, matching to_string(ConsensusPhase); and the header
now records that the end-of-open attributes are absent on recovered and
simulated rounds, and that tx_sets_acquired can skew either way because
handleWrongLedger clears currPeerPositions_ but not acquired_.

Tests: the minimum-open-time assertion used prevRoundTime=10s, where
openTime=1s trips the too-fast branch as well, so deleting the ledgerMinClose
check entirely left it green. Replaced with prevRoundTime=2s, which isolates
the branch. Added the others-closed boundary, which is strict and was
untested in either direction, its integer truncation for odd prevProposers,
and its precedence over the no-transactions and minimum-open branches.
2026-08-24 20:40:00 +01:00
Pratik Mankawde
779bf0ca98 docs(telemetry): drop the runbook row for a panel this branch removes
The dashboard inventory listed "Peer TX Receive Rate", but this branch
deletes that panel from transaction-overview.json, so an operator
following the runbook finds no such chart. The row is stale from
phase-7 onward and is still present at the tip of the chain, so
nothing downstream repairs it.

Remove the row rather than retarget it: the neighbouring
"Transaction Receive vs Suppressed" row already documents the
tx.receive rate and its panel exists on every branch, so retargeting
would have produced two rows for one chart.
2026-08-24 20:29:36 +01:00
Pratik Mankawde
6eaf7316e5 feat(telemetry): record the ledger close reason on consensus.phase.open
The open phase ended for one of four distinct reasons, but
shouldCloseLedger() collapsed them into a bool, so a trace could say when a
phase ended and never why. "The network closed without us" and "nothing was
waiting" are the same span today.

Add whyCloseLedger(), which holds the decision and returns
LedgerCloseReason. shouldCloseLedger() keeps its exact signature and becomes
a one-line delegation, so its callers and unit tests are untouched and the
branch logic is not duplicated. phaseOpen() calls whyCloseLedger() directly;
both emit the same journal and CLOG output, so only one is called.

New attributes on consensus.phase.open, both set once on the closing tick:

  close_reason         anomaly | others_closed | idle | normal
  proposers_validated  trusted peers that had already validated the prior
                       ledger, reusing the value the decision was made on

Absent on the simulate() close path, which bypasses the decision rather than
having a reason invented for it.

Skipped has_open_transactions: hasOpenTransactions() is
!getOpenLedger().empty(), which is false on a quiet network for most of a
round, and close_reason=idle already implies it. The sibling
consensus.ledger_close span carries tx_count_open, which is the same fact
with a count instead of a boolean.

shouldCloseLedger() now has no production caller; it stays exported so the
public API and its tests are unchanged.

Tests pin every input vector from should_close_ledger to its literal reason,
including that the anomaly check outranks others-closed, and cover the
inclusive idle boundary either side by one millisecond.
2026-08-24 20:13:39 +01:00
Pratik Mankawde
2ae47c66aa fix(telemetry): assert ios_latency and correct the rpc_size rename attribution 2026-08-24 20:09:04 +01:00
Pratik Mankawde
31619219cc feat(telemetry): add set-once state attrs to consensus phase spans
consensus.phase.open and consensus.establish carried almost no state of
their own. Span attributes are not inherited, so the ledger context on the
parent consensus.round span does not describe either child, and the few
attributes the establish span did carry are rewritten on every iteration
and therefore only ever report the final value.

Add seven attributes that are read from state already in scope, are
written exactly once, and are not duplicates of the parent round span:

  consensus.phase.open (start)
    start_reason            initial, or recovered on a handleWrongLedger
                            re-entry, which emplaces a SECOND phase.open
                            span under the same round
    previous_close_agree    feeds the sinceClose branch in phaseOpen()
    peer_positions_at_open  positions in hand after playbackProposals(),
                            the head start the round began with
    early_close_triggered   the round skipped the timer because enough
                            peers had already closed

  consensus.phase.open (end)
    tx_sets_acquired        candidate tx sets held at close, read before
                            our own position is added; a low count against
                            a high peer_positions_at_close means tx-set
                            fetches did not land, not disagreement

  consensus.establish (start)
    disputes_count_initial  disputes carried in from the positions held at
                            close, as opposed to disputes_count, which is
                            overwritten each iteration

  consensus.establish (end)
    avalanche_state         terminal close-time convergence regime; the
                            derived avalanche_threshold is a weight and
                            cannot be inverted back to the state

The avalanche label is mapped by a new constexpr avalancheStateLabel() in
ConsensusSpanNames.h rather than an inline switch, so the four labels stay
under the naming check's L1 ownership and are unit-testable.

Deliberately not added: ledger_seq and consensus_mode, which would only
copy the parent round span's values down; tx set size and position hash,
which the TxSet concept does not expose portably across RCLTxSet and the
csf simulator; and the peer-unchanged and dead-node counters, whose
underlying state is reset mid-round and so would report a misleading value.

Behaviour is unchanged. The early-close condition is hoisted into a named
local so the annotation happens before timerEntry(), which can reach
closeLedger() and end the open-phase span.

Tests pin the wire strings for every new key and value and cover all four
enumerators of the avalanche mapping. They need no telemetry runtime: the
csf simulator returns an invalid round span context and a null Telemetry,
so consensus spans there are null guards and attribute writes are no-ops.
2026-08-24 19:54:15 +01:00
Pratik Mankawde
2afae6655d test(telemetry): assert xrpl_node_id reaches the metric series 2026-08-24 19:53:43 +01:00
Pratik Mankawde
14badbfdd7 docs(telemetry): record rpc_size_bytes and the other unasserted histograms 2026-08-24 19:46:18 +01:00
Pratik Mankawde
d1b80e47a2 test(telemetry): void span baselines captured on the old span ladder 2026-08-24 19:35:00 +01:00
Pratik Mankawde
1282645289 test(telemetry): invalidate job-queue baselines captured on the old ladder
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.
2026-08-21 12:49:56 +01:00
Pratik Mankawde
bda6c1323e Merge branch 'pratik/otel-phase9-metric-gap-fill' into pratik/otel-phase10-workload-validation 2026-08-21 12:47:12 +01:00
Pratik Mankawde
6e2b2da772 fix(telemetry): resolve microsecond latencies below 100us
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.
2026-08-21 12:46:56 +01:00
Pratik Mankawde
7735d725fb Merge branch 'pratik/otel-phase8-log-correlation' into pratik/otel-phase9-metric-gap-fill
# Conflicts:
#	docker/telemetry/grafana/dashboards/rpc-pathfinding.json
#	src/libxrpl/telemetry/Telemetry.cpp
2026-08-21 12:33:13 +01:00
Pratik Mankawde
8169894ada Merge branch 'pratik/otel-phase7-native-metrics' into pratik/otel-phase8-log-correlation 2026-08-21 12:30:50 +01:00
Pratik Mankawde
24094e427b fix(telemetry): give each histogram unit its own bucket ladder
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.
2026-08-21 12:30:38 +01:00
Pratik Mankawde
a24db2e995 docs: Document the [telemetry] TLS path readability check
Bring the three documentation surfaces in line with the new parse-time check:

- The @throws clause on makeTelemetrySetup now names the third failure
  condition and records that an empty path is skipped.
- cfg/xrpld-example.cfg states, under all three TLS keys, that with enabled=1
  and use_tls=1 a path that does not exist or cannot be read stops startup. The
  tls_ca_cert wording still says that empty selects the system CA store, since
  only a path that is set is checked.
- The runbook troubleshooting entry gains a third bullet for the "cannot be
  read" message, whose remedy is the path or its permissions rather than the
  certificate and key pairing.

Documentation only; no behaviour change.
2026-08-21 12:29:35 +01:00
Pratik Mankawde
251cd181a7 fix: Reject unreadable [telemetry] TLS certificate paths at startup
With telemetry enabled and use_tls=1, makeTelemetrySetup now reads each
non-empty tls_ca_cert / tls_client_cert / tls_client_key path and refuses to
start when the file is missing or cannot be read. The message names the config
key, the path and the OS error, instead of leaving the problem to surface much
later as an opaque TLS handshake failure inside the exporter.

Reading the file with getFileContents, as the gRPC server already does for its
own ssl_cert and ssl_key pair, proves the file is both present and readable; an
existence test alone would miss a permissions problem. The contents are
discarded.

Both gates are deliberate. The check is skipped when enabled is 0, so a stale
cert line still cannot stop a node from booting, and when use_tls is 0, where
the exporter never opens the files. An empty path stays valid; for tls_ca_cert
it selects the system CA store.

Six GTest cases cover the three keys that can fail, the all-readable case, and
each gate on its own.
2026-08-21 12:29:02 +01:00
Pratik Mankawde
de64a7ab7a Merge branch 'pratik/otel-phase9-metric-gap-fill' into pratik/otel-phase10-workload-validation 2026-08-21 12:13:37 +01:00
Pratik Mankawde
63dc5cce65 Merge branch 'pratik/otel-phase8-log-correlation' into pratik/otel-phase9-metric-gap-fill 2026-08-21 12:13:29 +01:00
Pratik Mankawde
0136291aa0 Merge branch 'pratik/otel-phase7-native-metrics' into pratik/otel-phase8-log-correlation 2026-08-21 12:13:19 +01:00
Pratik Mankawde
76c9051203 feat(insight): let an Event declare what it measures
beast::insight::Event documents itself as carrying "a millisecond time, or
other integral value", but both backends assumed the first case: the OTel
bridge declared every instrument with unit `ms` and StatsD tagged every
sample `|ms`. One Event does not measure time -- ServerHandler's "size"
records the serialized RPC response length -- so it exported as
rpc_size_milliseconds and inherited the millisecond bucket ladder. A quarter
of its samples landed above that ladder's top edge, and since Prometheus
returns the second-highest edge for a quantile in the `+Inf` bucket, its p95
panel showed a flat 5.00 kB rather than a measurement.

Adds beast::insight::Unit (Millis, Bytes) plus otelUnitCode(), carried on
EventImpl and selectable at makeEvent(). Naming the unit at creation is what
lets a backend pick the export unit and, through it, the bucket ladder.

- Collector gains a virtual makeEvent(name, Unit) whose default delegates to
  the millisecond overload, so a collector that cannot act on a unit keeps
  working unchanged. NullCollector and the Groups wrapper override it.
- The Groups override matters most: call sites reach a collector through a
  Group, so forwarding only the prefixed name would silently drop the unit.
  A test covers that hop specifically.
- Event gains notify(std::uint64_t) for non-duration samples, replacing
  ServerHandler's `Event::value_type{response.size()}` -- wrapping a byte
  count in a std::chrono::milliseconds compiles but reads as a duration to
  everything downstream.
- EventImpl::value_type stays std::chrono::milliseconds. Widening it would
  change the wire value of every existing StatsD timer, and metrics needing
  finer resolution use the OTel-native microsecond instruments.

The StatsD collector deliberately keeps emitting `|ms`: that path is retired
here (its UDP port is commented out of the compose file and the integration
test fails if anything listens on 8125), so changing its wire format would
alter a legacy contract with no consumer and no way to verify it.

The exported name does not change yet -- OTelEventImpl still hardcodes its
unit. That follows with the unit-keyed histogram views.
2026-08-21 12:11:32 +01:00
Pratik Mankawde
cbfbea67f2 feat(telemetry): own every histogram ladder in one tested header
The bucket edges for the OTel histograms lived as file-local `namespace {}`
constants, unreachable from any test, and they drifted from the collector's
spanmetrics ladder they were specified to match. The millisecond ladder
stayed capped at 5 s after the collector side was extended to 30 s, so any
quantile above 5 s read back as a flat 5000 -- Prometheus returns the
second-highest edge for a quantile in the `+Inf` bucket, which looks like a
measurement rather than an error.

Adds include/xrpl/telemetry/HistogramBuckets.h as the single owner of the
ladders, with a constexpr validator plus static_asserts so a descending or
duplicated edge cannot compile, and gtest coverage that pins the floor and
ceiling against the measured distributions:

- kMillisecondBuckets carries every representable collector edge and extends
  to 120 s, because the updatepaths job type averages ~60 s and a 30 s
  ceiling would censor it exactly as 5 s does today. Sub-millisecond
  collector edges are omitted: beast::insight::Event rounds durations up to
  whole milliseconds, so they would collect nothing.
- kByteBuckets is new, for Events whose samples are sizes rather than
  durations. Edges follow the measured RPC response distribution (mean
  2131 B, half under 1 kB, tail mean bounded at 7538 B) rather than a guess,
  so the resolution sits between 512 B and 64 kB.

No behaviour change yet -- nothing consumes the header until the views are
rewired.
2026-08-21 11:51:22 +01:00
Pratik Mankawde
0edabc2daa Merge branch 'pratik/otel-phase9-metric-gap-fill' into pratik/otel-phase10-workload-validation 2026-08-20 19:16:39 +01:00
Pratik Mankawde
e54d81a644 style(telemetry): dim the Locust annotation colour
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.
2026-08-20 19:16:21 +01:00
Pratik Mankawde
91a85ebcc8 Merge branch 'pratik/otel-phase9-metric-gap-fill' into pratik/otel-phase10-workload-validation 2026-08-20 19:12:11 +01:00
Pratik Mankawde
3ed5baf485 Merge branch 'pratik/otel-phase8-log-correlation' into pratik/otel-phase9-metric-gap-fill 2026-08-20 19:11:28 +01:00
Pratik Mankawde
4b017dbade fix(telemetry): make the log-trace correlation checks meaningful
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.
2026-08-20 19:11:18 +01:00
Pratik Mankawde
4dc413dcde Merge branch 'pratik/otel-phase9-metric-gap-fill' into pratik/otel-phase10-workload-validation 2026-08-20 19:10:07 +01:00
Pratik Mankawde
1a280603b8 style(telemetry): keep the JMeter annotation colour as it was
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.
2026-08-20 19:10:00 +01:00
Pratik Mankawde
2fea0d44f9 Merge branch 'pratik/otel-phase9-metric-gap-fill' into pratik/otel-phase10-workload-validation 2026-08-20 18:33:59 +01:00