[prometheus-kube-stack] New cAdvisorMetricRelabelings are a bit too restrictive by default

[dotdc]

Describe the bug a clear and concise description of what the bug is.

Hi,

I think two of the new rules introduced in 37.0.0 by @SuperQ are a bit too restrictive to be enabled by default.

Rules concerned :

      # Drop cgroup metrics with no pod.
      - sourceLabels: [id, pod]
        action: drop
        regex: '.+;'
      # Drop cgroup metrics with no container.
      - sourceLabels: [id, container]
        action: drop
        regex: '.+;'

Theses 2 rules drop every metrics without a pod and/or a container.
This will make some basic queries like container_cpu_usage_seconds_total{id="/"} to stop working out of the box.
I would at least comment theses two to limit the number of impacted users.

What do you think?

What's your helm version?

3.9.0

What's your kubectl version?

1.24.2

Which chart?

prometheus-kube-stack

What's the chart version?

37.2.0

What happened?

No response

What you expected to happen?

No response

How to reproduce it?

No response

Enter the changed values of values.yaml?

No response

Enter the command that you execute and failing/misfunctioning.

container_cpu_usage_seconds_total{id="/"}

Anything else we need to know?

No response

[SuperQ]

Adding my comments from the PR:

These are mostly generated by systemd cgroups, which can be pretty noisy for Kubernetes users. This is why I added them as the default.

The goal here is to monitor Kubernetes resources.

Also, I'm not sure what use of monitoring the CPU of the root cgroup is, when node_cpu_seconds_total covers that.

[dotdc]

Good point, can't remember why I didn't used node_cpu_seconds_total to calculate global cluster usage, will check again and update this.

[BeckYeh]

Becasue this metrics used in prometheus-adapter for nodes resource query（ kubectl top nodes ）.
So by default I hope don't add this filter.

[SuperQ]

Ooof, the prometheus-adapter seems to have a bunch of incorrect queries built in. Those should be fixed.

[SuperQ]

kubernetes-sigs/prometheus-adapter#516 should fix up the prometheus-adapter bug.

[dotdc]

Thank you @SuperQ, I replaced all my affected queries using node_cpu_seconds_total instead of container_cpu_usage_seconds_total and LGTM.

Can I close the issue or should we wait to see if other related charts and projects are using similar queries?

[SuperQ]

I think we can close it. Other projects can find this issue for advice on how to fix their query use.