prometheus-community / helm-charts

Prometheus community Helm charts
Apache License 2.0
5.04k stars 5k forks source link

[prometheus] v15.* - labels modified for metrics #1613

Closed kaajrot closed 2 years ago

kaajrot commented 2 years ago

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

Change in label names for metrics

target_label: namespace used to be kubernetes_namespace and in the metrics, the actual namespace now shows as exported_namespace. This has led to queries on grafana failing.

What's your helm version?

version.BuildInfo{Version:"v3.7.0", GitCommit:"eeac83883cb4014fe60267ec6373570374ce770b", GitTreeState:"clean", GoVersion:"go1.17"}

What's your kubectl version?

Client Version: version.Info{Major:"1", Minor:"22", GitVersion:"v1.22.2", GitCommit:"8b5a19147530eaac9476b0ab82980b4088bbc1b2", GitTreeState:"clean", BuildDate:"2021-09-15T21:31:32Z", GoVersion:"go1.16.8", Compiler:"gc", Platform:"darwin/amd64"}

Which chart?

https://github.com/prometheus-community/helm-charts/releases/tag/prometheus-15.0.1

What's the chart version?

15.0.1

What happened?

On upgrading chart from 14.9.2 to 15.0.1, the metrics have changed and the target label is being modified

target_label: namespace used to be kubernetes_namespace and in the metrics, the actual namespace now shows as exported_namespace. This has led to queries on grafana failing.

How can this be mitigated or resolved?

example output with 15.0.1 -

kube_pod_container_info{app_kubernetes_io_component="metrics", app_kubernetes_io_instance="prometheus", app_kubernetes_io_managed_by="Helm", app_kubernetes_io_name="kube-state-metrics", app_kubernetes_io_part_of="kube-state-metrics", app_kubernetes_io_version="2.2.4", container="autoscaler", container_id="containerd://9572ee0f3400b5b49ddbe3005ea983a637d08fbe792863e845d29a27b3eb9a32", exported_namespace="kube-system", helm_sh_chart="kube-state-metrics-4.0.2", image="mcr.microsoft.com/oss/kubernetes/autoscaler/cluster-proportional-autoscaler:1.8.3", image_id="sha256:cfb93326278fdad0d75931baef3412cb5eb59ab292bd993d50a448ef2e5eadac", instance="10.1.192.202:8080", job="kubernetes-service-endpoints", namespace="kube-infrastructure", node="aks-system210426-28105498-vmss000003", pod="coredns-autoscaler-5f85dc856b-wp76j", service="prometheus-kube-state-metrics", uid="aafd66fc-e6f3-4b48-9123-d8811c2f268f"}

example output with 14.9.2 -

kube_pod_container_info{app_kubernetes_io_instance="prometheus", app_kubernetes_io_managed_by="Helm", app_kubernetes_io_name="kube-state-metrics", container="autoscaler", container_id="containerd://9572ee0f3400b5b49ddbe3005ea983a637d08fbe792863e845d29a27b3eb9a32", helm_sh_chart="kube-state-metrics-3.5.2", image="mcr.microsoft.com/oss/kubernetes/autoscaler/cluster-proportional-autoscaler:1.8.3", image_id="sha256:cfb93326278fdad0d75931baef3412cb5eb59ab292bd993d50a448ef2e5eadac", instance="10.1.192.190:8080", job="kubernetes-service-endpoints", kubernetes_name="prometheus-kube-state-metrics", kubernetes_namespace="kube-infrastructure", kubernetes_node="aks-system210426-28105498-vmss000003", namespace="kube-system", pod="coredns-autoscaler-5f85dc856b-wp76j", uid="aafd66fc-e6f3-4b48-9123-d8811c2f268f"}

What you expected to happen?

Upgrade is successful with no modifications to metric labels

How to reproduce it?

No response

Enter the changed values of values.yaml?

No response

Enter the command that you execute and failing/misfunctioning.

The upgrade works fine and the prometheus works as well. just that the labels are modified.

Anything else we need to know?

No response

mnorrsken commented 2 years ago

I have the same problem and there is also another issue on this: #1548

qaiserali commented 2 years ago

Hi,

I have the same issue. Any idea how to solve this?

pierluigilenoci commented 2 years ago

@monotek could you please take a look?

qaiserali commented 2 years ago

Ok, I found a fix for this issue. In the new version of Prometheus, the target label is set as "namespace" instead of kubernetes_namespace which was the case in the previous versions.

So your relabel configs under the serverFiles in values.yaml should look like below

`- action: labelmap regex: __meta_kubernetes_podlabel(.+)

These target_labels need to be fixed for all job_name under the serverFiles in values.yaml file.

pierluigilenoci commented 2 years ago

@qaiserali this is not enough.

IMHO your workaround works for some pod metrics but not for metrics scraped by kube-state-metrics like kube_deployment_status_replicas.

Ref: https://github.com/kubernetes/kube-state-metrics/blob/master/docs/deployment-metrics.md

qaiserali commented 2 years ago

@pierluigilenoci I can't see an issue with metrics scraped by kube-state-metrics, and target labels are correctly populated.

mnorrsken commented 2 years ago

I think because #1520 doesnt change kubernetes-nodes-cadvisor labeling, the collisions aren't even consistent.

It means a query like container_memory_rss / kube_pod_container_resource_requests now needs relabeling on one end. Also I don't see how the namespace of the kube-state-metrics container is more important than the actual pod/whatever namespace, but I suspect it is because of some prometheus-operator thing (which I don't use).

I would try to make a PR to fix this but I am thoroughly confused by the same-sounding-labels kubernetes_namespace, namespace, exported_namespace... 😖

ekhaydarov commented 2 years ago

It would be good to simply add the option of the old naming convention. This just causes unnecessary work for everyone.

Also i might have missed something but these changes are not mentioned in the charts/kube-prometheus-stack#upgrading-chart

pierluigilenoci commented 2 years ago

@mnorrsken let me explain. I agree with the relabeling. It makes total sense to me that if Prometheus scraps kube-state-metrics the metrics themselves report the kube-state-metrics namespace. Even if the information is related to other pods. Logical and reasonable.

The problem in my opinion was not to carefully consider the impact that the change had on the chart of the pure Prometheus. It seems to me that the attention is mostly focused on the full stack chart, formerly called Prometheus Operator.

@qaiserali you don't see the problem because probably in your case the kube-state-metrics relabeling hasn't broken anything. For those who use the pure chart of Prometheus, it happened in the transition between versions 14 and 15.

@ekhaydarov for me this passage also makes sense, only it was not well communicated and documented only for the full stack chart. Supporting a legacy version of labels I don't know how much it makes sense, it's something you can't support forever, and sooner or later you have to deal with it. Easier to "remove the tooth" and adapt who is using Prometheus metrics.

ekhaydarov commented 2 years ago

Thanks for that response and it does make sense. Looking at this holistically I feel like namespace is a very commonly used thing now and everyone should take care to articulate that key with adding extra prefix to make sure its clearly identified. ie kubernetes_namespace -> namespace should have actually went to prometheus_namespace. In our case we also had the airflow exporter namespace named simply like that. On our end we also will clarify with a prefix of airflow_namespace to remove the chance of collisions in future

mnorrsken commented 2 years ago

@pierluigilenoci Thanks, I understand the labeling now. It would be easy to fix these things in a deployment if the helm chart scrape configuration was more "modular", but I understand it's a limit of prometheus configuration design too. As it is now the entire configuration has to be re-made to make any changes if I understand it correctly.

ekhaydarov commented 2 years ago

Sorry on a similar note, but let me know if i need to raise a different issue on this. kube-prometheus-stack also has the metrics updated to the new convention. I cannot seem to figure out how to change the default scrae configs. there is nothing in values.yaml that reference prometheus.yaml or serverConfig. Reading additionalScarpeConfigs the default config comes from prometheus operator yet I cannot find anything in there to modify prometheus.yaml.

Any help on that would be appreciated

stale[bot] commented 2 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Any further update will cause the issue/pull request to no longer be considered stale. Thank you for your contributions.

pierluigilenoci commented 2 years ago

Still an issue.

krzysztof-magosa commented 2 years ago

for us cluster-autoscaler stopped working after upgrade to 15.x, also lens and kubectl stopped displaying proper metrics, any idea how to solve that?

pierluigilenoci commented 2 years ago

@krzysztof-magosa I see no correlation between Prometheus and the symptoms you are describing. For example, Cluster Autoscaler uses Metrics Server to get CPU/Memory metrics. Kubectl does the same with the top command.

krzysztof-magosa commented 2 years ago

@pierluigilenoci we don't use Metrics Server - so far autoscaler used metrics from Prometheus - we still have many clusters using prometheus chart 14.x where it works that way without issues.

krzysztof-magosa commented 2 years ago

and another thing is - in our case all node metrics are not correct - they look like sum of all nodes in cluster

wiardvanrij commented 2 years ago

I'm wondering why we are not using honor_labels. For example kube_deployment_status_replicas_available now gives the namespace of kube-state-metrics, which makes no sense to me. Also perhaps it's an idea to warn users when upgrading to v15.x? This was a big surprise for me.

marianobilli commented 2 years ago

Waiting for @ekadas PR