Skip to content

Commit cc856bb

Browse files
pohlypohly
committed
update to logr 1.3.0
LogSink.Enabled is sensitive to stack unwinding. logr v1.2.0 got this wrong by calling LogSink.Enabled at different depths for Logger.Info and Logger.Enabled, therefore this test fails with that version. logr v1.3.0 changes the behavior to the more sane "LogSink.Enabled needs to skip one level, regardless where it is called". The Discard log sink implementation changed such that a discard logger doesn't have a sink at all anymore. Go < 1.18 is no longer supported by logr.
1 parent 02e7b69 commit cc856bb

File tree

10 files changed

+112
-14
lines changed

10 files changed

+112
-14
lines changed

‎contextual_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ func ExampleSetLogger() {
4141
// Output:
4242
// logger after SetLogger: *klog.klogger
4343
// logger after SetLoggerWithOptions with ContextualLogger(false): *klog.klogger
44-
// logger after SetLoggerWithOptions with ContextualLogger(true): logr.discardLogSink
44+
// logger after SetLoggerWithOptions with ContextualLogger(true): <nil>
4545
}
4646

4747
func ExampleFlushLogger() {

‎examples/go.mod

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ module k8s.io/klog/examples
33
go 1.13
44

55
require (
6-
github.com/go-logr/logr v1.2.2
6+
github.com/go-logr/logr v1.3.0
77
github.com/go-logr/zapr v1.2.3
88
github.com/golang/glog v0.0.0-20160126235308-23def4e6c14b
99
go.uber.org/goleak v1.1.12

‎examples/go.sum

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,9 +3,9 @@ github.com/benbjohnson/clock v1.1.0/go.mod h1:J11/hYXuz8f4ySSvYwY0FKfm+ezbsZBKZx
33
github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
44
github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c=
55
github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
6-
github.com/go-logr/logr v1.2.0/go.mod h1:jdQByPbusPIv2/zmleS9BjJVeZ6kBagPoEUsqbVz/1A=
7-
github.com/go-logr/logr v1.2.2 h1:ahHml/yUpnlb96Rp8HCvtYVPY8ZYpxq3g7UYchIYwbs=
86
github.com/go-logr/logr v1.2.2/go.mod h1:jdQByPbusPIv2/zmleS9BjJVeZ6kBagPoEUsqbVz/1A=
7+
github.com/go-logr/logr v1.3.0 h1:2y3SDp0ZXuc6/cjLSZ+Q3ir+QB9T/iG5yYRXqsagWSY=
8+
github.com/go-logr/logr v1.3.0/go.mod h1:9T104GzyrTigFIr8wt5mBrctHMim0Nb2HLGrmQ40KvY=
99
github.com/go-logr/zapr v1.2.3 h1:a9vnzlIBPQBBkeaR9IuMUfmVOrQlkoC4YfPoFkX3T7A=
1010
github.com/go-logr/zapr v1.2.3/go.mod h1:eIauM6P8qSvTw5o2ez6UEAfGjQKrxQTl5EoK+Qa2oG4=
1111
github.com/golang/glog v0.0.0-20160126235308-23def4e6c14b h1:VKtxabqXZkF25pY9ekfRL6a582T4P37/31XEstQ5p58=

‎go.mod

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,4 +2,4 @@ module k8s.io/klog/v2
22

33
go 1.13
44

5-
require github.com/go-logr/logr v1.2.0
5+
require github.com/go-logr/logr v1.3.0

‎go.sum

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,2 +1,2 @@
1-
github.com/go-logr/logr v1.2.0 h1:QK40JKJyMdUDz+h+xvCsru/bJhvG0UxvePV0ufL/AcE=
2-
github.com/go-logr/logr v1.2.0/go.mod h1:jdQByPbusPIv2/zmleS9BjJVeZ6kBagPoEUsqbVz/1A=
1+
github.com/go-logr/logr v1.3.0 h1:2y3SDp0ZXuc6/cjLSZ+Q3ir+QB9T/iG5yYRXqsagWSY=
2+
github.com/go-logr/logr v1.3.0/go.mod h1:9T104GzyrTigFIr8wt5mBrctHMim0Nb2HLGrmQ40KvY=

‎klogr.go

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -58,8 +58,7 @@ func (l *klogger) Info(level int, msg string, kvList ...interface{}) {
5858
}
5959

6060
func (l *klogger) Enabled(level int) bool {
61-
// Skip this function and logr.Logger.Info where Enabled is called.
62-
return VDepth(l.callDepth+2, Level(level)).Enabled()
61+
return VDepth(l.callDepth+1, Level(level)).Enabled()
6362
}
6463

6564
func (l *klogger) Error(err error, msg string, kvList ...interface{}) {

‎klogr/klogr.go

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -144,8 +144,7 @@ func (l *klogger) Info(level int, msg string, kvList ...interface{}) {
144144
}
145145

146146
func (l *klogger) Enabled(level int) bool {
147-
// Skip this function and logr.Logger.Info where Enabled is called.
148-
return klog.VDepth(l.callDepth+2, klog.Level(level)).Enabled()
147+
return klog.VDepth(l.callDepth+1, klog.Level(level)).Enabled()
149148
}
150149

151150
func (l *klogger) Error(err error, msg string, kvList ...interface{}) {

‎klogr_helper_test.go

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
/*
2+
Copyright 2023 The Kubernetes Authors.
3+
4+
Licensed under the Apache License, Version 2.0 (the "License");
5+
you may not use this file except in compliance with the License.
6+
You may obtain a copy of the License at
7+
8+
http://www.apache.org/licenses/LICENSE-2.0
9+
10+
Unless required by applicable law or agreed to in writing, software
11+
distributed under the License is distributed on an "AS IS" BASIS,
12+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
See the License for the specific language governing permissions and
14+
limitations under the License.
15+
*/
16+
17+
package klog_test
18+
19+
import (
20+
"testing"
21+
22+
"k8s.io/klog/v2"
23+
)
24+
25+
func testVerbosity(t *testing.T, logger klog.Logger) {
26+
// This runs with -v=5 -vmodule=klog_helper_test=10.
27+
logger.V(11).Info("v11 not visible from klogr_helper_test.go")
28+
if logger.V(11).Enabled() {
29+
t.Error("V(11).Enabled() in klogr_helper_test.go should have returned false.")
30+
}
31+
logger.V(10).Info("v10 visible from klogr_helper_test.go")
32+
if !logger.V(10).Enabled() {
33+
t.Error("V(10).Enabled() in klogr_helper_test.go should have returned true.")
34+
}
35+
}

‎klogr_test.go

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,67 @@
1+
/*
2+
Copyright 2023 The Kubernetes Authors.
3+
4+
Licensed under the Apache License, Version 2.0 (the "License");
5+
you may not use this file except in compliance with the License.
6+
You may obtain a copy of the License at
7+
8+
http://www.apache.org/licenses/LICENSE-2.0
9+
10+
Unless required by applicable law or agreed to in writing, software
11+
distributed under the License is distributed on an "AS IS" BASIS,
12+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
See the License for the specific language governing permissions and
14+
limitations under the License.
15+
*/
16+
17+
package klog_test
18+
19+
import (
20+
"bytes"
21+
"flag"
22+
"regexp"
23+
"testing"
24+
25+
"k8s.io/klog/v2"
26+
)
27+
28+
func TestVerbosity(t *testing.T) {
29+
state := klog.CaptureState()
30+
defer state.Restore()
31+
32+
var fs flag.FlagSet
33+
klog.InitFlags(&fs)
34+
if err := fs.Set("v", "5"); err != nil {
35+
t.Fatalf("unexpected error: %v", err)
36+
}
37+
if err := fs.Set("vmodule", "klogr_helper_test=10"); err != nil {
38+
t.Fatalf("unexpected error: %v", err)
39+
}
40+
if err := fs.Set("logtostderr", "false"); err != nil {
41+
t.Fatalf("unexpected error: %v", err)
42+
}
43+
var buffer bytes.Buffer
44+
klog.SetOutput(&buffer)
45+
logger := klog.Background()
46+
47+
// -v=5 is in effect here.
48+
logger.V(6).Info("v6 not visible from klogr_test.go")
49+
if logger.V(6).Enabled() {
50+
t.Error("V(6).Enabled() in klogr_test.go should have returned false.")
51+
}
52+
logger.V(5).Info("v5 visible from klogr_test.go")
53+
if !logger.V(5).Enabled() {
54+
t.Error("V(5).Enabled() in klogr_test.go should have returned true.")
55+
}
56+
57+
// Now test with -v=5 -vmodule=klogr_helper_test=10.
58+
testVerbosity(t, logger)
59+
60+
klog.Flush()
61+
expected := `^.*v5 visible from klogr_test.go.*
62+
.*v10 visible from klogr_helper_test.go.*
63+
`
64+
if !regexp.MustCompile(expected).Match(buffer.Bytes()) {
65+
t.Errorf("Output did not match regular expression.\nOutput:\n%s\n\nRegular expression:\n%s\n", buffer.String(), expected)
66+
}
67+
}

‎textlogger/textlogger.go

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -77,9 +77,7 @@ func (l *tlogger) WithCallDepth(depth int) logr.LogSink {
7777
}
7878

7979
func (l *tlogger) Enabled(level int) bool {
80-
// Skip this function and the Logger.Info call, then
81-
// also any additional stack frames from WithCallDepth.
82-
return l.config.vstate.Enabled(verbosity.Level(level), 2+l.callDepth)
80+
return l.config.vstate.Enabled(verbosity.Level(level), 1+l.callDepth)
8381
}
8482

8583
func (l *tlogger) Info(_ int, msg string, kvList ...interface{}) {

0 commit comments

Comments
 (0)