Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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
13 changes: 13 additions & 0 deletions docs/sources/grafana.md
Original file line number Diff line number Diff line change
Expand Up @@ -980,6 +980,19 @@ Use that doc for:
is a loud 400 on this target ("regex label selectors on __name__ are not
supported") and is left on the native path for that reason; exact
`{__name__="A"} / B` is detected and rerouted.
- **PromQL `#` comments are removed before anything reads the expression.** A
comment runs to the end of its line, and both translation paths collapse
whitespace, so comments are stripped first, while that extent is still exact.
Otherwise a comment swallowed the rest of the expression and the truncated
text became the emitted query — Kibana reported "Couldn't parse Elasticsearch
ES|QL query" on a panel the run had scored as migrated. A `#` inside a string
literal is a label value, not a comment, and is preserved in all three PromQL
string forms (`"…"`, `'…'`, and backquoted `` `…` ``). Comment text never
decides routing either: prose mentioning `or`, `topk(` or
`histogram_quantile(` no longer disqualifies a panel from the native path, and
a comment can no longer hide a distinct-metric ratio from the vector-matching
rule above. An expression that is nothing but comments has no query to emit
and is reported `not_feasible` rather than shipping an empty selector.
- **Range-vector windows and counter typing.** Passing `--es-url` adds
validation and schema discovery; it does not change which translation strategy
a range-vector panel gets. `rate()` / `irate()` / `increase()` stay on the
Expand Down
58 changes: 51 additions & 7 deletions observability_migration/adapters/source/grafana/panels.py
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,7 @@
_parse_fragment,
_safe_alias,
_split_top_level_csv,
_strip_promql_comments,
_summary_mode_from_metadata,
_union_group_fields,
_unique_safe_alias,
Expand Down Expand Up @@ -1773,6 +1774,11 @@ def _clean_promql_for_native_with_state(
callers that still need to parse the expression must clean without this flag.
"""
had_bare_variable = False
# Comments go first, while their end-of-line extent is still intact: the
# whitespace collapse at the end of this function would otherwise let a
# comment swallow the rest of the expression and emit that as the native
# query (issue #443).
expr = _strip_promql_comments(expr)
expr = substitute_grafana_range_macros(expr)
# #273: a Grafana adaptive-window macro on rate()/increase() means "size the
# lookback to the view", so drop the window and let the native PROMQL command
Expand Down Expand Up @@ -2387,6 +2393,18 @@ def can_use_native_promql(promql_expr, runtime_features=None):
"""Return True if the expression is within the server-supported PromQL subset."""
if not promql_expr or not promql_expr.strip():
return False
# Every gate below asks a question about query *structure*, so none of them
# may read comment prose: on the raw expression a comment mentioning
# ``or`` / ``topk(`` / ``histogram_quantile(`` / ``{}`` / ``$var`` tripped
# the matching gate and needlessly degraded a native-able panel, while the
# gates that clean first (notably the issue-#376 vector-matching guard)
# analysed text the comment had already truncated and so missed the
# construct they exist to refuse (issue #443).
promql_expr = _strip_promql_comments(promql_expr)
if not promql_expr.strip():
# Nothing but comments: there is no query to emit, so decline instead of
# building ``value=(# note)``, which Kibana rejects at parse time.
return False
if (
_promql_label_matcher_has_template_variable(promql_expr)
and not is_feature_supported(runtime_features, PROMQL_LABEL_MATCHER_PARAMS)
Expand Down Expand Up @@ -2730,7 +2748,16 @@ def _translate_panel_native_promql(
return None

target = targets_with_expr[0][0]
expr = target.get("expr", "")
# ``raw_expr`` is what the operator authored in Grafana and is reported back
# to them verbatim as "Original query"; ``expr`` is the structural form.
# Everything below treats ``expr`` as structure — or-collapsing, the empty
# selector guard, live metric discovery, label recording and the routing
# gate — so comment prose would otherwise be read as query text: a comment
# naming a metric made live discovery report it missing, and one containing
# ``{}`` tripped the empty-selector guard, each declining native emission
# with a note describing something the query never did (issue #443).
raw_expr = target.get("expr", "")
expr = _strip_promql_comments(raw_expr)
collapsed_expr = collapse_or_for_native_promql(
expr, resolver=resolver, rule_pack=rule_pack
)
Expand All @@ -2742,9 +2769,16 @@ def _translate_panel_native_promql(
"side only when the left lacks samples",
)
expr = collapsed_expr
expr = _strip_ignored_promql_label_matchers(
expr, getattr(rule_pack, "ignored_labels", None)
)
# A rewritten expression has no comment-preserving form, and the two
# rewrites below already report themselves: this one through the note
# just above, the label strip through the rule pack. Carrying them into
# ``raw_expr`` too keeps the recorded source exactly what it has always
# been apart from the comment, so parity comparisons still execute the
# expression the panel actually migrated.
raw_expr = collapsed_expr
ignored_labels = getattr(rule_pack, "ignored_labels", None)
expr = _strip_ignored_promql_label_matchers(expr, ignored_labels)
raw_expr = _strip_ignored_promql_label_matchers(raw_expr, ignored_labels)
if _PROMQL_EMPTY_METRICLESS_SELECTOR_RE.search(_strip_promql_string_literals(expr)):
# Stripping an ignored label removed the sole matcher, leaving an empty
# metricless selector (``{}``) — invalid PromQL. Decline native
Expand Down Expand Up @@ -3030,7 +3064,12 @@ def _translate_panel_native_promql(

query_ir = QueryIR()
query_ir.source_language = "promql"
query_ir.source_expression = expr
# The source expression is provenance, not structure: it is what the
# operator compares against their Grafana panel, so it keeps the comment
# the structural passes above had to drop. ``clean_expression`` beside it
# is the cleaned counterpart, and the ES|QL path records the raw text the
# same way.
query_ir.source_expression = raw_expr
query_ir.clean_expression = cleaned_expr
query_ir.panel_type = panel_type
query_ir.datasource_type = datasource.get("type", "")
Expand Down Expand Up @@ -3060,7 +3099,8 @@ def _translate_panel_native_promql(
kibana_type,
"migrated_with_warnings" if metric_map_note else "migrated",
confidence,
promql_expr=expr,
# Reported to the operator as "Original query"; see ``raw_expr`` above.
promql_expr=raw_expr,
# Record the *emitted* panel query, not the bare ``PROMQL …`` command.
# Gauge/metric native panels append a trailing ``| EVAL _gauge_*`` (or
# other constants) to ``native_panel["query"]`` after
Expand Down Expand Up @@ -3116,7 +3156,11 @@ def _translate_multi_target_native_promql(
target_fragments = []

for target, _ in targets_with_expr:
expr = target.get("expr", "")
# Comment-blind for the same reason as the single-target path above.
# Unreachable while the combiner is disabled by the early return, so
# this is here to keep the bug from returning with the feature rather
# than to change behavior today.
expr = _strip_promql_comments(target.get("expr", ""))
runtime_features = getattr(rule_pack, "runtime_features", {})
_record_passthrough_native_labels(expr, resolver)
if (
Expand Down
69 changes: 69 additions & 0 deletions observability_migration/adapters/source/grafana/promql.py
Original file line number Diff line number Diff line change
Expand Up @@ -1012,6 +1012,63 @@ def _grafana_param_name(value: str) -> str | None:
return name or None


def _strip_promql_comments(expr):
"""Remove PromQL ``#`` comments, preserving each comment's newline.

A comment runs from an unquoted ``#`` to the end of its line, so it has to
go while that extent is still exact — before any caller collapses newlines.
Both cleaning entry points flatten whitespace, and doing that first let a
comment swallow the rest of the expression: on the native path the truncated
text became the emitted query, and on the ES|QL path comment prose was read
as query structure (issue #443).

The newline itself is kept so the remaining operands stay separated once
whitespace is collapsed; dropping it would join ``sum(a)`` and ``+ sum(b)``
into different text than Prometheus sees.

A ``#`` inside a string literal is a label value, not a comment, so the scan
tracks quote state across all three PromQL string forms: double, single, and
backquoted raw. The regex-based ``_strip_promql_string_literals`` helpers
cannot stand in here because they do not know the backquoted form. A
backslash is treated as an escape inside every form, matching
``promql-parser``; that also errs toward staying in string state, which
keeps text rather than deleting it.
"""
text = str(expr or "")
if "#" not in text:
return text
out = []
quote = ""
index = 0
length = len(text)
while index < length:
char = text[index]
if quote:
out.append(char)
if char == "\\" and index + 1 < length:
out.append(text[index + 1])
index += 2
continue
if char == quote:
quote = ""
index += 1
continue
if char in "\"'`":
quote = char
out.append(char)
index += 1
continue
if char == "#":
newline = text.find("\n", index)
if newline == -1:
break
index = newline
continue
out.append(char)
index += 1
return "".join(out)


def substitute_grafana_range_macros(expr):
"""Expand Grafana range macros before generic template-variable handling."""
result = expr
Expand Down Expand Up @@ -1130,6 +1187,12 @@ def _normalize_count_scalar(expr):
def preprocess_grafana_macros(expr, rule_pack=None):
"""Replace Grafana-specific macros with valid PromQL placeholders."""
default_window = (rule_pack.default_rate_window if rule_pack else "5m") or "5m"
# Comments first, while their end-of-line extent is still exact. Everything
# downstream treats this result as query structure — the complexity
# classifier and warning patterns scan it, so a comment merely *mentioning*
# ``predict_linear`` used to raise "predict_linear has no ES|QL equivalent"
# against a plain ``sum(rate(...))`` (issue #443).
expr = _strip_promql_comments(expr)
expr = _normalize_count_scalar(expr)
# Grafana's dynamic step macros ($__interval / $__rate_interval /
# $__auto_interval_* / $interval) resolve at render time from the selected
Expand Down Expand Up @@ -1588,6 +1651,12 @@ def template_vars_in_label_selectors(expr):
def classify_promql_complexity(expr, rule_pack=None):
"""Classify a PromQL expression's translation complexity."""
rule_pack = rule_pack or RulePackConfig()
# The rule-pack patterns match function and operator names anywhere in the
# text, so a comment that merely names an untranslatable construct would be
# reported as if the expression used it (issue #443). Callers inside the
# pipeline pass an already-cleaned expression; this keeps the exported
# helper honest for the ones that do not.
expr = _strip_promql_comments(expr)
for rule in rule_pack.not_feasible_patterns:
if re.search(rule.pattern, expr, re.IGNORECASE):
return "not_feasible", rule.reason
Expand Down
Loading
Loading