mirror of
https://github.com/XRPLF/rippled.git
synced 2026-08-28 17:50:55 +00:00
fix(telemetry): read span names from the Tempo name intrinsic
The span reverse-coverage check has never evaluated. It reported "no span
names were reported (backend unreachable or empty)" on a run where Tempo
demonstrably held data -- the same run resolved a logged trace id to 32
spans.
Root cause: the tag-values query asked for `span.name`. A span's name is a
TraceQL intrinsic, not a span-scoped attribute, so `span.name` resolves to
an attribute nothing sets. Tempo answers 200 with an empty tagValues list,
which is indistinguishable from an empty backend and never raises, so the
surrounding try/except stayed silent.
Verified against tempo 2.9.4 holding exactly one span named
probe.reverse.coverage, with the collector in front of it:
/api/v2/search/tag/span.name/values -> {"tagValues":[]}
/api/v2/search/tag/name/values -> that span's name
/api/v2/search/tag/resource.service.name/values -> xrpld
The third line is the control: the span was in Tempo, so the first line's
emptiness was the wrong tag rather than no data. Cross-checked against a
populated Tempo elsewhere, whose span scope lists real attributes
(command, ledger_seq, tx_hash) and no name tag at all, while the bare
intrinsic returns the whole span inventory.
This is pre-existing, not a regression in the reverse check: the same URL
fed the operations diagnostic before that check existed, and the last
green run before it also logged "Tempo operations (0 total)". The check
faithfully reported an empty input; the input was broken.
The neighbouring resource.service.name query is correctly scoped and is
left alone.
This commit is contained in:
@@ -465,11 +465,16 @@ async def validate_spans(
|
||||
# function. Note the tag-values API is not service-scoped, so in a stack
|
||||
# where something other than xrpld also sent traces this list would be a
|
||||
# superset; on the harness only xrpld exports spans.
|
||||
#
|
||||
# The tag is the bare intrinsic `name`, NOT `span.name`. A span's name is a
|
||||
# TraceQL intrinsic, not a span-scoped attribute, so `span.name` resolves to
|
||||
# an attribute nothing sets and Tempo answers 200 with an empty tagValues
|
||||
# list -- indistinguishable from an empty backend, which is how the wrong
|
||||
# tag went unnoticed. Verified against tempo 2.9.4 holding one span:
|
||||
# `span.name` -> {"tagValues":[]}, `name` -> that span's name.
|
||||
emitted_span_names: list[str] = []
|
||||
try:
|
||||
async with session.get(
|
||||
f"{tempo_url}/api/v2/search/tag/span.name/values"
|
||||
) as resp:
|
||||
async with session.get(f"{tempo_url}/api/v2/search/tag/name/values") as resp:
|
||||
ops_data = await resp.json()
|
||||
tag_values = ops_data.get("tagValues", [])
|
||||
emitted_span_names = [tv.get("value", "") for tv in tag_values]
|
||||
|
||||
@@ -3621,14 +3621,14 @@ The counts are not hard-coded in the validator — it iterates the inventory fil
|
||||
so those files are authoritative. The figures below are the inventory as it
|
||||
stands today.
|
||||
|
||||
| Category | Checks | Description |
|
||||
| ---------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
|
||||
| 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 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 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. |
|
||||
| Category | Checks | Description |
|
||||
| ---------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
|
||||
| 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 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 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 `name` intrinsic's 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
|
||||
|
||||
|
||||
Reference in New Issue
Block a user