Is this a BUG REPORT or FEATURE REQUEST?:
/kind bug
What happened:
The Node Metrics documentation has a description of kube_node_labels, but kube_node_labels is not export labels when using v2.0.0-alpha.1.
kube-state-metrics:v2.0.0-alpha.1
Labels information is not export
# HELP kube_node_labels Kubernetes labels converted to Prometheus labels.
# TYPE kube_node_labels gauge
- kube_node_labels 1
- kube_node_labels 1
- kube_node_labels 1
- kube_node_labels 1
# HELP kube_node_role The role of a cluster node.
kube-state-metrics:v1.9.7
Using v1.9.7 in same environment exports Labels.
# HELP kube_node_labels Kubernetes labels converted to Prometheus labels.
# TYPE kube_node_labels gauge
+ kube_node_labels{node="kind-worker",label_beta_kubernetes_io_arch="amd64",label_beta_kubernetes_io_os="linux",label_kubernetes_io_arch="amd64",label_kubernetes_io_hostname="kind-worker",label_kubernetes_io_os="linux"} 1
+ kube_node_labels{node="kind-control-plane",label_beta_kubernetes_io_arch="amd64",label_beta_kubernetes_io_os="linux",label_ingress_ready="true",label_kubernetes_io_arch="amd64",label_kubernetes_io_hostname="kind-control-plane",label_kubernetes_io_os="linux",label_node_role_kubernetes_io_master=""} 1
+ kube_node_labels{node="kind-worker3",label_beta_kubernetes_io_arch="amd64",label_beta_kubernetes_io_os="linux",label_kubernetes_io_arch="amd64",label_kubernetes_io_hostname="kind-worker3",label_kubernetes_io_os="linux"} 1
+ kube_node_labels{node="kind-worker2",label_beta_kubernetes_io_arch="amd64",label_beta_kubernetes_io_os="linux",label_kubernetes_io_arch="amd64",label_kubernetes_io_hostname="kind-worker2",label_kubernetes_io_os="linux"} 1
# HELP kube_node_role The role of a cluster node.
What you expected to happen:
kube_node_labels is exported Labels when using v2.0.0-alpha.1.
How to reproduce it (as minimally and precisely as possible):
```yaml
kind: Cluster
apiVersion: kind.x-k8s.io/v1alpha4
nodes:
- role: control-plane
image: kindest/node:v1.19.1
- role: worker
image: kindest/node:v1.19.1
- role: worker
image: kindest/node:v1.19.1
- role: worker
image: kindest/node:v1.19.1
```
Anything else we need to know?:
The other "kube_XXXX_labels" metrics are in the same state. I have confirmed that the following metrics are the same in my environment.
# TYPE kube_daemonset_labels gauge
kube_daemonset_labels 1
# TYPE kube_deployment_labels gauge
kube_deployment_labels 1
# TYPE kube_endpoint_labels gauge
kube_endpoint_labels 1
# TYPE kube_ingress_labels gauge
kube_ingress_labels 1
# TYPE kube_job_labels gauge
kube_job_labels 1
# TYPE kube_namespace_labels gauge
kube_namespace_labels 1
# TYPE kube_node_labels gauge
kube_node_labels 1
# TYPE kube_pod_labels gauge
kube_pod_labels 1
# TYPE kube_replicaset_labels gauge
kube_replicaset_labels 1
# TYPE kube_secret_labels gauge
kube_secret_labels 1
# TYPE kube_service_labels gauge
kube_service_labels 1
# TYPE kube_statefulset_labels gauge
kube_statefulset_labels 1
Environment:
kubectl version): v1.19.3v2.0.0-alpha.1Hello @yosshi825, in v2.0.0-alpha the kube_*_labels had a rework. To avoid cardinality explosion caused by an unbounded number of labels, it was decided not to expose all the labels by default but only the one specified via the CLI.
You can now safely specify all the labels you want to expose to Prometheus via the --labels-allow-list flag.
That being said, I noticed a bug from what you reported. We should only gate the label_* Prometheus labels which are coming from Kubernetes and not the other ones. In my opinion, we should still expose the default labels of the metrics family, in your case the node label.
@lilic @tariq1890 may I have your opinion on my previous statement?
I happened to stumble upon this bug/feature as I was working on metric discovery. It might be useful to mention this in the documentation.
@dgrisonnet I agree that only the label_* labels should be gated, otherwise it's impossible to relate them back without also specifying their "deployment/node/etc." label and namespace. I would also say that it would be nicer to not require the label_ prefix part as it adds quite some redundancy when more labels are specified. After all, this seems to be limited to the *_label metrics that all start the labels with label_ anyways. It would also be useful to allow some pattern matching on both sides of the =, even though this would open up the door again to cardinality explosion. But I do think that it should be possible to do *=[*] to get the old behavior, which I think is acceptable as users can choose whether to get the old behavior or not. Though for me a *=[label1,label2,label3] would be sufficient for my current workflow.
Edit:
Now I notice it was written in the release notes, I should have seen that. Though it might still be useful to document it a bit more.
@dsonck92 where would you propose this to be documented, what is a better place? We are happy to add this! If you can think of a place please feel free to open a PR! :)
Well, considering there's already an "Conflict resolution in label names" in the README.md, probably a new heading below that. I can create a PR with something I would think helps people avoid the pitfall. Of course keeping in mind potential changes as suggested.
@dsonck92 that sounds great, go ahead thanks!
I've created it here #1279.
@dgrisonnet ah, I was just chasing the "node" label (where was it magically coming from, because now it's lost), and I think I found the answer in your https://github.com/kubernetes/kube-state-metrics/issues/1270#issuecomment-712986441 comment. Was there any followup to that remark of yours? Can we have it back?
@ntavares Well, as my documentation PR should improve, you can get the labels you wish by allowing them specifically, keeping in mind you need to do that for every label metric and every label you want, including the label_ prefix.
--labels-allow-list=kube_node_labels=[label_your_label,label_other_label]
I don't see it happen soon that all labels are exposed by default (nor would I want to see that) as cardinality is a fair point: It might explode into a lot of labels, especially as some projects (e.g. cert-manager) add labels by themselves that could be picked up (though not associated with nodes). I still think there should be some additional usability like not requiring the label_ prefix as, well, the label metrics will always give labels, and will always prefix the labels with label_. And I would appreciate the wildcard options as it would further remove the need to specify a lot of specific cases. Key point being that it might be used wrongly, but at least people have to do it on purpose.
@dsonck92 I believe your last comment was in regards to another thread? :) I noticed now that, on that other thread, I forgot to mention your remark earlier here, and now reiterated, that:
I would also say that it would be nicer to not require the label_ prefix part as it adds quite some redundancy when more labels are specified.
I couldn't agree more. Our KSM command-line now looks gigantic..... having to repeat label_ for all of them is totally useless as, as you mention, we're listing labels, which will always be prefixed with label_.....
@ntavares yes, @lilic is currently working on a fix.
If I'm not wrong, it is a blocker for the 2.0 release so the default labels should be back in the official release.
@ntavares ah, yes, you mean the node default label (without label_ prefix) one that should have been included by default. Yes it should get added back in. In the meantime it is possible to mark them in the same allow list which was why I initially added this as a remark to the documentation. In fact, in my opinion, I wrote the documentation to be as accurate to the current state of master as possible as it would have been trivial to fix the documentation as soon as a fix was out. Hence why I did add, to the example, --labels-allow-list kube_node_labels=[node,namespace,labels_your_own] as anyone trying out alpha.x would hit the same problem.
I'm more with the "do what you can now and quickly to help as much as possible" regarding documentation instead of delaying the documentation till bugs are fixed. As people will read the docs first, then fail and eventually come to the issue.
Yes I am currently working on a fix, will open a PR sometime this week if all goes well. The new format will be a bit different from the current one, due to some performance regressions we noticed that the new --labels-allow-list flag introduced. Note that will fix the that it will always have the resource name and namespace there by default, this was a bug.
New format should be something like this:
--labels-allow-list nodes=[your-actual-label-name-as-seen-in-k8s, your-other-label]
Will mention this issue on the PR when I open it!
Wait, does that also mean it's with dashes, slashes and dots? (Which I think would be nice)
Yes, that is the current plan, note this will be converted to Prometheus format always later. The idea was that it's easier for users to know the k8s label name and pass that name to allowlist, rather than converting that by hand before. It also is more performance efficient for reasons which will hopefully be evident in the PR.
But yes the current plan is to have something like this to work yes:
--labels-allow-list="pods=["app.kubernetes.io/component"]"
Like I said it's still WIP, so can't promise it will be that format but will detail any reasons if something does not work in the PR. :)
Yeah, understandable that it will get transformed. I can see this might also allow users to prevent the label collisions mentioned in the readme file. As, for example, you would be able to indicate you want the one with dots instead of dashes.
I would still stress that it's useful to allow wildcards on at least the labels side as now you could say 'keep all app.kubernetes.io/*' and just use the proper label "namespaces" (or how the docs calls them on k8s).
Yeah there is an issue opened for this, I understand the need but its out of the scope of my work currently. But we can discuss that in that issue, currently only want to fix the bug and introduce the more performant way of allowing labels, the PR will be large enough as is :)
This is great stuff fellows, thanks for the quick reaction. And @dsonck92 thanks a bunch for the "node" example, it did actually occur to me to try, but lost my faith on it somehow, great that it works again; thanks for the tip.
Just wanted to confirm that this bug is still present in 2.0.0-alpha.3.
Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.
If this issue is safe to close now please do so with /close.
Send feedback to sig-contributor-experience at kubernetes/community.
/lifecycle stale
Stale issues rot after 30d of inactivity.
Mark the issue as fresh with /remove-lifecycle rotten.
Rotten issues close after an additional 30d of inactivity.
If this issue is safe to close now please do so with /close.
Send feedback to sig-contributor-experience at kubernetes/community.
/lifecycle rotten
Rotten issues close after 30d of inactivity.
Reopen the issue with /reopen.
Mark the issue as fresh with /remove-lifecycle rotten.
Send feedback to sig-contributor-experience at kubernetes/community.
/close
@fejta-bot: Closing this issue.
In response to this:
Rotten issues close after 30d of inactivity.
Reopen the issue with/reopen.
Mark the issue as fresh with/remove-lifecycle rotten.Send feedback to sig-contributor-experience at kubernetes/community.
/close
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.
Most helpful comment
Yes I am currently working on a fix, will open a PR sometime this week if all goes well. The new format will be a bit different from the current one, due to some performance regressions we noticed that the new --labels-allow-list flag introduced. Note that will fix the that it will always have the resource name and namespace there by default, this was a bug.
New format should be something like this:
Will mention this issue on the PR when I open it!