prometheus / prometheus

The Prometheus monitoring system and time series database.
https://prometheus.io/
Apache License 2.0
55.6k stars 9.14k forks source link

promql: setting multiple regexp's for `__name__` doesn't work anymore #13596

Closed muff1nman closed 8 months ago

muff1nman commented 8 months ago

What did you do?

The following promql is a valid expression in previous versions of promtool and when evaluated via prometheus:

groups:
  - name: kube-resources.rules
    rules:
      - record: ksm:resource_count
        expr: |
          count by (job, ksm_resource, namespace) (
            max without (instance) (
              label_replace(
                {__name__!~"kube_pod_.*",__name__=~"kube_.*_info",job="~kube-state-metrics",namespace!=""},
                "ksm_resource",
                "$1",
                "__name__",
                "kube_(.*)_info"
              )
            )
          )

What did you expect to see?

promtool check rules should successfully validate the given rules.

What did you see instead? Under which circumstances?

However with the latest promtool this expression is no longer validating succesfully:

$ promtool check rules ~/Documents/kube-resource.yaml 
Checking /home/ademaria/Documents/kube-resource.yaml
  FAILED:
/home/ademaria/Documents/kube-resource.yaml: 5:15: group "kube-resources.rules", rule 1, "ksm:resource_count": could not parse expression: 4:7: parse error: metric name must not be set twice: "kube_pod_.*" or "kube_.*_info"

System information

No response

Prometheus version

No response

Prometheus configuration file

No response

Alertmanager version

No response

Alertmanager configuration file

No response

Logs

No response

tjhop commented 8 months ago

Noticed this as well, and it seems this was an explicit change as part of UTF-8 support and some promql grammar/exposition format changes:

https://github.com/prometheus/prometheus/commit/a28d7865ad#diff-0accfa26d7e8bfecc27f0c77f5f7ecc5d2417cf16edfa7b8ca9d9ab5b822aff0R794-R805

https://github.com/prometheus/prometheus/issues/13095

beorn7 commented 8 months ago

Oops, yeah, that was introduced in https://github.com/prometheus/prometheus/pull/13271 . @ywwg there are legitimate uses of specifying the name multiple times inside the braces (to apply multiple regexp's). I'm afraid this will not only break promtool but the expression parsing in general.

beorn7 commented 8 months ago

Verified that this breaks parsing those expressions in general. @ywwg could you have a look here?

ywwg commented 8 months ago

I think the fix here would be that the metric name cannot exist twice in the same selector for the equality operator -- can't do ="foo" and ="bar". OR, do we just want to remove the restriction completely and if someone does foo{"bar"} (two metric name equality selectors with different values), we just let them?

ywwg commented 8 months ago

looking at the code, I think it did already check for foo{__name__="bar"}, but did not check for something like {__name__="foo",__name__="bar"}