Add metrics of kube_pod_status_ready_time and kube_pod_status_containers_ready_time #1837

liangyuanpeng · 2022-09-18T09:33:42Z

What this PR does / why we need it:
Base PR #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

Co-authored-by: Szymon Grzemski sz.grzemski@gmail.com
Signed-off-by: Lan Liang gcslyp@gmail.com

coleary-hyperscience · 2022-10-17T18:36:05Z

It would be great to have this PR merged as it is blocking efforts on my team.

internal/store/pod_test.go

azalenski-branch · 2022-10-20T12:47:42Z

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!

k8s-ci-robot · 2022-10-20T13:25:25Z

@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 · 2022-10-20T13:25:41Z

internal/store/pod_test.go

 						},
 					},
 				},
 			},
 			Want: `
 				# HELP kube_pod_status_ready [STABLE] Describes whether the pod is ready to serve requests.
-				# TYPE kube_pod_status_ready gauge
+		        # HELP kube_pod_status_ready_time Readiness achieved time in unix timestamp for a pod.


Add test for kube_pod_status_container_ready_time

CatherineF-dev · 2022-10-20T13:26:13Z

/reopen

Sry, I only want to close that comment..

k8s-ci-robot · 2022-10-20T13:26:21Z

@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.

liangyuanpeng · 2022-10-20T15:51:02Z

I'm working for rebase commit and finish it tomorrow.

Co-authored-by: Szymon Grzemski <sz.grzemski@gmail.com> Signed-off-by: Lan Liang <gcslyp@gmail.com>

fpetkovski · 2022-10-21T06:23:34Z

docs/pod-metrics.md

@@ -12,6 +12,8 @@
 | kube_pod_nodeselectors| Gauge | Describes the Pod nodeSelectors | |  `pod`=&lt;pod-name&gt; <br> `namespace`=&lt;pod-namespace&gt; <br> `nodeselector_NODE_SELECTOR`=&lt;NODE_SELECTOR&gt; <br> `uid`=&lt;pod-uid&gt; | EXPERIMENTAL | Opt-in |
 | kube_pod_status_phase | Gauge | The pods current phase | | `pod`=&lt;pod-name&gt; <br> `namespace`=&lt;pod-namespace&gt; <br> `phase`=&lt;Pending\|Running\|Succeeded\|Failed\|Unknown&gt; <br> `uid`=&lt;pod-uid&gt; | STABLE | - |
 | kube_pod_status_ready | Gauge | Describes whether the pod is ready to serve requests | | `pod`=&lt;pod-name&gt; <br> `namespace`=&lt;pod-namespace&gt; <br> `condition`=&lt;true\|false\|unknown&gt; <br> `uid`=&lt;pod-uid&gt; | STABLE | - |
+| kube_pod_status_ready_time | Gauge | Time when pod passed readiness probes. | seconds | `pod`=&lt;pod-name&gt; <br> `namespace`=&lt;pod-namespace&gt; <br> `uid`=&lt;pod-uid&gt; | STABLE |
+| kube_pod_status_container_ready_time | Gauge | Time when the container of the pod entered Ready state. | seconds | `pod`=&lt;pod-name&gt; <br> `namespace`=&lt;pod-namespace&gt; <br> `uid`=&lt;pod-uid&gt; | STABLE |


Let's start with making the metrics experimental, and we can graduate them over time.

Agree.

You can use generator.NewFamilyGeneratorWithStability instead of generator.NewFamilyGenerator to state it's experimental. (basemetrics.ALPHA)
Example PR: #1844

dgrisonnet · 2022-11-03T17:51:55Z

I am fine with these new metrics. To address some initial concerns that were raised in #1465 (comment), 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?

logicalhan

/lgtm

k8s-ci-robot · 2022-11-03T17:54:08Z

[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.

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

k8s-ci-robot · 2022-11-04T00:37:15Z

@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.

fpetkovski · 2022-11-04T09:34:43Z

@liangyuanpeng would you mind resolving conflicts against master?

liangyuanpeng · 2022-11-16T09:10:40Z

Sorry for my late and would be work for it in this week.

rumstead · 2022-12-05T17:26:55Z

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?

akifkhan01 · 2022-12-09T05:27:33Z

@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

liangyuanpeng · 2022-12-09T10:04:30Z

Sorry for my late and i still want to work for it and as soon as push it.

akifkhan01 · 2022-12-09T10:17:17Z

Thanks @liangyuanpeng for the response and being on top of it.

akifkhan01 · 2022-12-19T11:50:32Z

@liangyuanpeng can I help in anyway to get this merged?

ryanrolds · 2022-12-22T18:28:30Z

I created PR on @liangyuanpeng's fork resolving the merge conflict. Hope it helps. 🤞

ryanrolds · 2022-12-27T16:50:33Z

At some point this may have to be resolved without further work from the OP.

dgrisonnet · 2023-01-02T11:04:13Z

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.

ryanrolds · 2023-01-11T00:31:17Z

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.

mrueg · 2023-01-12T10:37:06Z

@liangyuanpeng are you able to proceed with this PR?
Otherwise we will continue on #1938 after mid next week.

mrueg · 2023-01-19T13:28:12Z

Closing this PR as it will be superseded by #1938

Thanks!

k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Sep 18, 2022

k8s-ci-robot requested review from dgrisonnet and fpetkovski September 18, 2022 09:33

k8s-ci-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Sep 18, 2022

liangyuanpeng force-pushed the pod_ready_time branch from a5cc434 to de36d25 Compare September 18, 2022 09:48

k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Sep 18, 2022

liangyuanpeng changed the title ~~Add metrics of kube_pod_status_ready_time and kube_pod_status_containers_ready_time~~ [WIP] Add metrics of kube_pod_status_ready_time and kube_pod_status_containers_ready_time Sep 18, 2022

k8s-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Sep 18, 2022

k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 14, 2022

k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 18, 2022

liangyuanpeng force-pushed the pod_ready_time branch from d9305d0 to bb4c9ff Compare October 18, 2022 08:56

CatherineF-dev reviewed Oct 18, 2022

View reviewed changes

internal/store/pod_test.go Show resolved Hide resolved

k8s-ci-robot closed this Oct 20, 2022

CatherineF-dev reviewed Oct 20, 2022

View reviewed changes

k8s-ci-robot reopened this Oct 20, 2022

Support pod_ready_time and pod_container_ready_time

f36b8e6

Co-authored-by: Szymon Grzemski <sz.grzemski@gmail.com> Signed-off-by: Lan Liang <gcslyp@gmail.com>

liangyuanpeng force-pushed the pod_ready_time branch from 85c7555 to f36b8e6 Compare October 21, 2022 04:08

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 Oct 21, 2022

liangyuanpeng changed the title ~~[WIP] Add metrics of kube_pod_status_ready_time and kube_pod_status_containers_ready_time~~ Add metrics of kube_pod_status_ready_time and kube_pod_status_containers_ready_time Oct 21, 2022

k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Oct 21, 2022

fpetkovski reviewed Oct 21, 2022

View reviewed changes

logicalhan approved these changes Nov 3, 2022

View reviewed changes

k8s-ci-robot assigned logicalhan Nov 3, 2022

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Nov 3, 2022

k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 4, 2022

ryanrolds mentioned this pull request Dec 29, 2022

Add metrics of kube_pod_status_ready_time and kube_pod_status_containers_ready_time redux #1938

Merged

mrueg closed this Jan 19, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add metrics of kube_pod_status_ready_time and kube_pod_status_containers_ready_time #1837

Add metrics of kube_pod_status_ready_time and kube_pod_status_containers_ready_time #1837

liangyuanpeng commented Sep 18, 2022 •

edited

Loading

coleary-hyperscience commented Oct 17, 2022

azalenski-branch commented Oct 20, 2022

k8s-ci-robot commented Oct 20, 2022

CatherineF-dev Oct 20, 2022

CatherineF-dev commented Oct 20, 2022 •

edited

Loading

k8s-ci-robot commented Oct 20, 2022

liangyuanpeng commented Oct 20, 2022

fpetkovski Oct 21, 2022

CatherineF-dev Oct 21, 2022 •

edited

Loading

dgrisonnet commented Nov 3, 2022

logicalhan left a comment

k8s-ci-robot commented Nov 3, 2022

k8s-ci-robot commented Nov 4, 2022

fpetkovski commented Nov 4, 2022

liangyuanpeng commented Nov 16, 2022 •

edited

Loading

rumstead commented Dec 5, 2022

akifkhan01 commented Dec 9, 2022 •

edited

Loading

liangyuanpeng commented Dec 9, 2022

akifkhan01 commented Dec 9, 2022 •

edited

Loading

akifkhan01 commented Dec 19, 2022

ryanrolds commented Dec 22, 2022

ryanrolds commented Dec 27, 2022 •

edited

Loading

dgrisonnet commented Jan 2, 2023 •

edited

Loading

ryanrolds commented Jan 11, 2023

mrueg commented Jan 12, 2023

mrueg commented Jan 19, 2023

Add metrics of kube_pod_status_ready_time and kube_pod_status_containers_ready_time #1837

Add metrics of kube_pod_status_ready_time and kube_pod_status_containers_ready_time #1837

Conversation

liangyuanpeng commented Sep 18, 2022 • edited Loading

coleary-hyperscience commented Oct 17, 2022

azalenski-branch commented Oct 20, 2022

k8s-ci-robot commented Oct 20, 2022

CatherineF-dev Oct 20, 2022

Choose a reason for hiding this comment

CatherineF-dev commented Oct 20, 2022 • edited Loading

k8s-ci-robot commented Oct 20, 2022

liangyuanpeng commented Oct 20, 2022

fpetkovski Oct 21, 2022

Choose a reason for hiding this comment

CatherineF-dev Oct 21, 2022 • edited Loading

Choose a reason for hiding this comment

dgrisonnet commented Nov 3, 2022

logicalhan left a comment

Choose a reason for hiding this comment

k8s-ci-robot commented Nov 3, 2022

k8s-ci-robot commented Nov 4, 2022

fpetkovski commented Nov 4, 2022

liangyuanpeng commented Nov 16, 2022 • edited Loading

rumstead commented Dec 5, 2022

akifkhan01 commented Dec 9, 2022 • edited Loading

liangyuanpeng commented Dec 9, 2022

akifkhan01 commented Dec 9, 2022 • edited Loading

akifkhan01 commented Dec 19, 2022

ryanrolds commented Dec 22, 2022

ryanrolds commented Dec 27, 2022 • edited Loading

dgrisonnet commented Jan 2, 2023 • edited Loading

ryanrolds commented Jan 11, 2023

mrueg commented Jan 12, 2023

mrueg commented Jan 19, 2023

liangyuanpeng commented Sep 18, 2022 •

edited

Loading

CatherineF-dev commented Oct 20, 2022 •

edited

Loading

CatherineF-dev Oct 21, 2022 •

edited

Loading

liangyuanpeng commented Nov 16, 2022 •

edited

Loading

akifkhan01 commented Dec 9, 2022 •

edited

Loading

akifkhan01 commented Dec 9, 2022 •

edited

Loading

ryanrolds commented Dec 27, 2022 •

edited

Loading

dgrisonnet commented Jan 2, 2023 •

edited

Loading