-
Notifications
You must be signed in to change notification settings - Fork 38.9k
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
kubelet: create top-level traces for pod sync and GC #114504
kubelet: create top-level traces for pod sync and GC #114504
Conversation
Welcome @vrutkovs! |
Hi @vrutkovs. Thanks for your PR. I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. 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. |
/sig node |
/assign |
ba391ce
to
84474d4
Compare
@@ -594,7 +597,7 @@ func TestGarbageCollectNotEnoughFreed(t *testing.T) { | |||
manager, fakeRuntime := newRealImageGCManager(policy, mockStatsProvider) | |||
|
|||
// Expect 95% usage and little of it gets freed. | |||
mockStatsProvider.EXPECT().ImageFsStats(ctx).Return(&statsapi.FsStats{ | |||
mockStatsProvider.EXPECT().ImageFsStats(gomock.Any()).Return(&statsapi.FsStats{ |
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.
Not sure about this one - we can't use ctx
as it gets mutated (trace attributes being added). Any better way other than replacing it with gomock.Any()
?
28d50fa
to
e7a4d15
Compare
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!
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
@sergeyevstifeev do you have the approver rights for the kubelet part? |
This starts new top level OpenTelemetry spans every time syncPod or image / container GC is invoked
e7a4d15
to
556d774
Compare
/test pull-kubernetes-node-e2e-containerd |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: ardaguclu, dashpole, mrunalp, sallyom, saschagrunert, swagatbora90, vrutkovs 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 |
/lgtm |
LGTM label has been added. Git tree hash: 2f5e755662cb10934aed17e7d7093a62d10af126
|
/milestone v1.27 |
This adds top level OpenTelemetry spans to most common goroutines periodically invoked by kubelet:
syncPod
ImageGCManager.GarbageCollect
ContainerGC.GarbageCollect
Other GRPC calls will be nested below these spans grouping CRI calls
What type of PR is this?
/kind feature
What this PR does / why we need it:
Creates top level OpenTelemetry spans whenever:
This would group kubelet GRPC span under a single span showing the source of these calls
Which issue(s) this PR fixes:
Part of #113414
Special notes for your reviewer:
Does this PR introduce a user-facing change?
Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:
Part of KEP kubernetes/enhancements#2831