From 9202a79f8d83d327922579f57010f77d9c21ef51 Mon Sep 17 00:00:00 2001 From: Giorgi Imerlishvili Date: Thu, 13 Aug 2026 03:59:41 +0400 Subject: [PATCH 1/3] fix(grafana): disclose source metrics silently dropped from migrated 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 #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 #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 #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. --- docs/sources/grafana.md | 10 +- .../grafana_1860_node_exporter_full/pack.yaml | 23 +- .../adapters/source/grafana/panels.py | 105 +++++++++ .../test_cross_index_xy_layers.py | 10 + tests/test_curated_packs.py | 207 ++++++++++++++++++ tests/test_grafana_extended.py | 143 ++++++++++++ 6 files changed, 487 insertions(+), 11 deletions(-) diff --git a/docs/sources/grafana.md b/docs/sources/grafana.md index eec3d27e..7f352725 100644 --- a/docs/sources/grafana.md +++ b/docs/sources/grafana.md @@ -111,7 +111,15 @@ behavior, and final Kibana geometry. Query fixes land through `panel.query_overrides`; scope-only or misleading variable controls can be suppressed or rewritten through curated-pack plugins; layout fixes land through `panel.layout_overrides` after the standard Kibana layout transform and before -final overlap cleanup. +final overlap cleanup. `query_overrides.status_override` is a ceiling, not an +unconditional assignment: if the panel's own targets reference a source metric +the hand-written override never emits, the panel downgrades to +`migrated_with_warnings` (confidence capped at `0.6`) with an explicit "Target +telemetry missing from curated override" reason, the same disclosure the +non-pack path uses for a metric that never made it into an otherwise-migrated +fused query ("Dropped from migrated query"). Both checks require live +field-caps discovery (`--es-url`) to resolve the metric's actual field name; +without it they no-op rather than guess. The console pipeline is **5 stages**, not 7: `[1/5] Extracting dashboards`, `[2/5] Translating dashboards`, `[3/5] Verification-packet ES|QL validation`, diff --git a/observability_migration/adapters/source/grafana/curated_packs/grafana_1860_node_exporter_full/pack.yaml b/observability_migration/adapters/source/grafana/curated_packs/grafana_1860_node_exporter_full/pack.yaml index 07887d4d..a036df5d 100644 --- a/observability_migration/adapters/source/grafana/curated_packs/grafana_1860_node_exporter_full/pack.yaml +++ b/observability_migration/adapters/source/grafana/curated_packs/grafana_1860_node_exporter_full/pack.yaml @@ -98,7 +98,7 @@ panel: | EVAL _gauge_min = 0, _gauge_max = 100, _gauge_goal = 85 status_override: migrated - title_match: "Pressure" - # Compact first-row summary: unpivot the three PSI rates to + # Compact first-row summary: unpivot the four PSI rates to # label/gauge_value rows and render as Lens metric tiles stacked in # one column (panel is only w=6). Use TBUCKET(20) so short windows # (15m) still have enough samples per bucket for IRATE. @@ -109,14 +109,14 @@ panel: TS metrics-* | WHERE (?node == "" OR ({{label:instance}} RLIKE ?node OR ({{label:instance}} IS NULL AND "" RLIKE ?node))) | WHERE (?job == "" OR ({{label:job}} RLIKE ?job OR ({{label:job}} IS NULL AND "" RLIKE ?job))) - | WHERE {{metric:node_pressure_cpu_waiting_seconds_total:counter}} IS NOT NULL OR {{metric:node_pressure_memory_waiting_seconds_total:counter}} IS NOT NULL OR {{metric:node_pressure_io_waiting_seconds_total:counter}} IS NOT NULL - | STATS CPU = MAX(IRATE({{metric:node_pressure_cpu_waiting_seconds_total:counter}})), Mem = MAX(IRATE({{metric:node_pressure_memory_waiting_seconds_total:counter}})), IO = MAX(IRATE({{metric:node_pressure_io_waiting_seconds_total:counter}})) BY time_bucket = TBUCKET(20, ?_tstart, ?_tend) - | WHERE CPU IS NOT NULL OR Mem IS NOT NULL OR IO IS NOT NULL + | WHERE {{metric:node_pressure_cpu_waiting_seconds_total:counter}} IS NOT NULL OR {{metric:node_pressure_memory_waiting_seconds_total:counter}} IS NOT NULL OR {{metric:node_pressure_io_waiting_seconds_total:counter}} IS NOT NULL OR {{metric:node_pressure_irq_stalled_seconds_total:counter}} IS NOT NULL + | STATS CPU = MAX(IRATE({{metric:node_pressure_cpu_waiting_seconds_total:counter}})), Mem = MAX(IRATE({{metric:node_pressure_memory_waiting_seconds_total:counter}})), IO = MAX(IRATE({{metric:node_pressure_io_waiting_seconds_total:counter}})), Irq = MAX(IRATE({{metric:node_pressure_irq_stalled_seconds_total:counter}})) BY time_bucket = TBUCKET(20, ?_tstart, ?_tend) + | WHERE CPU IS NOT NULL OR Mem IS NOT NULL OR IO IS NOT NULL OR Irq IS NOT NULL | SORT time_bucket DESC | LIMIT 2 | SORT time_bucket ASC | LIMIT 1 - | EVAL __labels = MV_APPEND(MV_APPEND("CPU", "I/O"), "Mem"), __values = MV_APPEND(MV_APPEND(COALESCE(TO_STRING(CPU), ""), COALESCE(TO_STRING(IO), "")), COALESCE(TO_STRING(Mem), "")) + | EVAL __labels = MV_APPEND(MV_APPEND(MV_APPEND("CPU", "I/O"), "Mem"), "Irq"), __values = MV_APPEND(MV_APPEND(MV_APPEND(COALESCE(TO_STRING(CPU), ""), COALESCE(TO_STRING(IO), "")), COALESCE(TO_STRING(Mem), "")), COALESCE(TO_STRING(Irq), "")) | EVAL __pairs = MV_ZIP(__labels, __values, "\t") | MV_EXPAND __pairs | EVAL label = MV_FIRST(SPLIT(__pairs, "\t")), gauge_value = TO_DOUBLE(MV_LAST(SPLIT(__pairs, "\t"))) * 100 @@ -232,19 +232,22 @@ panel: - title_match: "CPU" # TBUCKET(20) keeps ~45s buckets on a 15m range so IRATE has enough # samples; TBUCKET(100) (~9s) gaps the newest points and is heavy. + # node_cpu_guest_seconds_total is a distinct exporter metric (not a + # `mode` value of node_cpu_seconds_total), so it needs its own OR arm + # in both the presence WHERE and the per-cpu rhs CASE guard. esql_query: | TS metrics-* | WHERE (?node == "" OR ({{label:instance}} RLIKE ?node OR ({{label:instance}} IS NULL AND "" RLIKE ?node))) | WHERE (?job == "" OR ({{label:job}} RLIKE ?job OR ({{label:job}} IS NULL AND "" RLIKE ?job))) - | WHERE {{metric:node_cpu_seconds_total:counter}} IS NOT NULL - | STATS system_lhs = SUM(CASE((labels.mode == "system"), IRATE({{metric:node_cpu_seconds_total:counter}}), NULL)), system_rhs = COUNT_DISTINCT(labels.cpu), user_lhs = SUM(CASE((labels.mode == "user"), IRATE({{metric:node_cpu_seconds_total:counter}}), NULL)), user_rhs = COUNT_DISTINCT(labels.cpu), nice_lhs = SUM(CASE((labels.mode == "nice"), IRATE({{metric:node_cpu_seconds_total:counter}}), NULL)), nice_rhs = COUNT_DISTINCT(labels.cpu), iowait_lhs = SUM(CASE((labels.mode == "iowait"), IRATE({{metric:node_cpu_seconds_total:counter}}), NULL)), iowait_rhs = COUNT_DISTINCT(labels.cpu), irq_lhs = SUM(CASE((labels.mode == "irq"), IRATE({{metric:node_cpu_seconds_total:counter}}), NULL)), irq_rhs = COUNT_DISTINCT(labels.cpu), softirq_lhs = SUM(CASE((labels.mode == "softirq"), IRATE({{metric:node_cpu_seconds_total:counter}}), NULL)), softirq_rhs = COUNT_DISTINCT(labels.cpu), steal_lhs = SUM(CASE((labels.mode == "steal"), IRATE({{metric:node_cpu_seconds_total:counter}}), NULL)), steal_rhs = COUNT_DISTINCT(labels.cpu), idle_lhs = SUM(CASE((labels.mode == "idle"), IRATE({{metric:node_cpu_seconds_total:counter}}), NULL)), idle_rhs = COUNT_DISTINCT(labels.cpu) BY time_bucket = TBUCKET(20, ?_tstart, ?_tend) - | EVAL System = (system_lhs / system_rhs), User = (user_lhs / user_rhs), Nice = (nice_lhs / nice_rhs), Iowait = (iowait_lhs / iowait_rhs), Irq = (irq_lhs / irq_rhs), Softirq = (softirq_lhs / softirq_rhs), Steal = (steal_lhs / steal_rhs), Idle = (idle_lhs / idle_rhs) + | WHERE {{metric:node_cpu_seconds_total:counter}} IS NOT NULL OR {{metric:node_cpu_guest_seconds_total:counter}} IS NOT NULL + | STATS system_lhs = SUM(CASE((labels.mode == "system"), IRATE({{metric:node_cpu_seconds_total:counter}}), NULL)), system_rhs = COUNT_DISTINCT(labels.cpu), user_lhs = SUM(CASE((labels.mode == "user"), IRATE({{metric:node_cpu_seconds_total:counter}}), NULL)), user_rhs = COUNT_DISTINCT(labels.cpu), nice_lhs = SUM(CASE((labels.mode == "nice"), IRATE({{metric:node_cpu_seconds_total:counter}}), NULL)), nice_rhs = COUNT_DISTINCT(labels.cpu), iowait_lhs = SUM(CASE((labels.mode == "iowait"), IRATE({{metric:node_cpu_seconds_total:counter}}), NULL)), iowait_rhs = COUNT_DISTINCT(labels.cpu), irq_lhs = SUM(CASE((labels.mode == "irq"), IRATE({{metric:node_cpu_seconds_total:counter}}), NULL)), irq_rhs = COUNT_DISTINCT(labels.cpu), softirq_lhs = SUM(CASE((labels.mode == "softirq"), IRATE({{metric:node_cpu_seconds_total:counter}}), NULL)), softirq_rhs = COUNT_DISTINCT(labels.cpu), steal_lhs = SUM(CASE((labels.mode == "steal"), IRATE({{metric:node_cpu_seconds_total:counter}}), NULL)), steal_rhs = COUNT_DISTINCT(labels.cpu), idle_lhs = SUM(CASE((labels.mode == "idle"), IRATE({{metric:node_cpu_seconds_total:counter}}), NULL)), idle_rhs = COUNT_DISTINCT(labels.cpu), guest_lhs = SUM(CASE(true, IRATE({{metric:node_cpu_guest_seconds_total:counter}}), NULL)), guest_rhs = COUNT_DISTINCT(CASE({{metric:node_cpu_guest_seconds_total:counter}} IS NOT NULL, labels.cpu, NULL)) BY time_bucket = TBUCKET(20, ?_tstart, ?_tend) + | EVAL System = (system_lhs / system_rhs), User = (user_lhs / user_rhs), Nice = (nice_lhs / nice_rhs), Iowait = (iowait_lhs / iowait_rhs), Irq = (irq_lhs / irq_rhs), Softirq = (softirq_lhs / softirq_rhs), Steal = (steal_lhs / steal_rhs), Idle = (idle_lhs / idle_rhs), Guest = (guest_lhs / guest_rhs) | EVAL __labels_ab = MV_APPEND("System - Processes executing in kernel mode", "User - Normal processes executing in user mode"), __labels_cd = MV_APPEND("Nice - Niced processes executing in user mode", "Iowait - Waiting for I/O to complete") | EVAL __labels_ef = MV_APPEND("Irq - Servicing interrupts", "Softirq - Servicing softirqs"), __labels_gh = MV_APPEND("Steal - Time spent in other operating systems when running in a virtualized environment", "Idle - Waiting for something to happen") - | EVAL __labels = MV_APPEND(MV_APPEND(__labels_ab, __labels_cd), MV_APPEND(__labels_ef, __labels_gh)) + | EVAL __labels = MV_APPEND(MV_APPEND(MV_APPEND(__labels_ab, __labels_cd), MV_APPEND(__labels_ef, __labels_gh)), "Guest - CPU time spent running a virtual CPU for guest operating systems under the control of the Linux kernel") | EVAL __values_ab = MV_APPEND(COALESCE(TO_STRING(System), ""), COALESCE(TO_STRING(User), "")), __values_cd = MV_APPEND(COALESCE(TO_STRING(Nice), ""), COALESCE(TO_STRING(Iowait), "")) | EVAL __values_ef = MV_APPEND(COALESCE(TO_STRING(Irq), ""), COALESCE(TO_STRING(Softirq), "")), __values_gh = MV_APPEND(COALESCE(TO_STRING(Steal), ""), COALESCE(TO_STRING(Idle), "")) - | EVAL __values = MV_APPEND(MV_APPEND(__values_ab, __values_cd), MV_APPEND(__values_ef, __values_gh)) + | EVAL __values = MV_APPEND(MV_APPEND(MV_APPEND(__values_ab, __values_cd), MV_APPEND(__values_ef, __values_gh)), COALESCE(TO_STRING(Guest), "")) | EVAL __pairs = MV_ZIP(__labels, __values, "~") | MV_EXPAND __pairs | EVAL series_group = MV_FIRST(SPLIT(__pairs, "~")), value = TO_DOUBLE(MV_LAST(SPLIT(__pairs, "~"))) diff --git a/observability_migration/adapters/source/grafana/panels.py b/observability_migration/adapters/source/grafana/panels.py index fe8aa6dd..05bb6456 100644 --- a/observability_migration/adapters/source/grafana/panels.py +++ b/observability_migration/adapters/source/grafana/panels.py @@ -3807,6 +3807,32 @@ def translate_panel(panel, datasource_index="metrics-*", esql_index=None, rule_p # uploaded without ``metric.max``, so Kibana auto-fit # the dial to ~0-2% instead of the Grafana 0-100 domain. _emitted_query = _native_panel.get("query", _curated_query) + # A curated override is hand-written and can omit a + # source metric the pack author never accounted for + # (issue #349). ``status_override`` must act as a + # ceiling on status/confidence, not an unconditional + # assignment, so a detected gap still surfaces -- the + # same discipline the general (non-pack) path applies + # for "Target telemetry missing" (issue #352). + _source_target_exprs = [ + str(_t.get("expr") or "") + for _t in panel.get("targets", []) or [] + if isinstance(_t, dict) + and _t.get("expr") + and not _t.get("hide") + ] + _dropped_curated_metrics = _source_metrics_absent_from_query( + _source_target_exprs, _emitted_query, resolver + ) + if _dropped_curated_metrics: + _append_unique( + _override_warnings, + "Target telemetry missing from curated override: " + + ", ".join(_dropped_curated_metrics), + ) + if _status == "migrated": + _status = "migrated_with_warnings" + _score = min(_score, 0.6) _panel_result = PanelResult( title, panel_type, _override_type, _status, _score, reasons=_override_warnings, @@ -4412,6 +4438,40 @@ def translate_panel(panel, datasource_index="metrics-*", esql_index=None, rule_p rule_pack, ): _append_unique(primary.warnings, recording_rule_note) + # Only check targets that were actually counted as migrated + # (``fused_series``): targets dropped for a live-missing metric or an + # incompatible grouping are already explained by the warnings above, so + # re-checking them here would double-report the same gap under a + # different reason (issue #352). This instead catches a target that WAS + # judged mergeable yet whose metric silently never made it into the + # final STATS/EVAL -- available but dropped by the translator, not a + # target-schema gap. + _migrated_target_exprs = [ + str(_series.metadata.get("target_source_expr") or _series.promql_expr or "") + for _series in (fused_series or [primary]) + ] + # A cross-index panel (issue #352 regression risk) splits fused targets + # across multiple Lens layers, each with its own ES|QL query + # (``primary.metadata["cross_index_layers"]``); ``primary.esql_query`` is + # only the first layer's query. Checking against that alone would falsely + # flag every target whose metric only appears in a later layer. + _cross_index_layers = primary.metadata.get("cross_index_layers") or [] + _all_layer_queries = "\n".join( + [primary.esql_query or ""] + + [ + str(_layer.get("query") or "") + for _layer in _cross_index_layers + if isinstance(_layer, dict) + ] + ) + _dropped_source_metrics = _source_metrics_absent_from_query( + _migrated_target_exprs, _all_layer_queries, resolver + ) + if _dropped_source_metrics: + _append_unique( + primary.warnings, + "Dropped from migrated query: " + ", ".join(_dropped_source_metrics), + ) panel_confidence = 0.85 if not primary.warnings else 0.6 status = "migrated" if not primary.warnings else "migrated_with_warnings" @@ -5666,6 +5726,51 @@ def _live_missing_metrics_for_expr(expr, resolver): return missing +def _source_metrics_absent_from_query(source_exprs, query_text, resolver): + """Prometheus metrics referenced by *source_exprs* that never appear in the + final emitted *query_text*. + + Complements ``_live_missing_metrics_for_expr``, which flags a metric that + is absent from the *target's schema* (a data gap). This instead flags a + metric the translator itself dropped while building the final query, even + though the metric is queryable -- e.g. a target folded into a multi-target + fusion whose column never made it into the emitted STATS/EVAL (issue + #352), or a curated ``query_overrides`` entry that omits a source metric + the pack author never accounted for (issue #349). Callers are responsible + for only passing exprs/metrics not already explained by another check + (live-missing metrics, incompatible-target drops) to avoid double + reporting the same gap under two different reasons. + + Requires live field-caps discovery to have actually run (same gate as + ``_live_missing_metrics_for_expr``): without a real target schema to + resolve field names against, a bare metric-name substring match against + the emitted query text is unreliable and would false-positive on curated + overrides/tests that legitimately rename or synthesize fields. + """ + if not resolver: + return [] + discovery_status = getattr(resolver, "discovery_status", lambda: {})() + if discovery_status.get("status") != "ok": + return [] + source_metrics: set[str] = set() + for expr in source_exprs or []: + source_metrics |= _metrics_in_expr(str(expr or "")) + if not source_metrics or not query_text: + return [] + resolve_metric = getattr(resolver, "resolve_metric_field", None) + missing: list[str] = [] + for metric in sorted(source_metrics): + candidates = {metric} + if callable(resolve_metric): + for prefer in ("gauge", "counter"): + resolved = resolve_metric(metric, prefer=prefer) + if resolved: + candidates.add(resolved) + if not any(candidate and candidate in query_text for candidate in candidates): + _append_unique(missing, metric) + return missing + + def _make_missing_telemetry_panel(yaml_panel, title, panel_type, missing_metrics): metrics_text = ", ".join(sorted(dict.fromkeys(missing_metrics))) yaml_panel["markdown"] = { diff --git a/tests/core/metric_mapping/test_cross_index_xy_layers.py b/tests/core/metric_mapping/test_cross_index_xy_layers.py index a65a60f4..0ef0196b 100644 --- a/tests/core/metric_mapping/test_cross_index_xy_layers.py +++ b/tests/core/metric_mapping/test_cross_index_xy_layers.py @@ -73,6 +73,16 @@ def test_mixed_target_index_emits_cross_index_layers(self) -> None: any("distinct data streams" in str(reason) for reason in (result.reasons or [])), result.reasons, ) + # Regression guard (issue #352's dropped-metric detector): both + # targets' metrics genuinely survive, just split across two Lens + # layers with two separate ES|QL queries. The detector must check + # every layer's query, not only the first -- otherwise whichever + # metric lands in the second layer looks "dropped" even though it's + # right there in ``esql.layers``. + self.assertFalse( + any("Dropped from migrated query" in str(reason) for reason in (result.reasons or [])), + result.reasons, + ) esql = yaml_panel.get("esql") or {} self.assertEqual(esql.get("type"), "line") layers = esql.get("layers") or [] diff --git a/tests/test_curated_packs.py b/tests/test_curated_packs.py index a5bb31e5..6b719fd5 100644 --- a/tests/test_curated_packs.py +++ b/tests/test_curated_packs.py @@ -232,6 +232,213 @@ def test_curated_rate_overrides_do_not_use_sub_scrape_adaptive_tbucket_100(): assert offenders == [] +# --------------------------------------------------------------------------- +# Curated override dropped-source-metric disclosure (issue #349) +# --------------------------------------------------------------------------- + +def test_curated_override_downgrades_when_source_metric_dropped(): + """``status_override: migrated`` must act as a ceiling, not an + unconditional assignment: if the panel's own targets reference a metric + the hand-written override never emits, the panel must downgrade to + ``migrated_with_warnings`` and name the dropped metric, matching the + tool's own behavior for non-pack panels ("Target telemetry missing").""" + rule_pack = RulePackConfig( + panel_query_overrides=[ + { + "title_match": "Two Series", + "esql_query": ( + "TS metrics-*\n" + "| WHERE {{metric:foo_total:counter}} IS NOT NULL\n" + "| STATS value = MAX(LAST_OVER_TIME({{metric:foo_total:counter}}))\n" + "| KEEP value" + ), + "status_override": "migrated", + } + ] + ) + resolver = SchemaResolver(rule_pack) + resolver._field_cache = { + "foo_total": {"double": {"type": "double"}}, + "bar_total": {"double": {"type": "double"}}, + } + resolver._discovery_attempted = True + resolver._discovery_status = "ok" + + panel = { + "type": "gauge", + "title": "Two Series", + "targets": [ + {"expr": "foo_total", "refId": "A"}, + {"expr": "bar_total", "refId": "B"}, + ], + } + + _yaml_panel, result = translate_panel(panel, rule_pack=rule_pack, resolver=resolver) + + assert result.status == "migrated_with_warnings", result.reasons + assert result.confidence <= 0.6 + assert any( + "bar_total" in reason and "curated override" in reason + for reason in result.reasons + ), result.reasons + + +def test_curated_override_status_ceiling_not_downgraded_when_no_gap(): + """Sanity companion: when the override legitimately covers every source + metric, ``status_override: migrated`` must NOT be downgraded.""" + rule_pack = RulePackConfig( + panel_query_overrides=[ + { + "title_match": "One Series", + "esql_query": ( + "TS metrics-*\n" + "| WHERE {{metric:foo_total:counter}} IS NOT NULL\n" + "| STATS value = MAX(LAST_OVER_TIME({{metric:foo_total:counter}}))\n" + "| KEEP value" + ), + "status_override": "migrated", + } + ] + ) + resolver = SchemaResolver(rule_pack) + resolver._field_cache = {"foo_total": {"double": {"type": "double"}}} + resolver._discovery_attempted = True + resolver._discovery_status = "ok" + + panel = { + "type": "gauge", + "title": "One Series", + "targets": [{"expr": "foo_total", "refId": "A"}], + } + + _yaml_panel, result = translate_panel(panel, rule_pack=rule_pack, resolver=resolver) + + assert result.status == "migrated" + assert result.confidence == 1.0 + assert result.reasons == [] + + +def test_curated_override_ignores_hidden_target_when_checking_dropped_metrics(): + """A ``hide: true`` target is a disabled/legacy alternate query Grafana + itself never renders (e.g. Node Exporter Full's real "RAM Used" panel + keeps an old MemFree-based formula hidden behind a visible + MemAvailable-based one for older node_exporter compatibility). The + dropped-metric check must only compare against targets a user actually + sees, or every such compatibility fallback falsely downgrades an + otherwise-clean curated override.""" + rule_pack = RulePackConfig( + panel_query_overrides=[ + { + "title_match": "RAM Used", + "esql_query": ( + "TS metrics-*\n" + "| WHERE {{metric:mem_available:gauge}} IS NOT NULL\n" + "| STATS value = MAX(LAST_OVER_TIME({{metric:mem_available:gauge}}))\n" + "| KEEP value" + ), + "status_override": "migrated", + } + ] + ) + resolver = SchemaResolver(rule_pack) + resolver._field_cache = { + "mem_free": {"double": {"type": "double"}}, + "mem_available": {"double": {"type": "double"}}, + } + resolver._discovery_attempted = True + resolver._discovery_status = "ok" + + panel = { + "type": "gauge", + "title": "RAM Used", + "targets": [ + {"expr": "mem_free", "refId": "A", "hide": True}, + {"expr": "mem_available", "refId": "B"}, + ], + } + + _yaml_panel, result = translate_panel(panel, rule_pack=rule_pack, resolver=resolver) + + assert result.status == "migrated" + assert result.confidence == 1.0 + assert result.reasons == [] + + +def test_1860_pressure_panel_includes_irq_series(): + """node_pressure_irq_stalled_seconds_total (issue #349) must be part of + the curated Pressure override, not silently dropped.""" + dashboard = {"gnetId": 1860, "title": "Node Exporter Full", "tags": ["prometheus"]} + resolved = resolve_pack_for_dashboard(dashboard, RulePackConfig()) + resolver = SchemaResolver(resolved) + resolver._field_cache = { + "node_pressure_cpu_waiting_seconds_total": {"double": {"type": "double"}}, + "node_pressure_memory_waiting_seconds_total": {"double": {"type": "double"}}, + "node_pressure_io_waiting_seconds_total": {"double": {"type": "double"}}, + "node_pressure_irq_stalled_seconds_total": {"double": {"type": "double"}}, + "instance": {"keyword": {"type": "keyword"}}, + "job": {"keyword": {"type": "keyword"}}, + } + resolver._discovery_attempted = True + resolver._discovery_status = "ok" + + panel = { + "type": "bargauge", + "title": "Pressure", + "targets": [ + {"expr": "irate(node_pressure_cpu_waiting_seconds_total[$__rate_interval])", "refId": "A"}, + {"expr": "irate(node_pressure_memory_waiting_seconds_total[$__rate_interval])", "refId": "B"}, + {"expr": "irate(node_pressure_io_waiting_seconds_total[$__rate_interval])", "refId": "C"}, + {"expr": "irate(node_pressure_irq_stalled_seconds_total[$__rate_interval])", "refId": "D"}, + ], + } + + yaml_panel, result = translate_panel(panel, rule_pack=resolved, resolver=resolver) + + assert result.status == "migrated", f"got {result.status}: {result.reasons}" + query = (yaml_panel or {}).get("esql", {}).get("query", "") + assert "node_pressure_irq_stalled_seconds_total" in query + assert '"Irq"' in query + + +def test_1860_cpu_panel_includes_guest_series(): + """node_cpu_guest_seconds_total (issue #349) must be part of the curated + CPU override, not silently dropped.""" + dashboard = {"gnetId": 1860, "title": "Node Exporter Full", "tags": ["prometheus"]} + resolved = resolve_pack_for_dashboard(dashboard, RulePackConfig()) + resolver = SchemaResolver(resolved) + resolver._field_cache = { + "node_cpu_seconds_total": {"double": {"type": "double"}}, + "node_cpu_guest_seconds_total": {"double": {"type": "double"}}, + "instance": {"keyword": {"type": "keyword"}}, + "job": {"keyword": {"type": "keyword"}}, + "cpu": {"keyword": {"type": "keyword"}}, + "mode": {"keyword": {"type": "keyword"}}, + } + resolver._discovery_attempted = True + resolver._discovery_status = "ok" + + targets = [ + { + "expr": f'avg(irate(node_cpu_seconds_total{{mode="{mode}"}}[$__rate_interval])) by (mode) * 100', + "refId": chr(65 + i), + } + for i, mode in enumerate( + ["system", "user", "nice", "iowait", "irq", "softirq", "steal", "idle"] + ) + ] + targets.append( + {"expr": "avg(irate(node_cpu_guest_seconds_total[$__rate_interval])) * 100", "refId": "I"} + ) + panel = {"type": "timeseries", "title": "CPU", "targets": targets} + + yaml_panel, result = translate_panel(panel, rule_pack=resolved, resolver=resolver) + + assert result.status == "migrated", f"got {result.status}: {result.reasons}" + query = (yaml_panel or {}).get("esql", {}).get("query", "") + assert "node_cpu_guest_seconds_total" in query + assert "Guest -" in query + + def test_1860_interrupts_detail_uses_interrupt_cpu_legend(): """Interrupts Detail must legend by interrupt/cpu, not empty type/info GROK.""" dashboard = {"gnetId": 1860, "title": "Node Exporter Full", "tags": ["prometheus"]} diff --git a/tests/test_grafana_extended.py b/tests/test_grafana_extended.py index b8835d1f..93327066 100644 --- a/tests/test_grafana_extended.py +++ b/tests/test_grafana_extended.py @@ -20,6 +20,7 @@ import sys import time import unittest +from unittest.mock import patch import yaml @@ -3331,6 +3332,148 @@ def test_incompatible_targets_warn(self): for r in result.reasons) self.assertTrue(has_drop, f"Should warn about dropped targets: {result.reasons}") + def test_different_metrics_merged_with_live_resolver_no_false_positive(self): + """Regression guard for issue #352's detector: a clean multi-target + fusion where every target's metric genuinely lands in the final + query must NOT be flagged, even with live field-caps discovery + available (``test_different_metrics_merged`` above only exercises + the no-discovery/offline resolver path).""" + rule_pack = rules.RulePackConfig() + resolver = schema.SchemaResolver(rule_pack) + resolver._field_cache = { + "foo_total": {"double": {"type": "double"}}, + "bar_total": {"double": {"type": "double"}}, + } + resolver._discovery_attempted = True + resolver._discovery_status = "ok" + + panel = _make_panel(1) + panel["targets"] = [ + {"expr": "rate(foo_total[5m])", "refId": "A"}, + {"expr": "rate(bar_total[5m])", "refId": "B"}, + ] + _, result = _translate_panel(panel, rule_pack=rule_pack, resolver=resolver) + self.assertIn("foo_total", result.esql_query) + self.assertIn("bar_total", result.esql_query) + self.assertFalse( + any("Dropped from migrated query" in r for r in result.reasons), + f"Unexpected false-positive dropped-metric warning: {result.reasons}", + ) + + def test_metric_silently_dropped_from_fused_query_is_disclosed(self): + """Issue #352: a target counted as migrated (its ref_id lands in + ``fused_series``) whose metric the translator itself never emits into + the final query must be disclosed, not silently reported clean. + + The real-world trigger required a specific live target schema the + reporter observed manually; this exercises the same code path + directly by making the multi-target query builder drop one target's + field from its own output, the exact shape of gap the detector must + catch regardless of which upstream code path causes it. + """ + rule_pack = rules.RulePackConfig() + resolver = schema.SchemaResolver(rule_pack) + resolver._field_cache = { + "foo_total": {"double": {"type": "double"}}, + "bar_total": {"double": {"type": "double"}}, + } + resolver._discovery_attempted = True + resolver._discovery_status = "ok" + + panel = _make_panel(1) + panel["targets"] = [ + {"expr": "rate(foo_total[5m])", "refId": "A"}, + {"expr": "rate(bar_total[5m])", "refId": "B"}, + ] + + real_builder = panels._build_multi_target_series_query + + def _drop_bar_from_emitted_query(translations): + merged = real_builder(translations) + if merged and "bar_total" in merged.get("query", ""): + merged = dict(merged) + merged["query"] = ( + merged["query"] + .replace(", B = RATE(bar_total)", "") + .replace("bar_total", "foo_total") + ) + return merged + + with patch.object( + panels, + "_build_multi_target_series_query", + side_effect=_drop_bar_from_emitted_query, + ): + _, result = _translate_panel(panel, rule_pack=rule_pack, resolver=resolver) + + self.assertEqual(result.status, "migrated_with_warnings") + self.assertTrue( + any( + "Dropped from migrated query" in r and "bar_total" in r + for r in result.reasons + ), + f"Expected a disclosed dropped-metric warning, got: {result.reasons}", + ) + + +# ========================================================================= +# Dropped-metric detection helper (issues #349, #352) +# ========================================================================= + +class TestSourceMetricsAbsentFromQuery(unittest.TestCase): + """Direct unit tests for ``_source_metrics_absent_from_query``, the + shared helper behind both the curated-pack (#349) and general (#352) + dropped-source-metric disclosures.""" + + def _resolver_with_fields(self, *field_names): + rule_pack = rules.RulePackConfig() + resolver = schema.SchemaResolver(rule_pack) + resolver._field_cache = {name: {"double": {"type": "double"}} for name in field_names} + resolver._discovery_attempted = True + resolver._discovery_status = "ok" + return resolver + + def test_detects_metric_missing_from_query_text(self): + resolver = self._resolver_with_fields("foo_total", "bar_total") + missing = panels._source_metrics_absent_from_query( + ["rate(foo_total[5m])", "rate(bar_total[5m])"], + "TS metrics-* | STATS a = RATE(foo_total)", + resolver, + ) + self.assertEqual(missing, ["bar_total"]) + + def test_no_gap_when_every_metric_present(self): + resolver = self._resolver_with_fields("foo_total", "bar_total") + missing = panels._source_metrics_absent_from_query( + ["rate(foo_total[5m])", "rate(bar_total[5m])"], + "TS metrics-* | STATS a = RATE(foo_total), b = RATE(bar_total)", + resolver, + ) + self.assertEqual(missing, []) + + def test_requires_successful_live_discovery(self): + """Without live field-caps discovery, a bare metric-name substring + match is unreliable (curated overrides may legitimately reference a + materialized field name that bears no textual resemblance to the raw + PromQL metric), so the helper must no-op rather than guess.""" + rule_pack = rules.RulePackConfig() + resolver = schema.SchemaResolver(rule_pack) + # No discovery attempted: resolver exists but is not authoritative. + missing = panels._source_metrics_absent_from_query( + ["rate(foo_total[5m])"], + "TS metrics-* | STATS a = MAX(LAST_OVER_TIME(some_other_field))", + resolver, + ) + self.assertEqual(missing, []) + + def test_no_op_without_resolver(self): + missing = panels._source_metrics_absent_from_query( + ["rate(foo_total[5m])"], + "TS metrics-* | STATS a = MAX(LAST_OVER_TIME(some_other_field))", + None, + ) + self.assertEqual(missing, []) + # ========================================================================= # Summary Panel Correctness From 3c0e6e6de863bfdc94399eb4037770ea62c5462f Mon Sep 17 00:00:00 2001 From: subham sarkar Date: Wed, 19 Aug 2026 13:53:21 +0530 Subject: [PATCH 2/3] fix(grafana): omit absent optional series from curated Pressure/CPU queries 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. --- docs/sources/grafana.md | 5 +- .../grafana_1860_node_exporter_full/pack.yaml | 5 + .../adapters/source/grafana/panels.py | 113 ++++++++++++++++- tests/test_curated_packs.py | 116 ++++++++++++++++++ 4 files changed, 234 insertions(+), 5 deletions(-) diff --git a/docs/sources/grafana.md b/docs/sources/grafana.md index 7f352725..f5e5705b 100644 --- a/docs/sources/grafana.md +++ b/docs/sources/grafana.md @@ -119,7 +119,10 @@ telemetry missing from curated override" reason, the same disclosure the non-pack path uses for a metric that never made it into an otherwise-migrated fused query ("Dropped from migrated query"). Both checks require live field-caps discovery (`--es-url`) to resolve the metric's actual field name; -without it they no-op rather than guess. +without it they no-op rather than guess. Metrics listed in the pack's +`live_optional_metrics` that field-caps proved absent are stripped from the +hand-written override so the rest of the panel can still render; those +omissions are not reported as pack gaps. The console pipeline is **5 stages**, not 7: `[1/5] Extracting dashboards`, `[2/5] Translating dashboards`, `[3/5] Verification-packet ES|QL validation`, diff --git a/observability_migration/adapters/source/grafana/curated_packs/grafana_1860_node_exporter_full/pack.yaml b/observability_migration/adapters/source/grafana/curated_packs/grafana_1860_node_exporter_full/pack.yaml index a036df5d..eb3c264a 100644 --- a/observability_migration/adapters/source/grafana/curated_packs/grafana_1860_node_exporter_full/pack.yaml +++ b/observability_migration/adapters/source/grafana/curated_packs/grafana_1860_node_exporter_full/pack.yaml @@ -64,6 +64,11 @@ query: - node_netstat_TcpExt_TCPRcvQDrop - node_netstat_Tcp_MaxConn - node_interrupts_total + # PSI irq and CPU guest are real source series on newer 1860 revisions, but + # referencing them when field-caps prove them absent makes Elasticsearch + # reject the whole Pressure / CPU panel (unknown column). + - node_pressure_irq_stalled_seconds_total + - node_cpu_guest_seconds_total label_candidates: instance: diff --git a/observability_migration/adapters/source/grafana/panels.py b/observability_migration/adapters/source/grafana/panels.py index 05bb6456..f46ecbb4 100644 --- a/observability_migration/adapters/source/grafana/panels.py +++ b/observability_migration/adapters/source/grafana/panels.py @@ -3824,6 +3824,20 @@ def translate_panel(panel, datasource_index="metrics-*", esql_index=None, rule_p _dropped_curated_metrics = _source_metrics_absent_from_query( _source_target_exprs, _emitted_query, resolver ) + # live_optional_metrics already stripped these because + # field-caps proved them absent. Re-flagging them as a + # pack omission fights that design and yellows panels + # (TCP Errors / TCPRcvQDrop) whose remaining series + # still render. + _optional_omitted = set( + _optional_metric_result.omitted_metrics or [] + ) + if _optional_omitted: + _dropped_curated_metrics = [ + metric + for metric in _dropped_curated_metrics + if metric not in _optional_omitted + ] if _dropped_curated_metrics: _append_unique( _override_warnings, @@ -5549,6 +5563,7 @@ class _CuratedOptionalMetricStripResult: class _CuratedOptionalMetricOmissionResult: query: str exhausted_metrics: list[str] = field(default_factory=list) + omitted_metrics: list[str] = field(default_factory=list) def _live_optional_metric_is_absent(metric_name: str, resolver) -> bool: @@ -5572,6 +5587,49 @@ def _split_top_level_boolean_terms(text: str, keyword: str) -> list[str]: return [part for part in parts if part] +def _esql_expr_references_aliases(expression: str, aliases: set[str]) -> bool: + """True when *expression* uses any identifier in *aliases* outside quotes.""" + if not expression or not aliases: + return False + for match in _ESQL_ALIAS_TOKEN_RE.finditer(expression): + token = match.group(0) + if token.startswith(("'", '"', "`")): + continue + if _canonical_esql_alias(token) in aliases: + return True + return False + + +def _tail_is_removed_unpivot_piece(tail: str, removed_aliases: set[str]) -> bool: + """True when an ``MV_APPEND(inner, tail)`` tail is a stripped optional series.""" + if _esql_expr_references_aliases(tail, removed_aliases): + return True + stripped = str(tail or "").strip() + if len(stripped) >= 2 and stripped[0] in {'"', "'"} and stripped[-1] == stripped[0]: + text = stripped[1:-1] + for alias in removed_aliases: + if text == alias or text.startswith(f"{alias} - "): + return True + return False + + +def _unwrap_removed_unpivot_mv_appends(expression: str, removed_aliases: set[str]) -> str: + """Peel ``MV_APPEND(inner, stripped_series)`` layers left by optional omit.""" + expr = str(expression or "").strip() + while True: + upper = expr.upper() + if not upper.startswith("MV_APPEND(") or not expr.endswith(")"): + return expr + body = expr[len("MV_APPEND("):-1] + parts = [part.strip() for part in _split_top_level_csv(body) if part.strip()] + if len(parts) != 2: + return expr + inner, tail = parts + if not _tail_is_removed_unpivot_piece(tail, removed_aliases): + return expr + expr = inner.strip() + + def _strip_optional_metric_token_from_curated_esql_result( query: str, metric_name: str, @@ -5591,9 +5649,13 @@ def _strip_optional_metric_token_from_curated_esql_result( upper = stripped.upper() if upper.startswith("WHERE "): predicates = _split_top_level_boolean_terms(stripped[6:].strip(), "OR") - kept_predicates = [ - predicate for predicate in predicates if not token_re.search(predicate) - ] + kept_predicates = [] + for predicate in predicates: + if token_re.search(predicate): + continue + if _esql_expr_references_aliases(predicate, removed_alias_set): + continue + kept_predicates.append(predicate) if kept_predicates: stripped_stages.append("WHERE " + " OR ".join(kept_predicates)) continue @@ -5626,6 +5688,42 @@ def _strip_optional_metric_token_from_curated_esql_result( rebuilt += f" BY {by_text}" stripped_stages.append(rebuilt) continue + if upper.startswith("EVAL "): + assignments = [ + part.strip() + for part in _split_top_level_csv(stripped[5:].strip()) + if part.strip() + ] + changed = True + while changed: + changed = False + kept_assignments: list[str] = [] + for assignment in assignments: + left, right = _split_top_level_assignment(assignment) + rhs = right if right is not None else assignment + rewritten = _unwrap_removed_unpivot_mv_appends( + rhs, removed_alias_set + ) + if rewritten != rhs: + assignment = ( + f"{left} = {rewritten}" if left else rewritten + ) + rhs = rewritten + changed = True + if token_re.search(rhs) or _esql_expr_references_aliases( + rhs, removed_alias_set + ): + alias = _canonical_esql_alias(left) if left else "" + if alias: + _append_unique(removed_aliases, alias) + removed_alias_set.add(alias) + changed = True + continue + kept_assignments.append(assignment) + assignments = kept_assignments + if assignments: + stripped_stages.append("EVAL " + ", ".join(assignments)) + continue if upper.startswith("KEEP ") and removed_aliases: keep_parts = [ part.strip() @@ -5669,18 +5767,25 @@ def _omit_absent_optional_metrics_from_curated_query_result( return _CuratedOptionalMetricOmissionResult(query=query) out = str(query) exhausted_metrics: list[str] = [] + omitted_metrics: list[str] = [] for metric_name in metrics: if not _live_optional_metric_is_absent(metric_name, resolver): continue strip_result = _strip_optional_metric_token_from_curated_esql_result(out, metric_name) + if strip_result.query != out: + _append_unique(omitted_metrics, metric_name) if strip_result.exhausted: _append_unique(exhausted_metrics, metric_name) return _CuratedOptionalMetricOmissionResult( query="", exhausted_metrics=exhausted_metrics, + omitted_metrics=omitted_metrics, ) out = strip_result.query - return _CuratedOptionalMetricOmissionResult(query=out) + return _CuratedOptionalMetricOmissionResult( + query=out, + omitted_metrics=omitted_metrics, + ) def _omit_absent_optional_metrics_from_curated_query(query, optional_metrics, resolver): diff --git a/tests/test_curated_packs.py b/tests/test_curated_packs.py index 6b719fd5..2f399b6b 100644 --- a/tests/test_curated_packs.py +++ b/tests/test_curated_packs.py @@ -439,6 +439,122 @@ def test_1860_cpu_panel_includes_guest_series(): assert "Guest -" in query +def test_1860_pressure_omits_irq_when_field_caps_absent(): + """PSI irq is not on every kernel. Referencing the unknown column makes + Elasticsearch reject the whole Pressure panel, so the override must drop + it via live_optional_metrics when field-caps prove it absent.""" + dashboard = {"gnetId": 1860, "title": "Node Exporter Full", "tags": ["prometheus"]} + resolved = resolve_pack_for_dashboard(dashboard, RulePackConfig()) + resolver = SchemaResolver(resolved) + resolver._field_cache = { + "node_pressure_cpu_waiting_seconds_total": {"double": {"type": "double"}}, + "node_pressure_memory_waiting_seconds_total": {"double": {"type": "double"}}, + "node_pressure_io_waiting_seconds_total": {"double": {"type": "double"}}, + "instance": {"keyword": {"type": "keyword"}}, + "job": {"keyword": {"type": "keyword"}}, + } + resolver._discovery_attempted = True + resolver._discovery_status = "ok" + + panel = { + "type": "bargauge", + "title": "Pressure", + "targets": [ + {"expr": "irate(node_pressure_cpu_waiting_seconds_total[$__rate_interval])", "refId": "A"}, + {"expr": "irate(node_pressure_memory_waiting_seconds_total[$__rate_interval])", "refId": "B"}, + {"expr": "irate(node_pressure_io_waiting_seconds_total[$__rate_interval])", "refId": "C"}, + ], + } + + yaml_panel, result = translate_panel(panel, rule_pack=resolved, resolver=resolver) + + assert result.status == "migrated", f"got {result.status}: {result.reasons}" + query = (yaml_panel or {}).get("esql", {}).get("query", "") + assert "node_pressure_cpu_waiting_seconds_total" in query + assert "irq_stalled" not in query + assert not any("curated override" in reason for reason in result.reasons) + + +def test_1860_cpu_omits_guest_when_field_caps_absent(): + """Guest is a distinct exporter metric. An unknown-column reference in the + same STATS as the eight CPU modes would take down the whole CPU panel.""" + dashboard = {"gnetId": 1860, "title": "Node Exporter Full", "tags": ["prometheus"]} + resolved = resolve_pack_for_dashboard(dashboard, RulePackConfig()) + resolver = SchemaResolver(resolved) + resolver._field_cache = { + "node_cpu_seconds_total": {"double": {"type": "double"}}, + "instance": {"keyword": {"type": "keyword"}}, + "job": {"keyword": {"type": "keyword"}}, + "cpu": {"keyword": {"type": "keyword"}}, + "mode": {"keyword": {"type": "keyword"}}, + } + resolver._discovery_attempted = True + resolver._discovery_status = "ok" + + targets = [ + { + "expr": f'avg(irate(node_cpu_seconds_total{{mode="{mode}"}}[$__rate_interval])) by (mode) * 100', + "refId": chr(65 + i), + } + for i, mode in enumerate( + ["system", "user", "nice", "iowait", "irq", "softirq", "steal", "idle"] + ) + ] + panel = {"type": "timeseries", "title": "CPU", "targets": targets} + + yaml_panel, result = translate_panel(panel, rule_pack=resolved, resolver=resolver) + + assert result.status == "migrated", f"got {result.status}: {result.reasons}" + query = (yaml_panel or {}).get("esql", {}).get("query", "") + assert "node_cpu_seconds_total" in query + assert "guest" not in query.lower() + assert not any("curated override" in reason for reason in result.reasons) + + +def test_curated_override_does_not_flag_stripped_optional_metrics(): + """A live_optional metric that field-caps proved absent is stripped so the + rest of the override can render. That is not a pack omission, so + status_override: migrated must not be downgraded with a "missing from + curated override" reason (TCP Errors / TCPRcvQDrop).""" + dashboard = {"gnetId": 1860, "title": "Node Exporter Full", "tags": ["prometheus"]} + resolved = resolve_pack_for_dashboard(dashboard, RulePackConfig()) + resolver = SchemaResolver(resolved) + resolver._field_cache = { + "metrics.node_netstat_TcpExt_ListenOverflows": {"double": {"type": "double"}}, + "metrics.node_netstat_TcpExt_ListenDrops": {"double": {"type": "double"}}, + "metrics.node_netstat_TcpExt_TCPSynRetrans": {"double": {"type": "double"}}, + "metrics.node_netstat_Tcp_RetransSegs": {"double": {"type": "double"}}, + "metrics.node_netstat_Tcp_InErrs": {"double": {"type": "double"}}, + "metrics.node_netstat_Tcp_OutRsts": {"double": {"type": "double"}}, + "metrics.node_netstat_TcpExt_TCPOFOQueue": {"double": {"type": "double"}}, + "labels.instance": {"keyword": {"type": "keyword"}}, + "labels.job": {"keyword": {"type": "keyword"}}, + } + resolver._discovery_attempted = True + resolver._discovery_status = "ok" + + panel = { + "type": "timeseries", + "title": "TCP Errors", + "targets": [ + {"expr": "irate(node_netstat_TcpExt_ListenOverflows[5m])", "refId": "A"}, + {"expr": "irate(node_netstat_TcpExt_ListenDrops[5m])", "refId": "B"}, + {"expr": "irate(node_netstat_TcpExt_TCPSynRetrans[5m])", "refId": "C"}, + {"expr": "irate(node_netstat_Tcp_RetransSegs[5m])", "refId": "D"}, + {"expr": "irate(node_netstat_Tcp_InErrs[5m])", "refId": "E"}, + {"expr": "irate(node_netstat_Tcp_OutRsts[5m])", "refId": "F"}, + {"expr": "irate(node_netstat_TcpExt_TCPRcvQDrop[5m])", "refId": "G"}, + {"expr": "irate(node_netstat_TcpExt_TCPOFOQueue[5m])", "refId": "H"}, + ], + } + + _yaml_panel, result = translate_panel(panel, rule_pack=resolved, resolver=resolver) + + assert result.status == "migrated", f"got {result.status}: {result.reasons}" + assert not any("curated override" in reason for reason in result.reasons), result.reasons + assert "TCPRcvQDrop" not in ((_yaml_panel or {}).get("esql") or {}).get("query", "") + + def test_1860_interrupts_detail_uses_interrupt_cpu_legend(): """Interrupts Detail must legend by interrupt/cpu, not empty type/info GROK.""" dashboard = {"gnetId": 1860, "title": "Node Exporter Full", "tags": ["prometheus"]} From ddfadcb9d5a581bb0b21808f9879fd3b909ac5b6 Mon Sep 17 00:00:00 2001 From: Giorgi Imerlishvili Date: Wed, 19 Aug 2026 13:59:10 +0400 Subject: [PATCH 3/3] fix(test): stop reusing the reserved "percentage" axis-label alias as 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. --- tests/test_migrate.py | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/tests/test_migrate.py b/tests/test_migrate.py index afedb3fd..182e682a 100644 --- a/tests/test_migrate.py +++ b/tests/test_migrate.py @@ -13105,11 +13105,17 @@ def test_extract_xy_appearance_hides_time_axis_title_from_override(self): def test_extract_xy_appearance_omits_line_area_style_for_bar(self): from observability_migration.targets.kibana.emit.display import extract_xy_appearance + # NOTE: "percentage" is deliberately NOT used here -- it is a reserved + # opaque Grafana-unit-id alias (see _OPAQUE_AXIS_TITLE_ALIASES / + # test_extract_axis_label_suppresses_grafana_unit_id_aliases) whose + # title is intentionally suppressed. This test's own purpose is + # unrelated: it only checks that bar charts keep axis config while + # dropping line/area-only appearance keys, so use ordinary label text. panel = { "fieldConfig": { "defaults": { "custom": { - "axisLabel": "percentage", + "axisLabel": "CPU usage", "drawStyle": "bars", "lineInterpolation": "smooth", "fillOpacity": 70, @@ -13118,7 +13124,7 @@ def test_extract_xy_appearance_omits_line_area_style_for_bar(self): } } appearance = extract_xy_appearance(panel, chart_type="bar") - self.assertEqual(appearance["y_left_axis"]["title"], "percentage") + self.assertEqual(appearance["y_left_axis"]["title"], "CPU usage") self.assertNotIn("line_style", appearance) self.assertNotIn("fill_opacity", appearance) area_appearance = extract_xy_appearance(panel, chart_type="area")