logr v1.4.1 + SetSlogLogger #396

pohly · 2024-01-03T15:00:53Z

What this PR does / why we need it:

logr v1.4.1 moved the slog support code into the main package, which is now used by klog instead of the deprecated slogr package.

SetSlogLogger syntactic sugar, but it's still useful because it hides logr from developers who only care about klog and slog.

Which issue(s) this PR fixes

Related-to: kubernetes/kubernetes#120696 (comment)

/cc @thockin

Release note:

add SetSlogLogger (available with Go >= 1.21)

k8s-ci-robot · 2024-01-03T15:01:02Z

This issue is currently awaiting triage.

If klog contributors determine this is a relevant issue, they will accept it by applying the triage/accepted label and provide further guidance.

The triage/accepted label can be added by org members by writing /triage accepted in a comment.

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.

pohly · 2024-01-03T15:01:46Z

klogr_slog_test.go

@@ -27,7 +27,8 @@ import (
 	"os"
 	"time"

-	"github.com/go-logr/logr/slogr"
+	"github.com/go-logr/logr"
+


This blank line should have been added already earlier.

This moves slog support into the main package. Go >1.18 is needed for generic type parameters.

pohly · 2024-01-03T15:40:29Z

.github/workflows/test.yml

@@ -4,7 +4,7 @@ jobs:
  test:
    strategy:
      matrix:
-        go-version: [1.18, 1.19, 1.20, 1.21]


We were testing with Go 1.2 (float 1.20 = 1.2)...

kaisoz · 2024-01-08T14:41:10Z

/lgtm

pohly · 2024-01-08T14:42:46Z

/hold

In case someone else wants to review.

ricardoapl · 2024-01-08T17:04:36Z

Looks good to me

mengjiao-liu · 2024-01-09T07:22:26Z

contextual_slog.go

+)
+
+// SetSlogLogger reconfigures klog to log through the slog logger. The logger must not be nil.
+func SetSlogLogger(logger *slog.Logger) {


Can you add a corresponding unit test for this function? This can also be used as an example. It is similar to https://github.com/kubernetes/kubernetes/pull/120696/files#diff-e284ecee88b71e68a5cb5630bad5227e3787043dd7ef880d55b5529294e98b47R34-R39

Good point, thanks for ensuring that I don't take shortcuts... 🥵

I added a ExampleSetSlogLogger in contextual_slog_example_test.go: https://github.com/kubernetes/klog/compare/37230168d33647d10a14966ab1a359d5424110ed..e4deee85f1cbc9ac407804b4a0fc1a38123b62f2

This is syntactic sugar, but it's still useful because it hides logr from developers who only care about klog and slog.

logr v1.4.0 moved the slog support functions in the main package and deprecated slogr.

mengjiao-liu

/lgtm

k8s-ci-robot · 2024-01-10T02:28:34Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: kaisoz, mengjiao-liu, pohly

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:

~~OWNERS~~ [pohly]

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

pohly · 2024-01-10T06:48:11Z

/hold cancel

Second review is in.

[![Mend Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com) This PR contains the following updates: | Package | Change | Age | Adoption | Passing | Confidence | |---|---|---|---|---|---| | [k8s.io/klog/v2](https://togithub.com/kubernetes/klog) | `v2.110.1` -> `v2.120.1` | [![age](https://developer.mend.io/api/mc/badges/age/go/k8s.io%2fklog%2fv2/v2.120.1?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![adoption](https://developer.mend.io/api/mc/badges/adoption/go/k8s.io%2fklog%2fv2/v2.120.1?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![passing](https://developer.mend.io/api/mc/badges/compatibility/go/k8s.io%2fklog%2fv2/v2.110.1/v2.120.1?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![confidence](https://developer.mend.io/api/mc/badges/confidence/go/k8s.io%2fklog%2fv2/v2.110.1/v2.120.1?slim=true)](https://docs.renovatebot.com/merge-confidence/) | --- > [!WARNING] > Some dependencies could not be looked up. Check the Dependency Dashboard for more information. --- ### Release Notes <details> <summary>kubernetes/klog (k8s.io/klog/v2)</summary> ### [`v2.120.1`](https://togithub.com/kubernetes/klog/releases/tag/v2.120.1): Prepare klog release for Kubernetes v1.30 (Take 2) [Compare Source](https://togithub.com/kubernetes/klog/compare/v2.120.0...v2.120.1) #### What's Changed - textlogger: allow caller to override stack unwinding by [@pohly](https://togithub.com/pohly) in [https://github.com/kubernetes/klog/pull/397](https://togithub.com/kubernetes/klog/pull/397) **Full Changelog**: kubernetes/klog@v2.120.0...v2.120.1 ### [`v2.120.0`](https://togithub.com/kubernetes/klog/releases/tag/v2.120.0): Prepare klog release for Kubernetes v1.30 (Take 1) [Compare Source](https://togithub.com/kubernetes/klog/compare/v2.110.1...v2.120.0) #### What's Changed - OWNERS: remove serathius, add mengjiao-liu, promote pohly by [@pohly](https://togithub.com/pohly) in [https://github.com/kubernetes/klog/pull/394](https://togithub.com/kubernetes/klog/pull/394) - docs: clarify relationship between different features by [@pohly](https://togithub.com/pohly) in [https://github.com/kubernetes/klog/pull/395](https://togithub.com/kubernetes/klog/pull/395) - Add SafePtr wrapper by [@kaisoz](https://togithub.com/kaisoz) in [https://github.com/kubernetes/klog/pull/393](https://togithub.com/kubernetes/klog/pull/393) - logr v1.4.1 + SetSlogLogger by [@pohly](https://togithub.com/pohly) in [https://github.com/kubernetes/klog/pull/396](https://togithub.com/kubernetes/klog/pull/396) #### New Contributors - [@kaisoz](https://togithub.com/kaisoz) made their first contribution in [https://github.com/kubernetes/klog/pull/393](https://togithub.com/kubernetes/klog/pull/393) **Full Changelog**: kubernetes/klog@v2.110.1...v2.120.0 </details> --- ### Configuration 📅 **Schedule**: Branch creation - "on tuesday" (UTC), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] If you want to rebase/retry this PR, check this box --- This PR has been generated by [Mend Renovate](https://www.mend.io/free-developer-tools/renovate/). View repository job log [here](https://developer.mend.io/github/open-telemetry/opentelemetry-collector-contrib).  --------- Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> Co-authored-by: opentelemetrybot <107717825+opentelemetrybot@users.noreply.github.com>

…1721) [![Mend Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com) This PR contains the following updates: | Package | Change | Age | Adoption | Passing | Confidence | |---|---|---|---|---|---| | [k8s.io/klog/v2](https://togithub.com/kubernetes/klog) | `v2.110.1` -> `v2.120.1` | [![age](https://developer.mend.io/api/mc/badges/age/go/k8s.io%2fklog%2fv2/v2.120.1?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![adoption](https://developer.mend.io/api/mc/badges/adoption/go/k8s.io%2fklog%2fv2/v2.120.1?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![passing](https://developer.mend.io/api/mc/badges/compatibility/go/k8s.io%2fklog%2fv2/v2.110.1/v2.120.1?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![confidence](https://developer.mend.io/api/mc/badges/confidence/go/k8s.io%2fklog%2fv2/v2.110.1/v2.120.1?slim=true)](https://docs.renovatebot.com/merge-confidence/) | --- > [!WARNING] > Some dependencies could not be looked up. Check the Dependency Dashboard for more information. --- ### Release Notes <details> <summary>kubernetes/klog (k8s.io/klog/v2)</summary> ### [`v2.120.1`](https://togithub.com/kubernetes/klog/releases/tag/v2.120.1): Prepare klog release for Kubernetes v1.30 (Take 2) [Compare Source](https://togithub.com/kubernetes/klog/compare/v2.120.0...v2.120.1) #### What's Changed - textlogger: allow caller to override stack unwinding by [@&open-telemetry#8203;pohly](https://togithub.com/pohly) in [https://github.com/kubernetes/klog/pull/397](https://togithub.com/kubernetes/klog/pull/397) **Full Changelog**: kubernetes/klog@v2.120.0...v2.120.1 ### [`v2.120.0`](https://togithub.com/kubernetes/klog/releases/tag/v2.120.0): Prepare klog release for Kubernetes v1.30 (Take 1) [Compare Source](https://togithub.com/kubernetes/klog/compare/v2.110.1...v2.120.0) #### What's Changed - OWNERS: remove serathius, add mengjiao-liu, promote pohly by [@&open-telemetry#8203;pohly](https://togithub.com/pohly) in [https://github.com/kubernetes/klog/pull/394](https://togithub.com/kubernetes/klog/pull/394) - docs: clarify relationship between different features by [@&open-telemetry#8203;pohly](https://togithub.com/pohly) in [https://github.com/kubernetes/klog/pull/395](https://togithub.com/kubernetes/klog/pull/395) - Add SafePtr wrapper by [@&open-telemetry#8203;kaisoz](https://togithub.com/kaisoz) in [https://github.com/kubernetes/klog/pull/393](https://togithub.com/kubernetes/klog/pull/393) - logr v1.4.1 + SetSlogLogger by [@&open-telemetry#8203;pohly](https://togithub.com/pohly) in [https://github.com/kubernetes/klog/pull/396](https://togithub.com/kubernetes/klog/pull/396) #### New Contributors - [@&open-telemetry#8203;kaisoz](https://togithub.com/kaisoz) made their first contribution in [https://github.com/kubernetes/klog/pull/393](https://togithub.com/kubernetes/klog/pull/393) **Full Changelog**: kubernetes/klog@v2.110.1...v2.120.0 </details> --- ### Configuration 📅 **Schedule**: Branch creation - "on tuesday" (UTC), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] If you want to rebase/retry this PR, check this box --- This PR has been generated by [Mend Renovate](https://www.mend.io/free-developer-tools/renovate/). View repository job log [here](https://developer.mend.io/github/open-telemetry/opentelemetry-collector-contrib).  --------- Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> Co-authored-by: opentelemetrybot <107717825+opentelemetrybot@users.noreply.github.com>

k8s-ci-robot requested a review from thockin January 3, 2024 15:00

k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Jan 3, 2024

k8s-ci-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Jan 3, 2024

pohly commented Jan 3, 2024

View reviewed changes

pohly changed the title ~~logt v1.4.1 + SetSlogLogger~~ logr v1.4.1 + SetSlogLogger Jan 3, 2024

pohly force-pushed the slog-helper branch 3 times, most recently from ee73172 to 98c28e2 Compare January 3, 2024 15:23

dependencies: logr v1.4.1

39afdba

This moves slog support into the main package. Go >1.18 is needed for generic type parameters.

pohly force-pushed the slog-helper branch from 98c28e2 to 3723016 Compare January 3, 2024 15:39

pohly commented Jan 3, 2024

View reviewed changes

k8s-ci-robot assigned kaisoz Jan 8, 2024

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jan 8, 2024

k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jan 8, 2024

mengjiao-liu reviewed Jan 9, 2024

View reviewed changes

pohly added 2 commits January 9, 2024 09:46

add SetSlogLogger

5d1d2d5

This is syntactic sugar, but it's still useful because it hides logr from developers who only care about klog and slog.

slog: use main logr package instead of logr/slogr

e4deee8

logr v1.4.0 moved the slog support functions in the main package and deprecated slogr.

pohly force-pushed the slog-helper branch from 3723016 to e4deee8 Compare January 9, 2024 08:46

k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed lgtm "Looks good to me", indicates that a PR is ready to be merged. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Jan 9, 2024

mengjiao-liu reviewed Jan 10, 2024

View reviewed changes

k8s-ci-robot assigned mengjiao-liu Jan 10, 2024

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jan 10, 2024

k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jan 10, 2024

k8s-ci-robot merged commit 2d08296 into kubernetes:main Jan 10, 2024
17 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

logr v1.4.1 + SetSlogLogger #396

logr v1.4.1 + SetSlogLogger #396

pohly commented Jan 3, 2024

k8s-ci-robot commented Jan 3, 2024

pohly Jan 3, 2024

pohly Jan 3, 2024

kaisoz commented Jan 8, 2024

pohly commented Jan 8, 2024

ricardoapl commented Jan 8, 2024

mengjiao-liu Jan 9, 2024

pohly Jan 9, 2024

mengjiao-liu left a comment

k8s-ci-robot commented Jan 10, 2024

pohly commented Jan 10, 2024

logr v1.4.1 + SetSlogLogger #396

logr v1.4.1 + SetSlogLogger #396

Conversation

pohly commented Jan 3, 2024

k8s-ci-robot commented Jan 3, 2024

pohly Jan 3, 2024

Choose a reason for hiding this comment

pohly Jan 3, 2024

Choose a reason for hiding this comment

kaisoz commented Jan 8, 2024

pohly commented Jan 8, 2024

ricardoapl commented Jan 8, 2024

mengjiao-liu Jan 9, 2024

Choose a reason for hiding this comment

pohly Jan 9, 2024

Choose a reason for hiding this comment

mengjiao-liu left a comment

Choose a reason for hiding this comment

k8s-ci-robot commented Jan 10, 2024

pohly commented Jan 10, 2024