From ff8629bb116a64ccd748ec3a97513a2a5022733d Mon Sep 17 00:00:00 2001 From: Pratik Mankawde <3397372+pratikmankawde@users.noreply.github.com> Date: Mon, 24 Aug 2026 20:50:21 +0100 Subject: [PATCH] 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. --- docker/telemetry/workload/README.md | 2 +- .../telemetry/workload/run-full-validation.sh | 13 +++++++++---- .../workload/xrpld-validator.cfg.template | 18 ++++++++++++++---- 3 files changed, 24 insertions(+), 9 deletions(-) diff --git a/docker/telemetry/workload/README.md b/docker/telemetry/workload/README.md index 9d96499051..4c0ce078ca 100644 --- a/docker/telemetry/workload/README.md +++ b/docker/telemetry/workload/README.md @@ -469,7 +469,7 @@ absence is a skip rather than a failure: The orchestrator (`run-full-validation.sh`) generates node configs with: - `[telemetry] enabled=1` with all five trace categories: `trace_rpc`, `trace_transactions`, `trace_consensus`, `trace_peer`, `trace_ledger` -- `[insight] server=otel` with `endpoint=http://localhost:4318/v1/metrics` and `prefix=xrpld` — `beast::insight` metrics reach Prometheus over OTLP, because the collector declares no `statsd` receiver +- `[insight] server=otel` with `endpoint=http://localhost:4318/v1/metrics` — `beast::insight` metrics reach Prometheus over OTLP, because the collector declares no `statsd` receiver. No `prefix` is set: it would be inert, since `OTelCollector` applies no prefix to instrument names and exported names are the lowercased raw names (`jobq_job_count`, not `xrpld_jobq_job_count`) - `[signing_support] true` — required for `tx_submitter.py` to submit signed transactions via WebSocket - `[ips]` (not `[ips_fixed]`) — ensures peer connections are counted in the PeerFinder active-peer gauges, exported as `peer_finder_active_inbound_peers` / `peer_finder_active_outbound_peers` (fixed peers are excluded from these counters by design). The `beast::insight` group/name pair is `Peer_Finder` / `Active_Inbound_Peers`; `formatName()` lowercases it for export. diff --git a/docker/telemetry/workload/run-full-validation.sh b/docker/telemetry/workload/run-full-validation.sh index ef3cc6cb45..5acbbfbfc5 100755 --- a/docker/telemetry/workload/run-full-validation.sh +++ b/docker/telemetry/workload/run-full-validation.sh @@ -329,12 +329,17 @@ trace_ledger=1 [insight] # Native OTel metrics via OTLP/HTTP. The collector has no StatsD receiver -# (metrics pipeline is [otlp, spanmetrics]), so beast::insight must export -# over OTLP for system metrics to reach Prometheus. prefix=xrpld matches the -# OTel resource service name and the metric names the dashboards query. +# (its metrics pipeline is [otlp, spanmetrics]), so beast::insight must export +# over OTLP for system metrics to reach Prometheus at all. server=otel is the +# only load-bearing key here -- it selects the OTel collector; endpoint is +# read but already matches the built-in default. +# +# No prefix is set on purpose. It would be inert: OTelCollector applies no +# prefix to instrument names, so exported names are the lowercased raw names +# (jobq_job_count, rpc_requests_total, total_bytes_in). The service is +# identified by the OTel resource service.name, not by a name prefix. server=otel endpoint=http://localhost:4318/v1/metrics -prefix=xrpld [rpc_startup] # info is required here, not cosmetic, and is the minimum that works. The diff --git a/docker/telemetry/workload/xrpld-validator.cfg.template b/docker/telemetry/workload/xrpld-validator.cfg.template index 875abd63ab..623b781707 100644 --- a/docker/telemetry/workload/xrpld-validator.cfg.template +++ b/docker/telemetry/workload/xrpld-validator.cfg.template @@ -4,6 +4,11 @@ # cfg inline. Kept as the reference layout for a validator run as a container, # whose entrypoint would substitute the placeholders below. # +# Because nothing reads this file, nothing catches it drifting from the cfg the +# runner actually generates. Any change to the [telemetry], [insight] or +# [rpc_startup] stanzas here must be mirrored in run-full-validation.sh, and +# vice versa. +# # Placeholders: # {{NODE_INDEX}} — Node number (1-based) # {{RPC_PORT}} — HTTP RPC port @@ -90,13 +95,18 @@ trace_peer=1 trace_ledger=1 # --- Native OTel metrics (beast::insight over OTLP/HTTP) --- -# The collector has no StatsD receiver (metrics pipeline is [otlp, spanmetrics]), -# so beast::insight exports natively over OTLP. prefix=xrpld matches the OTel -# resource service name and the metric names the dashboards query. +# The collector has no StatsD receiver (its metrics pipeline is +# [otlp, spanmetrics]), so beast::insight must export natively over OTLP for +# system metrics to reach Prometheus at all. `server=otel` is the only +# load-bearing key here -- it selects the OTel collector. +# +# No `prefix` is set on purpose. It would be inert: OTelCollector applies no +# prefix to instrument names, so exported names are the lowercased raw names +# (`jobq_job_count`, `rpc_requests_total`, `total_bytes_in`). The service is +# identified by the OTel resource `service.name`, not by a name prefix. [insight] server=otel endpoint={{OTEL_METRICS_ENDPOINT}} -prefix=xrpld [rpc_startup] { "command": "log_level", "severity": "{{LOG_LEVEL}}" }