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

Support new daemonset.updated metric name from kube-state-metrics #12103

Closed
wants to merge 3 commits into from

Conversation

rewolf
Copy link

@rewolf rewolf commented May 30, 2022

What does this PR do?

Adds a new metric mapping to daemonset.updated from the kube-state-metrics kube_daemonset_status_updated_number_scheduled metric (previously called kube_daemonset_updated_number_scheduled).

Motivation

In kubernetes/kube-state-metrics@8031a70, the metric name was changed from kube_daemonset_updated_number_scheduled to kube_daemonset_status_updated_number_scheduled to be consistent with other metric names.
This PR adds the new mapping and leaves the old mapping for backward compatibility.

Additional Notes

n/a

Review checklist (to be filled by reviewers)

  • Feature or bugfix MUST have appropriate tests (unit, integration, e2e)
  • PR title must be written as a CHANGELOG entry (see why)
  • Files changes must correspond to the primary purpose of the PR as described in the title (small unrelated changes should have their own PR)
  • PR must have changelog/ and integration/ labels attached

In kubernetes/kube-state-metrics@8031a70, the metric name was changed from kube_daemonset_updated_number_scheduled to kube_daemonset_status_updated_number_scheduled.
@rewolf rewolf marked this pull request as ready for review May 30, 2022 03:06
@rewolf rewolf requested review from a team as code owners May 30, 2022 03:06
@rewolf
Copy link
Author

rewolf commented May 30, 2022

I was not able to add a changelog label to the PR.

@codecov
Copy link

codecov bot commented May 30, 2022

Codecov Report

Merging #12103 (a730833) into master (c943a73) will not change coverage.
Report is 3330 commits behind head on master.
The diff coverage is 100.00%.

Additional details and impacted files
Flag Coverage Δ
kubernetes_state 89.35% <100.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

@clamoriniere
Copy link
Contributor

hi @rewolf

The metrics was renamed in kube-state-metrics version >= 2. So the Datadog check that support v2 is kubernetes_state_core. Because the python check that you have modified only support kube-state-metrics <= 1.9.x. More information about the reason we have build this new check here: https://www.datadoghq.com/blog/kube-state-metrics-v2-monitoring-datadog/

Also the metric change is already done in the kuberneres_state_core check: DataDog/datadog-agent#11369

I would suggest you to migrate to the kubernetes_state_core check: https://docs.datadoghq.com/integrations/kubernetes_state_core/

@rewolf
Copy link
Author

rewolf commented May 30, 2022

Also the metric change is already done in the kuberneres_state_core check: DataDog/datadog-agent#11369

Must have missed that when searching for references!

Thanks for the explanation @clamoriniere.
Looks like we'll have to look at migrating to kubernetes_state_core sooner rather than later, although it's going to take us a while to assess the affected scope.

I understand your suggestion and viewpoint, but if this is only one of a few breaking changes between ksm v1 and v2, it'd be great to get support for the new metric backported to this kubernetes_state check with a merge of this PR and give us time to properly prepare for migration to kubernetes_state_core.

@clamoriniere
Copy link
Contributor

Hi @rewolf

I understand your suggestion and viewpoint, but if this is only one of a few breaking changes between ksm v1 and v2, it'd be great to get support for the new metric backported to this kubernetes_state check with a merge of this PR and give us time to properly prepare for migration to kubernetes_state_core.

As I explained this metric renaming (kube_daemonset_updated_number_scheduled to kube_daemonset_status_updated_number_scheduled) was introduced in kube-state-metrics v2. But the Datadog kubernetes_state check only support scrapping kube-state-metrics <= 1.9.x. So this metric renaming doesnt applied to the kubernetes_state check.

The migration from kubernetes_state to kubernetes_state_core is not fully transparent, but it is worth doing it if you are using a recent Kubernetes version. It provided new useful metrics with much better performances.

If it is ok with you, I think we can close this PR.

@clamoriniere
Copy link
Contributor

hi @rewolf,
I'm closing this PR, feel free to reopen it if you want react on my previous comment

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants