Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
63 changes: 55 additions & 8 deletions docs/sources/grafana.md
Original file line number Diff line number Diff line change
Expand Up @@ -449,6 +449,49 @@ selected.
gap. Controls have no `PanelResult`-style per-item tracking of their own, so
`control_warnings` is dashboard-scoped rather than per-control.

### Interval, Custom, And Other Non-Query Variables (Issue #356)

Grafana `interval` variables (a dropdown of durations, e.g. `20s,1m,5m`) have
no Kibana control equivalent and are intentionally skipped by
`interval_variable_rule` — Kibana's time picker controls the *displayed*
range, which is the variable's most common use. `custom` variables (a static
comma-separated value list) are also skipped by default; if one is referenced
as `$var`/`?var` inside a panel query, `_ensure_param_controls` (issue #131)
synthesizes a binding control after translation, but a `custom` variable never
referenced that way has nothing to bind.

Neither skip is safe when the variable is doing more than that. Dashboard
9852's `RateInterval` is the sharp counter-example: 16 targets use it as the
**rate window** (`rate(node_disk_written_bytes_total[$RateInterval])`), which
has nothing to do with the time picker — Grafana keeps the rate window fixed
at, e.g., `1m` regardless of the displayed range so the line stays smooth. A
duration variable used this way, or any other variable type that ends up with
no control *and* no `?var` binding, is not equivalent to "handled by the time
picker" — it silently hands control of the window to a translator-chosen
substitute. ES|QL panels pick a `TBUCKET` bucket-width; native PROMQL panels
typically inline a fixed range (dashboard 9852's disk panels become
`rate(...[5m])` even when Grafana's current `RateInterval` was `1m`). Neither
tracks the source value, and either can differ from it in either direction.

`translate_dashboard` therefore runs one disclosure pass after every control
has been synthesized (variable translation, `_ensure_param_controls`,
late-bound group controls, `?var` retargeting): for every templating-list
variable that is not bound to a control — checking both the ES|QL
`variable_name` key and the classic options/range `_source_variable_name` key
(a classic control never sets `variable_name`, so looking at that key alone
would falsely flag a working dropdown as dropped) — it checks whether any
panel's *original* PromQL `expr` still references `$var` / `${var}`. If so, it
appends a `control_warnings` entry naming the variable, so the loss is printed
under `CONTROL WARNINGS` and recorded in the JSON report / migration manifest
/ preflight report, matching every other control degradation on this page.
`interval` variables get a specific message calling out both the ES|QL
`TBUCKET` substitute and the native PROMQL fixed-range inline; every other
type gets a generic "referenced but dropped" message. A variable that is
genuinely unused by every panel is never warned about — there is nothing lost
to disclose. This is disclosure only: the variable is not migrated into a
working control (that would require parameterizing the ES|QL duration literal,
which is unverified and out of scope for this fix).

### Variable Label Filters (`metric{label="$var"}` → `?var`)

When a dashboard's templating list defines named variables used in PromQL label
Expand Down Expand Up @@ -498,14 +541,18 @@ target can **bind ES|QL named parameters** for that migration pass:

The split is therefore *per target capability*, never per individual variable.

**Multi-select.** A variable (ES|QL) control binds its selection into a scalar
parameter position (`== ?var` / `RLIKE ?var`), which cannot accept a
multi-value selection, so a Grafana multi-select variable is emitted as a
single-select control. That loss is reported (not silent) as a
`control_warnings` entry (`"variable '<name>' was multi-select in Grafana but
binds a scalar ES|QL parameter in Kibana; emitted a single-select control"`).
Regular options controls, which do not bind a scalar parameter, keep the source
`multi` flag.
**Multi-select.** A Grafana `multi: true` variable stays multi-select in
Kibana. Scalar `== ?var` / `RLIKE ?var` cannot bind an array, so the matcher is
emitted as `MV_CONTAINS(TO_STRING(?var), ".*") OR MV_CONTAINS(TO_STRING(?var), field)`
with `single_select: false`. The `".*"` sentinel preserves Grafana's All option.
`TO_STRING` (issue #353) keeps that guardrail type-safe: Elasticsearch infers
the bound parameter's type from the JSON values Kibana sends, so numeric-looking
options (CPU indices, ports, PIDs) can arrive as an integer array and fail
compile-time type verification against the keyword sentinel/field. Some Kibana
builds still send those options as keyword strings (`["0","1"]`); `TO_STRING` on
an already-keyword value is a no-op, so the wrap is unconditional. Matching is
exact rather than regex (`RLIKE` rejects a computed pattern). Regular options
controls, which do not bind an ES|QL parameter, keep the source `multi` flag.

**Value-list filters.** A `label_values(metric{device!="nbd1"}, device)`
variable restricts its option list to series matching the selector. The
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ panel:
- title_match: "Memory Usage"
esql_query: |
TS metrics-*
| WHERE (MV_CONTAINS(?instance, ".*") OR MV_CONTAINS(?instance, {{control:instance}}))
| WHERE (MV_CONTAINS(TO_STRING(?instance), ".*") OR MV_CONTAINS(TO_STRING(?instance), {{control:instance}}))
| WHERE {{metric:redis_memory_used_bytes:gauge}} IS NOT NULL OR {{metric:redis_memory_max_bytes:gauge}} IS NOT NULL
| STATS used = AVG(LAST_OVER_TIME({{metric:redis_memory_used_bytes:gauge}})), max = AVG(LAST_OVER_TIME({{metric:redis_memory_max_bytes:gauge}})) BY time_bucket = TBUCKET(75, ?_tstart, ?_tend)
| EVAL value = (used / max) * 100.0
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ panel:
- title_match: "Memory Usage"
esql_query: |
TS metrics-*
| WHERE (MV_CONTAINS(?instance, ".*") OR MV_CONTAINS(?instance, {{control:instance}}))
| WHERE (MV_CONTAINS(TO_STRING(?instance), ".*") OR MV_CONTAINS(TO_STRING(?instance), {{control:instance}}))
| WHERE {{metric:redis_memory_used_bytes:gauge}} IS NOT NULL OR {{metric:redis_memory_max_bytes:gauge}} IS NOT NULL
| STATS used = AVG(LAST_OVER_TIME({{metric:redis_memory_used_bytes:gauge}})), max = AVG(LAST_OVER_TIME({{metric:redis_memory_max_bytes:gauge}})) BY time_bucket = TBUCKET(75, ?_tstart, ?_tend)
| EVAL value = (used / max) * 100.0
Expand All @@ -104,7 +104,7 @@ panel:
esql_query: |
TS metrics-*
| WHERE @timestamp >= ?_tstart AND @timestamp <= ?_tend
| WHERE (MV_CONTAINS(?instance, ".*") OR MV_CONTAINS(?instance, {{control:instance}}))
| WHERE (MV_CONTAINS(TO_STRING(?instance), ".*") OR MV_CONTAINS(TO_STRING(?instance), {{control:instance}}))
| WHERE {{metric:redis_net_input_bytes_total:counter}} IS NOT NULL OR {{metric:redis_net_output_bytes_total:counter}} IS NOT NULL
| STATS input = SUM(RATE({{metric:redis_net_input_bytes_total:counter}})), output = SUM(RATE({{metric:redis_net_output_bytes_total:counter}})) BY time_bucket = TBUCKET(2 minute)
| KEEP time_bucket, input, output
Expand All @@ -114,7 +114,7 @@ panel:
esql_query: |
TS metrics-*
| WHERE @timestamp >= ?_tstart AND @timestamp <= ?_tend
| WHERE (MV_CONTAINS(?instance, ".*") OR MV_CONTAINS(?instance, {{control:instance}}))
| WHERE (MV_CONTAINS(TO_STRING(?instance), ".*") OR MV_CONTAINS(TO_STRING(?instance), {{control:instance}}))
| WHERE {{metric:redis_keyspace_hits_total:counter}} IS NOT NULL OR {{metric:redis_keyspace_misses_total:counter}} IS NOT NULL
| STATS hits = AVG(IRATE({{metric:redis_keyspace_hits_total:counter}})), misses = AVG(IRATE({{metric:redis_keyspace_misses_total:counter}})) BY time_bucket = TBUCKET(2 minute), labels.instance
| KEEP time_bucket, `labels.instance`, hits, misses
Expand All @@ -124,7 +124,7 @@ panel:
esql_query: |
TS metrics-*
| WHERE @timestamp >= ?_tstart AND @timestamp <= ?_tend
| WHERE (MV_CONTAINS(?instance, ".*") OR MV_CONTAINS(?instance, {{control:instance}}))
| WHERE (MV_CONTAINS(TO_STRING(?instance), ".*") OR MV_CONTAINS(TO_STRING(?instance), {{control:instance}}))
| WHERE {{metric:redis_commands_duration_seconds_total:counter}} IS NOT NULL OR {{metric:redis_commands_total:counter}} IS NOT NULL
| STATS dur = SUM(IRATE({{metric:redis_commands_duration_seconds_total:counter}})), cnt = SUM(IRATE({{metric:redis_commands_total:counter}})) BY time_bucket = TBUCKET(2 minute), labels.cmd
| EVAL computed_value = (dur / cnt)
Expand All @@ -135,7 +135,7 @@ panel:
esql_query: |
TS metrics-*
| WHERE @timestamp >= ?_tstart AND @timestamp <= ?_tend
| WHERE (MV_CONTAINS(?instance, ".*") OR MV_CONTAINS(?instance, {{control:instance}}))
| WHERE (MV_CONTAINS(TO_STRING(?instance), ".*") OR MV_CONTAINS(TO_STRING(?instance), {{control:instance}}))
| WHERE {{metric:redis_commands_duration_seconds_total:counter}} IS NOT NULL
| STATS redis_commands_duration_seconds_total = SUM(IRATE({{metric:redis_commands_duration_seconds_total:counter}})) BY time_bucket = TBUCKET(2 minute), labels.cmd
| KEEP time_bucket, `labels.cmd`, redis_commands_duration_seconds_total
Expand Down
116 changes: 115 additions & 1 deletion observability_migration/adapters/source/grafana/panels.py
Original file line number Diff line number Diff line change
Expand Up @@ -8431,8 +8431,10 @@ def _query_param_names(query):

_ESQL_FIELD_CONTROL_RE = re.compile(r"\?\?(?P<name>[A-Za-z][A-Za-z0-9_]*)")
_ESQL_VALUE_PARAM_FIELD_PATTERNS = (
# ``?var`` may be wrapped in ``TO_STRING(...)`` (issue #353's multi-select
# guardrail type-fix); match with or without that wrapper.
lambda name: re.compile(
rf"MV_CONTAINS\(\s*\?{re.escape(name)}\s*,\s*(?P<field>`[^`]+`|[A-Za-z_][A-Za-z0-9_.]*)\s*\)"
rf"MV_CONTAINS\(\s*(?:TO_STRING\(\s*)?\?{re.escape(name)}\s*\)?\s*,\s*(?P<field>`[^`]+`|[A-Za-z_][A-Za-z0-9_.]*)\s*\)"
),
lambda name: re.compile(
rf"(?P<field>`[^`]+`|[A-Za-z_][A-Za-z0-9_.]*)\s+(?:RLIKE|LIKE|==|!=|>=|<=|>|<)\s+\?{re.escape(name)}\b"
Expand Down Expand Up @@ -8928,6 +8930,115 @@ def _repeat_variable_name(value):
_VARIABLE_REFERENCE_RE = re.compile(r"\$\{([A-Za-z_][A-Za-z0-9_]*)(?::[^}]*)?\}|\$([A-Za-z_][A-Za-z0-9_]*)")


def _variable_names_referenced_in_panels(panels) -> set[str]:
"""Grafana variable names referenced by any panel target's raw PromQL.

Scans ``$var`` / ``${var}`` / ``${var:fmt}`` tokens in each target's
``expr`` *before* any translation-time rewrite, so it reflects what the
source dashboard actually used the variable for -- independent of
whether the migrated query still carries an equivalent reference. Used
by :func:`_disclose_dropped_referenced_variables` (issue #356) to tell a
variable that is genuinely unused from one whose loss changes query
behavior.
"""
names: set[str] = set()
for panel in panels or []:
if not isinstance(panel, dict):
continue
targets = panel.get("targets")
if not isinstance(targets, list):
continue
for target in targets:
if not isinstance(target, dict):
continue
expr = target.get("expr")
if not isinstance(expr, str) or not expr:
continue
for match in _VARIABLE_REFERENCE_RE.finditer(expr):
name = match.group(1) or match.group(2)
if name:
names.add(name)
return names


def _disclose_dropped_referenced_variables(variables, controls, panels, control_warnings):
"""Warn when a variable referenced by panel queries never became a
control (issue #356).

``interval`` variables are the sharpest case: Grafana's own docs and this
codebase's ``interval_variable_rule`` frame them as "handled by Kibana's
time picker", but a variable used as a rate/range-vector window
(``rate(x[$RateInterval])``) has nothing to do with the displayed time
range. Dropping it does not just remove a dropdown -- it silently hands
control of the rate window to a translator-chosen substitute: ES|QL
panels use the TBUCKET bucket-width heuristic, and native PROMQL panels
typically inline a fixed range (e.g. ``[5m]``). Neither tracks the
Grafana value (AGENTS.md: degrade gracefully, do not hide a semantic
gap).

Generalised to any Grafana variable type that ends up with no bound
control, since the same silent loss applies to any of them -- e.g. a
``query`` variable hidden with ``hide: 2`` skips straight past
``query_variable_rule`` with no control and no warning today. Must run
after every control-synthesis pass (``_ensure_param_controls``,
late-bound group controls, ``?var`` retargeting) so a variable that one
of those passes did bind is correctly excluded. Also skips a variable
that some earlier pass already named in a ``control_warnings`` entry
(for example ``query_variable_rule``'s "could not resolve source field"
or ``textbox_variable_rule``'s "no direct Kibana control equivalent") --
that is already disclosed, just in more specific language, and a second
generic entry would only add noise.
"""
if control_warnings is None:
return
# A control's owning variable name is ``variable_name`` for ES|QL
# parameter-binding controls, but a classic (non-ESQL) options/range
# control -- built directly from ``context.control`` in
# ``query_variable_rule`` and friends -- never sets that key; it only
# gets ``_CONTROL_SOURCE_VARIABLE_NAME`` attached afterwards in
# ``translate_variables``. Checking only ``variable_name`` here would
# falsely flag every variable that resolved to a working classic control
# as "dropped". Mirrors ``_covered_control_variable_refs``'s lookup.
bound_names = {
name
for control in controls or []
if isinstance(control, dict)
for name in (
control.get("variable_name"),
control.get(_CONTROL_SOURCE_VARIABLE_NAME),
)
if name
}
referenced = _variable_names_referenced_in_panels(panels)
for variable in variables or []:
if not isinstance(variable, dict):
continue
name = variable.get("name")
if not name or name in bound_names or name not in referenced:
continue
if any(f"'{name}'" in warning for warning in control_warnings):
continue
var_type = variable.get("type") or "unknown"
if var_type == "interval":
control_warnings.append(
f"variable '{name}' (type 'interval') is used by panel queries as a "
f"rate/range window (e.g. '[${name}]') but was dropped during migration "
"-- no Kibana control was emitted, and Kibana's time picker only "
"controls the displayed range, not this window. The migrated query no "
"longer uses the Grafana interval: ES|QL panels pick a TBUCKET "
"bucket-width, and native PROMQL panels typically inline a fixed range "
"(e.g. [5m]), either of which can be narrower or wider than the source "
"value"
)
else:
control_warnings.append(
f"variable '{name}' (type '{var_type}') is referenced by panel queries "
"but was dropped during migration -- no Kibana control or query "
"parameter was emitted for it, so it no longer has any effect on "
"query behavior"
)


def _resolve_variable_values(variable: dict) -> tuple[list[str], str]:
"""Return ``(values, source)`` for a Grafana templating variable.

Expand Down Expand Up @@ -10495,6 +10606,9 @@ def translate_dashboard(dashboard, datasource_index="metrics-*", esql_index=None
control_warnings=result.control_warnings,
)
controls = _retarget_esql_param_controls_to_panel_bindings(controls, flat_panels)
_disclose_dropped_referenced_variables(
variables, controls, all_panels, result.control_warnings
)
rewritten_panel_results = _rewrite_variable_warnings(
result.panel_results,
_covered_control_variable_refs(controls),
Expand Down
17 changes: 15 additions & 2 deletions observability_migration/adapters/source/grafana/promql.py
Original file line number Diff line number Diff line change
Expand Up @@ -1747,16 +1747,29 @@ def _mv_contains_filter(label, param_name, negate=False, allow_empty_match_all=F
preference: ES|QL ``RLIKE`` requires a literal pattern and rejects a
computed one, so ``RLIKE MV_CONCAT(?var, "|")`` -- which would have
rebuilt Grafana's own ``(a|b)`` alternation -- is not expressible.

``?param`` is wrapped in ``TO_STRING(...)`` (issue #353): Elasticsearch
infers a bound ES|QL parameter's type from the JSON values Kibana sends,
not from the control's keyword-typed option-list query. A variable whose
values happen to look numeric (CPU/core indices, ports, PIDs, status
codes) can bind ``?param`` as an integer array; ``MV_CONTAINS`` requires
both arguments to share a type, so the ``".*"`` sentinel (keyword) and
the keyword label field both fail to type-check against it -- a
compile-time verification error, not a runtime one, so it fails the whole
query. Some Kibana versions send those same options as keyword strings
(``["0", "1"]``); ``TO_STRING`` on an already-keyword parameter is a
no-op, so wrapping unconditionally is safe either way.
"""
param = f"TO_STRING(?{param_name})"
clauses = []
if allow_empty_match_all:
# Kibana leaves an unselected multi-values control bound as an empty
# list. For Grafana includeAll variables that empty state must behave
# like the source default All selection ([".*"]), not like "match
# nothing" which blanks the dashboard on first load.
clauses.append(f"MV_COUNT(?{param_name}) == 0")
clauses.append(f'MV_CONTAINS(?{param_name}, ".*")')
clauses.append(f"MV_CONTAINS(?{param_name}, {label})")
clauses.append(f'MV_CONTAINS({param}, ".*")')
clauses.append(f"MV_CONTAINS({param}, {label})")
expr = "(" + " OR ".join(clauses) + ")"
return f"NOT {expr}" if negate else expr

Expand Down
7 changes: 6 additions & 1 deletion observability_migration/core/verification/parity_oracle.py
Original file line number Diff line number Diff line change
Expand Up @@ -1180,7 +1180,12 @@ def _exact_control_param_names(esql: str) -> set[str]:



_MV_CONTAINS_PARAM_RE = re.compile(r"MV_CONTAINS\s*\(\s*\?([A-Za-z_][A-Za-z0-9_]*)", re.IGNORECASE)
_MV_CONTAINS_PARAM_RE = re.compile(
# ``?var`` may be wrapped in ``TO_STRING(...)`` (issue #353's multi-select
# guardrail type-fix); match with or without that wrapper.
r"MV_CONTAINS\s*\(\s*(?:TO_STRING\s*\(\s*)?\?([A-Za-z_][A-Za-z0-9_]*)",
re.IGNORECASE,
)


def _mv_contains_param_names(esql: str) -> set[str]:
Expand Down
Loading
Loading