ARCHIVE/rippled
1
0
Fork 0
mirror of https://github.com/XRPLF/rippled.git synced 2026-10-04 18:58:14 +00:00
Code Issues Packages Projects Releases Wiki Activity

feat(telemetry): gate log-trace correlation in CI with per-leg diagnostics

Browse Source 
The two log-correlation checks have never executed in CI: the workflow
hardcoded --skip-loki, so validate_telemetry.py never constructed
log.trace_id_present or log.trace_id_cross_reference. A green Telemetry
Validation therefore carried no evidence that a log line reaches Loki with
trace context. Drop the flag so both checks run and can fail the job.

Correlation spans four independent legs and a failed check names none of
them, so run-full-validation.sh now prints a per-leg diagnostic after the
suite whenever the checks are enabled:

  node      per-node debug.log line count, the count matching the injected
            trace_id/span_id shape, one sample line, and the severity mix,
            so "no log at all", "log level too high" and "no active sampled
            span" are distinguishable
  mount     the container-side listing of /var/log/xrpld, taken with the
            collector's own mounts and uid. That image is built from
            scratch and carries no shell, so the listing runs in a
            throwaway container with --volumes-from, not via docker exec
  collector the receiver's watched files, logs-pipeline warnings, and the
            internal log-record counters, read from inside the container's
            network namespace because that endpoint binds to the
            container's own localhost and its port is not published
  loki      the exact query used, the label inventory, and entry counts for
            the stream selector with and without the line filter, so "Loki
            has nothing" and "Loki has lines but none carry a trace id" are
            distinguishable

The diagnostics are non-fatal by construction: every leg runs in its own
subshell with errexit off, each docker and curl call is guarded, and the
coordinator always returns success. Verified with no containers and no Loki
reachable, with an emptied PATH, and with a leg forced to exit non-zero.

validate_telemetry.py gains a matching diagnostic beside the checks,
following _log_prometheus_metric_names: warnings only, never a check
result. Its stream selector and line filter move into module constants
that the shell diagnostic reads back, so the two cannot drift into
describing different queries.

No check was widened or auto-passed, and LOG_QUERY_WINDOW_SECONDS stays at
four hours; a wider window would let a check pass on a previous run's logs.
This commit is contained in:
Pratik Mankawde
2026-08-25 18:20:38 +01:00
parent 59a0595a6e
commit c65cb0e2a8
6 changed files with 502 additions and 41 deletions
Download Patch File Download Diff File Expand all files Collapse all files

39
docs/telemetry-runbook.md
View File

@@ -2774,7 +2774,7 @@ The sampled check is normally satisfied on a self-rooted consensus round — hea
With all four satisfied, `info` is the minimum level at which the `log.trace_id_present` and `log.trace_id_cross_reference` checks pass by construction, and it is what the correlation-checking harnesses generate: the cfgs written by [run-full-validation.sh](../docker/telemetry/workload/run-full-validation.sh) and [integration-test.sh](../docker/telemetry/integration-test.sh) each set `enabled=1`, `trace_consensus=1` and `log_level info` together. `benchmark.sh` deliberately does not — it stays at `warning` to keep log I/O out of the overhead measurement, and it runs no correlation check. At `warning` and above that pair is suppressed and correlation becomes incidental — dependent on a `warn`-or-worse line happening to fire inside some active span.
> **CI does not exercise either check.** `log.trace_id_present` and `log.trace_id_cross_reference` never run in CI — see [Coverage gap](#ci-workflow) for why. Cover them locally, without `--skip-loki`, after any change to log formatting, span activation, the `filelog` receiver or the Loki exporter:
> **CI exercises both checks.** `log.trace_id_present` and `log.trace_id_cross_reference` are gated on every CI run — see [CI workflow](#ci-workflow) for the invocation and the per-leg diagnostics printed alongside them. Run the same thing locally after any change to log formatting, span activation, the `filelog` receiver or the Loki exporter:
>
> ```bash
> docker/telemetry/workload/run-full-validation.sh --xrpld .build/xrpld
@@ -3602,7 +3602,7 @@ Harness options (`run-full-validation.sh`):
| `--xrpld PATH` | `.build/xrpld` | Binary to run. Also settable via the `XRPLD` env var. |
| `--nodes NUM` | `5` | Size of the local validator cluster. |
| `--profile NAME` | `full-validation` | Load profile from `workload-profiles.json` (`full-validation`, `quick-smoke`, `stress`). This is the **only** thing that sets load shape. |
| `--skip-loki` | off | Skip the log-trace correlation checks. CI always passes this. |
| `--skip-loki` | off | Skip the log-trace correlation checks and their per-leg diagnostics. Local exploration only; CI does not pass this. |
| `--skip-regression` | off | Skip timing capture and the baseline comparison. Local exploration only. |
| `--with-benchmark` | off | Also run `benchmark.sh` (telemetry-off vs telemetry-on overhead) after validation. |
| `--cleanup` | — | Tear everything down and exit. |
@@ -3625,9 +3625,9 @@ stands today.
| ---------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
| Spans | Every **required** entry in `expected_spans.json` — 41 span types at the time of writing: 25 required, 16 marked `"optional": true` | Span name found in Tempo carrying its `required_attributes`, plus the declared parent-child relationships. An `"optional": true` entry that does not fire is recorded as a skip, not a failure — it needs traffic the harness may not generate (HTTP/JSON-RPC client, gRPC client, missing-ledger fetch, mode transitions) or that it deliberately no longer generates (path-finding RPC — see "Pathfinding is not exercised" in [the workload README](../docker/telemetry/workload/README.md)). |
| Metrics | Every entry in every asserted category of `expected_metrics.json` — 84 checks across 25 asserting categories at the time of writing: 79 metric names plus 5 `required_labels` checks | SpanMetrics, `beast::insight` gauges/counters exported over OTLP, and the `MetricsRegistry` OTLP metrics. Each must have > 0 Prometheus series; none are optional. A category may also declare `required_labels`, and each label there becomes one additional check that at least one of that category's series carries it with a non-empty value (matched as `<label>!=""`, because Prometheus cannot distinguish an absent label from an empty one). Those labels were declared but never actually read until the check was generalised, so they were documented as required while going unverified; `spanmetrics` contributes 4 and `job_queue` 1. The separate `not_asserted` group lists metrics deliberately left out of the gate because they are workload-gated or defect-gated; it has neither a `metrics` nor a `required_labels` key, so the validator skips it entirely. |
| Logs | 2 checks | `trace_id`/`span_id` present in Loki, and a Tempo trace id resolves in Loki. Skipped in CI, which runs `--skip-loki`. |
| Logs | 2 checks | `trace_id`/`span_id` present in Loki, and a logged trace id resolves in Tempo. Gated in CI. `run-full-validation.sh` prints a four-leg diagnostic (node, mount, collector, Loki) after the suite whenever these run, so a failure names the leg that broke. |
| Parity | 10 checks | 6 span attributes the external-parity dashboard panels read, plus 4 metric value-sanity bounds. |
| Dashboards | Every uid in `expected_metrics.json` under `grafana_dashboards.uids` — currently all 15 provisioned dashboards | Each listed dashboard loads and reports a panel count. This is a provisioning check only: it does **not** execute the panels' queries, so a dashboard can pass while individual panels render empty. `log-derived-insights` is Loki-backed, so under `--skip-loki` only its provisioning is meaningfully covered. |
| Dashboards | Every uid in `expected_metrics.json` under `grafana_dashboards.uids` — currently all 15 provisioned dashboards | Each listed dashboard loads and reports a panel count. This is a provisioning check only: it does **not** execute the panels' queries, so a dashboard can pass while individual panels render empty. `log-derived-insights` is Loki-backed, so only its provisioning is covered here; its data path is covered by the two log checks instead. |
| Reverse coverage | 2 checks — `metric.reverse_coverage` and `span.reverse_coverage` | The only checks that run in the opposite direction: they read the full emitted inventory (the Prometheus `__name__` label values, the Tempo `span.name` tag values) and name everything the contract never mentions, sorted and one per line in the log. **Warn only — `passed` is hardcoded `True` in `_reverse_coverage_result`, so these can never fail CI.** Downstream branches legitimately add telemetry an upstream contract has not seen, and a hard failure would redden all of them. A metric family is accounted for by a `metrics` entry, by a `not_asserted.metrics_excluded` key, or by an anchored regex under the top-level `accounted_patterns` list — which exists for families whose membership is derived mechanically from a table in the code (the per-job-type job-queue instruments, the overlay per-category traffic cross product) plus the Prometheus scrape plumbing that is not xrpld telemetry. Histogram `_bucket`/`_count`/`_sum` names fold onto their base family before matching. Spans need no pattern list: the check reuses the forward matcher, so `rpc.command.*` covers every command it expands to. |
### Running Individual Tools
@@ -3808,20 +3808,23 @@ container as the main CI, so Conan and ccache hit the shared caches), and
the workflow file, `docker/telemetry/**`, and the telemetry sources under
`include/xrpl/telemetry/**`, `src/libxrpl/telemetry/**` and
`src/xrpld/telemetry/**`. There is no cron schedule.
- **Invocation**: `run-full-validation.sh --xrpld <binary> --skip-loki`, so the
default `full-validation` profile is used and the Loki checks are skipped.
- **Coverage gap — log-trace correlation is never checked in CI**: the
`--skip-loki` above is hardcoded at
[telemetry-validation.yml:237](../.github/workflows/telemetry-validation.yml#L237),
and `validate_telemetry.py` creates `log.trace_id_present` and
`log.trace_id_cross_reference` only inside an `if not skip_loki` branch
([:1657](../docker/telemetry/workload/validate_telemetry.py#L1657)), so those
two checks are not merely skipped — they are never constructed and never appear
in the report. `docker/telemetry/integration-test.sh` (which has its own
`check_log_correlation()`) is run by no workflow at all. A green
`Telemetry Validation` therefore carries no evidence about correlation; run it
locally without the flag to cover it:
`docker/telemetry/workload/run-full-validation.sh --xrpld .build/xrpld`.
- **Invocation**: `run-full-validation.sh --xrpld <binary>`, so the default
`full-validation` profile is used and no category is skipped.
- **Log-trace correlation is gated**: `--skip-loki` is not passed
([telemetry-validation.yml:237](../.github/workflows/telemetry-validation.yml#L237)),
so `log.trace_id_present` and `log.trace_id_cross_reference` are constructed and
can fail the job. Correlation spans four independent legs — node, mount,
collector, Loki — and a failed check names none of them, so
`run-full-validation.sh` prints a per-leg diagnostic after the suite whenever
these checks are enabled: per-node counts of `debug.log` lines carrying the
injected `trace_id`/`span_id` shape plus the severity mix, the container-side
listing of `/var/log/xrpld` taken with the collector's own mounts and uid, the
`filelog` receiver's watched files, logs-pipeline warnings and internal
log-record counters, and Loki's entry counts for the stream selector with and
without the line filter. The diagnostics are non-fatal by construction: each
leg is isolated and a missing container or unreachable endpoint prints a note.
`docker/telemetry/integration-test.sh` (which has its own
`check_log_correlation()`) is still run by no workflow.
- **Inputs**: only `run_benchmark` changes behaviour. `rpc_rate`, `rpc_duration`,
`tx_tps` and `tx_duration` are inert, as noted in their descriptions.
- **Results**: reports are uploaded as the `telemetry-validation-reports`