From b7d0b700c65d8b173ea85b3ac83cbcfc9aef685f Mon Sep 17 00:00:00 2001 From: Pratik Mankawde <3397372+pratikmankawde@users.noreply.github.com> Date: Mon, 6 Jul 2026 21:04:07 +0100 Subject: [PATCH] fix(telemetry): correct runbook pathfind spans + mTLS validation - Add PathFinding Spans section to the runbook Span Reference and remove the incorrect "not emitted / later-phase" annotations: pathfind.request/compute/discover/update_all are emitted since Phase 2 (upstream of Phase 5). - Runbook label names use the bare spanmetrics dimensions (command, rpc_status, local, consensus_mode); fix stale docs. - 05-configuration-reference: note tls_client_cert/key require use_tls=1. - TelemetryConfig: reject mTLS cert paths when use_tls=0 so telemetry is not silently sent in plaintext. Co-Authored-By: Claude Opus 4.8 --- .../05-configuration-reference.md | 4 +- docs/telemetry-runbook.md | 56 ++++++++++++------- src/libxrpl/telemetry/TelemetryConfig.cpp | 10 ++++ 3 files changed, 47 insertions(+), 23 deletions(-) diff --git a/OpenTelemetryPlan/05-configuration-reference.md b/OpenTelemetryPlan/05-configuration-reference.md index ff1aa1bc98..e8b9bc0027 100644 --- a/OpenTelemetryPlan/05-configuration-reference.md +++ b/OpenTelemetryPlan/05-configuration-reference.md @@ -21,8 +21,8 @@ The authoritative `[telemetry]` example lives in `cfg/xrpld-example.cfg`. Teleme | `endpoint` | string | `http://localhost:4318/v1/traces` | OTLP/HTTP collector endpoint | | `use_tls` | bool | `false` | Enable TLS for exporter connection | | `tls_ca_cert` | string | `""` | Path to CA certificate file | -| `tls_client_cert` | string | `""` | Path to node's client certificate (PEM) for mutual TLS; empty = one-way TLS | -| `tls_client_key` | string | `""` | Path to private key (PEM) for `tls_client_cert`; required when it is set | +| `tls_client_cert` | string | `""` | Path to node's client certificate (PEM) for mutual TLS; requires `use_tls=1`; empty = one-way TLS | +| `tls_client_key` | string | `""` | Path to private key (PEM) for `tls_client_cert`; requires `use_tls=1`; required when the cert is set | | `batch_size` | uint | `512` | Spans per export batch | | `batch_delay_ms` | uint | `5000` | Max delay before sending batch (ms) | | `max_queue_size` | uint | `2048` | Maximum queued spans | diff --git a/docs/telemetry-runbook.md b/docs/telemetry-runbook.md index c614795433..264f9b6ed6 100644 --- a/docs/telemetry-runbook.md +++ b/docs/telemetry-runbook.md @@ -94,6 +94,15 @@ All spans instrumented in xrpld, grouped by subsystem: | `txq.accept_tx` | TxQ.cpp | `tx_hash`, `retries_remaining`, `ter_code`, `txq_status` | Per-transaction apply during accept | | `txq.cleanup` | TxQ.cpp | `ledger_seq` | Post-close cleanup of expired queue entries | +### PathFinding Spans + +| Span Name | Source File | Attributes | Description | +| --------------------- | --------------------------------- | -------------------------------------------------- | ------------------------------------------------------- | +| `pathfind.request` | PathFind.cpp / RipplePathFind.cpp | `pathfind_source_account`, `pathfind_dest_account` | Path-find RPC entry (accounts hashed; set when present) | +| `pathfind.compute` | PathRequest.cpp | `pathfind_fast`, `pathfind_dest_currency` | Path computation for one request (`doUpdate`) | +| `pathfind.discover` | PathRequest.cpp | `pathfind_search_level`, `pathfind_num_paths` | Graph exploration (one per RPC call in `findPaths`) | +| `pathfind.update_all` | PathRequestManager.cpp | `pathfind_ledger_index`, `pathfind_num_requests` | Async recomputation of active requests on ledger close | + ### Consensus Spans | Span Name | Source File | Attributes | Description | @@ -104,7 +113,7 @@ All spans instrumented in xrpld, grouped by subsystem: | `consensus.ledger_close` | RCLConsensus.cpp | `ledger_seq`, `consensus_mode` | Ledger close event | | `consensus.establish` | Consensus.h | `converge_percent`, `establish_count`, `proposers` | Establish phase duration (child of round) | | `consensus.update_positions` | Consensus.h | `converge_percent`, `proposers`, `disputes_count` | Position update and dispute resolution (see Events below) | -| `consensus.check` | Consensus.h | `agree_count`, `disagree_count`, `converge_percent`, `have_close_time_consensus`, `threshold_percent`, `consensus_result` | Consensus threshold check | +| `consensus.check` | Consensus.h | `agree_count`, `disagree_count`, `converge_percent`, `have_close_time_consensus`, `threshold_percent`, `proposers_finished`, `consensus_stalled`, `establish_count`, `consensus_result` | Consensus threshold check | | `consensus.accept` | RCLConsensus.cpp | `proposers`, `round_time_ms`, `quorum`, `disputes_count`, `consensus_state` | Ledger accepted by consensus | | `consensus.accept.apply` | RCLConsensus.cpp | `ledger_seq`, `close_time`, `close_time_correct`, `close_resolution_ms`, `consensus_state`, `proposing`, `round_time_ms`, `parent_close_time`, `close_time_self`, `close_time_vote_bins`, `resolution_direction`, `tx_count` | Ledger application with close time details (see Events below) | | `consensus.validation.send` | RCLConsensus.cpp | `ledger_seq`, `proposing`, `ledger_hash`, `full_validation`, `validation_sign_time` | Validation sent after accept (follows-from link) | @@ -338,7 +347,7 @@ all its normal attributes, it just lacks a cross-node parent link. {name="tx.receive"} && status != error # Find proposals received with cross-node parent context -{name="consensus.proposal.receive"} && nestedSetParent > 0 +{} >> {name="consensus.proposal.receive"} # Trace a transaction across the network by its hash {name=~"tx\\..*"} | tx_hash = "" @@ -374,14 +383,19 @@ Every metric carries these standard labels: | `service_name` | Resource attribute | `xrpld` | | `span_kind` | Span kind | `SPAN_KIND_INTERNAL` | -Additionally, span attributes configured as dimensions in the collector become metric labels (dots → underscores): +Additionally, span attributes configured as dimensions in the collector +become metric labels. The span attribute keys are already underscore form +(the naming convention forbids dots), so the label name matches the attribute +name verbatim. Prometheus' dots → underscores sanitization only fires for +dotted attribute names (e.g. resource attributes like `service.name`), which +does not apply to these dimensions. -| Span Attribute | Metric Label | Applies To | -| ---------------- | --------------------- | ------------------------------ | -| `command` | `xrpl_rpc_command` | `rpc.command.*` spans | -| `rpc_status` | `xrpl_rpc_status` | `rpc.command.*` spans | -| `consensus_mode` | `xrpl_consensus_mode` | `consensus.ledger_close` spans | -| `local` | `xrpl_tx_local` | `tx.process` spans | +| Span Attribute | Metric Label | Applies To | +| ---------------- | ---------------- | ------------------------------ | +| `command` | `command` | `rpc.command.*` spans | +| `rpc_status` | `rpc_status` | `rpc.command.*` spans | +| `consensus_mode` | `consensus_mode` | `consensus.ledger_close` spans | +| `local` | `local` | `tx.process` spans | ### Histogram Buckets @@ -481,21 +495,21 @@ Three dashboards are pre-provisioned in `docker/telemetry/grafana/dashboards/`: ### RPC Performance (`rpc-performance`) -| Panel | Type | PromQL | Labels Used | -| --------------------------- | ---------- | -------------------------------------------------------------------------------------------------------------------------------------------------- | --------------------------------- | -| RPC Request Rate by Command | timeseries | `sum by (xrpl_rpc_command) (rate(traces_span_metrics_calls_total{span_name=~"rpc.command.*"}[5m]))` | `xrpl_rpc_command` | -| RPC Latency p95 by Command | timeseries | `histogram_quantile(0.95, sum by (le, xrpl_rpc_command) (rate(traces_span_metrics_duration_milliseconds_bucket{span_name=~"rpc.command.*"}[5m])))` | `xrpl_rpc_command` | -| RPC Error Rate | bargauge | Error spans / total spans × 100, grouped by `xrpl_rpc_command` | `xrpl_rpc_command`, `status_code` | -| RPC Latency Heatmap | heatmap | `sum(increase(traces_span_metrics_duration_milliseconds_bucket{span_name=~"rpc.command.*"}[5m])) by (le)` | `le` (bucket boundaries) | +| Panel | Type | PromQL | Labels Used | +| --------------------------- | ---------- | ----------------------------------------------------------------------------------------------------------------------------------------- | ------------------------ | +| RPC Request Rate by Command | timeseries | `sum by (command) (rate(traces_span_metrics_calls_total{span_name=~"rpc.command.*"}[5m]))` | `command` | +| RPC Latency p95 by Command | timeseries | `histogram_quantile(0.95, sum by (le, command) (rate(traces_span_metrics_duration_milliseconds_bucket{span_name=~"rpc.command.*"}[5m])))` | `command` | +| RPC Error Rate | bargauge | Error spans / total spans × 100, grouped by `command` | `command`, `status_code` | +| RPC Latency Heatmap | heatmap | `sum(increase(traces_span_metrics_duration_milliseconds_bucket{span_name=~"rpc.command.*"}[5m])) by (le)` | `le` (bucket boundaries) | ### Transaction Overview (`transaction-overview`) -| Panel | Type | PromQL | Labels Used | -| --------------------------------- | ---------- | -------------------------------------------------------------------------------------------- | --------------- | -| Transaction Processing Rate | timeseries | `rate(traces_span_metrics_calls_total{span_name="tx.process"}[5m])` and `tx.receive` | `span_name` | -| Transaction Processing Latency | timeseries | `histogram_quantile(0.95 / 0.50, ... {span_name="tx.process"})` | — | -| Transaction Path Distribution | piechart | `sum by (xrpl_tx_local) (rate(traces_span_metrics_calls_total{span_name="tx.process"}[5m]))` | `xrpl_tx_local` | -| Transaction Receive vs Suppressed | timeseries | `rate(traces_span_metrics_calls_total{span_name="tx.receive"}[5m])` | — | +| Panel | Type | PromQL | Labels Used | +| --------------------------------- | ---------- | ------------------------------------------------------------------------------------ | ----------- | +| Transaction Processing Rate | timeseries | `rate(traces_span_metrics_calls_total{span_name="tx.process"}[5m])` and `tx.receive` | `span_name` | +| Transaction Processing Latency | timeseries | `histogram_quantile(0.95 / 0.50, ... {span_name="tx.process"})` | — | +| Transaction Path Distribution | piechart | `sum by (local) (rate(traces_span_metrics_calls_total{span_name="tx.process"}[5m]))` | `local` | +| Transaction Receive vs Suppressed | timeseries | `rate(traces_span_metrics_calls_total{span_name="tx.receive"}[5m])` | — | ### Consensus Health (`consensus-health`) diff --git a/src/libxrpl/telemetry/TelemetryConfig.cpp b/src/libxrpl/telemetry/TelemetryConfig.cpp index 66de3933b7..102ae3291c 100644 --- a/src/libxrpl/telemetry/TelemetryConfig.cpp +++ b/src/libxrpl/telemetry/TelemetryConfig.cpp @@ -114,6 +114,16 @@ makeTelemetrySetup( "(set both for mutual TLS, or neither for one-way TLS)."); } + // Mutual TLS only takes effect when TLS is on. Certificate paths set with + // use_tls=0 would be silently ignored and the exporter would connect in + // plaintext, so reject that contradiction instead of failing open. + if (!setup.tlsClientCertPath.empty() && !setup.useTls) + { + Throw( + "[telemetry] tls_client_cert/tls_client_key require use_tls=1 " + "(set use_tls=1 to enable mutual TLS, or remove the cert paths)."); + } + // Head sampling is intentionally fixed at 1.0 (sample everything) and is // not read from config. A per-node ratio would let nodes make divergent // keep/drop decisions for the same distributed trace, producing broken