Skip to content

Commit

Permalink
Fix IncreaseLevel being reset after With
Browse files Browse the repository at this point in the history
Fixes #810

We missed implementing the increase-level logic in With
and since Core was embedded, we wended up using With from
the original core.

To avoid this sort of issue, avoid embedding and implement all
Core methods explicitly. This lets us consider the behaviour
of each method explicitly.
  • Loading branch information
prashantv committed Apr 3, 2020
1 parent ec2dc45 commit ee180f3
Show file tree
Hide file tree
Showing 3 changed files with 36 additions and 5 deletions.
13 changes: 11 additions & 2 deletions increase_level_test.go
Expand Up @@ -29,10 +29,13 @@ import (
"go.uber.org/zap/zaptest/observer"
)

func newLoggedEntry(level zapcore.Level, msg string) observer.LoggedEntry {
func newLoggedEntry(level zapcore.Level, msg string, fields ...zapcore.Field) observer.LoggedEntry {
if len(fields) == 0 {
fields = []zapcore.Field{}
}
return observer.LoggedEntry{
Entry: zapcore.Entry{Level: level, Message: msg},
Context: []zapcore.Field{},
Context: fields,
}
}

Expand Down Expand Up @@ -75,9 +78,15 @@ func TestIncreaseLevel(t *testing.T) {
errorLogger.Warn("ignored warn log")
errorLogger.Error("increase level error log")

withFields := errorLogger.With(String("k", "v"))
withFields.Debug("ignored debug log with fields")
withFields.Warn("ignored warn log with fields")
withFields.Error("increase level error log with fields")

assert.Equal(t, []observer.LoggedEntry{
newLoggedEntry(WarnLevel, "original warn log"),
newLoggedEntry(ErrorLevel, "increase level error log"),
newLoggedEntry(ErrorLevel, "increase level error log with fields", String("k", "v")),
}, logs.AllUntimed(), "unexpected logs")

assert.Empty(t, errorOut.String(), "expect no error output")
Expand Down
17 changes: 14 additions & 3 deletions zapcore/increase_level.go
Expand Up @@ -23,8 +23,7 @@ package zapcore
import "fmt"

type levelFilterCore struct {
Core

core Core
level LevelEnabler
}

Expand All @@ -46,10 +45,22 @@ func (c *levelFilterCore) Enabled(lvl Level) bool {
return c.level.Enabled(lvl)
}

func (c *levelFilterCore) With(fields []Field) Core {
return &levelFilterCore{c.core.With(fields), c.level}
}

func (c *levelFilterCore) Check(ent Entry, ce *CheckedEntry) *CheckedEntry {
if !c.Enabled(ent.Level) {
return ce
}

return c.Core.Check(ent, ce)
return c.core.Check(ent, ce)
}

func (c *levelFilterCore) Write(ent Entry, fields []Field) error {
return c.core.Write(ent, fields)
}

func (c *levelFilterCore) Sync() error {
return c.core.Sync()
}
11 changes: 11 additions & 0 deletions zapcore/increase_level_test.go
Expand Up @@ -26,6 +26,7 @@ import (

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"go.uber.org/zap"
. "go.uber.org/zap/zapcore"
"go.uber.org/zap/zaptest/observer"
)
Expand All @@ -35,6 +36,7 @@ func TestIncreaseLevel(t *testing.T) {
coreLevel Level
increaseLevel Level
wantErr bool
with []Field
}{
{
coreLevel: InfoLevel,
Expand All @@ -49,6 +51,11 @@ func TestIncreaseLevel(t *testing.T) {
coreLevel: InfoLevel,
increaseLevel: ErrorLevel,
},
{
coreLevel: InfoLevel,
increaseLevel: ErrorLevel,
with: []Field{zap.String("k", "v")},
},
{
coreLevel: ErrorLevel,
increaseLevel: DebugLevel,
Expand Down Expand Up @@ -82,6 +89,10 @@ func TestIncreaseLevel(t *testing.T) {
return
}

if len(tt.with) > 0 {
filteredLogger = filteredLogger.With(tt.with)
}

require.NoError(t, err)

for l := DebugLevel; l <= FatalLevel; l++ {
Expand Down

0 comments on commit ee180f3

Please sign in to comment.