Use structured logs

[yuanchen8911]

Logging in Kubernetes 1.19 has been migrated to the structured format. Two new methods are added to k8s.io/klog/v2 library: InfoS and ErrorS. Should we update the scheduler plugins' logging calls to the structured logs and use the structured format in future logging?

Refs

• https://kubernetes.io/blog/2020/09/04/kubernetes-1-19-introducing-structured-logs/
• https://github.com/kubernetes/klog

[damemi]

Yeah, this would be a good thing to work on. We've already done some work to switch our logs to structured in the descheduler too

[Huang-Wei]

@yuanchen8911 SGTM. But before starting, it'd be good to have a document listing basic principles to guide developers how and when to use structural logging. Moreover, if we want to transfer to structural logging entirely, we'd better come up with a CI script to guard that as well.

BTW: I recalled @damemi raised an issue in k/k to improve logging, right?

[damemi]

@Huang-Wei yep, that issue is here: kubernetes/kubernetes#91633

[seanmalloy]

/kind cleanup

[fejta-bot]

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-contributor-experience at kubernetes/community.
/lifecycle stale

[seanmalloy]

/remove-lifecycle stale

[fejta-bot]

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-contributor-experience at kubernetes/community.
/lifecycle stale

[seanmalloy]

/remove-lifecycle stale

[JaneLiuL]

Could I try to fix this issue as the good first issue that I can try to contribute to this gitrepository?
/assign

[Huang-Wei]

This issue needs help :)

I run a search across the codebase and separate the migration work evenly into 6 parts. If anyone is interested, just leave a comment below that which part you want to take:

(the number indicates the occurrences of klog in that file)

• Part 1 (Migrate to structured logging #247)

  cmd/controller/app/server.go:4
  test/util/fake.go:3
  test/util/aggregator.go:2
  test/integration/elasticquota_controller_test.go:5
  test/integration/noderesourcetopology_test.go:2
  test/integration/capacity_scheduling_test.go:2
  test/integration/base.go:3
  test/integration/coscheduling_test.go:2
  

• Part 2 (Migrate to structured logging Part 2 #245)

  pkg/crossnodepreemption/cross_node_preemption.go:3
  pkg/coscheduling/core/core.go:9
  pkg/coscheduling/coscheduling.go:15
  

• Part 3 (Migrate structure logging 3 #267)

  pkg/trimaran/handler.go:2
  pkg/trimaran/loadvariationriskbalancing/analysis.go:5
  pkg/trimaran/loadvariationriskbalancing/collector.go:7
  pkg/trimaran/loadvariationriskbalancing/loadvariationriskbalancing.go:7
  pkg/trimaran/targetloadpacking/targetloadpacking.go:11
  pkg/trimaran/targetloadpacking/targetloadpacking_test.go:2
  

• Part 4 (Migrate to Structured Logs in pkg/controller #250)

  pkg/controller/elasticquota.go:16
  pkg/controller/podgroup.go:12
  pkg/controller/elasticquota_test.go:2
  

• Part 5 (Migrate noderesources and noderesourcetopology directories to structured logging #259)

  pkg/noderesources/allocatable.go:2
  pkg/noderesources/resource_allocation.go:6
  pkg/noderesourcetopology/filter_test.go:2
  pkg/noderesourcetopology/filter.go:14
  

• Part 6 (Structured Logging migration: modify part logs of controller #235)

  pkg/capacityscheduling/capacity_scheduling.go:24
  

• Leftover (Leftover for structured logging migration #273)

[Huang-Wei]

The structured logging KEP explains the motivation and internal details. And the migration doc is a practical guide to explain how to perform the migration.

[pravarag]

I can take the Part 1 for the logging issue as mentioned above :)

[arvidOtt]

I am working on part 2.

[CIPHERTron]

I'm willing to take up Part 4 🙌🏻

[sayantani11]

I'm willing to take part 3

[yharish991]

i can take part 5

[Darshnadas]

I am willing to take part 6

[Huang-Wei]

@Darshnadas Part 6 had an owner already :)

[Huang-Wei]

/reopen

[k8s-ci-robot]

@Huang-Wei: Reopened this issue.

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.

[Darshnadas]

Yes, I did not notice it earlier. No issues :)

[jyz0309]

I am willing to take part 5. @Huang-Wei

[Huang-Wei]

I am willing to take part 5. @Huang-Wei

it's claimed by @yharish991. @yharish991 are you still working part 5?

[jyz0309]

I am willing to take part 5. @Huang-Wei

it's claimed by @yharish991. @yharish991 are you still working part 5?

oh...I really sorry that I really have not noticed that. @yharish991 If you still work on this. I will close my PR.

[Huang-Wei]

oh...I really sorry that I really have not noticed that.

That's no problem.

@yharish991 If you still work on this. I will close my PR.

Let's wait a couple of days (until this weekend). If he is not working on it, I will go review your PR.

[yharish991]

@jyz0309 @Huang-Wei i'm almost done with this work, and was about to open a PR, but since @jyz0309 already opened a PR, let's review his PR :) I'm good with that :)

[jyz0309]

@jyz0309 @Huang-Wei i'm almost done with this work, and was about to open a PR, but since @jyz0309 already opened a PR, let's review his PR :) I'm good with that :)

No. Just open your PR. :) I have closed my PR.😉

[yharish991]

@Huang-Wei opened the pr for part 5 #259

[pravarag]

I'll be working on the CI script part as mentioned here in this comment: #120 (comment) .

[pravarag]

@Huang-Wei @damemi few questions on adding the CI check for structured logging:

1. The verify script will scan the diffs for the logging changes, if any. Is there a klog CLI which can be run during the job to verify the changes? Or is there any other way to filter for all the rules and structural changes in the commit.
2. I'm guessing the ideal place to put the verify script will be this location:https://github.com/kubernetes-sigs/scheduler-plugins/tree/master/hack
3. For tracking the progress, shall I continue in this issue itself or open a new issue for the same?

Any other suggestions you have which I can work on, please let me know 🙂

[Huang-Wei]

Re 1: klog is more a SDK, so I don't think there is a CLI available. We don't need to get the changes in a commit. Instead, check the whole project. For sure, some folders like /vendor can be ignored. Ref: https://github.com/kubernetes/kubernetes/blob/master/hack/verify-gofmt.sh

Re 2: Nope, we should put it under <project>/hack/.

Re 3: Please open an new issue for tracking.

[pravarag]

I've created the following issue: #261 for tracking further details/changes to it.

[Huang-Wei]

/reopen

[k8s-ci-robot]

@Huang-Wei: Reopened this issue.

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.

[Huang-Wei]

All items have been completed. Close this issue.