feat: Add new hpa metrics to prevent prometheus timeseries duplication

[CountryTk]

What this PR does / why we need it:

Added 4 new hpa metrics to prevent duplicated timeseries events like described in this issue: https://github.com/kubernetes/kube-state-metrics/issues/2403

Added new metrics are:

• kube_horizontalpodautoscaler_spec_target_container_metric
• kube_horizontalpodautoscaler_spec_target_object_metric
• kube_horizontalpodautoscaler_status_target_container_metric
• kube_horizontalpodautoscaler_status_target_object_metric

How does this change affect the cardinality of KSM: (increases, decreases or does not change cardinality) Cardinality is increased because of new metrics

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 # https://github.com/kubernetes/kube-state-metrics/issues/2403

FYI: I've also tested this change in our prelive cluster and for us it fixed the issue

[linux-foundation-easycla[bot]]

The committers listed above are authorized under a signed CLA.

• :white_check_mark: login: CountryTk / name: Tõnis Kõiv (112ec602096988b0d011639d609168d2f4512424)

[k8s-ci-robot]

Welcome @CountryTk!

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. :smiley:

[richabanker]

/triage accepted @CatherineF-dev @rexagod could you take a look here if possible, thanks!

[CountryTk]

Hey, could this please be reviewed @CatherineF-dev @rexagod

[CatherineF-dev]

Ok! In reviewing

[CatherineF-dev]

Overall LGTM. Two small comments.

[CountryTk]

Overall LGTM. Two small comments.

Thanks for the review, I've implemented your suggestions @CatherineF-dev

[rexagod]

Thank you for the patch.

I believe this identifies a resource-agnostic pitfall where we loop over certain nested fields without including a primary key in the overall generated metrics' label-sets.

I'll take a closer look tomorrow but so far this lgtm.

[CatherineF-dev]

Could you help enable allowed changes from maintainers?

I want to make a small change.

Or could you apply these small changes? https://github.com/kubernetes/kube-state-metrics/pull/2631/commits/96d948a6b0f00fb53f4922d2286607e6c84bf9e9

[CountryTk]

Could you help enable allowed changes from maintainers?

I want to make a small change.

Or could you apply these small changes? 96d948a

For me it shows maintainer edit access is enabled.

Anyway, I've added your suggested changes in the latest commit.

Question, wouldn't the earlier kube_horizontalpodautoscaler_spec_target_metric still be there after this is in, and cause the error to still show up?

Nope because kube_horizontalpodautoscaler_spec_target_metric is only going to have PodsMetricSourceType, ResourceMetricSourceType and ExternalMetricSourceType now, which didn't cause those duplicated errors.

ContainerResourceMetricSourceType and ObjectMetricSourceType have separate functions with added labels to prevent duplicated data

[rexagod]

I'll bring this up in the call tomorrow.

[CountryTk]

I'll bring this up in the call tomorrow.

Hey, any updates?

[CountryTk]

Is there any chance we can get this merged?

[CountryTk]

@rexagod Any possibility we can get this merged?

[dudicoco]

Hi @CountryTk,

Looks like this PR does not fix external metric types. Currently when you have multiple external metrics kube-state-metrics cannot deduplicate them properly. I believe the correct way to deduplicate them is according to the selector.matchLabels field.

[k8s-triage-robot]

The Kubernetes project currently lacks enough contributors to adequately respond to all PRs.

This bot triages PRs according to the following rules:

• After 90d of inactivity, lifecycle/stale is applied
• After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
• After 30d of inactivity since lifecycle/rotten was applied, the PR is closed

You can:

• Mark this PR as fresh with /remove-lifecycle stale
• Close this PR with /close
• Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

[dkulchinsky]

hoping this is still being considered

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: CountryTk Once this PR has been reviewed and has the lgtm label, please assign mrueg 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:

• OWNERS

Approvers can indicate their approval by writing /approve in a comment Approvers can cancel approval by writing /approve cancel in a comment

[CountryTk]

hoping this is still being considered

I hope the same.

Hi @CountryTk,

Looks like this PR does not fix external metric types. Currently when you have multiple external metrics kube-state-metrics cannot deduplicate them properly. I believe the correct way to deduplicate them is according to the selector.matchLabels field.

True, the company I worked for during the time of making this PR had no external metric types which is why I didn't touch upon those, feel free to make improvements. I just hope this PR gets merged