search

kubernetes-monitoring / kubernetes-mixin

A set of Grafana dashboards and Prometheus alerts for Kubernetes.
Apache License 2.0
2.12k stars 597 forks source link

feat: add suppport for empty cadvisorSelector #944

Closed lorenzofelletti closed 1 month ago

lorenzofelletti commented 5 months ago

Add support for cadvisorSelector being empty without the resulting generated queries having invalid syntax like {, label="something"}. That could also be easily extended to support more labels being empty.

povilasv commented 2 months ago

I guess question from my side what is the use case of optional selector? I feel like "job" label is almost mandatory concept in Prometheus, so I'm not sure what are we solving here?

lorenzofelletti commented 2 months ago

The thing I'm trying to solve is when I have cases in which I don't know which value this is, because different clusters use different values. If that was a regex for example, I could have set it to something like .+, but this is not. So in my case I set it to empty, and what happens is that the resulting expressions have invalid syntaxes, such as double commas ({,,something="value"}) for example.

Although I do agree that this PR is probably not the best way to solve the issue. I am happy to close this, and open an issue to generally discuss the possibility to add support for empty selectors without generating invalid syntaxes in output.

povilasv commented 2 months ago

I don't really think we should support empty selectors, you are working on data and you should know where your data should come from.

If you are not you might be mixing matching data you shouldn't. If you have two jobs scraping cadvisor, the dashboards will look weird

lorenzofelletti commented 2 months ago

what I meant was that each individual cluster has is own key=value pair, and I'd like to generate the rules only once, without having N different configuration for the N different combination of that pair.

povilasv commented 2 months ago

Not sure if this is common use case, and if we want to complicate our side to support your inconsitent clusters.

Why each individual cluster has their own key=value pair?

lorenzofelletti commented 2 months ago

Why each individual cluster has their own key=value pair?

I don't really have a good answer for that...