Commit Graph

589 Commits

Author SHA1 Message Date
Pratik Mankawde
0ee97a7951 docs(telemetry): follow the close-time attr rename in the key lists
Update the consensus and ledger key inventories and the spanmetrics
dimension comment in both collector configs, so they name the emitted
keys. The comment's last_close_time reference is a server_info gauge, not
the span attribute, and is left as it is.
2026-09-04 12:39:05 +01:00
Pratik Mankawde
a5d5d0911f merge: bring the metrics_endpoint key forward from phase-8
Telemetry.cpp conflicted. Phase-9 rewrote the metrics pipeline into
makeTracerResource()/makeMetricsResource()/initMetrics() further down the
class, so its side of the region is empty and phase-8's private helper
block does not apply. Resolved to phase-9's structure; phase-8's own
hunks outside the region (the deleted kTracesPath/kMetricsPath, the
verbatim traces URL, the two-endpoint startup log) merged in.

Phase-9's initMetrics() still derives the metrics URL by suffix-swap.
That is fixed in the next commit, not here.
2026-09-03 16:45:09 +01:00
Pratik Mankawde
4587986a93 merge: bring the metrics_endpoint key forward from phase-7 2026-09-03 16:40:59 +01:00
Pratik Mankawde
7b845392d4 feat(telemetry): give metrics its own endpoint key
One [telemetry] key served both OTLP signals, and the metrics URL was
derived from it by suffix-swap: strip a trailing slash, strip a known
signal path if present, append the wanted one. Anything not ending
/v1/traces therefore posted metrics to the traces path, and the OTLP
version was pinned in code where an operator could not reach it.

Adds metrics_endpoint alongside traces_endpoint. Both are full URLs used
verbatim, so traces and metrics can go to different collectors, or to one
whose OTLP paths are not the defaults. signalEndpoint(), kTracesPath and
kMetricsPath are gone; nothing derives an endpoint from another.

The startup log names both URLs, since with two independent endpoints
there was otherwise no way to see where metrics were going.

Also drops exporter=otlp_http from the shipped config and the test
fixture. No branch in the chain reads an `exporter` key: it was a real
Setup member in the first phase-1b implementation, removed when only
OTLP/HTTP was wired up, and already deleted from TESTING.md once on the
same grounds.
2026-09-03 16:39:04 +01:00
Pratik Mankawde
3aff6d4d64 merge: bring the traces_endpoint rename forward from phase-8
Eighteen conflict regions across nine files. Resolved by asking, per
region, which side is the better final state rather than by taking a
branch wholesale.

Telemetry.cpp keeps phase-9's two resource builders. phase-8 offered a
single makeResource() with no node identity; phase-9 splits it into
makeTracerResource() and makeMetricsResource() because the metrics
provider is built in the constructor, before setNodeId() runs, so
xrpl.node.id can only be stamped unconditionally on the tracer side.
Collapsing them would have dropped that attribute, which is what keeps
per-node traces from folding into one identity.

Telemetry.h and the config test compose both sides: phase-9's nodeId
member and its assertion, plus the renamed endpoint.

xrpld-telemetry.cfg keeps phase-9's devnet identity and its
metrics_endpoint, renames the traces key, and drops exporter=otlp_http.
Nothing reads an `exporter` key on any branch in the chain: it was a real
Setup member in the first phase-1b implementation, removed when only
OTLP/HTTP was wired up, and already deleted from TESTING.md once on the
same grounds. The cfg line was the last carrier.

The docs keep phase-9's versions, which are both fuller and more
accurate: the incoming runbook listed the consensus strategy values as
"random" where the code compares against "attribute".

OTelCollector.cpp had five comment-only regions in a file phase-7 owns,
so those take the upstream side.

MetricsRegistry.h's usage example named a member that no longer exists
and the wrong arity; it now matches the real three-argument call and says
where the endpoint comes from.
2026-09-03 16:06:38 +01:00
Pratik Mankawde
9fd5414fe6 merge: bring the traces_endpoint rename forward from phase-7
Ten conflict regions in four files, none of them caused by the rename.
phase-8's own commit had rewritten comments in files phase-7 owns
(Unit.h, HistogramBuckets.h, OTelCollector.cpp) and in Telemetry.cpp,
while the same sweep ran independently on phase-7.

Resolved every region to phase-7's side on ownership grounds: those files
belong to phase-7 or earlier, so a downstream branch should not carry
divergent copies. phase-8 changed comments only in all four, verified
against the merge base, so no code was dropped.

The one structural region: phase-7 had refactored addUnitView from an
inline lambda into a member function, and phase-8 still held the lambda.
Keeping phase-8's would have shadowed the member.

Wording phase-8 had that is worth restoring on phase-7 -- the "legacy"
qualifier on the prefix parameter, and the rejected-alternative note on
the bucket ladders -- is recorded outside the tree for a follow-up.
2026-09-03 15:43:24 +01:00
Pratik Mankawde
8d1eacc5fc merge: bring the traces_endpoint rename forward from phase-6
Three conflicts, all composed rather than resolved by taking a side:

- TelemetryConfig.cpp: phase-6 kept networkTypeFromId file-local with
  [[nodiscard]]; phase-7 had relocated it to public scope for
  Application.cpp. Kept phase-7's relocation, so one definition remains.
  The [[nodiscard]] survives on the declaration in Telemetry.h.
- Telemetry.cpp x2: phase-7 added getMeter overrides, phase-6 added
  [[nodiscard]] to the startSpan below them. Kept both, and put
  [[nodiscard]] on getMeter too.
- TESTING.md: phase-7 had the right metric name (span_calls_total, which
  the spanmetrics namespace produces) but the wrong label. Its
  xrpl.rpc.command appears nowhere else in the branch; the attribute is
  bare `command`, which is what the dashboards query. Took phase-7's
  metric with the correct label.

Both signalEndpoint call sites follow the renamed member. signalEndpoint
itself is left in place: removing it and adding metrics_endpoint is a
design change, not part of propagating a rename.
2026-09-03 15:38:47 +01:00
Pratik Mankawde
8c695b69a5 fix(telemetry): follow the traces_endpoint rename in the node configs
The rename arrived from phase-1b by merge. Four files still wrote the old
key, which the parser no longer reads, so each would have silently
fallen back to the default collector URL.

integration-test.sh is the load-bearing one: it generates the node config
the test harness starts, so the stale key would have pointed the node at
localhost regardless of the compose network. xrpld-telemetry.cfg is the
standalone node config; the other two document the key.

Note this cfg has a second, divergent variant on the devnet branches that
needs the same fix there.
2026-09-03 15:21:01 +01:00
Pratik Mankawde
91c7137373 merge: bring the traces_endpoint rename forward from phase-5 2026-09-03 15:20:30 +01:00
Pratik Mankawde
e57d3c2f4b merge: bring the traces_endpoint rename forward from phase3-tx-tracing 2026-09-03 15:15:33 +01:00
Pratik Mankawde
65a679b7cd merge: bring the traces_endpoint rename forward from phase2-rpc-tracing 2026-09-03 15:15:33 +01:00
Pratik Mankawde
4baac61dd9 merge: bring the traces_endpoint rename forward from phase-1c 2026-09-03 15:14:20 +01:00
Pratik Mankawde
f68cf0d009 refactor(telemetry): name the traces endpoint for its signal
[telemetry] endpoint carried one OTLP signal while its name implied it
covered every signal. That asymmetry is what let the metrics URL be
guessed later by rewriting this one's path suffix, so anything not
ending /v1/traces silently posted metrics to the traces path.

Renames the key to traces_endpoint and Setup::exporterEndpoint to
tracesEndpoint. The default value is unchanged and the URL is still used
verbatim, with no path derived from it. The startup log line and the
compose-file example name the new key, the latter being where an
operator copies it from.

No metrics_endpoint is added here: this branch has no metrics pipeline,
so the key would parse into a member nothing reads.
2026-09-03 15:09:19 +01:00
Pratik Mankawde
d687903c3f fix(telemetry): correct the metric names, guard the arithmetic, tidy the collector
Harness and docs:

- integration-test.sh queried traces_span_metrics_* for spanmetrics, but this
  branch sets the connector namespace to "span", so those two checks matched
  nothing and failed. The dashboards and runbook had moved; the script had not.
- The same script queried eight native metric names with a product prefix and
  capitals that formatName() cannot produce: it lowercases, maps '.' and ' ' to
  '_', and prepends nothing. Corrected against the runbook tables.
- TESTING.md carried the same stale spanmetrics names and a jq example reading
  a Prometheus label that does not exist.
- The runbook now records where each part of a derived metric name comes from,
  since only the namespace is ours to choose.

Collector:

- OTelCounterImpl::increment silently dropped a negative amount. An OTel
  counter takes unsigned deltas, so assert and let a release build under-count
  rather than wrap.
- OTelGaugeImpl::increment computed current + amount in int64, which is
  undefined on overflow, and the clamp ran afterwards so it could not help.
  Check the headroom first. set() now clamps rather than casting a uint64 above
  INT64_MAX to a negative, which is what made underflow reachable.
- The meter scope was two bare literals. They are constants now, and
  Telemetry.cpp static_asserts them equal to kMeterName and kMeterVersion:
  beast cannot include the telemetry header, so a build failure is the only way
  to catch the copies drifting.
- formatName uses views::transform and ranges::to, as Backend.cpp already does.
- Unused constructor parameters take [[maybe_unused]] instead of (void) casts.
- The destructor logged "shutting down" and "stopped" with nothing between.

initMetrics was 79 lines doing four jobs. The exporter and the histogram views
are separate functions now, addUnitView is a member rather than a lambda
capturing this, and the export interval and timeout are named. It also derived
the metrics URL from the traces URL by suffix swap, which sent metrics to the
traces path whenever the configured URL had any other shape; both URLs now come
from one rule that handles a bare host, a trailing slash and either signal path.
2026-09-03 10:52:42 +01:00
Pratik Mankawde
5f53745fbf docs(telemetry): correct the TESTING guide and the span attribute tables
Review feedback on the testing guide:

- rm -rf targeted data/, but this config writes under docker/telemetry/data/,
  so teardown did nothing and a second run reused the old NuDB and SQLite
  state. Corrected at both sites, including the Test 2 keygen node, which
  launches with the same config.
- The standalone span table said consensus.* does not fire. It does:
  ledger_accept drives a simulated round, so consensus.round, .phase.open,
  .ledger_close, .accept and .accept.apply all appear. Only .establish,
  .update_positions, .check, .proposal.* , .validation.receive and
  .mode_change cannot. The test intro claimed the same thing and now agrees
  with the table.
- Three blocks duplicated content the file already had. Test 1 now points at
  the shared Verification Queries section as Test 2 already did, and the
  Test 2 submit block checks engine_result like Test 1 does.
- The numbered step list was a copy of the script's own Step N headers and had
  drifted by four entries, so it now points at those headers instead.

Also corrects the runbook's ledger and peer span tables against the code:
ledger.build was credited with tx_count and tx_failed, which tx.apply sets,
and was missing its three close-time attributes; peer.validation.receive was
missing ledger_hash and full_validation. The five source line numbers in those
two tables were stale, so they now name the file only, as the other nineteen
rows do.
2026-09-03 10:52:04 +01:00
Pratik Mankawde
4f4c9e8e3e fix(telemetry): correct span scope, span-guard names and the span docs
Review feedback, plus a sweep of the branch for the same defects elsewhere.

Scope: the ledger.validate guard was a plain local, so it stayed alive until
checkAccept returned and the flag-ledger upgrade check ran inside the measured
span. That check reads every trusted validation of the parent, so one span in
256 became a duration outlier for work unrelated to promoting a ledger. The
span is now scoped to the promotion. tryAdvance stays inside it because it only
sets a flag and posts a job.

Attributes: tx.apply now carries ledger_seq, which the runbook already
documented. The parent ledger.build span has it, but a child cannot be selected
by its parent's attributes, so the span could not be found by ledger.

Guard names: each span guard is now named after the span it holds, so
proposalReceiveSpan, validationReceiveSpan, storeSpan and validateSpan. The
name "span" previously meant the trace root in one inbound-message handler and
the job-queue handle in its sibling, which taught a reader the opposite of the
truth in the next function.

Comments: the StatsD gauge rationale now sits with the initialiser it explains
rather than in the constructor. The peer span header described its trust flags
as shared when they are in fact re-declared to match the consensus keys; the
duplication is intentional and the wording was not.

Docs: the ledger and peer span tables disagreed with the code, crediting
ledger.build with attributes that are set on tx.apply and omitting several that
it does set, and all five source-file line numbers in them were stale. The
testing guide listed attribute keys that exist nowhere in the code, so its
catalog now points at the runbook instead of keeping a second copy that drifts.
2026-09-02 15:30:30 +01:00
Pratik Mankawde
566a734f13 Merge branch 'pratik/otel-phase8-log-correlation' into pratik/otel-phase9-metric-gap-fill
Conflict in docker/telemetry/integration-test.sh: the incoming side removes
the inert [insight] prefix, and this branch had added service_instance_id
to the same block. Resolved by taking both -- prefix=rippled dropped,
service_instance_id=Node-${i} kept.
2026-08-27 13:00:11 +01:00
Pratik Mankawde
0fcd690cf9 Merge branch 'pratik/otel-phase7-native-metrics' into pratik/otel-phase8-log-correlation 2026-08-27 12:59:13 +01:00
Pratik Mankawde
f9d482ba2b fix(telemetry): drop insight keys the OTel collector discards from devnet cfg
The devnet config was missed when the mainnet one was corrected. On the
OTel path prefix is inert, because formatName() ignores it, and
service_instance_id is read and then discarded -- OTelCollector.cpp does
`(void)instanceId`. The service_instance_id label Prometheus shows comes
from [telemetry] service_instance_id, which this file still sets, so
dashboards keep filtering by node.

The comment removed here claimed every insight-backed panel goes empty
without the [insight] copy of the key. That is not the case.
2026-08-27 12:58:57 +01:00
Pratik Mankawde
95fb21b5d5 docs(telemetry): drop the inert insight prefix from the OTel examples
OTelCollector routes every instrument name through a static formatName()
that only lowercases the name and maps '.' and space to '_'. The sole read
of prefix_ is the startup log line at OTelCollector.cpp:802, and all four
instrument factories go through formatName(), so no prefix can ever reach
an exported name. StatsDCollector does prepend it, so the StatsD example
keeps the key and now states why.

Covers the three server=otel blocks in the 09 reference and the config
integration-test.sh generates. This branch introduces OTelCollector, so it
is where the inert examples first appear; phase-6's examples are all
server=statsd and stay as they are.
2026-08-27 12:58:45 +01:00
Pratik Mankawde
4f056496f8 Merge branch 'pratik/otel-phase7-native-metrics' into pratik/otel-phase8-log-correlation 2026-08-27 12:32:54 +01:00
Pratik Mankawde
fa23fb51ea fix(telemetry): drop insight keys the OTel collector discards
prefix and service_instance_id are read and thrown away on this path, so
the node's identity label comes from [telemetry] instead. The old comment
claimed the insight copy was required or panels would be empty.
2026-08-27 12:13:46 +01:00
Pratik Mankawde
cb92d59f11 fix(telemetry): drop the inert insight prefix from the OTel path
formatName() never reads prefix, so setting it here does nothing and the
exported names are bare and lowercase. Leaving it invites queries written
against xrpld_jobq_job_count, which match no series.

The StatsD examples keep it, because that path does apply it to the name.
2026-08-27 12:12:31 +01:00
Pratik Mankawde
f71357d826 Merge branch 'pratik/otel-phase8-log-correlation' into pratik/otel-phase9-metric-gap-fill
# Conflicts:
#	OpenTelemetryPlan/05-configuration-reference.md
#	OpenTelemetryPlan/09-data-collection-reference.md
#	OpenTelemetryPlan/Phase4_taskList.md
#	docker/telemetry/grafana/dashboards/ledger-data-sync.json
#	docs/telemetry-runbook.md
2026-08-25 15:14:56 +01:00
Pratik Mankawde
520e8e5895 Merge branch 'pratik/otel-phase7-native-metrics' into pratik/otel-phase8-log-correlation 2026-08-25 15:07:36 +01:00
Pratik Mankawde
cc5e098928 fix(telemetry): rate the overlay traffic bargauge instead of ranking totals
The panel described itself as showing which categories consume the most
receive bandwidth, and told operators to watch for a category dominating
during ongoing sync, but it ranked raw cumulative counters. That ranks by
lifetime total, so a sync burst days earlier still dominates the bars and
the stated guidance cannot work.

Wrapping the existing selector in rate() does not work: rate() drops
__name__, so the many *_bytes_in series collapse to one labelset and the
query fails with "vector cannot contain metrics with the same labelset".
Grouping with sum by (__name__) does not help either, because Prometheus
strips __name__ from aggregation output regardless of the grouping. Both
were tried against a local Prometheus with synthetic multi-category
counters and both error.

Instead recover the category into a normal `series` label before rating,
and use a subquery so the renamed series can be rated. Verified against
the same synthetic data: rates match a plain single-metric rate control
to within subquery resolution, ranking order is correct, and both the
category and node labels survive.

Also set unit to Bps, matching the ten sibling rate panels in this
dashboard (this was the only byte panel still on decbytes and the only
one plotting raw), and drop "Heatmap" from the title since the panel is
a bargauge.

Note the subquery evaluates the selector at several steps, so it costs
more than the previous instant query across the traffic categories.
2026-08-24 21:10:28 +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
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
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
62ef753804 feat(telemetry): split perf-run annotations by load driver
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.
2026-08-20 18:33:19 +01:00
Pratik Mankawde
597b0c2bab Merge branch 'pratik/otel-phase8-log-correlation' into pratik/otel-phase9-metric-gap-fill
# Conflicts:
#	docker/telemetry/xrpld-telemetry.cfg
#	src/libxrpl/beast/insight/OTelCollector.cpp
#	src/libxrpl/telemetry/Telemetry.cpp
#	src/xrpld/app/main/Application.cpp
2026-08-20 16:43:32 +01:00
Pratik Mankawde
8fac7fc08c Merge branch 'pratik/otel-phase7-native-metrics' into pratik/otel-phase8-log-correlation 2026-08-20 16:38:03 +01:00
Pratik Mankawde
4278014ab0 fix(telemetry): order the metrics pipeline by instrument kind
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.
2026-08-20 16:36:41 +01:00
Pratik Mankawde
070d29b465 feat(telemetry): add the xrpl.node.id resource attribute
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.
2026-08-19 19:50:40 +01:00
Pratik Mankawde
4a361a496d fix(telemetry): carry the per-node id on spans, not only the resource
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.
2026-08-19 15:44:32 +01:00
Pratik Mankawde
0607d969bc Merge branch 'pratik/otel-phase8-log-correlation' into pratik/otel-phase9-metric-gap-fill
# Conflicts:
#	docker/telemetry/grafana/dashboards/node-health.json
#	docs/telemetry-runbook.md
2026-08-17 19:37:50 +01:00
Pratik Mankawde
bbc57202f6 Merge branch 'pratik/otel-phase7-native-metrics' into pratik/otel-phase8-log-correlation 2026-08-17 19:36:57 +01:00
Pratik Mankawde
ac71480a62 fix(telemetry): count mode transitions with tiling buckets, not overlapping ones
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.
2026-08-17 19:36:41 +01:00
Pratik Mankawde
a80c7849fc Merge branch 'pratik/otel-phase8-log-correlation' into pratik/otel-phase9-metric-gap-fill
# Conflicts:
#	docker/telemetry/grafana/dashboards/node-health.json
#	docs/telemetry-runbook.md
2026-08-17 18:04:50 +01:00
Pratik Mankawde
b19e429636 Merge branch 'pratik/otel-phase7-native-metrics' into pratik/otel-phase8-log-correlation 2026-08-17 18:02:59 +01:00
Pratik Mankawde
c8f9ab9597 fix(telemetry): plot operating-mode transitions per interval, not raw counters
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.
2026-08-17 18:02:13 +01:00
Pratik Mankawde
bc99a4edcd Merge branch 'pratik/otel-phase8-log-correlation' into pratik/otel-phase9-metric-gap-fill
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.
2026-08-17 12:07:56 +01:00
Pratik Mankawde
f3681ad920 Merge branch 'pratik/otel-phase7-native-metrics' into pratik/otel-phase8-log-correlation
Carries phase-6's integration-test and telemetry-doc fixes forward. Merged
cleanly with no conflicts.
2026-08-17 12:06:12 +01:00
Pratik Mankawde
b77f516f95 Merge branch 'pratik/otel-phase6-statsd' into pratik/otel-phase7-native-metrics
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.
2026-08-17 12:05:10 +01:00
Pratik Mankawde
3a3ae1c3ae fix(telemetry): name the job-queue depth metric jobq_job_count
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
2026-08-17 12:02:46 +01:00
Pratik Mankawde
301149d131 fix(telemetry): keep filelog offset storage out of the shared collector config
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.
2026-08-17 11:44:30 +01:00
Pratik Mankawde
d1d766a8d1 Merge branch 'pratik/otel-phase8-log-correlation' into pratik/otel-phase9-metric-gap-fill
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.
2026-08-15 17:52:42 +01:00