Skip to content

fix(grafana): disclose source metrics silently dropped from migrated queries - #362

Merged
giorgi-imerlishvili-elastic merged 3 commits into
elastic:mainfrom
giorgi-imerlishvili-elastic:fix/dropped-source-metric-disclosure
Aug 19, 2026
Merged

fix(grafana): disclose source metrics silently dropped from migrated queries#362
giorgi-imerlishvili-elastic merged 3 commits into
elastic:mainfrom
giorgi-imerlishvili-elastic:fix/dropped-source-metric-disclosure

Conversation

@giorgi-imerlishvili-elastic

Copy link
Copy Markdown
Collaborator

Issues fixed

Validity

Both issues are valid. Confirmed empirically against the real "Node Exporter Full" (grafana.com id 1860, revision 37) dashboard: the curated pack's "Pressure" and "CPU" panel overrides genuinely omit two metrics present in the panel's real Grafana targets (node_pressure_irq_stalled_seconds_total, node_cpu_guest_seconds_total) while still reporting status_override: migrated / confidence 1.0.

Root cause

Code paths checked

  • observability_migration/adapters/source/grafana/panels.py: curated-override branch of translate_panel (status/confidence assignment), the multi-target fusion pipeline (fused_series, cross-index layer construction), and the existing "live-missing-metric" / "incompatible-grouping-drop" warning paths (to avoid double-reporting the same gap under two messages).
  • observability_migration/adapters/source/grafana/curated_packs/grafana_1860_node_exporter_full/pack.yaml: the Pressure and CPU panel overrides.
  • observability_migration/core/verification/translation_oracle/structure.py: the STATS_CASE_BARE_TS_MIX structural-oracle rule, relevant to a bug found during review (see below).

Fix

  1. New shared helper _source_metrics_absent_from_query(source_exprs, query_text, resolver) in panels.py: given a panel's original PromQL target expressions and the final emitted query text, flags any source metric (or its resolve_metric_field-resolved candidate name) that never appears in the output. Gated on successful live field-caps discovery (resolver.discovery_status()["status"] == "ok") — without it, a bare substring match against an unverified schema would be unreliable, so the helper no-ops.
  2. Curated path (Curated pack status_override: migrated reports full confidence while the hand-written ES|QL drops source series #349): called right before PanelResult construction. status_override: migrated is now a ceiling — a detected gap downgrades to migrated_with_warnings and caps confidence at 0.6, with a "Target telemetry missing from curated override: <metric>" reason. Excludes hide: true targets (a disabled/legacy fallback query Grafana itself never renders — found via live verification, see below).
  3. General path (General translator drops source panel targets without any warning #352): called after fusion, checking only targets already counted in fused_series (to avoid double-reporting a gap already explained by an existing live-missing-metric or incompatible-grouping warning) against all emitted query layers, including every layer of a cross-index panel (a metric legitimately landing in the second Lens layer must not look "dropped" just because the first layer's query was checked alone — also found via review, see below). Appends "Dropped from migrated query: <metric>".
  4. Fixed the two Node Exporter Full pack gaps issue Curated pack status_override: migrated reports full confidence while the hand-written ES|QL drops source series #349 named directly: added node_pressure_irq_stalled_seconds_total as a 4th "Irq" tile to the Pressure bargauge, and node_cpu_guest_seconds_total as a "Guest" series to the CPU per-mode breakdown.
  5. Documented the ceiling semantics in docs/sources/grafana.md.

Side effects considered

  • False positive: hidden targets. During live verification I found the curated-path check was flagging a hide: true target's metric (node_memory_MemFree_bytes, a legacy MemFree-based formula the real "RAM Used" panel keeps disabled behind a newer MemAvailable-based one). Fixed by excluding hidden targets from the source-metric set; added test_curated_override_ignores_hidden_target_when_checking_dropped_metrics.
  • False positive: cross-index layers. Independent model review found the general-path check only compared against the first Lens layer's query for a cross-index panel (targets spanning distinct ES|QL indexes get split into separate layers/queries). Fixed by checking every layer; strengthened the existing test_cross_index_xy_layers.py test with a regression assertion (confirmed it fails without the fix).
  • Structural bug in the new CPU query. Independent model review also caught that the new guest_lhs aggregation used a bare SUM(IRATE(...)) alongside seven CASE-wrapped sibling assignments in the same STATS, tripping the codebase's own STATS_CASE_BARE_TS_MIX structural-oracle rule (an ERROR-level check created after a real historical incident where Elasticsearch rejected this exact mix — see docs/design/open-problems.md). It executed fine against my local ES version, which is exactly the kind of environment-dependent landmine that rule exists to catch. Fixed by CASE-wrapping it to match its siblings; confirmed the structural oracle now reports zero errors on the fully resolved query.
  • Two other pre-existing curated panels ("Processes Memory", "Hardware temperature monitor") now surface genuine, previously-silent disclosures from this same detector — confirmed these are real textual metric mismatches unrelated to this PR's own changes, just newly visible now that detection is switched on. Left as-is since remediating them is outside what issues Curated pack status_override: migrated reports full confidence while the hand-written ES|QL drops source series #349/General translator drops source panel targets without any warning #352 asked for.
  • Verified the common/clean case is unaffected: a curated override with status_override: migrated and no gap stays migrated/confidence 1.0 with no new warning.

Tests

  • New: test_source_metrics_absent_from_query-style unit tests in tests/test_grafana_extended.py (TestSourceMetricsAbsentFromQuery), test_curated_override_downgrades_when_source_metric_dropped, test_curated_override_status_ceiling_not_downgraded_when_no_gap, test_curated_override_ignores_hidden_target_when_checking_dropped_metrics in tests/test_curated_packs.py, and a strengthened assertion in tests/core/metric_mapping/test_cross_index_xy_layers.py.
  • Full suite: 5744 passed, 3 skipped. make lint and make typecheck clean.

Kibana visual verification

Performed directly (with prior authorization) against a local Kibana/Elasticsearch stack: downloaded the real "Node Exporter Full" (1860, revision 37) dashboard from grafana.com, migrated it on this branch, uploaded the resulting native artifact, and seeded synthetic telemetry matching the new contract via obs-migrate seed-sample-data.

  • Pressure panel now renders 4 tiles (CPU, I/O, Irq, Mem) — confirms the new PSI metric is live.
  • CPU panel's legend now includes "Guest - CPU time spent running a virtual CPU..." as a 9th series alongside System/User/Nice/Iowait/Irq/Softirq/Steal/Idle — confirms the new metric renders and the CASE-wrap fix didn't break the panel.
  • RAM Used renders cleanly with no false warning after the hidden-target fix.

Review

Independent review performed via a subagent on a different model (GPT 5.6 Sol xhigh) per the team's cross-model review workflow. First pass found the two false-positive/structural bugs described above (both fixed and re-verified live); second pass confirmed Ready to merge.

…queries

Two related gaps let a panel report itself as cleanly "migrated" while a
real Grafana target's metric never made it into the emitted ES|QL:

- Curated-pack overrides (issue elastic#349): `status_override: migrated`
  unconditionally set status/confidence, bypassing any check that the
  hand-written query actually covers every metric the panel's real targets
  reference. A pack author could forget a metric and the panel would still
  claim confidence 1.0.
- The general multi-target fusion path (issue elastic#352): no check compared a
  panel's original PromQL target metrics against what actually survived
  into the final fused query, so a target judged "mergeable" could still
  have its metric silently dropped during query construction.

Both paths now share `_source_metrics_absent_from_query`, gated on
successful live field-caps discovery to avoid false positives from an
unverified schema. `status_override` is now a ceiling, not an unconditional
assignment. The curated-path check excludes hidden (`hide: true`) targets
(a disabled/legacy fallback query Grafana itself never renders) and the
general-path check covers every Lens layer of a cross-index panel, not just
the first -- both false-positive traps found via live Kibana verification
and an independent model review before merge.

Also fixes the two Node Exporter Full (1860) curated-pack gaps issue elastic#349
named directly: `node_pressure_irq_stalled_seconds_total` was missing from
the Pressure panel's PSI tiles and `node_cpu_guest_seconds_total` was
missing from the CPU panel's per-mode breakdown. The CPU panel's new Guest
aggregation is CASE-wrapped to match its sibling assignments, avoiding the
STATS_CASE_BARE_TS_MIX structural-oracle class of query that Elasticsearch
can reject.
…ueries

Adding irq/guest to the 1860 pack referenced unknown columns on clusters
without those fields, which made Elasticsearch reject the whole panel.
Treat them as live-optional, strip leftover unpivot EVAL/WHERE aliases,
and stop reporting stripped optional metrics as pack omissions.
… incidental sample data

A same-day commit (4cb7726) reserved the literal axis-label text
"percentage" as an opaque Grafana-unit-id alias whose title is
intentionally suppressed (so unit-inferred titles like "%" can take over
instead) and added a correct, dedicated test for that behavior. It didn't
touch this unrelated, pre-existing test, which happened to reuse the same
literal string purely as incidental sample text for testing something
else entirely (that bar charts keep axis config while omitting
line/area-only appearance keys) -- breaking it on main and therefore on
every open PR whose CI merges against main.

Swap the incidental fixture text for an ordinary, non-reserved label so
the test again exercises its own actual intent without colliding with the
new opaque-alias behavior.
@giorgi-imerlishvili-elastic
giorgi-imerlishvili-elastic merged commit 757ea9e into elastic:main Aug 19, 2026
13 checks passed
giorgi-imerlishvili-elastic added a commit to giorgi-imerlishvili-elastic/observability-migration-platform that referenced this pull request Aug 21, 2026
… verification script (elastic#350)

All 6 dashboard_sha256 pins in curated_packs/registry.yaml were wrong
(didn't match a canonical-JSON hash of the pinned grafana.com revision),
and gnet_id 11835 pinned gnet_revision 4, which has never existed on
grafana.com (that dashboard only has revision 1) -- silently defeating
the provenance check these fields exist for.

- Correct dashboard_sha256 for the 5 packs whose gnet_revision resolves to
  a real download; re-pin 11835 to revision 1 with an explicit callout
  that this can't be independently confirmed to match the pack authors'
  original source, since revision 4 never existed.
- Add scripts/verify_curated_pack_pins.py, a maintainer/CI-only command
  (network required, not part of `make test`) that re-downloads each
  pinned revision and re-checks its canonical hash, mirroring the
  existing fetch_community_corpus.py pattern. Hardened against a
  malformed download response aborting the whole run, and against an
  empty/unparseable registry silently reporting a false "0/0 verified"
  pass.
- Document these fields as maintainer-verified provenance pins, not a
  migration-time gate: a pristine grafana.com download structurally
  differs from any real Grafana-instance export (mutated id/uid/version),
  so comparing against operator dashboards would mismatch on every real
  migration. Rewrote docs/design/curated-dashboard-packs.md's "drift
  detection" section, which described a revision-comparison mechanism and
  two CLI commands that were never actually implemented, to match this.
- Fix a stale duplicate gnet_revision in the 11835 pack's own
  fidelity_manifest.yaml left behind by the re-pin, and add a test
  guarding every pack's manifest against the registry to prevent this
  drift from recurring silently.

Depends on elastic#362 (dropped-source-metric disclosure) for the new
docs/sources/grafana.md wording to describe behavior that actually exists
on main; should land at or after that PR.
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.

2 participants