Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Stable metrics "kube_pod_container_resource_[limits|requests]" have cpu/memory units removed in v2 #1263

Closed
mcavoyk opened this issue Oct 14, 2020 · 4 comments · Fixed by #1278
Labels
kind/documentation Categorizes issue or PR as related to documentation. v2 version 2

Comments

@mcavoyk
Copy link

mcavoyk commented Oct 14, 2020

Is this a BUG REPORT or FEATURE REQUEST?:
/kind bug

What happened:
In the latest v1 version (v1.9.7), kube_pod_container_resource_[limits|requests] is marked as stable, and the duplicates kube_pod_container_resource_[limits|requests]_[cpu_cores|memory_bytes] are marked as deprecated. The deprecated metrics were removed with #1004.

#1168 then removes the memory/cpu units from the previously stable kube_pod_container_resource_[limits|requests] metrics and re-adds the deprecated and removed metrics.

What you expected to happen:
I had expected kube_pod_container_resource_[limits|requests] to retain the memory/cpu units in v2 and the kube_pod_container_resource_[limits|requests]_[cpu_cores|memory_bytes] to be removed, instead this was flipped with what feels like little communication and no v1 version which marked these metrics as either not deprecated, experimental, or otherwise.

The changelog for v2 doesn't explain these changes particularly well. This change seems like it should need a v1.10 with more of these distinctions made clear, or v2 with a flag to re-enable the cpu/memory units on kube_pod_container_resource_[limits|requests].

Environment:

  • Kubernetes version (use kubectl version): v1.17
  • Kube-state-metrics image version: v2.0.0-alpha.1
@k8s-ci-robot k8s-ci-robot added the kind/bug Categorizes issue or PR as related to a bug. label Oct 14, 2020
@mcavoyk
Copy link
Author

mcavoyk commented Oct 14, 2020

Similiar to #1245, but this is more targeted around the communication of these changes and how the latest v1 version seems to indicate the opposite of these changes.

@lilic
Copy link
Member

lilic commented Oct 19, 2020

@mcavoyk hey thanks for the issue, yes the CHANGELOG is rather large and a lot PRs went in, we can help clarify it no problem. What would make most sense for you to understand the changes more clear, e.g. what should we write instead?

I remove bug as your last comment indicated this is more of a CHANGELOG issue? Feel free to correct me. Also note this a prerelease so all this feedback is very helpful :)

@lilic lilic added kind/documentation Categorizes issue or PR as related to documentation. v2 version 2 and removed kind/bug Categorizes issue or PR as related to a bug. labels Oct 19, 2020
@lilic
Copy link
Member

lilic commented Oct 26, 2020

@mcavoyk thanks for reporting the issue and bringing this issue to light, reason for closure is in the PR:

After some talk with Frederic who initially did the work for reworking resource metrics we decided to revert the PR, as the KEP that will introduce more detailed resource metrics has been accepted now kubernetes/enhancements#1916 this better aligns the resource metrics to the ones proposed in that KEP, this will prevent users from needing to do triple migration if they want to switch any alerting/recording rules to new metrics and avoid potential confusion. If we were to keep current metrics with the unit suffix the migration work would double, this way potentially only metric name would need to be adjusted.

Let me know if something is not clear!

@mcavoyk
Copy link
Author

mcavoyk commented Oct 28, 2020

@lilic Yes, that clarifies this for me, I appreciate the quick response 👍

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
kind/documentation Categorizes issue or PR as related to documentation. v2 version 2
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants