Commit Graph

369 Commits

Author SHA1 Message Date
Pratik Mankawde
c92bb3e02e Merge branch 'pratik/otel-phase7-native-metrics' into pratik/otel-phase8-log-correlation 2026-09-14 20:34:51 +01:00
Pratik Mankawde
975238d7d4 docs(telemetry): fix the node log path, log level and Grafana span link
The config template wrote each node's log to a lowercase node{N} directory
while setting service_instance_id=Node-{N}. The collector takes the node name
from the log file's parent directory and stamps it as the Loki
service_instance_id label, so the logs carried a name no trace or metric
shared and nothing joined. Use Node-{N} and state the rule.

The template also set log_level to warning. Nothing in the pipeline filters on
severity; the constraint is that a log line carries trace context only when it
is emitted inside an active span. At warning the only such statements in the
consensus accept span are a catch path a healthy round never takes and a
periodic censorship warning. At info the CNF Val / CNF buildLCL pair writes one
line per accepted ledger, which is what makes this test's Step 1 findable.

Grafana 13 offers the link per span, labelled "Logs for this span", in the
span's Links row — not per trace. Fix the step and the expected-results row.

The example log line quoted a message that does not exist. The real in-span
RPC statement logs at debug, so the severity code is DBG; say which line to
look for under each test, since Test 2 now logs at info.

Drop the reference to workload/validate_telemetry.py: that file is not part of
this branch, and its instant-endpoint call uses seconds, so the nanoseconds
claim applied only to query_range.
2026-09-11 11:30:39 +01:00
Pratik Mankawde
e1ef6ba183 docs(telemetry): drop the inert insight endpoint from the test config template
On the OTel path only [insight] server is load-bearing. CollectorManager reads
endpoint and hands it to OTelCollector, which logs it at startup and routes
nothing with it; the real export endpoint is [telemetry] metrics_endpoint,
which the template already sets. service_instance_id and service_name in that
section are read and discarded.

Leaving the line invited an operator to reconcile a mismatch that has no
effect. integration-test.sh already emits only server=otel with the same
explanation, so the two now agree.
2026-09-11 11:30:17 +01:00
Pratik Mankawde
6f28d46682 merge: bring the component renames forward from phase7-native-metrics
Two conflicts, both where this branch's log-pipeline additions sat next to the
upstream spanmetrics -> span_metrics rename: the config header comment, which
this branch extended with a logs line, and the integration test, where the
log-correlation step precedes the span-metrics step. This branch's content is
kept in both and the rename carried into it.
2026-09-09 19:20:45 +01:00
Pratik Mankawde
366bcaa328 merge: bring the component renames forward from phase6-statsd
Four conflicts, all where this branch's replacement of the StatsD path with
native OTLP met the upstream spanmetrics -> span_metrics rename. This branch's
design wins in every case; the rename is carried into its text rather than
reverting it, so the connector, its pipeline references, the header comment,
the TESTING.md summary and the runbook all use span_metrics while keeping the
native-OTLP wording.

One addition beyond a straight take-a-side: publish the collector's health
check port. This branch restored the health_check extension and its own
TESTING.md polls http://localhost:13133/ to decide the collector is ready, but
the port was never published on this side of docker-compose.yml, so that check
could not pass from the host. Verified the merged config loads with no
deprecation warnings and that 13133 is published exactly once.

No metric name changed: traces_span_metrics_* already read that way before the
rename, which only ever touched component names and prose.
2026-09-09 19:19:22 +01:00
Pratik Mankawde
988015dc65 merge: bring the OTLP gRPC exporter rename forward from phase5-docs-deployment
Conflict in docker/telemetry/otel-collector-config.yaml, in the service
pipelines: this branch renamed the deprecated spanmetrics connector to
span_metrics and adds the statsd metrics pipeline, while upstream renamed the
deprecated otlp exporter to otlp_grpc. Both kept.

With both renames present the collector now starts with no deprecation
warnings at all, which was the point of the pair.
2026-09-09 19:17:07 +01:00
Pratik Mankawde
c0024c7c57 Merge branch 'pratik/otel-phase3-tx-tracing' into pratik/otel-phase4-consensus-tracing 2026-09-09 19:16:08 +01:00
Pratik Mankawde
b98a9d3d13 Merge branch 'pratik/otel-phase2-rpc-tracing' into pratik/otel-phase3-tx-tracing 2026-09-09 19:16:08 +01:00
Pratik Mankawde
4dc706c606 merge: bring the OTLP gRPC exporter rename forward from phase1c-rpc-integration
Conflict in docker/telemetry/otel-collector-config.yaml, in the traces
pipeline: this branch added the attributes/hash processor while upstream
renamed the deprecated otlp exporter to otlp_grpc. Both sides kept — the
processor list keeps attributes/hash and the exporter list takes the new name.
Checked that the exporter definition key was renamed to match the reference,
and that the collector still loads the merged config.
2026-09-09 19:15:52 +01:00
Pratik Mankawde
8b1e577931 fix(telemetry): tear down the docker volumes on every integration-test run
`docker compose down` keeps the `tempo-data` named volume, so a previous
run's traces stay in Tempo and can satisfy this run's span searches. Pass
-v in cleanup(), and tear the stack down before starting it: a run that
reaches the summary deliberately leaves the stack up, so nothing else
clears it.
2026-09-09 16:10:46 +01:00
Pratik Mankawde
5cb829a755 merge: bring the lock-free ValidationTracker forward from phase7-native-metrics
No conflicts. Carries the lock-free tracker, the metrics_endpoint scheme guard
with its repaired test set, and phase-6's Tempo time bounds.
2026-09-09 15:49:43 +01:00
Pratik Mankawde
c622da5a76 refactor(telemetry): use the current name for the span metrics connector
The pinned collector warns on every start that "spanmetrics" is a deprecated
alias for "span_metrics". Rename the connector, its pipeline references and
the prose that names it.

The derived metric names are untouched. They come from the connector's
`namespace` setting rather than its component name, so occurrences inside a
metric name such as traces_spanmetrics_calls_total are deliberately left as
they are; renaming those would break every span panel. The rename is applied
only to the bare word, never where it is joined to a metric name by
underscores.

This branch introduces the connector, so the change belongs here.
2026-09-09 15:40:11 +01:00
Pratik Mankawde
87457446be refactor(telemetry): use the current name for the OTLP gRPC exporter
The pinned collector warns on every start that "otlp" is a deprecated alias
for "otlp_grpc". Rename the trace exporter to otlp_grpc/tempo.

Only the exporter is affected. The otlp RECEIVER keeps its name: it serves
both gRPC and HTTP under one component and is not deprecated, verified by
renaming the exporter alone and seeing the warning stop.

This belongs on this branch because it introduces the exporter, and it is the
last of four deprecated aliases in the collector config; the other three are
owned by later branches in the chain.
2026-09-09 15:40:09 +01:00
Pratik Mankawde
cbb8581997 fix(telemetry): make the log pipeline actually deliver, and fix its docs
Addresses the open review findings on this branch.

The log root was never delivered at all. Docker creates a missing bind-mount
source as root, Config::getDebugLogFile() only warns when it cannot create the
network subdirectory inside it, and Application carries on. The node therefore
looked healthy while writing no debug.log, and Loki stayed empty with no error
at any layer. docker/telemetry/data/logs has in fact been root-owned in a
working checkout since it was first created. A one-shot xrpld-logdir-init
service now creates the directory and hands it to XRPLD_UID/XRPLD_GID,
following the pattern the storage-init service already uses.

Ingested logs carried no node identity, so a multi-node stack collapsed into
one indistinguishable stream while every dashboard filters on
service_instance_id. The receiver now sets include_file_path and lifts the
per-node directory onto the resource attribute service.instance.id, which is
on the allow-list Loki promotes to an indexed stream label. A record attribute
would only become structured metadata and could not be used in a selector.
For that to join anything the directory name has to equal the emitter's
service_instance_id, so the node directories are renamed to match: node$i
becomes Node-$i, and the standalone config writes to logs/xrpld-standalone.

The integration test aborted before reporting. Under set -o pipefail the
grep | head -1 pipeline is killed by SIGPIPE once the log exceeds the pipe
buffer, so the run exited 141 somewhere past a few hundred matching lines and
read as a flaky test. grep -m1 stops on its own. The test also verified the
local file and Tempo but never that a line reached Loki, which is the one hop
this branch adds, so a bounded Loki assertion is added alongside a readiness
wait.

Documentation fixes: the Tempo cross-check counted .data, but Tempo returns
OTLP shape so the array is batches and one trace can span several; the Loki
step used the instant /query endpoint, which rejects a bare log selector with
HTTP 400 and a text/plain body, so jq could never parse it and the step never
printed a number even when ingestion worked. The filelog comment claimed six
fractional digits where the node always emits nine. The two flowcharts used
<br/>, carried no legend, and advertised GetSpan(), which Log.cpp deliberately
avoids in favour of reading the thread-local context directly.

Finally, rename the deprecated collector component names: the pinned
collector warns on every start that otlphttp and filelog are aliases for
otlp_http and file_log. Alloy's otelcol.exporter.otlphttp and
otelcol.receiver.filelog are that product's own component names and are not
deprecated, so they are left alone.
2026-09-09 15:11:49 +01:00
Pratik Mankawde
ea4f7fd0d4 fix(telemetry): restore health_check and batch the metrics pipeline
The health_check extension was present on the previous branch and dropped
here with no replacement, while this branch's own TESTING.md still polls
http://localhost:13133/ to decide the collector is ready. That check has had
no listener since, so the documented readiness step cannot pass.

Also add batch to the metrics pipeline. Without it the OTLP metric path
exports one request per instrument; the added delay is bounded by the batch
timeout, well under the Prometheus scrape interval.

Both belong here rather than downstream: this branch owns the metrics
pipeline and is the one that regressed the extension.
2026-09-09 15:11:25 +01:00
Pratik Mankawde
cbd6cdbbdd merge: bring the integration-test bounds forward from phase6-statsd
Two files conflicted and both were composed rather than taken from one side.

integration-test.sh: kept this branch's spanmetrics names, since the collector
sets namespace: "span" here and traces_span_metrics_* matches nothing, and took
phase-6's --max-time on every probe. The Tempo time bound needed restoring by
hand: RUN_START, the check_span guard and the start/end parameters are on
phase-6 and absent here, so a plain resolution kept phase-6's comment about
bounding the search while shipping no bound. All four pieces are back.

TESTING.md: kept server=otel with the metrics endpoint. Phase-6's template sets
server=statsd and documents prefix, which this branch's OTel path ignores.
2026-09-09 15:03:08 +01:00
Pratik Mankawde
d972772747 docs(telemetry): match the manual test template to the OTel insight path
This branch switches integration-test.sh to [insight] server=otel and adds an
assertion that no StatsD listener is needed, but TESTING.md still described the
metrics it verifies as StatsD-derived and its manual node-config template had no
[insight] stanza at all.

CollectorManagerImp falls through to NullCollector when server is neither statsd
nor otel, so a reader building configs from that template got zero
beast::insight metrics. The template also omitted service_instance_id and
metrics_endpoint, which the script writes; without the former every node is
indistinguishable in the $node dashboard filter.
2026-09-09 13:14:15 +01:00
Pratik Mankawde
6976539274 fix(telemetry): bound the integration-test probes and align its config template
curl applies no overall timeout of its own, so a server that accepts the
connection and then stops answering parks a poll loop for the rest of the run
and the loop's attempt count stops bounding anything. Add a CURL_MAX_TIME
ceiling and apply it to all 18 executable probes in integration-test.sh.

TESTING.md's manual node-config template also disagreed with what the script
writes, so a reader following it could not reproduce the automated path:

- no [insight] stanza, so no beast::insight metric leaves the node at all and
  Step 10b's ten rippled_* assertions cannot pass
- [ips_fixed] listed all six peer ports including the node's own

The log level is deliberately untouched: the template and the script agree on
warning here.
2026-09-09 13:13:41 +01:00
Pratik Mankawde
30d165da43 fix(telemetry): bound integration-test assertions to the run under test
check_statsd_metric queried rippled_rpc_requests, which no pipeline
produces: the collector's statsd receiver runs with is_monotonic_counter,
so the Prometheus exporter appends _total. A wrong name returns zero
series rather than an error, so the assertion could not be told apart
from a broken pipeline. All eight assertions were re-derived from how
each metric is created in code; this was the only counter.

Tempo searches carried no start/end, and tempo-data is a named volume
that `docker compose down` preserves under a one-hour block retention, so
the 17 span assertions could pass on an earlier local run's traces. Bound
every search to this run, and tear the stack down with -v before starting
so no earlier data is present to match. The service-name check now
matches a whole line, because the tag-values endpoint ignores start/end.

Add a gtest for the StatsD gauge that publishes its initial zero and for
the counter that must publish nothing. Assert two metrics the harness
never checked: a traffic-category gauge no message reaches, and
io_context latency.
2026-09-08 16:43:41 +01:00
Pratik Mankawde
771ea277a5 merge: bring telemetry config and doc fixes forward from phase7-native-metrics 2026-09-03 20:30:33 +01:00
Pratik Mankawde
f180b70640 fix(telemetry): correct drain, init and naming defects; add export cadence keys
Addresses review findings on the native-metrics work.

StatsDCollector::onTimer drained the send buffer inside the polling_ gate. That
gate holds back hook handlers until the application's services are built, but
sendBuffers() is socket I/O. StatsDEventImpl derives only from EventImpl, so it
never enters metrics_ and posts straight to the buffer; its |ms timings piled up
before onCollectionReady and were dropped after onCollectionStopping. The drain
now runs every tick, and outside metricsLock_, so onCollectionStopping no longer
waits on a UDP flush.

TelemetryImpl's constructor left meterProvider_ set when initMetrics() threw.
initMetrics publishes globally as its last step, so a throw left getMeter()
callers holding a provider nothing else could reach. Reset it in the catch.

~ApplicationImp caught only std::exception around telemetry shutdown while the
callees reach third-party SDK code, so a foreign exception would have terminated
the process. Added a logging catch-all.

ValidationTracker's hard trim evicted by unordered_map bucket order. It now
evicts oldest-first, so the entry dropped under pressure is the one least likely
to still reconcile.

The GetMeter test restored the global meter provider only on the success path,
and ASSERT_TRUE early-returns past it. Uses xrpl::ScopeExit instead.

The hook debounce window is a named constant rather than a bare 500 in a
comparison, and the metric export cadence becomes operator-configurable through
metric_export_interval_ms and metric_export_timeout_ms. Both are range-checked:
the SDK warns and silently substitutes its own 60s/30s defaults when the timeout
is not below the interval, so an unchecked value would slow export rather than
speed it up. Parsing uses a signed representation because lexical_cast<uint32_t>
accepts a leading minus and wraps it.

Naming corrections: CollectorManager documented exported_instance, which no OTel
dashboard uses; node-health queried job_count where the exported name is
jobq_job_count; network-traffic and overlay-traffic-detail referenced an
undeclared DS_PROMETHEUS variable; the counter table omitted the _total suffix
the Prometheus exporter appends; the plan docs and task list carried an xrpld_
prefix formatName never applies; and OTelCollector::New()'s contract promised its
instanceId, serviceName and networkType arguments were read, contradicting the
definition that marks them unused.
2026-09-03 20:20:13 +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
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
0fcd690cf9 Merge branch 'pratik/otel-phase7-native-metrics' into pratik/otel-phase8-log-correlation 2026-08-27 12:59:13 +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
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
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
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
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
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
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
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