From ee0d923ee24bfd018047894e0e5df2eefcef391d Mon Sep 17 00:00:00 2001 From: Prashant Varanasi Date: Sun, 22 Dec 2019 16:51:57 -0800 Subject: [PATCH 1/5] [WIP] Add option to increase the level of a logger We've had a few requests on how to change the level of a logger. With the current APIs, it's not possible to reduce the log level (e.g., go from Info to Debug), but it is possible to increase the level using `WrapCore` with a custom filtering core. Since it's a common request, I think we should add an option to make this easy to use. I want to make sure "increase" is part of the API to avoid confusion on whether it can reduce the log level. Fixes #774. --- logger_test.go | 15 ++++++++++++++ options.go | 7 +++++++ zapcore/level_filter.go | 45 +++++++++++++++++++++++++++++++++++++++++ 3 files changed, 67 insertions(+) create mode 100644 zapcore/level_filter.go diff --git a/logger_test.go b/logger_test.go index a9f705788..e6b0d96e7 100644 --- a/logger_test.go +++ b/logger_test.go @@ -392,6 +392,21 @@ func TestLoggerReplaceCore(t *testing.T) { }) } +func TestLoggerIncreaseLevel(t *testing.T) { + withLogger(t, DebugLevel, opts(IncreaseLevel(WarnLevel)), func(logger *Logger, logs *observer.ObservedLogs) { + logger.Info("logger.Info") + logger.Warn("logger.Warn") + logger.Error("logger.Error") + require.Equal(t, 2, logs.Len(), "Expected no-op core to write no logs.") + assert.Equal( + t, + logs.AllUntimed()[0].Entry.Message, + "logger.Warn", + "Expected first logged message to be warn level message", + ) + }) +} + func TestLoggerHooks(t *testing.T) { hook, seen := makeCountingHook() withLogger(t, DebugLevel, opts(Hooks(hook)), func(logger *Logger, logs *observer.ObservedLogs) { diff --git a/options.go b/options.go index 7a6b0fca1..9ee608908 100644 --- a/options.go +++ b/options.go @@ -107,3 +107,10 @@ func AddStacktrace(lvl zapcore.LevelEnabler) Option { log.addStack = lvl }) } + +// IncreaseLevel increase the level of the logger. +func IncreaseLevel(lvl zapcore.LevelEnabler) Option { + return WrapCore(func(c zapcore.Core) zapcore.Core { + return zapcore.NewLevelCore(c, lvl) + }) +} diff --git a/zapcore/level_filter.go b/zapcore/level_filter.go new file mode 100644 index 000000000..869dd62e4 --- /dev/null +++ b/zapcore/level_filter.go @@ -0,0 +1,45 @@ +// Copyright (c) 2019 Uber Technologies, Inc. +// +// Permission is hereby granted, free of charge, to any person obtaining a copy +// of this software and associated documentation files (the "Software"), to deal +// in the Software without restriction, including without limitation the rights +// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +// copies of the Software, and to permit persons to whom the Software is +// furnished to do so, subject to the following conditions: +// +// The above copyright notice and this permission notice shall be included in +// all copies or substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN +// THE SOFTWARE. + +package zapcore + +type levelFilterCore struct { + Core + level LevelEnabler +} + +// NewLevelCore creates a core that can be used to increase the level of an existing +// Core. It cannot be used to decrease the logging level, as it acts as a filter +// before calling the underlying core. +func NewLevelCore(core Core, level LevelEnabler) Core { + return &levelFilterCore{core, level} +} + +func (c *levelFilterCore) Enabled(lvl Level) bool { + return c.level.Enabled(lvl) +} + +func (c *levelFilterCore) Check(ent Entry, ce *CheckedEntry) *CheckedEntry { + if !c.Enabled(ent.Level) { + return ce + } + + return c.Core.Check(ent, ce) +} From 1653481fa7a4f7b4dad3702b413bb34e40961bdb Mon Sep 17 00:00:00 2001 From: Prashant Varanasi Date: Fri, 24 Jan 2020 10:55:05 -0800 Subject: [PATCH 2/5] Rename to IncreaseLevel, add tests --- increase_level_test.go | 81 ++++++++++++++ options.go | 18 +++- .../{level_filter.go => increase_level.go} | 21 ++-- zapcore/increase_level_test.go | 102 ++++++++++++++++++ 4 files changed, 212 insertions(+), 10 deletions(-) create mode 100644 increase_level_test.go rename zapcore/{level_filter.go => increase_level.go} (67%) create mode 100644 zapcore/increase_level_test.go diff --git a/increase_level_test.go b/increase_level_test.go new file mode 100644 index 000000000..e37f1c2da --- /dev/null +++ b/increase_level_test.go @@ -0,0 +1,81 @@ +// Copyright (c) 2020 Uber Technologies, Inc. +// +// Permission is hereby granted, free of charge, to any person obtaining a copy +// of this software and associated documentation files (the "Software"), to deal +// in the Software without restriction, including without limitation the rights +// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +// copies of the Software, and to permit persons to whom the Software is +// furnished to do so, subject to the following conditions: +// +// The above copyright notice and this permission notice shall be included in +// all copies or substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN +// THE SOFTWARE. + +package zap + +import ( + "bytes" + "testing" + + "github.com/stretchr/testify/assert" + "go.uber.org/zap/zapcore" + "go.uber.org/zap/zaptest/observer" +) + +func newLoggedEntry(level zapcore.Level, msg string) observer.LoggedEntry { + return observer.LoggedEntry{ + Entry: zapcore.Entry{Level: level, Message: msg}, + Context: []zapcore.Field{}, + } +} + +func TestIncreaseLevelTryDecrease(t *testing.T) { + errorOut := &bytes.Buffer{} + opts := []Option{ + ErrorOutput(zapcore.AddSync(errorOut)), + } + withLogger(t, WarnLevel, opts, func(logger *Logger, logs *observer.ObservedLogs) { + logger.Warn("original warn log") + + debugLogger := logger.WithOptions(IncreaseLevel(DebugLevel)) + debugLogger.Debug("ignored debug log") + debugLogger.Warn("increase level warn log") + debugLogger.Error("increase level error log") + + assert.Equal(t, []observer.LoggedEntry{ + newLoggedEntry(WarnLevel, "original warn log"), + newLoggedEntry(WarnLevel, "increase level warn log"), + newLoggedEntry(ErrorLevel, "increase level error log"), + }, logs.AllUntimed(), "unexpected logs") + assert.Equal(t, "failed to IncreaseLevel: invalid increase level, as info is allowed by increased level, but not by existing core", errorOut.String(), "unexpected error output") + }) +} + +func TestIncreaseLevel(t *testing.T) { + errorOut := &bytes.Buffer{} + opts := []Option{ + ErrorOutput(zapcore.AddSync(errorOut)), + } + withLogger(t, WarnLevel, opts, func(logger *Logger, logs *observer.ObservedLogs) { + logger.Warn("original warn log") + + errorLogger := logger.WithOptions(IncreaseLevel(ErrorLevel)) + errorLogger.Debug("ignored debug log") + errorLogger.Warn("ignored warn log") + errorLogger.Error("increase level error log") + + assert.Equal(t, []observer.LoggedEntry{ + newLoggedEntry(WarnLevel, "original warn log"), + newLoggedEntry(ErrorLevel, "increase level error log"), + }, logs.AllUntimed(), "unexpected logs") + + assert.Empty(t, errorOut.String(), "expect no error output") + }) +} diff --git a/options.go b/options.go index 9ee608908..dd3b6b2b2 100644 --- a/options.go +++ b/options.go @@ -20,7 +20,11 @@ package zap -import "go.uber.org/zap/zapcore" +import ( + "fmt" + + "go.uber.org/zap/zapcore" +) // An Option configures a Logger. type Option interface { @@ -108,9 +112,15 @@ func AddStacktrace(lvl zapcore.LevelEnabler) Option { }) } -// IncreaseLevel increase the level of the logger. +// IncreaseLevel increase the level of the logger. It has no effect if +// the passed in level tries to decrease the level of the logger. func IncreaseLevel(lvl zapcore.LevelEnabler) Option { - return WrapCore(func(c zapcore.Core) zapcore.Core { - return zapcore.NewLevelCore(c, lvl) + return optionFunc(func(log *Logger) { + core, err := zapcore.NewIncreaseLevelCore(log.core, lvl) + if err != nil { + fmt.Fprintf(log.errorOutput, "failed to IncreaseLevel: %v", err) + } else { + log.core = core + } }) } diff --git a/zapcore/level_filter.go b/zapcore/increase_level.go similarity index 67% rename from zapcore/level_filter.go rename to zapcore/increase_level.go index 869dd62e4..7876a9f15 100644 --- a/zapcore/level_filter.go +++ b/zapcore/increase_level.go @@ -1,4 +1,4 @@ -// Copyright (c) 2019 Uber Technologies, Inc. +// Copyright (c) 2020 Uber Technologies, Inc. // // Permission is hereby granted, free of charge, to any person obtaining a copy // of this software and associated documentation files (the "Software"), to deal @@ -20,16 +20,25 @@ package zapcore +import "fmt" + type levelFilterCore struct { Core level LevelEnabler } -// NewLevelCore creates a core that can be used to increase the level of an existing -// Core. It cannot be used to decrease the logging level, as it acts as a filter -// before calling the underlying core. -func NewLevelCore(core Core, level LevelEnabler) Core { - return &levelFilterCore{core, level} +// NewIncreaseLevelCore creates a core that can be used to increase the level of +// an existing Core. It cannot be used to decrease the logging level, as it acts +// as a filter before calling the underlying core. If level decreases the log level, +// an error is returned. +func NewIncreaseLevelCore(core Core, level LevelEnabler) (Core, error) { + for l := _maxLevel; l >= _minLevel; l-- { + if !core.Enabled(l) && level.Enabled(l) { + return nil, fmt.Errorf("invalid increase level, as %v is allowed by increased level, but not by existing core", l) + } + } + + return &levelFilterCore{core, level}, nil } func (c *levelFilterCore) Enabled(lvl Level) bool { diff --git a/zapcore/increase_level_test.go b/zapcore/increase_level_test.go new file mode 100644 index 000000000..9254092c5 --- /dev/null +++ b/zapcore/increase_level_test.go @@ -0,0 +1,102 @@ +// Copyright (c) 2020 Uber Technologies, Inc. +// +// Permission is hereby granted, free of charge, to any person obtaining a copy +// of this software and associated documentation files (the "Software"), to deal +// in the Software without restriction, including without limitation the rights +// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +// copies of the Software, and to permit persons to whom the Software is +// furnished to do so, subject to the following conditions: +// +// The above copyright notice and this permission notice shall be included in +// all copies or substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN +// THE SOFTWARE. + +package zapcore_test + +import ( + "fmt" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + . "go.uber.org/zap/zapcore" + "go.uber.org/zap/zaptest/observer" +) + +func TestIncreaseLevel(t *testing.T) { + tests := []struct { + coreLevel Level + increaseLevel Level + wantErr bool + wantLogLevels []Level + }{ + { + coreLevel: InfoLevel, + increaseLevel: DebugLevel, + wantErr: true, + }, + { + coreLevel: InfoLevel, + increaseLevel: InfoLevel, + }, + { + coreLevel: InfoLevel, + increaseLevel: ErrorLevel, + }, + { + coreLevel: ErrorLevel, + increaseLevel: DebugLevel, + wantErr: true, + }, + { + coreLevel: ErrorLevel, + increaseLevel: InfoLevel, + wantErr: true, + }, + { + coreLevel: ErrorLevel, + increaseLevel: WarnLevel, + wantErr: true, + }, + { + coreLevel: ErrorLevel, + increaseLevel: PanicLevel, + }, + } + + for _, tt := range tests { + msg := fmt.Sprintf("increase %v to %v", tt.coreLevel, tt.increaseLevel) + t.Run(msg, func(t *testing.T) { + logger, _ := observer.New(tt.coreLevel) + + filteredLogger, err := NewIncreaseLevelCore(logger, tt.increaseLevel) + if tt.wantErr { + require.Error(t, err) + assert.Contains(t, err.Error(), "invalid increase level") + return + } + + require.NoError(t, err) + + for l := DebugLevel; l <= FatalLevel; l++ { + enabled := filteredLogger.Enabled(l) + ce := filteredLogger.Check(Entry{Level: l}, nil) + + if l >= tt.increaseLevel { + assert.True(t, enabled, "expect %v to be enabled", l) + assert.NotNil(t, ce, "expect non-nil Check") + } else { + assert.False(t, enabled, "expect %v to be disabled", l) + assert.Nil(t, ce, "expect nil Check") + } + } + }) + } +} From 056412ce6322aeac3ac59f78597d42d05de7eb0b Mon Sep 17 00:00:00 2001 From: Prashant Varanasi Date: Wed, 29 Jan 2020 21:21:47 -0800 Subject: [PATCH 3/5] Update zapcore/increase_level.go Co-Authored-By: Abhinav Gupta --- zapcore/increase_level.go | 1 + 1 file changed, 1 insertion(+) diff --git a/zapcore/increase_level.go b/zapcore/increase_level.go index 7876a9f15..0d7cf6e99 100644 --- a/zapcore/increase_level.go +++ b/zapcore/increase_level.go @@ -24,6 +24,7 @@ import "fmt" type levelFilterCore struct { Core + level LevelEnabler } From cea780b71ec773f2273e22d094be2cc0e3b4e811 Mon Sep 17 00:00:00 2001 From: Prashant Varanasi Date: Wed, 29 Jan 2020 21:21:57 -0800 Subject: [PATCH 4/5] Update zapcore/increase_level_test.go Co-Authored-By: Abhinav Gupta --- zapcore/increase_level_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/zapcore/increase_level_test.go b/zapcore/increase_level_test.go index 9254092c5..8b8613375 100644 --- a/zapcore/increase_level_test.go +++ b/zapcore/increase_level_test.go @@ -35,7 +35,6 @@ func TestIncreaseLevel(t *testing.T) { coreLevel Level increaseLevel Level wantErr bool - wantLogLevels []Level }{ { coreLevel: InfoLevel, From a36ef684a79e1a67a87bac2b3656ac16a29f0780 Mon Sep 17 00:00:00 2001 From: Prashant Varanasi Date: Wed, 29 Jan 2020 21:26:29 -0800 Subject: [PATCH 5/5] CR comments --- increase_level_test.go | 6 +++++- logger_test.go | 2 +- zapcore/increase_level.go | 2 +- 3 files changed, 7 insertions(+), 3 deletions(-) diff --git a/increase_level_test.go b/increase_level_test.go index e37f1c2da..d3e111464 100644 --- a/increase_level_test.go +++ b/increase_level_test.go @@ -54,7 +54,11 @@ func TestIncreaseLevelTryDecrease(t *testing.T) { newLoggedEntry(WarnLevel, "increase level warn log"), newLoggedEntry(ErrorLevel, "increase level error log"), }, logs.AllUntimed(), "unexpected logs") - assert.Equal(t, "failed to IncreaseLevel: invalid increase level, as info is allowed by increased level, but not by existing core", errorOut.String(), "unexpected error output") + assert.Equal(t, + `failed to IncreaseLevel: invalid increase level, as level "info" is allowed by increased level, but not by existing core`, + errorOut.String(), + "unexpected error output", + ) }) } diff --git a/logger_test.go b/logger_test.go index e6b0d96e7..dabcb95a3 100644 --- a/logger_test.go +++ b/logger_test.go @@ -397,7 +397,7 @@ func TestLoggerIncreaseLevel(t *testing.T) { logger.Info("logger.Info") logger.Warn("logger.Warn") logger.Error("logger.Error") - require.Equal(t, 2, logs.Len(), "Expected no-op core to write no logs.") + require.Equal(t, 2, logs.Len(), "expected only warn + error logs due to IncreaseLevel.") assert.Equal( t, logs.AllUntimed()[0].Entry.Message, diff --git a/zapcore/increase_level.go b/zapcore/increase_level.go index 0d7cf6e99..a42135c15 100644 --- a/zapcore/increase_level.go +++ b/zapcore/increase_level.go @@ -35,7 +35,7 @@ type levelFilterCore struct { func NewIncreaseLevelCore(core Core, level LevelEnabler) (Core, error) { for l := _maxLevel; l >= _minLevel; l-- { if !core.Enabled(l) && level.Enabled(l) { - return nil, fmt.Errorf("invalid increase level, as %v is allowed by increased level, but not by existing core", l) + return nil, fmt.Errorf("invalid increase level, as level %q is allowed by increased level, but not by existing core", l) } }