Skip to content

Commit

Permalink
Merge branch 'uber-go:master' into feature/custom-mapping-level
Browse files Browse the repository at this point in the history
  • Loading branch information
arukiidou committed Feb 26, 2024
2 parents 6b841ca + 8f5ee80 commit 5a63490
Show file tree
Hide file tree
Showing 8 changed files with 214 additions and 28 deletions.
6 changes: 4 additions & 2 deletions .github/workflows/go.yml
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,9 @@ jobs:
run: make cover

- name: Upload coverage to codecov.io
uses: codecov/codecov-action@v3
uses: codecov/codecov-action@v4
env:
CODECOV_TOKEN: ${{ secrets.CODECOV_TOKEN }}

lint:
name: Lint
Expand All @@ -56,7 +58,7 @@ jobs:
go-version: 1.22.x
cache: false # managed by golangci-lint

- uses: golangci/golangci-lint-action@v3
- uses: golangci/golangci-lint-action@v4
name: Install golangci-lint
with:
version: latest
Expand Down
14 changes: 14 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,20 @@ All notable changes to this project will be documented in this file.

This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).

## 1.27.0 (20 Feb 2024)
Enhancements:
* [#1378][]: Add `WithLazy` method for `SugaredLogger`.
* [#1399][]: zaptest: Add `NewTestingWriter` for customizing TestingWriter with more flexibility than `NewLogger`.
* [#1406][]: Add `Log`, `Logw`, `Logln` methods for `SugaredLogger`.
* [#1416][]: Add `WithPanicHook` option for testing panic logs.

Thanks to @defval, @dimmo, @arxeiss, and @MKrupauskas for their contributions to this release.

[#1378]: https://github.com/uber-go/zap/pull/1378
[#1399]: https://github.com/uber-go/zap/pull/1399
[#1406]: https://github.com/uber-go/zap/pull/1406
[#1416]: https://github.com/uber-go/zap/pull/1416

## 1.26.0 (14 Sep 2023)
Enhancements:
* [#1297][]: Add Dict as a Field.
Expand Down
Binary file modified assets/logo.png
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
39 changes: 21 additions & 18 deletions logger.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ type Logger struct {

development bool
addCaller bool
onPanic zapcore.CheckWriteHook // default is WriteThenPanic
onFatal zapcore.CheckWriteHook // default is WriteThenFatal

name string
Expand Down Expand Up @@ -345,27 +346,12 @@ func (log *Logger) check(lvl zapcore.Level, msg string) *zapcore.CheckedEntry {
// Set up any required terminal behavior.
switch ent.Level {
case zapcore.PanicLevel:
ce = ce.After(ent, zapcore.WriteThenPanic)
ce = ce.After(ent, terminalHookOverride(zapcore.WriteThenPanic, log.onPanic))
case zapcore.FatalLevel:
onFatal := log.onFatal
// nil or WriteThenNoop will lead to continued execution after
// a Fatal log entry, which is unexpected. For example,
//
// f, err := os.Open(..)
// if err != nil {
// log.Fatal("cannot open", zap.Error(err))
// }
// fmt.Println(f.Name())
//
// The f.Name() will panic if we continue execution after the
// log.Fatal.
if onFatal == nil || onFatal == zapcore.WriteThenNoop {
onFatal = zapcore.WriteThenFatal
}
ce = ce.After(ent, onFatal)
ce = ce.After(ent, terminalHookOverride(zapcore.WriteThenFatal, log.onFatal))
case zapcore.DPanicLevel:
if log.development {
ce = ce.After(ent, zapcore.WriteThenPanic)
ce = ce.After(ent, terminalHookOverride(zapcore.WriteThenPanic, log.onPanic))
}
}

Expand Down Expand Up @@ -430,3 +416,20 @@ func (log *Logger) check(lvl zapcore.Level, msg string) *zapcore.CheckedEntry {

return ce
}

func terminalHookOverride(defaultHook, override zapcore.CheckWriteHook) zapcore.CheckWriteHook {
// A nil or WriteThenNoop hook will lead to continued execution after
// a Panic or Fatal log entry, which is unexpected. For example,
//
// f, err := os.Open(..)
// if err != nil {
// log.Fatal("cannot open", zap.Error(err))
// }
// fmt.Println(f.Name())
//
// The f.Name() will panic if we continue execution after the log.Fatal.
if override == nil || override == zapcore.WriteThenNoop {
return defaultHook
}
return override
}
124 changes: 124 additions & 0 deletions logger_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -836,6 +836,130 @@ func TestLoggerFatalOnNoop(t *testing.T) {
assert.Equal(t, 1, exitStub.Code, "must exit with status 1 for WriteThenNoop")
}

func TestLoggerCustomOnPanic(t *testing.T) {
tests := []struct {
msg string
level zapcore.Level
opts []Option
finished bool
want []observer.LoggedEntry
recoverValue any
}{
{
msg: "panic with nil hook",
level: PanicLevel,
opts: opts(WithPanicHook(nil)),
finished: false,
want: []observer.LoggedEntry{
{
Entry: zapcore.Entry{Level: PanicLevel, Message: "foobar"},
Context: []Field{},
},
},
recoverValue: "foobar",
},
{
msg: "panic with noop hook",
level: PanicLevel,
opts: opts(WithPanicHook(zapcore.WriteThenNoop)),
finished: false,
want: []observer.LoggedEntry{
{
Entry: zapcore.Entry{Level: PanicLevel, Message: "foobar"},
Context: []Field{},
},
},
recoverValue: "foobar",
},
{
msg: "no panic with goexit hook",
level: PanicLevel,
opts: opts(WithPanicHook(zapcore.WriteThenGoexit)),
finished: false,
want: []observer.LoggedEntry{
{
Entry: zapcore.Entry{Level: PanicLevel, Message: "foobar"},
Context: []Field{},
},
},
recoverValue: nil,
},
{
msg: "dpanic no panic in development mode with goexit hook",
level: DPanicLevel,
opts: opts(WithPanicHook(zapcore.WriteThenGoexit), Development()),
finished: false,
want: []observer.LoggedEntry{
{
Entry: zapcore.Entry{Level: DPanicLevel, Message: "foobar"},
Context: []Field{},
},
},
recoverValue: nil,
},
{
msg: "dpanic panic in development mode with noop hook",
level: DPanicLevel,
opts: opts(WithPanicHook(zapcore.WriteThenNoop), Development()),
finished: false,
want: []observer.LoggedEntry{
{
Entry: zapcore.Entry{Level: DPanicLevel, Message: "foobar"},
Context: []Field{},
},
},
recoverValue: "foobar",
},
{
msg: "dpanic no exit in production mode with goexit hook",
level: DPanicLevel,
opts: opts(WithPanicHook(zapcore.WriteThenPanic)),
finished: true,
want: []observer.LoggedEntry{
{
Entry: zapcore.Entry{Level: DPanicLevel, Message: "foobar"},
Context: []Field{},
},
},
recoverValue: nil,
},
{
msg: "dpanic no panic in production mode with panic hook",
level: DPanicLevel,
opts: opts(WithPanicHook(zapcore.WriteThenPanic)),
finished: true,
want: []observer.LoggedEntry{
{
Entry: zapcore.Entry{Level: DPanicLevel, Message: "foobar"},
Context: []Field{},
},
},
recoverValue: nil,
},
}

for _, tt := range tests {
t.Run(tt.msg, func(t *testing.T) {
withLogger(t, InfoLevel, tt.opts, func(logger *Logger, logs *observer.ObservedLogs) {
var finished bool
recovered := make(chan any)
go func() {
defer func() {
recovered <- recover()
}()

logger.Log(tt.level, "foobar")
finished = true
}()

assert.Equal(t, tt.recoverValue, <-recovered, "unexpected value from recover()")
assert.Equal(t, tt.finished, finished, "expect goroutine finished state doesn't match")
assert.Equal(t, tt.want, logs.AllUntimed(), "unexpected logs")
})
})
}
}

func TestLoggerCustomOnFatal(t *testing.T) {
tests := []struct {
msg string
Expand Down
15 changes: 15 additions & 0 deletions options.go
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,21 @@ func IncreaseLevel(lvl zapcore.LevelEnabler) Option {
})
}

// WithPanicHook sets a CheckWriteHook to run on Panic/DPanic logs.
// Zap will call this hook after writing a log statement with a Panic/DPanic level.
//
// For example, the following builds a logger that will exit the current
// goroutine after writing a Panic/DPanic log message, but it will not start a panic.
//
// zap.New(core, zap.WithPanicHook(zapcore.WriteThenGoexit))
//
// This is useful for testing Panic/DPanic log output.
func WithPanicHook(hook zapcore.CheckWriteHook) Option {
return optionFunc(func(log *Logger) {
log.onPanic = hook
})
}

// OnFatal sets the action to take on fatal logs.
//
// Deprecated: Use [WithFatalHook] instead.
Expand Down
24 changes: 24 additions & 0 deletions sugar.go
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,12 @@ func (s *SugaredLogger) Level() zapcore.Level {
return zapcore.LevelOf(s.base.core)
}

// Log logs the provided arguments at provided level.
// Spaces are added between arguments when neither is a string.
func (s *SugaredLogger) Log(lvl zapcore.Level, args ...interface{}) {
s.log(lvl, "", args, nil)
}

// Debug logs the provided arguments at [DebugLevel].
// Spaces are added between arguments when neither is a string.
func (s *SugaredLogger) Debug(args ...interface{}) {
Expand Down Expand Up @@ -180,6 +186,12 @@ func (s *SugaredLogger) Fatal(args ...interface{}) {
s.log(FatalLevel, "", args, nil)
}

// Logf formats the message according to the format specifier
// and logs it at provided level.
func (s *SugaredLogger) Logf(lvl zapcore.Level, template string, args ...interface{}) {
s.log(lvl, template, args, nil)
}

// Debugf formats the message according to the format specifier
// and logs it at [DebugLevel].
func (s *SugaredLogger) Debugf(template string, args ...interface{}) {
Expand Down Expand Up @@ -223,6 +235,12 @@ func (s *SugaredLogger) Fatalf(template string, args ...interface{}) {
s.log(FatalLevel, template, args, nil)
}

// Logw logs a message with some additional context. The variadic key-value
// pairs are treated as they are in With.
func (s *SugaredLogger) Logw(lvl zapcore.Level, msg string, keysAndValues ...interface{}) {
s.log(lvl, msg, nil, keysAndValues)
}

// Debugw logs a message with some additional context. The variadic key-value
// pairs are treated as they are in With.
//
Expand Down Expand Up @@ -270,6 +288,12 @@ func (s *SugaredLogger) Fatalw(msg string, keysAndValues ...interface{}) {
s.log(FatalLevel, msg, nil, keysAndValues)
}

// Logln logs a message at provided level.
// Spaces are always added between arguments.
func (s *SugaredLogger) Logln(lvl zapcore.Level, args ...interface{}) {
s.logln(lvl, args, nil)
}

// Debugln logs a message at [DebugLevel].
// Spaces are always added between arguments.
func (s *SugaredLogger) Debugln(args ...interface{}) {
Expand Down
20 changes: 12 additions & 8 deletions sugar_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -311,9 +311,10 @@ func TestSugarStructuredLogging(t *testing.T) {
logger.With(context...).Warnw(tt.msg, extra...)
logger.With(context...).Errorw(tt.msg, extra...)
logger.With(context...).DPanicw(tt.msg, extra...)
logger.With(context...).Logw(WarnLevel, tt.msg, extra...)

expected := make([]observer.LoggedEntry, 5)
for i, lvl := range []zapcore.Level{DebugLevel, InfoLevel, WarnLevel, ErrorLevel, DPanicLevel} {
expected := make([]observer.LoggedEntry, 6)
for i, lvl := range []zapcore.Level{DebugLevel, InfoLevel, WarnLevel, ErrorLevel, DPanicLevel, WarnLevel} {
expected[i] = observer.LoggedEntry{
Entry: zapcore.Entry{Message: tt.expectMsg, Level: lvl},
Context: expectedFields,
Expand Down Expand Up @@ -343,9 +344,10 @@ func TestSugarConcatenatingLogging(t *testing.T) {
logger.With(context...).Warn(tt.args...)
logger.With(context...).Error(tt.args...)
logger.With(context...).DPanic(tt.args...)
logger.With(context...).Log(InfoLevel, tt.args...)

expected := make([]observer.LoggedEntry, 5)
for i, lvl := range []zapcore.Level{DebugLevel, InfoLevel, WarnLevel, ErrorLevel, DPanicLevel} {
expected := make([]observer.LoggedEntry, 6)
for i, lvl := range []zapcore.Level{DebugLevel, InfoLevel, WarnLevel, ErrorLevel, DPanicLevel, InfoLevel} {
expected[i] = observer.LoggedEntry{
Entry: zapcore.Entry{Message: tt.expect, Level: lvl},
Context: expectedFields,
Expand Down Expand Up @@ -379,9 +381,10 @@ func TestSugarTemplatedLogging(t *testing.T) {
logger.With(context...).Warnf(tt.format, tt.args...)
logger.With(context...).Errorf(tt.format, tt.args...)
logger.With(context...).DPanicf(tt.format, tt.args...)
logger.With(context...).Logf(ErrorLevel, tt.format, tt.args...)

expected := make([]observer.LoggedEntry, 5)
for i, lvl := range []zapcore.Level{DebugLevel, InfoLevel, WarnLevel, ErrorLevel, DPanicLevel} {
expected := make([]observer.LoggedEntry, 6)
for i, lvl := range []zapcore.Level{DebugLevel, InfoLevel, WarnLevel, ErrorLevel, DPanicLevel, ErrorLevel} {
expected[i] = observer.LoggedEntry{
Entry: zapcore.Entry{Message: tt.expect, Level: lvl},
Context: expectedFields,
Expand Down Expand Up @@ -415,9 +418,10 @@ func TestSugarLnLogging(t *testing.T) {
logger.With(context...).Warnln(tt.args...)
logger.With(context...).Errorln(tt.args...)
logger.With(context...).DPanicln(tt.args...)
logger.With(context...).Logln(InfoLevel, tt.args...)

expected := make([]observer.LoggedEntry, 5)
for i, lvl := range []zapcore.Level{DebugLevel, InfoLevel, WarnLevel, ErrorLevel, DPanicLevel} {
expected := make([]observer.LoggedEntry, 6)
for i, lvl := range []zapcore.Level{DebugLevel, InfoLevel, WarnLevel, ErrorLevel, DPanicLevel, InfoLevel} {
expected[i] = observer.LoggedEntry{
Entry: zapcore.Entry{Message: tt.expect, Level: lvl},
Context: expectedFields,
Expand Down

0 comments on commit 5a63490

Please sign in to comment.