Skip to content

fix(prometheus): accept selector strings in label matches - #1340

Open
matheusleite-plank wants to merge 2 commits into
grafana:mainfrom
matheusleite-plank:fix/prometheus-selector-strings
Open

matheusleite-plank wants to merge 2 commits into
grafana:mainfrom
matheusleite-plank:fix/prometheus-selector-strings

Conversation

@matheusleite-plank

Copy link
Copy Markdown

What this changes

list_prometheus_label_values and list_prometheus_label_names reject Prometheus selector strings in matches. LLM clients often send the PromQL form ("matches": ["{job=\"api\"}"]) instead of the {"filters": [...]} object, and the call fails with:

invalid arguments: json: cannot unmarshal string into ListPrometheusLabelValuesParams.matches.0 of type tools.Selector

This adds Selector.UnmarshalJSON, which accepts either form. A JSON string is parsed with the existing parseSelectorStrings (already used for label_selectors in the alerting tools), so it handles missing braces, unquoted values and all four matcher operators the same way. Objects decode exactly as before, and null keeps its previous behavior.

The advertised input schema is unchanged: tools/list output is identical before and after (79 tools), so there is no token cost.

How you tested it

  • New TestSelectorUnmarshalJSON in tools/prometheus_unit_test.go: selector string, string without braces, =~ / !~ operators, filters object, null, invalid string, and a full ListPrometheusLabelValuesParams decode.
  • go test -tags unit ./..., gofmt -l ., golangci-lint run (v2.14.0, 0 issues), lint-jsonschema and lint-openapi pass.
  • End to end with the Docker image over streamable HTTP, against Grafana 12.2 with a Prometheus datasource, main vs this branch:
matches main This branch
["{job=\"prometheus\"}"] invalid arguments error ["prometheus"]
[{"filters":[{"name":"job","value":"prometheus","type":"="}]}] ["prometheus"] ["prometheus"]
["{job=\"nope\"}"] invalid arguments error []
["not a selector{"] invalid arguments error invalid arguments: invalid label selector ...
list_prometheus_label_names with a string selector invalid arguments error label list

Checklist

  • make lint and make test-unit pass locally
  • Commits are signed (required to merge — see CONTRIBUTING.md)
  • CLA signed (required to merge)
  • README tool table updated, with RBAC permissions and scopes, if this adds a tool
  • Any write operation respects --disable-write
  • Any datasource query execution respects --disable-query (and --disable-write too, if the query is passed through unfiltered)

@matheusleite-plank
matheusleite-plank requested a review from a team as a code owner October 15, 2026 18:44
@cla-assistant

cla-assistant Bot commented Oct 15, 2026 •

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

Comment thread tools/prometheus.go
if err := json.Unmarshal(data, &selector); err != nil {
return err
}
parsed, err := parseSelectorStrings([]string{selector})

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] Parse valid Prometheus selectors before applying tolerant normalization

Routing every string through parseSelectorStrings rejects valid selectors because that helper normalizes label-filter syntax before parsing:

  • up becomes {up}, and http_requests_total{job="api"} becomes {http_requests_total{job="api"}}. Both fail to parse.
  • {path="/search?q=abc"} becomes {path="/search?q="abc""} because the unquoted-value repair also matches = inside an already quoted value.

I reproduced these at 48207c4 using the new Selector decoder and verified that Prometheus's ParseMetricSelector accepts the original inputs. Since both label tools decode matches through this method, these valid string inputs still fail before reaching the backend.

Could we parse the original string first and apply tolerant normalization only if strict parsing fails? Regression tests should include a bare metric name, a metric-prefixed selector, and a quoted label value containing =. The existing focused tests pass but do not cover these cases.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, confirmed all three. The decoder now parses the original string with promqlParser.ParseMetricSelector first and only falls back to the tolerant parseSelectorStrings when strict parsing fails. Added regression tests for a bare metric name, a metric-prefixed selector, and = inside a quoted value, plus an unquoted-value case to cover the fallback.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants