Update logging guidelines

[serathius]

Fixes kubernetes/kubernetes#105975

cc @pohly @shivanshu1333

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: serathius

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:

• contributors/devel/sig-instrumentation/OWNERS [serathius]

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

[sftim]

Hi @serathius

Thanks for this PR. I reviewed this using the docs style guide for context.
I wasn't able to understand all of the new text so I have guessed at the intended meaning. I've added suggestions and where I wasn't sure I've marked these with ?s.

I hope this review is useful. Please feel free to select only the suggestions you feel are right.

[sftim]

Is this correct?

Suggested change

	For projects that want to integrate with Kubernetes or write same log format we recommend using [klogr](https://github.com/kubernetes/klog/tree/main/klogr), reimplementation of klog format that exposes [logr](https://github.com/go-logr/logr) interface.
	For projects that want to integrate with Kubernetes or write same log format we recommend using [klogr](https://github.com/kubernetes/klog/tree/main/klogr). The `klogr` library exposes an interface that
	follows [logr](https://github.com/go-logr/logr) conventions, and emits log entries that are equivalent to `klog`.

[pohly]

I don't think we should recommend using klogr, or at least not without further explanations. For example, a library should accept a logr.Logger instance and let the caller decide whether that is klogr or something else.

But right now we are not ready yet to propose how such a library should accept such a logger - we need a decision about kubernetes/enhancements#3078 first.

[serathius]

@sftim Awesome job on the suggestions! Applied most of them. I only need to rewrite klogr section as @pohly mentioned we should not recommend it.

One of the goals of this PR was to start splitting migration guide vs logging good practices. As 1/3 of K8s codebase is already migrated we need to put more weight on good practices document vs migration. Currently migration document contains both migration instruction and some good practices. I was hoping that in future we will be able to rewrite good practices document to list of concrete rules with examples, and make the migration guide just reference selected rules.
What do you think about this idea?

[sftim]

What do you think about this idea?

SGTM, no strong preference either way.

[k8s-triage-robot]

The Kubernetes project currently lacks enough contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

• After 90d of inactivity, lifecycle/stale is applied
• After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
• After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

• Mark this issue or PR as fresh with /remove-lifecycle stale
• Mark this issue or PR as rotten with /lifecycle rotten
• Close this issue or PR with /close
• Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

[serathius]

/remove-lifecycle stale
/assign @pohly

[pohly]

I will have to update this section to cover the replacement of those calls with logging through Logger. This goes beyond a simple PR review, so let's get this merged first even if it is not fully up-to-date.

[serathius]

sg

[serathius]

@pohly addressed all the comments. PTAL

[pohly]

One last typo fix for my own suggestion, then we can merge. Can squash before your next push?

[serathius]

Done

[pohly]

/lgtm