Skip to content

fix(grafana): route distinct-metric PromQL arithmetic off native PROMQL - #438

Merged
shmsr merged 3 commits into
elastic:mainfrom
giorgi-imerlishvili-elastic:fix/376-elementwise-distinct-metric-promql
Sep 7, 2026
Merged

shmsr merged 3 commits into
elastic:mainfrom
giorgi-imerlishvili-elastic:fix/376-elementwise-distinct-metric-promql

Conversation

@giorgi-imerlishvili-elastic

Copy link
Copy Markdown
Collaborator

Fixes #376.

Is the issue valid?

Yes. Reproduced on the unmodified base at 0a5340ad64b43764d74d8ae3a64fe22a8bb23a56, and confirmed against a live Elasticsearch 9.5.0-SNAPSHOT rather than taken on faith.

Root cause

Elasticsearch's native PROMQL command keeps __name__ in the implicit vector-matching key; Prometheus excludes it when matching binary-operator operands. Two different metric names can therefore never match, so A / B returns zero rows while the source panel shows real values — and the panel was still reported migrated at confidence 0.9 with no warning.

Bisected against the same index:

Expression Result
A / B alone 16 rows each
A / B (distinct names) 0 rows
B / B (same name) 16 rows
sum(A) / sum(B) 8 rows
B * 2 (vector × scalar) 16 rows

Probing further showed the divergence is wider than the report: Elasticsearch also propagates __name__ through rate(), abs(), *_over_time() and scalar arithmetic, where Prometheus drops it. So rate(A[5m]) / rate(B[5m]) is broken too, not just the bare-selector form. Aggregations do drop the name (and topk/bottomk preserve it), which is why sum(A)/sum(B) was unaffected and must stay native.

Code paths checked

The fix

Option 1 from the issue — detect and re-route.

promql_has_unmatchable_distinct_metric_binop walks the AST and resolves the set of metric names each operand's result carries (_ast_result_metric_names). When both sides are determinate, non-scalar and disjoint, the operation can never match, so can_use_native_promql declines it and the panel degrades to the existing per-key ES|QL STATS/EVAL translator, carrying an operator-visible note.

Excluded by design: set operators (and/or/unless), where distinct names are normal and correct; explicit on()/ignoring() matchers and __name__ regex selectors, which Elasticsearch rejects loudly with a 400 and so never fail silently; and anything indeterminate, which suppresses the flag rather than triggering it.

Alerts get the same reroute for a stronger reason — a rule that never fires is quieter than an empty panel. _has_source_faithful_query keeps them automated via ES|QL, but only promises a query when the translator can actually emit one, since it has no ES|QL form for changes(), absent() or predict_linear(). It and _generate_esql_for_alert now share a single feasibility predicate so they cannot disagree.

Side effects considered

  • Panel status changes shape, not just label. Statefulset replicas moves migrated/0.9 → migrated_with_warnings/0.6. That reads as a downgrade but is an upgrade in correctness: the old status described a query returning nothing.
  • Artifact diff is contained. Across 51 panels of dashboard 13332, the normalized native JSON changes by 10 lines in exactly one panel (query, x, y, breakdown_by). Two HPA panels were already on the ES|QL path and only picked up the explanatory note; their queries are byte-identical.
  • Aggregated forms stay native — verified, so no fidelity loss for sum(A)/sum(B).
  • Alert regression caught and fixed. Tightening can_use_native_promql initially made these alerts fall to manual, breaking test_group_interval_reaches_kibana_schedule with KeyError: 'schedule'. That was a real regression, not a stale test.
  • Predicate cost measured, since _has_source_faithful_query is called ~9 times per alert: 0.588 ms vs 0.184 ms baseline for affected alerts, and zero probes for unaffected ones.
  • Probe/real-call parity proven across 4 expressions × 4 resolver configurations (including a match-everything custom rule pack and label rewrites): 0 divergences, now pinned by a test.

Tests

make test 6260 passed, 53 skipped, 497 subtests. make lint (ruff + 502 source headers + skill mirror/structure) and make typecheck (mypy, 10 source files) both clean.

No pre-existing failures on the base. One environment caveat: test_bump_version_script.py fails under a restrictive sandbox with PermissionError creating a .cursor temp directory — an environment artifact, passing normally.

Kibana visual verification

Chrome DevTools MCP against a local 9.5.0-SNAPSHOT stack.

Dashboard obs-migrate-kube-state-metrics-v2 opened in view mode with a cache-bypassing hard reload. The previously-empty Statefulset replicas panel now renders a line at 100% with a prometheus series, matching Grafana.

Because the panel is driven by dashboard variables, a green default-state render alone would not prove filters work, so the namespace control was exercised from monitoring → production: the series switched prometheus → postgres and still rendered at 100%, correlating control selection with the affected query. Console showed only a CSP violation from the MCP's own injected script (an artifact, not an application error), and none after the interaction. All 89 xhr/fetch requests returned 200, including every esql_async call. Neighbouring Cluster tiles held at 1.82% / 12.50% / 3.13%, and the two not_feasible changes() panels kept their expected "Migration Required" state.

Re-verified after the follow-up review round by re-running the migration and diffing the normalized native artifact against the uploaded one: identical, so the saved object was unchanged.

Elasticsearch's native PROMQL command keeps `__name__` in the implicit
vector-matching key, while Prometheus excludes it. Element-wise arithmetic
between two different metric names with no explicit matcher therefore can
never match: `A / B` returns zero rows where the source panel shows real
values. The panel was still reported `migrated` at confidence 0.9 with no
warning, so the dashboard shipped looking clean and rendered empty — the
silent semantic gap the "degrade gracefully" rule exists to prevent.

Probing a live 9.5.0-SNAPSHOT showed the divergence is wider than the bare
selector form in the report: Elasticsearch also propagates `__name__` through
rate(), abs(), *_over_time() and scalar arithmetic, where Prometheus drops it,
so `rate(A[5m]) / rate(B[5m])` is broken too. Aggregations do drop it, which
is why `sum(A)/sum(B)` was unaffected and must stay on the native path.

Detect the shape on the PromQL AST by resolving which metric names each
operand's result carries, and decline the native path when both sides are
determinate, non-scalar and disjoint. Those panels fall through to the
existing per-key ES|QL STATS/EVAL translator, which computes the real ratio,
and carry a note explaining the reroute. This reverses the routing elastic#138/elastic#146
introduced for this subset only: they assumed the native command evaluated the
implicit label-set match the way Prometheus does, and it does not.

Alerts need the same treatment for a stronger reason — a rule that never fires
is quieter than an empty panel — so `_has_source_faithful_query` keeps them
automated through the ES|QL route instead of dropping them to manual. It
promises a query only when the translator can actually emit one: being
steered off the native path is not enough, since the translator has no ES|QL
form for changes(), absent() or predict_linear(). Both it and
`_generate_esql_for_alert` now share one predicate so they cannot disagree.

Explicit on()/ignoring() matchers and `__name__` regex selectors are left
alone: Elasticsearch rejects both loudly with a 400, so they never fail
silently and are already detectable.
The new probe could promise a source-faithful query for control-bound
$var matchers that generation then left empty. Fail that path closed.
Also skip the colocated per-document renderer when live co-occurrence
proves A and B never share a document, so prometheus-rw ratios like
Grafana 763 no longer render empty.
Keep the sparse-document agg(A op B) ES|QL path from this PR and the
agg(A or B) classification rewrite from elastic#436; the two bullets overlap
in docs/sources/grafana.md.
@shmsr
shmsr merged commit 52e748c into elastic:main Sep 7, 2026
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Grafana: element-wise distinct-metric PromQL arithmetic via native PROMQL silently returns zero rows (panel reported clean, renders empty)

2 participants