kube-state-metrics
kube-state-metrics copied to clipboard
kubernetes
[WIP] Add metrics of kube_pod_status_ready_time and kube_pod_status_containers_ready_time
What this PR does / why we need it: Base PR https://github.com/kubernetes/kube-state-metrics/pull/1482
Ref: Adds kube_pod_status_ready_time and kube_pod_status_containers_ready_time metrics to get the information provided by Pod Lifecycle's PodConditions array.
Maybe we should support param to open this feature or not, juse like feature-gates.
How does this change affect the cardinality of KSM: (increases, decreases or does not change cardinality) increases metrics number that Pod number * 2
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 #1465,#1830
It would be great to have this PR merged as it is blocking efforts on my team.
Would love to see this PR merged, as those metrics are valuable in determining pod startup times and issues related to it.
If needed can definitely help with making sure the code meets all the requirements.
@dgrisonnet @fpetkovski Do you think you can review this in the upcoming days? Thanks!
@CatherineF-dev: Closed this PR.
In response to this:
/close
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.
@CatherineF-dev: Reopened this PR.
In response to this:
/reopen
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.
I am fine with these new metrics. To address some initial concerns that were raised in https://github.com/kubernetes/kube-state-metrics/issues/1465#issuecomment-855922085, it is very unlikely that these metrics will surface in kubelet today due to their cardinality, so I think they are a good fit for ksm.
@liangyuanpeng there is still one unresolved comment, could you have a look at it when you have some times on your hands?
[APPROVALNOTIFIER] This PR is NOT APPROVED
This pull-request has been approved by: liangyuanpeng, logicalhan
Once this PR has been reviewed and has the lgtm label, please assign fpetkovski for approval by writing /assign @fpetkovski in a comment. For more information see the Kubernetes Code Review Process.
The full list of commands accepted by this bot can be found here.
Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment
@liangyuanpeng: 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.
Sorry for my late and would be work for it in this week.
@liangyuanpeng anything I can do to help get this over the finish line?
@liangyuanpeng can we please get this merged, it would be of great help to us. I see that there is only merge conflict to be resolved as a last step. We are also waiting on this to use this metric in our k8s clusters.
This will enable our customers/users to get better insights into the pod behaviour and fine tune their respective apps wherever necessary.
cc @fpetkovski @logicalhan
Sorry for my late and i still want to work for it and as soon as push it.
I created PR on @liangyuanpeng's fork resolving the merge conflict. Hope it helps. 🤞
At some point this may have to be resolved without further work from the OP.
Since @liangyuanpeng expressed their intention to push this effort over the finish line I'd rather wait a bit more for them to get the time they need to work on it again rather than prevent them to finish the work they started.
I would only reconsider that if we are close to a release and the PR hasn't been merged already, but for now there is no hurry at all so don't feel pressured.
I had to preform a rebase and deal with merge conflicts (new method was inserted in the same spot as methods added by these PRs) in the other PR. I'm not going to say I won't keep up with the rebases and merge conflicts in the other PR, but I would appreciate not having to spend that time.
@liangyuanpeng are you able to proceed with this PR? Otherwise we will continue on https://github.com/kubernetes/kube-state-metrics/pull/1938 after mid next week.
Closing this PR as it will be superseded by https://github.com/kubernetes/kube-state-metrics/pull/1938
Thanks!