add container probe duration metrics #104484

jackfrancis · 2021-08-20T19:39:32Z

What type of PR is this?

/kind feature

What this PR does / why we need it:

This PR adds duration metrics for kubelet container probes.

Which issue(s) this PR fixes:

Fixes #101035

Special notes for your reviewer:

Does this PR introduce a user-facing change?

add container probe duration metrics

Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:

add container probe duration metrics

jackfrancis · 2021-08-20T19:40:10Z

@SergeyKanzhelev how does this look as a draft change for probe duration metrics? are we on the right track?

mmiranda96 · 2021-09-20T20:46:06Z

/triage accepted
/priority important-soon

Could you update the proper unit tests for the new feature? Also, I'm not 100% sure on this, but it looks like the jobs are flaking. Rerunning them...
/retest

mmiranda96 · 2021-09-20T20:51:17Z

Also, please edit the section Does this PR introduce a user-facing change?, since it does introduce a new change.

k8s-ci-robot · 2021-09-26T08:35:30Z

@pacoxu: /release-note-edit must be used with a release note block.

In response to this:

/release-note-edit
add container probe duration metrics

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.

pacoxu · 2021-09-26T08:37:43Z

/release-note-edit

add container probe duration metrics

ehashman · 2021-09-27T17:50:48Z

/sig instrumentation

jackfrancis · 2022-01-21T00:51:05Z

pkg/kubelet/prober/worker_test.go

@@ -474,3 +475,50 @@ func TestStartupProbeDisabledByStarted(t *testing.T) {
 	expectContinue(t, w, w.doProbe(), msg)
 	expectResult(t, w, results.Success, msg)
 }
+
+func TestGetPodLabelName(t *testing.T) {


@dgrisonnet thx for the continual feedback!

Does the below set of cases prove out everything we want?

gentle ping @dgrisonnet @SergeyKanzhelev

This PR is ready for another "consider for merge" review

The new changes looks good from my side, but I would love to have a second pair of eyes on these assumptions: #104484 (comment)

pacoxu · 2022-02-10T13:55:59Z

/assign

jackfrancis · 2022-02-15T18:10:26Z

gentle ping @pacoxu

thank you!

pacoxu · 2022-02-16T07:17:13Z

I did some testing. Here are four examples including static pod and general pods.

[root@daocloud ~]# kubectl get pod --show-labels -A
NAMESPACE          NAME                                       READY   STATUS             RESTARTS         AGE     LABELS
default            liveness-exec                              0/1     CrashLoopBackOff   23 (54s ago)     75m     test=liveness
calico-system      calico-kube-controllers-59c45ff85c-x2gts   1/1     Running            10 (27d ago)     56d     k8s-app=calico-kube-controllers,pod-template-hash=59c45ff85c
calico-system      calico-node-d2jpf                          1/1     Running            4 (27d ago)      56d     controller-revision-hash=86b6bcc6dc,k8s-app=calico-node,pod-template-generation=1
calico-system      calico-typha-65cdbb598d-644x5              1/1     Running            7 (27d ago)      56d     k8s-app=calico-typha,pod-template-hash=65cdbb598d
kube-system        kube-apiserver-daocloud                    1/1     Running            0                81m     component=kube-apiserver,tier=control-plane

The metrics for pods above

prober_probe_duration_seconds_sum{container="liveness",namespace="default",pod="liveness-exec",probe_type="Liveness"} 6.563395632
prober_probe_duration_seconds_sum{container="calico-kube-controllers",namespace="calico-system",pod="calico-kube-controllers",probe_type="Liveness"} 25.634286655000015
prober_probe_duration_seconds_sum{container="calico-kube-controllers",namespace="calico-system",pod="calico-kube-controllers",probe_type="Readiness"} 26.072259959999986
prober_probe_duration_seconds_sum{container="calico-node",namespace="calico-system",pod="calico-node",probe_type="Liveness"} 0.6949132140000004
prober_probe_duration_seconds_sum{container="calico-node",namespace="calico-system",pod="calico-node",probe_type="Readiness"} 60.58489301800003
prober_probe_duration_seconds_sum{container="calico-typha",namespace="calico-system",pod="calico-typha",probe_type="Liveness"} 0.7015105350000005
prober_probe_duration_seconds_sum{container="calico-typha",namespace="calico-system",pod="calico-typha",probe_type="Readiness"} 0.6845843959999995
prober_probe_duration_seconds_sum{container="kube-apiserver",namespace="kube-system",pod="kube-apiserver-daocloud",probe_type="Liveness"} 3.810594282999996
prober_probe_duration_seconds_sum{container="kube-apiserver",namespace="kube-system",pod="kube-apiserver-daocloud",probe_type="Readiness"} 44.2331933840001
prober_probe_duration_seconds_sum{container="kube-apiserver",namespace="kube-system",pod="kube-apiserver-daocloud",probe_type="Startup"} 0.006615101

pacoxu

/lgtm

jackfrancis · 2022-03-08T21:11:12Z

@SergeyKanzhelev can we merge this?

jackfrancis · 2022-05-09T20:05:08Z

@SergeyKanzhelev @pacoxu @dgrisonnet should we take advantage of the beginning of the 1.25 cycle and add this to the milestone and merge?

pacoxu · 2022-06-28T03:32:34Z

kindly ping @dgrisonnet
/cc @dashpole @mrunalp
Do you think this metric is acceptable?

dgrisonnet · 2022-06-28T12:50:34Z

I somehow missed the pings, sorry about that. I think this PR is good to go now that my initial concerns have been addressed.

/unhold
/lgtm

jackfrancis · 2022-07-07T06:42:50Z

gentle ping @mrunalp

dashpole · 2022-07-11T14:16:49Z

/approve

k8s-ci-robot · 2022-07-29T18:43:28Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dashpole, jackfrancis, logicalhan, mrunalp, SergeyKanzhelev

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~pkg/kubelet/prober/OWNERS~~ [mrunalp]
~~pkg/kubelet/server/OWNERS~~ [mrunalp]

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

jackfrancis force-pushed the prober-duration-metrics branch from c7f256e to c837bb9 Compare August 20, 2021 19:41

k8s-ci-robot requested review from andrewsykim and mtaufen August 20, 2021 19:42

k8s-ci-robot added sig/node Categorizes an issue or PR as relevant to SIG Node. and removed do-not-merge/needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. labels Aug 20, 2021

pacoxu added this to Waiting on Author in SIG Node PR Triage Sep 2, 2021

pacoxu moved this from Waiting on Author to Triage in SIG Node PR Triage Sep 2, 2021

mmiranda96 moved this from Triage to Waiting on Author in SIG Node PR Triage Sep 20, 2021

This comment has been minimized.

Sign in to view

k8s-ci-robot added release-note Denotes a PR that will be considered when it comes time to generate release notes. and removed do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. labels Sep 26, 2021

pacoxu moved this from Waiting on Author to Needs Reviewer in SIG Node PR Triage Sep 26, 2021

k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Jan 21, 2022

jackfrancis commented Jan 21, 2022

View reviewed changes

jackfrancis mentioned this pull request Feb 8, 2022

Create a duration metric for probes #101035

Closed

k8s-ci-robot assigned pacoxu Feb 10, 2022

pacoxu reviewed Feb 16, 2022

View reviewed changes

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

ehashman moved this from Needs Reviewer to Needs Approver in SIG Node PR Triage Feb 28, 2022

k8s-ci-robot requested review from dashpole and mrunalp June 28, 2022 03:32

k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jun 28, 2022

mrunalp approved these changes Jul 29, 2022

View reviewed changes

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jul 29, 2022

mrunalp moved this from Needs Approver to Done in SIG Node PR Triage Jul 29, 2022

k8s-ci-robot merged commit 126c076 into kubernetes:master Jul 29, 2022

k8s-ci-robot added this to the v1.25 milestone Jul 29, 2022

jackfrancis deleted the prober-duration-metrics branch November 4, 2022 18:03

jackfrancis mentioned this pull request Jan 24, 2023

remove ExecProbeTimeout feature gate #115227

Closed

tzneal mentioned this pull request Feb 14, 2023

Metric prober_probe_total reports invalid pod field #115766

Closed

a7i mentioned this pull request Jun 8, 2023

[BUG] kubelet prober metrics on k8s 1.25+ DataDog/datadog-agent#17523

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add container probe duration metrics #104484

add container probe duration metrics #104484

jackfrancis commented Aug 20, 2021 •

edited by k8s-ci-robot

jackfrancis commented Aug 20, 2021

mmiranda96 commented Sep 20, 2021

mmiranda96 commented Sep 20, 2021

This comment has been minimized.

This comment has been minimized.

k8s-ci-robot commented Sep 26, 2021

pacoxu commented Sep 26, 2021

ehashman commented Sep 27, 2021

jackfrancis Jan 21, 2022

jackfrancis Feb 1, 2022

dgrisonnet Feb 1, 2022

pacoxu commented Feb 10, 2022

jackfrancis commented Feb 15, 2022

pacoxu commented Feb 16, 2022

pacoxu left a comment

jackfrancis commented Mar 8, 2022

jackfrancis commented May 9, 2022

pacoxu commented Jun 28, 2022

dgrisonnet commented Jun 28, 2022

jackfrancis commented Jul 7, 2022

dashpole commented Jul 11, 2022

k8s-ci-robot commented Jul 29, 2022

add container probe duration metrics #104484

add container probe duration metrics #104484

Conversation

jackfrancis commented Aug 20, 2021 • edited by k8s-ci-robot

What type of PR is this?

What this PR does / why we need it:

Which issue(s) this PR fixes:

Special notes for your reviewer:

Does this PR introduce a user-facing change?

Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:

jackfrancis commented Aug 20, 2021

mmiranda96 commented Sep 20, 2021

mmiranda96 commented Sep 20, 2021

This comment has been minimized.

This comment has been minimized.

k8s-ci-robot commented Sep 26, 2021

pacoxu commented Sep 26, 2021

ehashman commented Sep 27, 2021

jackfrancis Jan 21, 2022

Choose a reason for hiding this comment

jackfrancis Feb 1, 2022

Choose a reason for hiding this comment

dgrisonnet Feb 1, 2022

Choose a reason for hiding this comment

pacoxu commented Feb 10, 2022

jackfrancis commented Feb 15, 2022

pacoxu commented Feb 16, 2022

pacoxu left a comment

Choose a reason for hiding this comment

jackfrancis commented Mar 8, 2022

jackfrancis commented May 9, 2022

pacoxu commented Jun 28, 2022

dgrisonnet commented Jun 28, 2022

jackfrancis commented Jul 7, 2022

dashpole commented Jul 11, 2022

k8s-ci-robot commented Jul 29, 2022

jackfrancis commented Aug 20, 2021 •

edited by k8s-ci-robot