fix(telemetry): address review findings in the workload validation harness

Fixes the review findings on this PR that belong to files it owns, plus
several defects found while verifying those fixes. Findings in files owned
by upstream branches are routed there and left untouched here.

Correctness:
- tx_submitter: advance the account sequence only on results that actually
  consume one (tes*, tec*, terQUEUED). tem*/tef*/tel* never reach the
  ledger, so advancing left a permanent gap that every later submit from
  that account inherited. Add a re-fetch hatch so a repeated non-consuming
  failure cannot livelock on the same sequence, and gate the account check
  on funded-ness rather than list length.
- validate_telemetry: filter spans by name before collecting attributes, so
  a per-span attribute contract can no longer be satisfied by a sibling
  span; require exact name equality for non-wildcard children and glob
  matching for wildcards; bounds-check every returned series instead of
  only the first.
- collect_system_metrics: select xrpld by argv[0] rather than a substring
  match on the whole command line, which averaged in unrelated processes
  and reported their RSS as xrpld's. Count genuine 0.0 CPU readings, use a
  clamped nearest-rank p99 index, and record RPC latency only on success.
- benchmark: return each verdict through a named variable instead of a
  command substitution, so the pass/fail counters survive and the exit gate
  can fire. Scale before dividing in the percentage math, which truncated a
  1.26% impact to 1.00% and cleared a 1% threshold.
- compare_to_baseline: fall back to the absolute bound when the baseline is
  not positive, so a 0 -> 500 ms jump is no longer "within bounds".
- rpc_load_generator: bound each connection to one in-flight recv(), drain
  in-flight requests before closing, use a nearest-rank percentile, and
  report delivery shortfall so an under-delivered run cannot pass with a 0%
  error rate.

Fail loudly instead of silently:
- run-full-validation: treat a consensus timeout and a missing validated
  ledger as fatal infrastructure errors, and fold the orchestrator and
  benchmark exit codes into the final status. A degraded cluster previously
  ran a full validation pass and reported misleading downstream failures.
- collect_system_metrics: warn per empty measurement source, emit
  metrics_complete, and exit non-zero instead of substituting zeros that
  pass every threshold. Require GNU date with %N rather than falling back
  to a per-sample python3 fork that costs more than the threshold it is
  measured against.
- benchmark: distinguish "could not measure" from "exceeded thresholds",
  install a cleanup trap so a failure cannot leak nodes and ports, and
  report an unusable baseline as inconclusive.
- workload_orchestrator: bound subprocess communicate() and fail the exit
  gate on per-phase errors.

Also pins the workload compose images to the versions the sibling stack
already uses, hash-pins the Python dependencies, restricts the validator
config template to loopback, corrects the dashboard and metric counts in
the reference docs, drops a span from the regression gate that cannot fire
under a WebSocket-only workload, and narrows the teardown pkill pattern so
it no longer matches processes that merely mention the work directory.

Verified with a full harness run against a local five-node cluster:
158 of 158 checks passed with no regressions detected.
This commit is contained in:
Pratik Mankawde
2026-08-14 19:59:19 +01:00
parent d3ff79121d
commit d059f21bf3
22 changed files with 2594 additions and 532 deletions

View File

@@ -1023,11 +1023,11 @@ See [Phase10_taskList.md](./Phase10_taskList.md) for detailed per-task breakdown
### Validation Check Inventory
`validate_telemetry.py` derives its check count at run time from
`expected_spans.json` (40 catalogued span types, 27 of them required, 67 unique
required attributes) and `expected_metrics.json` (36 metric entries, 14 dashboard
uids). No fixed total is asserted here, because any manifest edit changes it; the
historical "71 checks" figure predates the Phase 7-9 metric families. The
categories are:
`expected_spans.json` (span types and their required attributes) and
`expected_metrics.json` (metric names and dashboard uids). Per the inventory note
above, no counts are quoted here — those two manifests are the only authority and
any edit to them changes the total. The historical "71 checks" figure predates the
later metric families. The categories are:
- **Service registration** — `xrpld` exists in Tempo
- **Span existence** — every required entry in `expected_spans.json`. Note that

View File

@@ -828,7 +828,9 @@ for how the tier attributes are set and reach metrics.
1. Open Grafana at **http://localhost:3000**
2. Navigate to **Dashboards → xrpld** folder
3. All 15 dashboards are auto-provisioned from `docker/telemetry/grafana/dashboards/`
(the Phase-10 harness asserts 14 of them — `log-derived-insights` is unasserted)
(the workload harness checks that all 15 provision and load; 14 of them also have
metric-data assertions — `log-derived-insights` is Loki-backed, so only its
provisioning is checked)
---
@@ -1175,17 +1177,18 @@ docker/telemetry/workload/benchmark.sh --xrpld .build/xrpld --duration 300
| Call-site `XRPL_METRIC_*` | 7 instruments | Prometheus query | `expected_metrics.json` |
| Per-job-type gauges | 105 (35 types × 3) | Prometheus `__name__` query | `expected_metrics.json` |
| SpanMetrics RED | 4 per span | Prometheus query | `expected_metrics.json` |
| Grafana dashboards | 14 of 15 on disk | Dashboard API "no data" check | `expected_metrics.json` |
| Grafana dashboards | all 15 on disk | Dashboard API load + panel count | `expected_metrics.json` |
| Log-trace links | Present | Loki query + Tempo reverse check | — |
> **These are the harness's numbers, not the code's, and three of them differ.**
> **These are the harness's numbers, not the code's, and two of them differ.**
> `docker/telemetry/workload/expected_spans.json` carries 40 span entries against
> the **41** families the code emits ([§1.1](#11-complete-span-inventory-41-spans)) —
> `rpc.ws_upgrade` has no entry — and 67 distinct required attributes (the
> manifest's own `total_unique_attributes: 58` field is stale).
> `expected_metrics.json` asserts 36 metric entries and 14 dashboard uids against
> the **15** dashboard JSONs in `docker/telemetry/grafana/dashboards/`;
> `log-derived-insights` is the unasserted one. The 35 native instruments match
> `expected_metrics.json` lists all **15** dashboard uids in
> `docker/telemetry/grafana/dashboards/`, so dashboard coverage does not differ;
> `log-derived-insights` is listed for the provisioning check only, and its panel
> data is asserted nowhere. The 35 native instruments match
> the tables in
> [§Phase 9: OTel SDK-Exported Metrics](#phase-9-otel-sdk-exported-metrics-metricsregistry)
> and the Phase 7+ section exactly, counting each labeled gauge family