diff --git a/docs/sources/grafana.md b/docs/sources/grafana.md index eec3d27e..aa4c8c94 100644 --- a/docs/sources/grafana.md +++ b/docs/sources/grafana.md @@ -631,6 +631,7 @@ is available at `examples/cue/grafana-rule-pack.cue`. - `label_join(v, dst, separator, src1, src2, ...)` translates to a post-`STATS` `| EVAL dst = CONCAT(src1, "separator", src2, ...)` when all source labels appear in the inner expression's `by()` clause. If any source label is absent from the `by()` clause, the panel stays `not_feasible` (the column would not exist in the `STATS` output and `CONCAT` cannot reference it). - Histogram mean idioms `sum(increase|rate(m_sum) / increase|rate(m_count))` approximate as a ratio of aggregates (`sum(m_sum)/sum(m_count)`) with an explicit warning; unrelated per-element ratios stay `not_feasible`. - Multi-target XY panels fuse when series share a compatible ES|QL shape. Summary panels (`stat` / `singlestat` / `gauge` / `bargauge` / table) use the same compatibility group and approximate multi-series stats as a summary table when needed. Grouping mismatches where one target's groups are a subset of another's (e.g. QoS `by (qos_class)` + ungrouped total) union the BY fields with a warning. Divergent label filters on otherwise identical measures CASE-inline into the shared `STATS` (including window-less `LAST_OVER_TIME`, used by Express-style status-class counters). `legendFormat` `{{label}}` placeholders on `rate`/`irate`/`increase` (and other TS paths covered by issue #99) are display hints — they become series aliases, not `BY` dimensions — so overlays like Redis in/out rates can share one panel. Targets that remain incompatible (Windows vs Linux metrics, complex `or`/`label_replace` trees) still keep the largest compatible group and warn; Windows-specific drop wording only applies when every dropped target is a `windows_*` metric. +- Composite/multi-target series (a fused native-PROMQL `value` column, the general ES|QL translator's scalar-expression `computed_value` column, or a curated-pack override that folds several source metrics into one `value` column with a `series_group` breakdown) never surface that internal column name to the operator. An unambiguous static `legendFormat` shared by every visible target takes priority; mixed or templated legends fall back to the panel title as the metric's label. A single such metric with a breakdown uses that label as the Y-axis title when Grafana left the axis unnamed and no uniform unit title (`%`, `Bytes`, …) can be inferred (issue #351). An explicit Grafana axis label still wins; opaque aliases such as `percentage` are ignored so a unit title can apply. A panel with no usable title/legend text falls back to the prior hidden-title behavior. - Kibana ES|QL visualizations are still effectively single-query / single data layer. Independent Grafana queries that cannot fuse into one wide ES|QL statement cannot be overlaid the way Grafana does; that is a platform limit, not a silent drop. - Mixed-datasource and mixed-query-language panels are still weaker than single-source Prometheus or Loki paths. - Verification is strongest when live Prometheus/Loki and Elasticsearch are available, but full measured source-vs-target comparison is still partial. diff --git a/observability_migration/adapters/source/grafana/panels.py b/observability_migration/adapters/source/grafana/panels.py index fe8aa6dd..3a4278b0 100644 --- a/observability_migration/adapters/source/grafana/panels.py +++ b/observability_migration/adapters/source/grafana/panels.py @@ -2082,7 +2082,37 @@ def _static_legend_label(legend_format): return label -def _label_native_promql_value_metric(yaml_panel, *, title, legend_format=""): +def _panel_static_legend_label(panel): + """Return the one unambiguous static legend shared by visible targets.""" + labels = [] + for target in panel.get("targets") or []: + if not isinstance(target, dict) or target.get("hide"): + continue + label = _static_legend_label(target.get("legendFormat", "")) + if not label: + return "" + if label not in labels: + labels.append(label) + return labels[0] if len(labels) == 1 else "" + + +_PLACEHOLDER_VALUE_METRIC_FIELDS = frozenset({"value", "computed_value"}) + + +def _label_placeholder_value_metric(yaml_panel, *, title, legend_format=""): + """Give a synthetic ``value``/``computed_value`` metric column a real label. + + Both the native-PROMQL path (single ``value=(...)`` column) and the + general ES|QL translator (single ``computed_value`` scalar-expression + column) collapse a panel's target(s) into one numeric column with a + placeholder name. Kibana's Lens then falls back to that raw column name + -- ``value``/``computed_value`` -- as the axis/legend label (issue #351). + Curated-pack overrides that fuse multiple metrics into one ``value`` + column via ``EVAL value = ...`` hit the same gap. + + Uses the same fallback as a single-target panel: the target's static + legend text if the operator set one, otherwise the panel title. + """ esql = yaml_panel.get("esql") if not isinstance(esql, dict): return @@ -2098,7 +2128,7 @@ def _label_native_promql_value_metric(yaml_panel, *, title, legend_format=""): for metric in metrics: if not isinstance(metric, dict): continue - if metric.get("field") != "value": + if metric.get("field") not in _PLACEHOLDER_VALUE_METRIC_FIELDS: continue metric.setdefault("label", fallback_label) @@ -2730,7 +2760,7 @@ def _translate_panel_native_promql( yaml_panel["esql"] = native_panel enrich_yaml_panel_display(yaml_panel, panel) - _label_native_promql_value_metric(yaml_panel, title=title, legend_format=legend_format) + _label_placeholder_value_metric(yaml_panel, title=title, legend_format=legend_format) _apply_series_override_axes(yaml_panel, panel, []) notes = list(panel_notes) + ["Native PROMQL: original PromQL reused via ES|QL PROMQL command"] @@ -3783,6 +3813,11 @@ def translate_panel(panel, datasource_index="metrics-*", esql_index=None, rule_p yaml_panel, panel, _override_warnings ) enrich_yaml_panel_display(yaml_panel, panel) + _label_placeholder_value_metric( + yaml_panel, + title=title, + legend_format=_panel_static_legend_label(panel), + ) _score = 1.0 if _status == "migrated" else 0.7 _override_notes = list(panel_notes) if _status == "migrated": @@ -4383,6 +4418,14 @@ def translate_panel(panel, datasource_index="metrics-*", esql_index=None, rule_p panel, metric_labels=metric_labels or None, ) + _label_placeholder_value_metric( + yaml_panel, + title=title, + # Same rule as curated overrides: only an unambiguous static legend + # shared by every visible target is safe on a fused series. Mixed + # legends fall back to the panel title rather than the primary target. + legend_format=_panel_static_legend_label(panel) or static_legend_label or "", + ) _apply_series_override_axes(yaml_panel, panel, primary.warnings) if yaml_panel.get("esql", {}).get("query"): primary.esql_query = yaml_panel["esql"]["query"] diff --git a/observability_migration/targets/kibana/compile.py b/observability_migration/targets/kibana/compile.py index 0b032716..2b2e2749 100644 --- a/observability_migration/targets/kibana/compile.py +++ b/observability_migration/targets/kibana/compile.py @@ -116,15 +116,30 @@ def _sync_dimension(field_name): breakdown["field"] = "series_group" changed = True format_cfg = None + label_cfg = None metrics = esql_config.get("metrics") if isinstance(metrics, list): for item in metrics: - if isinstance(item, dict) and isinstance(item.get("format"), dict): + if not isinstance(item, dict): + continue + if format_cfg is None and isinstance(item.get("format"), dict): format_cfg = copy.deepcopy(item["format"]) - break + if ( + label_cfg is None + and str(item.get("field") or item.get("column") or "").strip() + in {"value", "computed_value"} + and str(item.get("label") or "").strip() + ): + label_cfg = item["label"] new_metric_item = {"field": "value"} if format_cfg: new_metric_item["format"] = format_cfg + # Preserve a caller-derived label (panel title / static legend + # fallback for the placeholder ``value`` column, issue #351) across + # this rebuild -- otherwise a post-validation query swap silently + # regresses a labeled axis back to the raw column name. + if label_cfg: + new_metric_item["label"] = label_cfg if esql_config.get("metrics") != [new_metric_item]: esql_config["metrics"] = [new_metric_item] changed = True diff --git a/observability_migration/targets/kibana/dashboards_api.py b/observability_migration/targets/kibana/dashboards_api.py index d5b62768..a1c3ae59 100644 --- a/observability_migration/targets/kibana/dashboards_api.py +++ b/observability_migration/targets/kibana/dashboards_api.py @@ -740,11 +740,19 @@ def _api_legend(legend: Any, kind: str) -> dict[str, Any] | None: } +# YAML/visual-IR spellings that put a series on the right-hand axis. Kept in +# sync with the ``axis`` mapping in :func:`_api_column` (role ``xy_y``). +_RIGHT_AXIS_VALUES = {"right", "y2"} + + def _infer_y_axis_title(metrics: list[Any]) -> str: """Return a Y-axis title inferred from a uniform metric format, or ''.""" left_formats = set() for m in metrics: - if not isinstance(m, dict) or m.get("axis") == "right": + if ( + not isinstance(m, dict) + or str(m.get("axis") or "") in _RIGHT_AXIS_VALUES + ): continue fmt = m.get("format") if isinstance(fmt, dict): @@ -774,11 +782,6 @@ def _xy_metrics(cfg: dict[str, Any]) -> list[Any]: return metrics -# YAML/visual-IR spellings that put a series on the right-hand axis. Kept in -# sync with the ``axis`` mapping in :func:`_api_column` (role ``xy_y``). -_RIGHT_AXIS_VALUES = {"right", "y2"} - - def _has_right_axis_series(cfg: dict[str, Any]) -> bool: """True when at least one series is plotted against the right-hand axis.""" return any( @@ -870,48 +873,63 @@ def _ensure_temporal_x_scale( return out +def _xy_config_has_breakdown(cfg: dict[str, Any]) -> bool: + """True when the XY config (or any layer) declares a breakdown dimension.""" + if cfg.get("breakdown") or cfg.get("breakdowns"): + return True + layers = cfg.get("layers") + if isinstance(layers, list): + for layer in layers: + if isinstance(layer, dict) and (layer.get("breakdown") or layer.get("breakdowns")): + return True + return False + + def _xy_axis_from_cfg(cfg: dict[str, Any]) -> dict[str, Any] | None: """Return the XY axis config, falling back to format-inferred Y title.""" axis = _api_xy_axis(_cfg_axis_source(cfg)) if axis and (axis.get("y") or {}).get("title"): return axis - metrics = cfg.get("metrics") if isinstance(cfg.get("metrics"), list) else [] + # Include ``layers[*].metrics`` so cross-data-stream XY panels get the same + # left-axis title decision as a top-level ``metrics`` list. + metrics = _xy_metrics(cfg) + left = [ + m for m in metrics + if ( + isinstance(m, dict) + and str(m.get("axis") or "") not in _RIGHT_AXIS_VALUES + ) + ] + + def _with_y_title(title: dict[str, Any]) -> dict[str, Any]: + if axis: + merged = dict(axis) + merged["y"] = {**merged.get("y", {}), **title} + return merged + return {"y": title} + inferred = _infer_y_axis_title(metrics) - if not inferred: - # No unit-derived title available. With two or more left-axis series - # Kibana falls back to the FIRST series' name, which mislabels the whole - # axis ("not expiring" on a chart plotting not-expiring AND expiring; - # "hits" on hits AND misses). Grafana showed no axis title at all here - # (axisLabel is empty), so hide it rather than let Kibana invent a - # wrong one. A single series keeps the default, where the column name - # does describe the axis. - left = [ - m for m in metrics - if isinstance(m, dict) and m.get("axis") != "right" - ] - if len(left) >= 2: - # ``visible: false`` alone hides the title; a companion ``text: ""`` - # names nothing and Kibana does not store it (see _api_axis_title). - hidden: dict[str, Any] = {"title": {"visible": False}} - if axis: - merged_hidden = dict(axis) - merged_hidden["y"] = {**merged_hidden.get("y", {}), **hidden} - return merged_hidden - return {"y": hidden} - if len(left) == 1 and _xy_single_metric_uses_placeholder_name(cfg, left[0]): - hidden = {"title": {"visible": False}} - if axis: - merged_hidden = dict(axis) - merged_hidden["y"] = {**merged_hidden.get("y", {}), **hidden} - return merged_hidden - return {"y": hidden} - return axis - y_title: dict[str, Any] = {"title": {"text": inferred, "visible": True}} - if axis: - merged = dict(axis) - merged["y"] = {**merged.get("y", {}), **y_title} - return merged - return {"y": y_title} + if inferred: + # A uniform unit title ("%"/ "Bytes") is more useful as an axis name + # than repeating the panel title on a composite ``value`` series. + return _with_y_title({"title": {"text": inferred, "visible": True}}) + if len(left) == 1 and _xy_single_metric_uses_placeholder_name(cfg, left[0]): + placeholder_label = str(left[0].get("label") or "").strip() + if placeholder_label: + return _with_y_title({"title": {"text": placeholder_label, "visible": True}}) + return _with_y_title({"title": {"visible": False}}) + # No unit-derived title available. With two or more left-axis series + # Kibana falls back to the FIRST series' name, which mislabels the whole + # axis ("not expiring" on a chart plotting not-expiring AND expiring; + # "hits" on hits AND misses). Grafana showed no axis title at all here + # (axisLabel is empty), so hide it rather than let Kibana invent a + # wrong one. A single series keeps the default, where the column name + # does describe the axis. + if len(left) >= 2: + # ``visible: false`` alone hides the title; a companion ``text: ""`` + # names nothing and Kibana does not store it (see _api_axis_title). + return _with_y_title({"title": {"visible": False}}) + return axis def _xy_single_metric_uses_placeholder_name(cfg: dict[str, Any], metric: Any) -> bool: @@ -924,9 +942,7 @@ def _xy_single_metric_uses_placeholder_name(cfg: dict[str, Any], metric: Any) -> """ if not isinstance(metric, dict): return False - breakdown = cfg.get("breakdown") - breakdowns = cfg.get("breakdowns") - if not breakdown and not breakdowns: + if not _xy_config_has_breakdown(cfg): return False field_name = str(metric.get("field") or metric.get("column") or "").strip() return field_name in {"value", "computed_value"} diff --git a/observability_migration/targets/kibana/emit/display.py b/observability_migration/targets/kibana/emit/display.py index 2de07a66..41d69909 100644 --- a/observability_migration/targets/kibana/emit/display.py +++ b/observability_migration/targets/kibana/emit/display.py @@ -13,6 +13,11 @@ _OPAQUE_AXIS_TITLE_ALIASES = { "aqu-sz", + # Community dashboards often copy Grafana's percent unit id into axisLabel. + # "%"/inferred unit titles are better; "counter" is left alone because + # hiding it lets Lens name the whole axis after the first series. + "percentage", + "percent", } GRAFANA_UNIT_TO_YAML: dict[str, dict[str, Any]] = { @@ -224,7 +229,7 @@ def sanitize_axis_title_text(label: str, *, unit: str = "") -> str: text = str(label or "").strip() if not text: return "" - if text in _OPAQUE_AXIS_TITLE_ALIASES: + if text.casefold() in _OPAQUE_AXIS_TITLE_ALIASES: return "" return text @@ -334,7 +339,14 @@ def humanize_metric_label(field_name: str, legend_format: str | None = None) -> return None text = re.sub(r"_+", " ", field_name).strip() text = re.sub(r"\s{2,}", " ", text) - if not text or text.lower() in ("series", "value", "metric", "time bucket"): + # "computed value" is the ES|QL translator's own synthetic scalar-expression + # column name (``computed_value``, the ``by()``-less binary-expression + # counterpart to native PROMQL's ``value``) -- humanizing it into "Computed + # Value" is exactly the misleading placeholder-as-title leak issue #351 + # covers, just re-capitalized rather than literal. Both placeholder names + # rely on the panel-title/legend fallback in + # ``_label_placeholder_value_metric`` instead. + if not text or text.lower() in ("series", "value", "computed value", "metric", "time bucket"): return None parts = text.split() label = " ".join(p if p.isupper() else p.capitalize() for p in parts) diff --git a/tests/targets/kibana/test_dashboards_api.py b/tests/targets/kibana/test_dashboards_api.py index 3d24ff4b..16c22d95 100644 --- a/tests/targets/kibana/test_dashboards_api.py +++ b/tests/targets/kibana/test_dashboards_api.py @@ -542,6 +542,206 @@ def test_xy_hidden_y_axis_title_for_single_breakdown_value_metric(): assert cfg["axis"]["y"]["title"] == {"visible": False} +def test_xy_shows_y_axis_title_for_labeled_breakdown_value_metric(): + """A labeled ``value`` composite metric (issue #351) names the axis instead of hiding it. + + The translator sets ``label`` on a placeholder ``value``/``computed_value`` + column as a fallback (panel title or static legend) when it fuses multiple + targets into one composite series. That label is a real title, unlike the + raw column name, so it should surface as the axis title text. + """ + cfg = _map({ + "type": "line", + "query": "FROM metrics-*", + "dimension": {"field": "time_bucket"}, + "breakdown": {"field": "series_group"}, + "metrics": [ + {"field": "value", "label": "Disk Space Used Basic", "format": {"type": "number", "suffix": "%"}} + ], + })["config"] + assert cfg["axis"]["y"]["title"] == {"text": "Disk Space Used Basic", "visible": True} + + +def test_xy_inferred_unit_title_wins_over_placeholder_label(): + """A uniform unit title is a better axis name than repeating the panel title. + + Issue #351 asked for the Grafana axis title *or unit*. Composite CPU panels + already format ticks as percent; stamping the panel name on the axis just + duplicates the chrome. + """ + cfg = _map({ + "type": "line", + "query": "FROM metrics-*", + "dimension": {"field": "time_bucket"}, + "breakdown": {"field": "instance"}, + "metrics": [ + { + "field": "computed_value", + "label": "CPU Idle Percentage", + "format": {"type": "percent"}, + } + ], + })["config"] + assert cfg["axis"]["y"]["title"] == {"text": "%", "visible": True} + + +def test_xy_placeholder_label_used_when_no_unit_title_can_be_inferred(): + """``number`` + suffix is not a unit type, so the placeholder label names the axis.""" + cfg = _map({ + "type": "line", + "query": "FROM metrics-*", + "dimension": {"field": "time_bucket"}, + "breakdown": {"field": "series_group"}, + "metrics": [ + { + "field": "value", + "label": "Network Traffic Basic", + "format": {"type": "bits", "suffix": "/s"}, + } + ], + })["config"] + assert cfg["axis"]["y"]["title"] == { + "text": "Network Traffic Basic", + "visible": True, + } + + +def test_xy_placeholder_label_does_not_override_explicit_axis_title(): + """An operator-authored Grafana axis title keeps highest precedence.""" + cfg = _map({ + "type": "line", + "query": "FROM metrics-*", + "dimension": {"field": "time_bucket"}, + "breakdown": {"field": "instance"}, + "metrics": [ + { + "field": "computed_value", + "label": "CPU Idle Percentage", + "format": {"type": "percent"}, + } + ], + "appearance": {"y_left_axis": {"title": "Operator CPU Axis"}}, + })["config"] + assert cfg["axis"]["y"]["title"] == { + "text": "Operator CPU Axis", + "visible": True, + } + + +def test_xy_placeholder_label_does_not_title_multi_metric_axis(): + """The issue #351 fallback is limited to one left-axis metric.""" + cfg = _map({ + "type": "line", + "query": "FROM metrics-*", + "dimension": {"field": "time_bucket"}, + "breakdown": {"field": "instance"}, + "metrics": [ + {"field": "value", "label": "Composite Value"}, + {"field": "latency", "label": "Latency"}, + ], + })["config"] + assert cfg["axis"]["y"]["title"] == {"visible": False} + + +def test_xy_placeholder_label_does_not_title_non_placeholder_metric(): + """A normal metric label remains Lens metric metadata, not an axis override.""" + cfg = _map({ + "type": "line", + "query": "FROM metrics-*", + "dimension": {"field": "time_bucket"}, + "breakdown": {"field": "instance"}, + "metrics": [{"field": "requests", "label": "Request Rate"}], + })["config"] + assert "y" not in cfg["axis"] + + +def test_xy_right_axis_placeholder_does_not_title_left_axis(): + """The supported ``y2`` alias must not enter left-axis title inference.""" + cfg = _map({ + "type": "line", + "query": "FROM metrics-*", + "dimension": {"field": "time_bucket"}, + "breakdown": {"field": "instance"}, + "metrics": [ + { + "field": "value", + "label": "Right-side Composite", + "format": {"type": "percent"}, + "axis": "y2", + } + ], + })["config"] + assert "y" not in cfg["axis"] + + +def test_xy_placeholder_label_reads_layer_metrics(): + """Cross-data-stream XY keeps metrics and breakdowns on layers.""" + axis = api._xy_axis_from_cfg({ + "type": "line", + "layers": [ + { + "breakdown": {"field": "series_group"}, + "metrics": [ + { + "field": "value", + "label": "Disk Space Used Basic", + "format": {"type": "number", "suffix": "%"}, + } + ], + } + ], + }) + assert axis is not None + assert axis["y"]["title"] == {"text": "Disk Space Used Basic", "visible": True} + + +def test_xy_inferred_unit_title_reads_layer_metrics(): + axis = api._xy_axis_from_cfg({ + "type": "line", + "breakdown": {"field": "series_group"}, + "layers": [ + { + "metrics": [ + { + "field": "value", + "label": "CPU Basic", + "format": {"type": "percent"}, + } + ] + } + ], + }) + assert axis is not None + assert axis["y"]["title"] == {"text": "%", "visible": True} + + +def test_xy_opaque_grafana_percentage_label_yields_inferred_unit_title(): + """Grafana ``axisLabel: percentage`` is an opaque unit id, not a real title.""" + cfg = _map({ + "type": "line", + "query": "FROM metrics-*", + "dimension": {"field": "time_bucket"}, + "breakdown": {"field": "series_group"}, + "metrics": [ + {"field": "value", "label": "CPU", "format": {"type": "percent"}} + ], + "appearance": {"y_left_axis": {"title": "percentage"}}, + })["config"] + assert cfg["axis"]["y"]["title"] == {"text": "%", "visible": True} + + +def test_xy_hidden_y_axis_title_for_breakdown_computed_value_metric(): + """``computed_value`` (the general ES|QL translator's placeholder) is covered too.""" + cfg = _map({ + "type": "line", + "query": "FROM metrics-*", + "dimension": {"field": "time_bucket"}, + "breakdown": {"field": "mountpoint"}, + "metrics": [{"field": "computed_value", "format": {"type": "number", "suffix": "%"}}], + })["config"] + assert cfg["axis"]["y"]["title"] == {"visible": False} + + def test_api_axis_title_drops_empty_text(): assert api._api_axis_title({"text": "", "visible": False}) == {"visible": False} assert api._api_axis_title({"text": ""}) is None @@ -553,6 +753,11 @@ def test_api_axis_title_suppresses_opaque_shorthand_text(): "visible": True, } assert api._yaml_axis_title("aqu-sz") is None + assert api._yaml_axis_title("percentage") is None + assert api._yaml_axis_title("counter") == { + "text": "counter", + "visible": True, + } def test_api_axis_title_keeps_legitimate_kebab_and_snake_text(): diff --git a/tests/targets/kibana/test_shared_compile.py b/tests/targets/kibana/test_shared_compile.py index 95440695..7c1c9a5c 100644 --- a/tests/targets/kibana/test_shared_compile.py +++ b/tests/targets/kibana/test_shared_compile.py @@ -32,8 +32,16 @@ def test_sync_esql_panel_fields_rebuilds_long_form_xy_breakdown(self): "query": "TS metrics-* | STATS Busy_System = AVG(v) BY time_bucket = TBUCKET(10, ?_tstart, ?_tend) | KEEP time_bucket, Busy_System", "dimension": {"field": "time_bucket", "data_type": "date"}, "metrics": [ - {"field": "Busy_System", "format": {"type": "percent"}}, - {"field": "Busy_User", "format": {"type": "percent"}}, + { + "field": "Busy_System", + "label": "Busy System", + "format": {"type": "percent"}, + }, + { + "field": "Busy_User", + "label": "Busy User", + "format": {"type": "percent"}, + }, ], "mode": "percentage", }, @@ -60,6 +68,55 @@ def test_sync_esql_panel_fields_rebuilds_long_form_xy_breakdown(self): [{"field": "value", "format": {"type": "percent"}}], ) + def test_sync_esql_panel_fields_preserves_placeholder_value_label_on_rebuild(self): + """A ``value``/``computed_value`` fallback label (issue #351) must survive. + + Post-validation query swaps rebuild the long-form XY metric list from + scratch (see ``test_sync_esql_panel_fields_rebuilds_long_form_xy_breakdown``). + Before this fix that rebuild kept ``format`` but dropped any ``label`` + the translator had derived for the placeholder ``value`` column, + silently regressing a labeled axis back to the raw column name. + """ + yaml_panel = { + "title": "Disk Space Used Basic", + "esql": { + "type": "line", + "query": "TS metrics-* | STATS computed_value = AVG(v) BY time_bucket = TBUCKET(75, ?_tstart, ?_tend) | KEEP time_bucket, computed_value", + "dimension": {"field": "time_bucket", "data_type": "date"}, + "metrics": [ + { + "field": "computed_value", + "label": "Disk Space Used Basic", + "format": {"type": "number", "suffix": "%"}, + } + ], + }, + } + new_query = ( + "TS metrics-* " + "| STATS computed_value = AVG(v) BY time_bucket = TBUCKET(75, ?_tstart, ?_tend), labels.mountpoint " + "| EVAL series_group = labels.mountpoint, value = computed_value " + "| KEEP time_bucket, series_group, value" + ) + + changed = shared_compile._sync_esql_panel_fields( + yaml_panel, + yaml_panel["esql"]["query"], + new_query, + ) + + self.assertTrue(changed) + self.assertEqual( + yaml_panel["esql"]["metrics"], + [ + { + "field": "value", + "format": {"type": "number", "suffix": "%"}, + "label": "Disk Space Used Basic", + } + ], + ) + def test_sync_esql_panel_fields_keeps_time_dimension_metadata_when_query_changes(self): yaml_panel = { "title": "Traffic", diff --git a/tests/test_curated_packs.py b/tests/test_curated_packs.py index a5bb31e5..90da2318 100644 --- a/tests/test_curated_packs.py +++ b/tests/test_curated_packs.py @@ -13,8 +13,10 @@ ) from observability_migration.adapters.source.grafana.panels import ( _apply_panel_layout_overrides_recursively, + _label_placeholder_value_metric, _materialize_curated_query_override, _omit_absent_optional_metrics_from_curated_query, + _panel_static_legend_label, _retarget_esql_param_controls_to_panel_bindings, _strip_optional_metric_token_from_curated_esql, translate_dashboard, @@ -492,6 +494,88 @@ def test_1860_cpu_busy_curated_override_avoids_boundary_bucket_last(): assert "STATS computed_value = LAST(computed_value, time_bucket)" not in query +def test_1860_disk_space_used_basic_labels_composite_value_metric(): + """Curated composite-series overrides must label their ``value`` column (#351). + + The "Disk Space Used Basic" override fuses ``node_filesystem_avail_bytes`` + and ``node_filesystem_size_bytes`` into one ``value`` column broken down + by ``series_group`` (mountpoint). With no ``label`` set, Lens falls back + to the raw column name ("value") as the y-axis title; the panel title is + the same fallback the single-target native-PROMQL path already uses. + """ + dashboard = {"gnetId": 1860, "title": "Node Exporter Full", "tags": ["prometheus"]} + resolved = resolve_pack_for_dashboard(dashboard, RulePackConfig()) + + panel = { + "type": "timeseries", + "title": "Disk Space Used Basic", + "fieldConfig": {"defaults": {"unit": "percent"}}, + "targets": [{"expr": "node_filesystem_avail_bytes", "refId": "A"}], + } + + yaml_panel, result = translate_panel(panel, rule_pack=resolved) + + assert result.status == "migrated", ( + f"Expected migrated via curated override, got {result.status}: {result.reasons}" + ) + metrics = yaml_panel["esql"]["metrics"] + assert [m.get("field") for m in metrics] == ["value"] + assert metrics[0].get("label") == "Disk Space Used Basic" + + +def test_1860_curated_composite_value_metric_prefers_static_legend_label(): + """Curated overrides keep the single-target static-legend precedence.""" + dashboard = {"gnetId": 1860, "title": "Node Exporter Full", "tags": ["prometheus"]} + resolved = resolve_pack_for_dashboard(dashboard, RulePackConfig()) + panel = { + "type": "timeseries", + "title": "Disk Space Used Basic", + "fieldConfig": {"defaults": {"unit": "percent"}}, + "targets": [ + { + "expr": "node_filesystem_avail_bytes", + "refId": "A", + "legendFormat": "Disk Used", + } + ], + } + + yaml_panel, result = translate_panel(panel, rule_pack=resolved) + + assert result.status == "migrated" + assert yaml_panel["esql"]["metrics"][0].get("label") == "Disk Used" + + +def test_panel_static_legend_label_rejects_mixed_static_and_dynamic_legends(): + panel = { + "targets": [ + {"legendFormat": "Disk Used"}, + {"legendFormat": "{{ mountpoint }}"}, + ] + } + + assert _panel_static_legend_label(panel) == "" + + +def test_placeholder_label_falls_back_to_title_when_visible_legends_disagree(): + """Fused series cannot pick one target's legend when visible legends differ.""" + yaml_panel = {"esql": {"metrics": [{"field": "value"}]}} + _label_placeholder_value_metric( + yaml_panel, + title="Network Traffic Basic", + legend_format=_panel_static_legend_label( + { + "targets": [ + {"legendFormat": "recv {{device}}"}, + {"legendFormat": "trans {{device}}"}, + ] + } + ), + ) + + assert yaml_panel["esql"]["metrics"][0]["label"] == "Network Traffic Basic" + + def test_find_14091_by_gnet_id(): entry = find_curated_pack(gnet_id=14091, title="", tags=[]) assert entry is not None diff --git a/tests/test_migrate.py b/tests/test_migrate.py index afedb3fd..4869b764 100644 --- a/tests/test_migrate.py +++ b/tests/test_migrate.py @@ -12188,6 +12188,28 @@ def test_multi_by_xy_composites_series_group_breakdown(self): result.reasons, ) + def test_computed_value_breakdown_panel_falls_back_to_panel_title_label(self): + """A scalar-expression panel (``computed_value``) with a breakdown must + get a real y-axis label instead of leaking the synthetic column name + (issue #351). With no legendFormat, the panel title is the fallback, + mirroring the single-target native-PROMQL path's existing behavior.""" + panel = { + "id": 1, + "title": "CPU Idle Percentage", + "type": "timeseries", + "gridPos": {"w": 24, "h": 8, "x": 0, "y": 0}, + "targets": [ + { + "refId": "A", + "expr": '100 - (avg by(instance) (rate(node_cpu_seconds_total{mode="idle"}[5m])) * 100)', + } + ], + } + yaml_panel, _result = self.translate_panel(panel) + esql = yaml_panel["esql"] + self.assertEqual([m.get("field") for m in esql["metrics"]], ["computed_value"]) + self.assertEqual(esql["metrics"][0].get("label"), "CPU Idle Percentage") + def test_xy_job_scope_prefers_instance_breakdown_without_composite_warning(self): warnings = [] panel = panels._build_esql_xy_panel( @@ -13055,6 +13077,17 @@ def test_extract_axis_label_suppresses_opaque_shorthand(self): } self.assertIsNone(extract_axis_config(panel)) + def test_extract_axis_label_suppresses_grafana_unit_id_aliases(self): + from observability_migration.targets.kibana.emit.display import extract_axis_config + panel = { + "fieldConfig": { + "defaults": { + "custom": {"axisLabel": "percentage"}, + } + } + } + self.assertIsNone(extract_axis_config(panel)) + def test_extract_axis_log_scale_modern(self): from observability_migration.targets.kibana.emit.display import extract_axis_config panel = {"fieldConfig": {"defaults": {"custom": {"scaleDistribution": {"type": "log"}}}}} @@ -13204,6 +13237,17 @@ def test_humanize_metric_label_simple_word(self): from observability_migration.targets.kibana.emit.display import humanize_metric_label self.assertIsNone(humanize_metric_label("active")) + def test_humanize_metric_label_excludes_computed_value_placeholder(self): + """``computed_value`` is a synthetic scalar-expression column name, the + general ES|QL translator's counterpart to native PROMQL's ``value`` + (both listed as placeholders in + ``dashboards_api._xy_single_metric_uses_placeholder_name``). Humanizing + it into "Computed Value" would leak the same misleading placeholder + title issue #351 covers, just re-capitalized rather than literal.""" + from observability_migration.targets.kibana.emit.display import humanize_metric_label + self.assertIsNone(humanize_metric_label("computed_value")) + self.assertIsNone(humanize_metric_label("value")) + def test_humanize_metric_label_empty(self): from observability_migration.targets.kibana.emit.display import humanize_metric_label self.assertIsNone(humanize_metric_label(""))