Skip to content
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(horizontalpodautoscaler): add selector labels where applicable for hpa spec|status target metrics #2621

Closed

Conversation

DP19
Copy link

@DP19 DP19 commented Feb 28, 2025

What this PR does / why we need it:
Similar to the issue found in #2408 since prometheus 2.52 it now complains about duplicate metric values. Since some of the hpa metrics for external, pod, and object metrics can have the same name but different selectors we should add these labels to help differentiate between these metrics (#2405)

How does this change affect the cardinality of KSM: (increases, decreases or does not change cardinality)
This may increase cardinality somewhat if these selectors change between hpa revisions but this should fix an issue with the duplicates that already exist

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 #2405

I didn't dig too deep but the quick search i did didn't yeild any other patterns for things like this. I just went with the approach of adding in metric labels all prepended with selectorlabel so if you have a hpa like the one shown in the issue:
#2405 (comment)

these metrics would look like:

kube_horizontalpodautoscaler_spec_target_metric{horizontalpodautoscaler="payments-gateway",metric_name="amqp_messages_unacknowledged",metric_target_type="AverageValue",namespace="payments",selectorlabel_queue="queue_one"} 40
kube_horizontalpodautoscaler_spec_target_metric{horizontalpodautoscaler="payments-gateway",metric_name="amqp_messages_unacknowledged",metric_target_type="AverageValue",namespace="payments",selectorlabel_queue="queue_two"} 40
kube_horizontalpodautoscaler_spec_target_metric{horizontalpodautoscaler="payments-gateway",metric_name="amqp_messages_unacknowledged",metric_target_type="AverageValue",namespace="payments",selectorlabel_queue="queue_three"} 40
...

where selectorlabel will be any number of labels within same with the selector.matchLabels field of the metric. This is the same for kube_horizontalpodautoscaler_status_target_metric as well

@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. labels Feb 28, 2025
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: DP19
Once this PR has been reviewed and has the lgtm label, please assign dgrisonnet for approval. For more information see the Code Review Process.

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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot
Copy link
Contributor

Welcome @DP19!

It looks like this is your first PR to kubernetes/kube-state-metrics 🎉. Please refer to our pull request process documentation to help your PR have a smooth ride to approval.

You will be prompted by a bot to use commands during the review process. Do not be afraid to follow the prompts! It is okay to experiment. Here is the bot commands documentation.

You can also check if kubernetes/kube-state-metrics has its own contribution guidelines.

You may want to refer to our testing guide if you run into trouble with your tests not passing.

If you are having difficulty getting your pull request seen, please follow the recommended escalation practices. Also, for tips and tricks in the contribution process you may want to read the Kubernetes contributor cheat sheet. We want to make sure your contribution gets all the attention it needs!

Thank you, and welcome to Kubernetes. 😃

@k8s-ci-robot k8s-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Feb 28, 2025
@DP19 DP19 force-pushed the selector-labels-for-hpa-target-metrics branch from 967eb0e to 2f9d426 Compare March 6, 2025 04:38
@k8s-ci-robot k8s-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Mar 6, 2025
@DP19 DP19 force-pushed the selector-labels-for-hpa-target-metrics branch from 2f9d426 to b14a346 Compare March 6, 2025 04:40
@dgrisonnet
Copy link
Member

/assign @CatherineF-dev
/triage accepted

@k8s-ci-robot k8s-ci-robot added triage/accepted Indicates an issue or PR is ready to be actively worked on. and removed needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. labels Mar 6, 2025
@CatherineF-dev
Copy link
Contributor

We think cardinality might be an issue.

Will #2614 fix the problem?

@DP19
Copy link
Author

DP19 commented Mar 6, 2025

We think cardinality might be an issue.

Will #2614 fix the problem?

Yup that should fix it as well. I'll close this out, thanks!

@DP19 DP19 closed this Mar 6, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. triage/accepted Indicates an issue or PR is ready to be actively worked on.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Duplicate sample for HPA metrics using multiple external metrics with same metric name
4 participants