Skip to content

Commit f5d68db

Browse files
authored
log: allow different log levels (#1210)
* log: allow different log levels With this feature we can use --zap-level 10 and this will allow log.V(10).Info("hello world") to be seen in the logs. Prior to this change debug (log.V(1)) was the lowest value attainable. We also disable sampling when using an integer less than -1 (i.e. beyond debug). The zap sampling code assumes the lowest number for sampling is DebugLevel (-1) which isn't the case in this scenario. Disabling sampling makes sense at this point because sampling is used for better performing logs which isn't required for trace level type logs. We also handle negative values being passed in. * log: add unit test for flags. Fix error value. * support more levels & remove debug level warning * add logger test * do not allow negative log levels at the CLI * disable sampling later in the call based on review * add note about disabling sampling to help * update sampling doc, fix log level help * make sample text consistent with log level
1 parent c6e209a commit f5d68db

File tree

5 files changed

+425
-10
lines changed

5 files changed

+425
-10
lines changed

doc/user/logging.md

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -38,10 +38,9 @@ In the above example, we add the zap flagset to the operator's command line flag
3838
By default, `zap.Logger()` will return a logger that is ready for production use. It uses a JSON encoder, logs starting at the `info` level, and has [sampling][zap_sampling] enabled. To customize the default behavior, users can use the zap flagset and specify flags on the command line. The zap flagset includes the following flags that can be used to configure the logger:
3939
* `--zap-devel` - Enables the zap development config (changes defaults to console encoder, debug log level, and disables sampling) (default: `false`)
4040
* `--zap-encoder` string - Sets the zap log encoding (`json` or `console`)
41-
* `--zap-level` string - Sets the zap log level (`debug`, `info`, or `error`)
42-
* `--zap-sample` - Enables zap's sampling mode
41+
* `--zap-level` string or integer - Sets the zap log level (`debug`, `info`, `error`, or an integer value greater than 0)
42+
* `--zap-sample` - Enables zap's sampling mode. Sampling will be disabled for integer log levels greater than 1.
4343

44-
**NOTE:** Although the `logr` interface supports multiple debug levels (e.g. `log.V(1).Info()`, `log.V(2).Info()`, etc.), zap supports only a single debug level with `log.V(1).Info()`. Log statements with higher debug levels will not be printed with the zap's `logr` backend.
4544

4645
## Creating a structured log statement
4746

pkg/log/zap/flags.go

Lines changed: 22 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -37,8 +37,8 @@ func init() {
3737
zapFlagSet = pflag.NewFlagSet("zap", pflag.ExitOnError)
3838
zapFlagSet.BoolVar(&development, "zap-devel", false, "Enable zap development mode (changes defaults to console encoder, debug log level, and disables sampling)")
3939
zapFlagSet.Var(&encoderVal, "zap-encoder", "Zap log encoding ('json' or 'console')")
40-
zapFlagSet.Var(&levelVal, "zap-level", "Zap log level (one of 'debug', 'info', 'error')")
41-
zapFlagSet.Var(&sampleVal, "zap-sample", "Enable zap log sampling")
40+
zapFlagSet.Var(&levelVal, "zap-level", "Zap log level (one of 'debug', 'info', 'error' or any integer value > 0)")
41+
zapFlagSet.Var(&sampleVal, "zap-sample", "Enable zap log sampling. Sampling will be disabled for integer log levels > 1")
4242
}
4343

4444
func FlagSet() *pflag.FlagSet {
@@ -91,11 +91,28 @@ type levelValue struct {
9191
func (v *levelValue) Set(l string) error {
9292
v.set = true
9393
lower := strings.ToLower(l)
94+
var lvl int
9495
switch lower {
95-
case "debug", "info", "error":
96-
return v.level.Set(l)
96+
case "debug":
97+
lvl = -1
98+
case "info":
99+
lvl = 0
100+
case "error":
101+
lvl = 2
102+
default:
103+
i, err := strconv.Atoi(lower)
104+
if err != nil {
105+
return fmt.Errorf("invalid log level \"%s\"", l)
106+
}
107+
108+
if i > 0 {
109+
lvl = -1 * i
110+
} else {
111+
return fmt.Errorf("invalid log level \"%s\"", l)
112+
}
97113
}
98-
return fmt.Errorf("invalid log level \"%s\"", l)
114+
v.level = zapcore.Level(int8(lvl))
115+
return nil
99116
}
100117

101118
func (v levelValue) String() string {

pkg/log/zap/flags_test.go

Lines changed: 202 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,202 @@
1+
// Copyright 2019 The Operator-SDK Authors
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// http://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
15+
package zap
16+
17+
import (
18+
"testing"
19+
20+
"github.com/stretchr/testify/assert"
21+
22+
"go.uber.org/zap/zapcore"
23+
)
24+
25+
func TestLevel(t *testing.T) {
26+
testCases := []struct {
27+
name string
28+
input string
29+
shouldErr bool
30+
expStr string
31+
expSet bool
32+
expLevel zapcore.Level
33+
}{
34+
{
35+
name: "debug level set",
36+
input: "debug",
37+
expStr: "debug",
38+
expSet: true,
39+
expLevel: zapcore.DebugLevel,
40+
},
41+
{
42+
name: "info level set",
43+
input: "info",
44+
expStr: "info",
45+
expSet: true,
46+
expLevel: zapcore.InfoLevel,
47+
},
48+
{
49+
name: "error level set",
50+
input: "error",
51+
expStr: "error",
52+
expSet: true,
53+
expLevel: zapcore.ErrorLevel,
54+
},
55+
{
56+
name: "negative number should error",
57+
input: "-10",
58+
shouldErr: true,
59+
expSet: false,
60+
},
61+
{
62+
name: "positive number level results in negative level set",
63+
input: "8",
64+
expStr: "Level(-8)",
65+
expSet: true,
66+
expLevel: zapcore.Level(int8(-8)),
67+
},
68+
{
69+
name: "non-integer should cause error",
70+
input: "invalid",
71+
shouldErr: true,
72+
expSet: false,
73+
},
74+
}
75+
76+
for _, tc := range testCases {
77+
t.Run(tc.name, func(t *testing.T) {
78+
lvl := levelValue{}
79+
err := lvl.Set(tc.input)
80+
if err != nil && !tc.shouldErr {
81+
t.Fatalf("Unknown error - %v", err)
82+
}
83+
if err != nil && tc.shouldErr {
84+
return
85+
}
86+
assert.Equal(t, tc.expSet, lvl.set)
87+
assert.Equal(t, tc.expLevel, lvl.level)
88+
assert.Equal(t, "level", lvl.Type())
89+
assert.Equal(t, tc.expStr, lvl.String())
90+
})
91+
}
92+
}
93+
94+
func TestSample(t *testing.T) {
95+
testCases := []struct {
96+
name string
97+
input string
98+
shouldErr bool
99+
expStr string
100+
expSet bool
101+
expValue bool
102+
}{
103+
{
104+
name: "enable sampling",
105+
input: "true",
106+
expStr: "true",
107+
expSet: true,
108+
expValue: true,
109+
},
110+
{
111+
name: "disable sampling",
112+
input: "false",
113+
expStr: "false",
114+
expSet: true,
115+
expValue: false,
116+
},
117+
{
118+
name: "invalid input",
119+
input: "notaboolean",
120+
shouldErr: true,
121+
expSet: false,
122+
},
123+
{
124+
name: "UPPERCASE true input",
125+
input: "true",
126+
expStr: "true",
127+
expSet: true,
128+
expValue: true,
129+
},
130+
{
131+
name: "MiXeDCase true input",
132+
input: "tRuE",
133+
shouldErr: true,
134+
expSet: false,
135+
},
136+
}
137+
138+
for _, tc := range testCases {
139+
t.Run(tc.name, func(t *testing.T) {
140+
sample := sampleValue{}
141+
err := sample.Set(tc.input)
142+
if err != nil && !tc.shouldErr {
143+
t.Fatalf("Unknown error - %v", err)
144+
}
145+
if err != nil && tc.shouldErr {
146+
return
147+
}
148+
assert.Equal(t, tc.expSet, sample.set)
149+
assert.Equal(t, tc.expValue, sample.sample)
150+
assert.Equal(t, "sample", sample.Type())
151+
assert.Equal(t, tc.expStr, sample.String())
152+
assert.True(t, sample.IsBoolFlag())
153+
})
154+
}
155+
}
156+
func TestEncoder(t *testing.T) {
157+
testCases := []struct {
158+
name string
159+
input string
160+
shouldErr bool
161+
expStr string
162+
expSet bool
163+
expEncoder zapcore.Encoder
164+
}{
165+
{
166+
name: "json encoder",
167+
input: "json",
168+
expStr: "json",
169+
expSet: true,
170+
expEncoder: jsonEncoder(),
171+
},
172+
{
173+
name: "console encoder",
174+
input: "console",
175+
expStr: "console",
176+
expSet: true,
177+
expEncoder: consoleEncoder(),
178+
},
179+
{
180+
name: "unknown encoder",
181+
input: "unknown",
182+
shouldErr: true,
183+
expEncoder: nil,
184+
},
185+
}
186+
for _, tc := range testCases {
187+
t.Run(tc.name, func(t *testing.T) {
188+
encoder := encoderValue{}
189+
err := encoder.Set(tc.input)
190+
if err != nil && !tc.shouldErr {
191+
t.Fatalf("Unknown error - %v", err)
192+
}
193+
if err != nil && tc.shouldErr {
194+
return
195+
}
196+
assert.Equal(t, tc.expSet, encoder.set)
197+
assert.Equal(t, "encoder", encoder.Type())
198+
assert.Equal(t, tc.expStr, encoder.String())
199+
assert.ObjectsAreEqual(tc.expEncoder, encoder.encoder)
200+
})
201+
}
202+
}

pkg/log/zap/logger.go

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ func Logger() logr.Logger {
3232

3333
func LoggerTo(destWriter io.Writer) logr.Logger {
3434
syncer := zapcore.AddSync(destWriter)
35-
conf := getConfig(destWriter)
35+
conf := getConfig()
3636

3737
conf.encoder = &logf.KubeAwareEncoder{Encoder: conf.encoder, Verbose: conf.level.Level() < 0}
3838
if conf.sample {
@@ -53,7 +53,7 @@ type config struct {
5353
opts []zap.Option
5454
}
5555

56-
func getConfig(destWriter io.Writer) config {
56+
func getConfig() config {
5757
var c config
5858

5959
// Set the defaults depending on the log mode (development vs. production)
@@ -80,5 +80,10 @@ func getConfig(destWriter io.Writer) config {
8080
c.sample = sampleVal.sample
8181
}
8282

83+
// Disable sampling when we are in debug mode. Otherwise, this will
84+
// cause index out of bounds errors in the sampling code.
85+
if c.level.Level() < -1 {
86+
c.sample = false
87+
}
8388
return c
8489
}

0 commit comments

Comments
 (0)