-
Notifications
You must be signed in to change notification settings - Fork 2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: adjust "volume name" label for kube_persistentvolumeclaim_* #2303
Conversation
Welcome @multani! |
2aa4fba
to
1c67ed2
Compare
Sorry for the noise, I was trying to understand what was the problem with the commit message 🤦 Should be good now :) |
Also update https://github.com/kubernetes/kube-state-metrics/blob/main/CHANGELOG.md Next: |
Done! 👍 |
@@ -4,7 +4,7 @@ | |||
| ---------------------------------------------------------- | ----------- | ------------------------------------------------------------------------------------------------------------------------- | ----------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | ------------ | | |||
| kube_persistentvolumeclaim_annotations | Gauge | Kubernetes annotations converted to Prometheus labels controlled via [--metric-annotations-allowlist](./cli-arguments.md) | | `persistentvolumeclaim`=<persistentvolumeclaim-name> <br> `namespace`=<persistentvolumeclaim-namespace> <br> `annotation_PERSISTENTVOLUMECLAIM_ANNOTATION`=<PERSISTENTVOLUMECLAIM_ANNOATION> | EXPERIMENTAL | | |||
| kube_persistentvolumeclaim_access_mode | Gauge | | | `access_mode`=<persistentvolumeclaim-access-mode> <br>`namespace`=<persistentvolumeclaim-namespace> <br> `persistentvolumeclaim`=<persistentvolumeclaim-name> | STABLE | | |||
| kube_persistentvolumeclaim_info | Gauge | | | `namespace`=<persistentvolumeclaim-namespace> <br> `persistentvolumeclaim`=<persistentvolumeclaim-name> <br> `storageclass`=<persistentvolumeclaim-storageclassname><br>`volumename`=<volumename><br>`volumemode`=<volumemode> | STABLE | | |||
| kube_persistentvolumeclaim_info | Gauge | | | `namespace`=<persistentvolumeclaim-namespace> <br> `persistentvolumeclaim`=<persistentvolumeclaim-name> <br> `storageclass`=<persistentvolumeclaim-storageclassname><br>`persistentvolume`=<volumename><br>`volumemode`=<volumemode> | STABLE | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This metric is STABLE, so can't change label name.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@CatherineF-dev what is the recommended approach here then?
I understand I can't change volumename
and I will revert it.
Should I keep this label and add a new persistentvolume
label (with the same value as volumename
) instead?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Have you tried updating your promQL to support join on different label names?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As mentioned in the parent issue, AFAIK it's difficult to join metrics on different label names.
We are already relabelling the kube_persistentvolumeclaim_annotations
metric to "fix" the label, but my hope with this pull request would be to bring this change upstream.
Is there a simpler way to join on different label names that would not require this change? (this discussion belongs more in the parent issue perhaps...)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@CatherineF-dev is there any other options to move this change forward?
/triage accepted |
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: multani The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Has anything changed in the release process I'm not aware of? We usually do not update the changelog within the PR. |
This changes the name of the volume name label, reported by the `kube_persistentvolumeclaim_*` metrics, to be the same label as the one in the `kube_persistentvolume_*` metrics. This makes it easier to combine the 2 related set of metrics together to being labels and values from one metric into another.
PR needs rebase. 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. |
The Kubernetes project currently lacks enough contributors to adequately respond to all PRs. This bot triages PRs according to the following rules:
You can:
Please send feedback to sig-contributor-experience at kubernetes/community. /lifecycle stale |
/remove-lifecycle stale Still looking for feedback on how to move this forward (cf. #2303 (comment)) |
The Kubernetes project currently lacks enough contributors to adequately respond to all PRs. This bot triages PRs according to the following rules:
You can:
Please send feedback to sig-contributor-experience at kubernetes/community. /lifecycle stale |
The Kubernetes project currently lacks enough active contributors to adequately respond to all PRs. This bot triages PRs according to the following rules:
You can:
Please send feedback to sig-contributor-experience at kubernetes/community. /lifecycle rotten |
What this PR does / why we need it:
This changes the name of the volume name label, reported by the
kube_persistentvolumeclaim_*
metrics, to be the same label as the one in thekube_persistentvolume_*
metrics.This makes it easier to combine the 2 related set of metrics together to being labels and values from one metric into another.
How does this change affect the cardinality of KSM: (increases, decreases or does not change cardinality)
Does not change cardinality
Which issue(s) this PR fixes (optional, in
fixes #<issue number>(, fixes #<issue_number>, ...)
format, will close the issue(s) when PR gets merged):Fixes #2288