Migrate pkg/kubelet/config to structured logging #100002

AfrouzMashayekhi · 2021-03-09T11:08:53Z

What type of PR is this?

/kind cleanup
/sig instrumentation
/sig node
/priority important-soon

What this PR does / why we need it:

Migrate pkg/kubelet/config to structured logging

Which issue(s) this PR fixes:

Ref #98976

Special notes for your reviewer:

part of kubernetes/enhancements#1602
As it is related to log, change the log here to structured log.

Does this PR introduce a user-facing change?

NONE

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

[KEP]: Structured Logging

k8s-ci-robot · 2021-03-09T11:09:02Z

Hi @AfrouzMashayekhi. 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 /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

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.

AfrouzMashayekhi · 2021-03-09T11:20:24Z

/cc @serathius

shubheksha · 2021-03-09T11:42:48Z

Hey @AfrouzMashayekhi, we normally don't use full stops after error messages. Logging both the object and the specific info about the same object seems repetitive. I'd also advice not modifying the error messages too much unless to clarify them further. If you could address this across your PR, I'm happy to take another look.

AfrouzMashayekhi · 2021-03-09T12:05:57Z

@shubheksha hope it's ok now

shubheksha · 2021-03-09T12:10:51Z

pkg/kubelet/config/file_linux.go

@@ -102,7 +102,7 @@ func (s *sourceFile) doWatch() error {
 func (s *sourceFile) produceWatchEvent(e *fsnotify.Event) error {
 	// Ignore file start with dots
 	if strings.HasPrefix(filepath.Base(e.Name), ".") {
-		klog.V(4).Infof("Ignored pod manifest: %s, because it starts with dots", e.Name)
+		klog.V(4).InfoS("Ignored pod manifest, because it starts with dots", "event", e.Name)


This is missing an "eventName" key

pkg/kubelet/config/http.go

pkg/kubelet/config/file.go

pkg/kubelet/config/config.go

shubheksha · 2021-03-09T12:14:11Z

pkg/kubelet/config/common.go


 	if pod.Namespace == "" {
 		pod.Namespace = metav1.NamespaceDefault
 	}
-	klog.V(5).Infof("Using namespace %q for pod %q from %s", pod.Namespace, pod.Name, source)
+	klog.V(5).InfoS("Used namespace for pod from source", "pod", klog.KObj(pod), "source", source)



Why are we logging the full object here?

In the KEP guide said we should reference Kubernetes objects.

klog.KObj doesn't log whole object, just a reference in standard format <namespace>/<name>. Main reason for using it is it both supports text and json format.

pkg/kubelet/config/file_linux.go

pkg/kubelet/config/config.go

pkg/kubelet/config/file.go

serathius · 2021-03-09T22:38:13Z

pkg/kubelet/config/config.go

@@ -254,16 +254,16 @@ func (s *podStorage) merge(source string, change interface{}) (adds, updates, de
 	switch update.Op {
 	case kubetypes.ADD, kubetypes.UPDATE, kubetypes.DELETE:
 		if update.Op == kubetypes.ADD {
-			klog.V(4).Infof("Adding new pods from source %s : %v", source, update.Pods)
+			klog.V(4).InfoS("Added new pods from source", "source", source, "pods", update.Pods)


Could you use format.Pods from #99799

AfrouzMashayekhi · 2021-03-10T05:15:06Z

/test pull-kubernetes-node-e2e

AfrouzMashayekhi · 2021-03-10T09:30:54Z

/retest

Co-authored-by: Shubheksha <shubheksha@users.noreply.github.com> Co-authored-by: Marek Siarkowicz <marek.siarkowicz@protonmail.com> Co-authored-by: Elana Hashman <ehashman@users.noreply.github.com>

AfrouzMashayekhi · 2021-03-11T12:53:32Z

/retest

AfrouzMashayekhi · 2021-03-11T14:58:17Z

/retest

kikisdeliveryservice

Verified requested changes made, review changes, checked files for completeness. Passing this to approvers

/lgtm

ehashman · 2021-03-11T23:53:54Z

/milestone v1.21

k8s-ci-robot · 2021-03-15T17:55:27Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: AfrouzMashayekhi, mrunalp

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/OWNERS~~ [mrunalp]

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 requested review from dims and Random-Liu March 9, 2021 11:09

k8s-ci-robot added the area/kubelet label Mar 9, 2021

k8s-ci-robot requested a review from serathius March 9, 2021 11:20

shubheksha added this to Waiting on Author in Structured Logging Migration for Kubelet, 1.21 Mar 9, 2021

AfrouzMashayekhi force-pushed the sl-kubelet-config branch from 66b4c98 to 3ab82cf Compare March 9, 2021 12:02

shubheksha suggested changes Mar 9, 2021

View reviewed changes

AfrouzMashayekhi commented Mar 9, 2021

View reviewed changes

pkg/kubelet/config/file_linux.go Outdated Show resolved Hide resolved

AfrouzMashayekhi force-pushed the sl-kubelet-config branch from 94c5e01 to 931f140 Compare March 9, 2021 13:06

AfrouzMashayekhi requested a review from shubheksha March 9, 2021 13:07

serathius reviewed Mar 9, 2021

View reviewed changes

pkg/kubelet/config/config.go Outdated Show resolved Hide resolved

serathius reviewed Mar 9, 2021

View reviewed changes

pkg/kubelet/config/config.go Outdated Show resolved Hide resolved

serathius reviewed Mar 9, 2021

View reviewed changes

pkg/kubelet/config/file.go Outdated Show resolved Hide resolved

AfrouzMashayekhi force-pushed the sl-kubelet-config branch from 8f2222c to 42c5fe5 Compare March 9, 2021 19:46

ehashman added this to Triage in SIG Node PR Triage Mar 9, 2021

serathius reviewed Mar 9, 2021

View reviewed changes

ehashman mentioned this pull request Mar 9, 2021

Kubelet migration to structured logs #98976

Closed

ehashman moved this from Triage to Waiting on Author in SIG Node PR Triage Mar 10, 2021

AfrouzMashayekhi force-pushed the sl-kubelet-config branch from f2b9861 to db96652 Compare March 10, 2021 08:54

SergeyKanzhelev added this to Triage in SIG Node CI/Test Board Mar 10, 2021

Migrate pkg/kubelet/config to structured logging

27a9904

Co-authored-by: Shubheksha <shubheksha@users.noreply.github.com> Co-authored-by: Marek Siarkowicz <marek.siarkowicz@protonmail.com> Co-authored-by: Elana Hashman <ehashman@users.noreply.github.com>

AfrouzMashayekhi force-pushed the sl-kubelet-config branch from db96652 to 27a9904 Compare March 11, 2021 11:33

kikisdeliveryservice moved this from Waiting on Author to Needs Reviewer in Structured Logging Migration for Kubelet, 1.21 Mar 11, 2021

kikisdeliveryservice moved this from Waiting on Author to Needs Reviewer in SIG Node PR Triage Mar 11, 2021

kikisdeliveryservice reviewed Mar 11, 2021

View reviewed changes

k8s-ci-robot assigned kikisdeliveryservice Mar 11, 2021

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Mar 11, 2021

kikisdeliveryservice moved this from Needs Reviewer to Needs Approver in Structured Logging Migration for Kubelet, 1.21 Mar 11, 2021

kikisdeliveryservice moved this from Needs Reviewer to Needs Approver in SIG Node PR Triage Mar 11, 2021

k8s-ci-robot added this to the v1.21 milestone Mar 11, 2021

mrunalp approved these changes Mar 15, 2021

View reviewed changes

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Mar 15, 2021

mrunalp moved this from Needs Approver to Done in Structured Logging Migration for Kubelet, 1.21 Mar 15, 2021

k8s-ci-robot merged commit f18caa8 into kubernetes:master Mar 15, 2021

SIG Node PR Triage automation moved this from Needs Approver to Done Mar 15, 2021

k8s-ci-robot added release-note-none Denotes a PR that doesn't merit a release note. and removed release-note Denotes a PR that will be considered when it comes time to generate release notes. labels Mar 29, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Migrate pkg/kubelet/config to structured logging #100002

Migrate pkg/kubelet/config to structured logging #100002

AfrouzMashayekhi commented Mar 9, 2021 •

edited by liggitt

k8s-ci-robot commented Mar 9, 2021

AfrouzMashayekhi commented Mar 9, 2021

shubheksha commented Mar 9, 2021

AfrouzMashayekhi commented Mar 9, 2021

shubheksha Mar 9, 2021

shubheksha Mar 9, 2021

AfrouzMashayekhi Mar 9, 2021

serathius Mar 9, 2021

serathius Mar 9, 2021

AfrouzMashayekhi commented Mar 10, 2021

AfrouzMashayekhi commented Mar 10, 2021

AfrouzMashayekhi commented Mar 11, 2021

AfrouzMashayekhi commented Mar 11, 2021

kikisdeliveryservice left a comment

ehashman commented Mar 11, 2021

k8s-ci-robot commented Mar 15, 2021

Migrate pkg/kubelet/config to structured logging #100002

Migrate pkg/kubelet/config to structured logging #100002

Conversation

AfrouzMashayekhi commented Mar 9, 2021 • edited by liggitt

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

k8s-ci-robot commented Mar 9, 2021

AfrouzMashayekhi commented Mar 9, 2021

shubheksha commented Mar 9, 2021

AfrouzMashayekhi commented Mar 9, 2021

shubheksha Mar 9, 2021

Choose a reason for hiding this comment

shubheksha Mar 9, 2021

Choose a reason for hiding this comment

AfrouzMashayekhi Mar 9, 2021

Choose a reason for hiding this comment

serathius Mar 9, 2021

Choose a reason for hiding this comment

serathius Mar 9, 2021

Choose a reason for hiding this comment

AfrouzMashayekhi commented Mar 10, 2021

AfrouzMashayekhi commented Mar 10, 2021

AfrouzMashayekhi commented Mar 11, 2021

AfrouzMashayekhi commented Mar 11, 2021

kikisdeliveryservice left a comment

Choose a reason for hiding this comment

ehashman commented Mar 11, 2021

k8s-ci-robot commented Mar 15, 2021

AfrouzMashayekhi commented Mar 9, 2021 •

edited by liggitt