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
Add metric for terminated pods with tracking finalizer #113176
Add metric for terminated pods with tracking finalizer #113176
Conversation
/assign @soltysh and can someone from @kubernetes/sig-instrumentation-approvers take a look? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/lgtm
/approve
(from sig instrumentation)
/hold for sig apps review |
6849381
to
7909b87
Compare
6e0541b
to
e004e57
Compare
/retest Opened #113207 for the flakies |
TerminatedPodsTrackingFinalizerTotal = metrics.NewCounterVec( | ||
&metrics.CounterOpts{ | ||
Subsystem: JobControllerSubsystem, | ||
Name: "terminated_pods_tracking_finalizer", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would suggest to add the _total suffix for consistency with other counter metrics. Seems to be the best practice by Prometheus: https://prometheus.io/docs/practices/naming/
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oops, that was the intention, but I forgot to rename it :)
Change-Id: I26f3169588c30ed82250cb7baff8e277f8d13bb7
e004e57
to
12d308f
Compare
Rebased and squashed |
/lgtm |
/triage accepted |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/lgtm
/hold cancel |
networking glitch when getting outh token on google API |
/priority important-longterm |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: alculquicondor, logicalhan, soltysh 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:
Approvers can indicate their approval by writing |
/retest |
/test pull-kubernetes-e2e-gce-ubuntu-containerd |
What type of PR is this?
/kind feature
/sig apps
/sig instrumentation
What this PR does / why we need it:
Add a counter metric to keep track of terminated Pods as they get and loose the tracking finalizer.
Which issue(s) this PR fixes:
Part of kubernetes/enhancements#2307
Special notes for your reviewer:
The KEP originally proposed a gauge, but it proved to be difficult to test whether the controller saw finalizers at all.
A counter with an event label (with values add and remove) can be used to construct a gauge.
Does this PR introduce a user-facing change?
Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.: