diff --git a/benchmarks/zap_sugar_bench_test.go b/benchmarks/zap_sugar_bench_test.go index 5ba330702..7dc25925e 100644 --- a/benchmarks/zap_sugar_bench_test.go +++ b/benchmarks/zap_sugar_bench_test.go @@ -50,11 +50,11 @@ func fakeSugarFormatArgs() (string, []interface{}) { } func newSugarLogger(lvl zapcore.Level, options ...zap.Option) *zap.SugaredLogger { - return zap.Sugar(zap.New(zapcore.WriterFacility( + return zap.New(zapcore.WriterFacility( benchEncoder(), &testutils.Discarder{}, lvl, - ), options...)) + ), options...).Sugar() } func BenchmarkZapSugarDisabledLevelsWithoutFields(b *testing.B) { diff --git a/common_test.go b/common_test.go index a09de4696..f057ac272 100644 --- a/common_test.go +++ b/common_test.go @@ -34,7 +34,7 @@ func opts(opts ...Option) []Option { // Here specifically to introduce an easily-identifiable filename for testing // stacktraces and caller skips. -func withLogger(t testing.TB, e zapcore.LevelEnabler, opts []Option, f func(Logger, *observer.ObservedLogs)) { +func withLogger(t testing.TB, e zapcore.LevelEnabler, opts []Option, f func(*Logger, *observer.ObservedLogs)) { var logs observer.ObservedLogs fac := observer.New(e, logs.Add, true) log := New(fac, opts...) @@ -42,7 +42,7 @@ func withLogger(t testing.TB, e zapcore.LevelEnabler, opts []Option, f func(Logg } func withSugar(t testing.TB, e zapcore.LevelEnabler, opts []Option, f func(*SugaredLogger, *observer.ObservedLogs)) { - withLogger(t, e, opts, func(logger Logger, logs *observer.ObservedLogs) { f(Sugar(logger), logs) }) + withLogger(t, e, opts, func(logger *Logger, logs *observer.ObservedLogs) { f(logger.Sugar(), logs) }) } func runConcurrently(goroutines, iterations int, wg *sync.WaitGroup, f func()) { diff --git a/http_handler_test.go b/http_handler_test.go index 0a2020198..33a20d3b4 100644 --- a/http_handler_test.go +++ b/http_handler_test.go @@ -38,7 +38,7 @@ import ( "github.com/stretchr/testify/require" ) -func newHandler() (AtomicLevel, Logger) { +func newHandler() (AtomicLevel, *Logger) { lvl := DynamicLevel() // XXX should be a discard facility fac := observer.New(lvl, func(observer.LoggedEntry) error { return nil }, false) diff --git a/logger.go b/logger.go index 4103006ab..34aeafc76 100644 --- a/logger.go +++ b/logger.go @@ -44,37 +44,11 @@ func defaultEncoderConfig() zapcore.EncoderConfig { } } -// A Logger enables leveled, structured logging. All methods are safe for +// A Logger provides fast, leveled, structured logging. All methods are safe for // concurrent use. -type Logger interface { - // Create a child logger, and optionally add some context to that logger. - With(...zapcore.Field) Logger - // Add a sub-scope to the logger's name. - Named(string) Logger - - // Check returns a CheckedEntry if logging a message at the specified level - // is enabled. It's a completely optional optimization; in high-performance - // applications, Check can help avoid allocating a slice to hold fields. - Check(zapcore.Level, string) *zapcore.CheckedEntry - - // Log a message at the given level. Messages include any context that's - // accumulated on the logger, as well as any fields added at the log site. - // - // Calling Panic should panic and calling Fatal should terminate the process, - // even if logging at those levels is disabled. - Debug(string, ...zapcore.Field) - Info(string, ...zapcore.Field) - Warn(string, ...zapcore.Field) - Error(string, ...zapcore.Field) - DPanic(string, ...zapcore.Field) - Panic(string, ...zapcore.Field) - Fatal(string, ...zapcore.Field) - - // Facility returns the destination that log entries are written to. - Facility() zapcore.Facility -} - -type logger struct { +// +// If you'd prefer a slower but less verbose logger, see the SugaredLogger. +type Logger struct { fac zapcore.Facility development bool @@ -90,7 +64,7 @@ type logger struct { // New returns a new logger with sensible defaults: logging at InfoLevel, // development mode off, errors written to standard error, and logs JSON // encoded to standard output. -func New(fac zapcore.Facility, options ...Option) Logger { +func New(fac zapcore.Facility, options ...Option) *Logger { if fac == nil { fac = zapcore.WriterFacility( zapcore.NewJSONEncoder(defaultEncoderConfig()), @@ -98,7 +72,7 @@ func New(fac zapcore.Facility, options ...Option) Logger { InfoLevel, ) } - log := &logger{ + log := &Logger{ fac: fac, errorOutput: zapcore.Lock(os.Stderr), addStack: LevelEnablerFunc(func(_ zapcore.Level) bool { return false }), @@ -109,7 +83,16 @@ func New(fac zapcore.Facility, options ...Option) Logger { return log } -func (log *logger) Named(s string) Logger { +// Sugar converts a Logger to a SugaredLogger. +func (log *Logger) Sugar() *SugaredLogger { + core := log.clone() + core.callerSkip += 2 + return &SugaredLogger{core} +} + +// Named adds a new path segment to the logger's name. Segments are joined by +// periods. +func (log *Logger) Named(s string) *Logger { if s == "" { return log } @@ -122,7 +105,9 @@ func (log *logger) Named(s string) Logger { return l } -func (log *logger) With(fields ...zapcore.Field) Logger { +// With creates a child logger and adds structured context to it. Fields added +// to the child don't affect the parent, and vice versa. +func (log *Logger) With(fields ...zapcore.Field) *Logger { if len(fields) == 0 { return log } @@ -131,11 +116,88 @@ func (log *logger) With(fields ...zapcore.Field) Logger { return l } -func (log *logger) Check(lvl zapcore.Level, msg string) *zapcore.CheckedEntry { +// Check returns a CheckedEntry if logging a message at the specified level +// is enabled. It's a completely optional optimization; in high-performance +// applications, Check can help avoid allocating a slice to hold fields. +func (log *Logger) Check(lvl zapcore.Level, msg string) *zapcore.CheckedEntry { return log.check(lvl, msg) } -func (log *logger) check(lvl zapcore.Level, msg string) *zapcore.CheckedEntry { +// Debug logs a message at DebugLevel. The message includes any fields passed +// at the log site, as well as any fields accumulated on the logger. +func (log *Logger) Debug(msg string, fields ...zapcore.Field) { + if ce := log.check(DebugLevel, msg); ce != nil { + ce.Write(fields...) + } +} + +// Info logs a message at InfoLevel. The message includes any fields passed +// at the log site, as well as any fields accumulated on the logger. +func (log *Logger) Info(msg string, fields ...zapcore.Field) { + if ce := log.check(InfoLevel, msg); ce != nil { + ce.Write(fields...) + } +} + +// Warn logs a message at WarnLevel. The message includes any fields passed +// at the log site, as well as any fields accumulated on the logger. +func (log *Logger) Warn(msg string, fields ...zapcore.Field) { + if ce := log.check(WarnLevel, msg); ce != nil { + ce.Write(fields...) + } +} + +// Error logs a message at ErrorLevel. The message includes any fields passed +// at the log site, as well as any fields accumulated on the logger. +func (log *Logger) Error(msg string, fields ...zapcore.Field) { + if ce := log.check(ErrorLevel, msg); ce != nil { + ce.Write(fields...) + } +} + +// DPanic logs a message at DPanicLevel. The message includes any fields passed +// at the log site, as well as any fields accumulated on the logger. +// +// If the logger is in development mode, it then panics (DPanic means +// "development panic"). This is useful for catching errors that are +// recoverable, but shouldn't ever happen. +func (log *Logger) DPanic(msg string, fields ...zapcore.Field) { + if ce := log.check(DPanicLevel, msg); ce != nil { + ce.Write(fields...) + } +} + +// Panic logs a message at PanicLevel. The message includes any fields passed +// at the log site, as well as any fields accumulated on the logger. +// +// The logger then panics, even if logging at PanicLevel is disabled. +func (log *Logger) Panic(msg string, fields ...zapcore.Field) { + if ce := log.check(PanicLevel, msg); ce != nil { + ce.Write(fields...) + } +} + +// Fatal logs a message at FatalLevel. The message includes any fields passed +// at the log site, as well as any fields accumulated on the logger. +// +// The logger then calls os.Exit(1), even if logging at FatalLevel is disabled. +func (log *Logger) Fatal(msg string, fields ...zapcore.Field) { + if ce := log.check(FatalLevel, msg); ce != nil { + ce.Write(fields...) + } +} + +// Facility returns the destination that logs entries are written to. +func (log *Logger) Facility() zapcore.Facility { + return log.fac +} + +func (log *Logger) clone() *Logger { + copy := *log + return © +} + +func (log *Logger) check(lvl zapcore.Level, msg string) *zapcore.CheckedEntry { // check must always be called directly by a method in the Logger interface // (e.g., Check, Info, Fatal). const callerSkipOffset = 2 @@ -188,55 +250,3 @@ func (log *logger) check(lvl zapcore.Level, msg string) *zapcore.CheckedEntry { return ce } - -func (log *logger) Debug(msg string, fields ...zapcore.Field) { - if ce := log.check(DebugLevel, msg); ce != nil { - ce.Write(fields...) - } -} - -func (log *logger) Info(msg string, fields ...zapcore.Field) { - if ce := log.check(InfoLevel, msg); ce != nil { - ce.Write(fields...) - } -} - -func (log *logger) Warn(msg string, fields ...zapcore.Field) { - if ce := log.check(WarnLevel, msg); ce != nil { - ce.Write(fields...) - } -} - -func (log *logger) Error(msg string, fields ...zapcore.Field) { - if ce := log.check(ErrorLevel, msg); ce != nil { - ce.Write(fields...) - } -} - -func (log *logger) DPanic(msg string, fields ...zapcore.Field) { - if ce := log.check(DPanicLevel, msg); ce != nil { - ce.Write(fields...) - } -} - -func (log *logger) Panic(msg string, fields ...zapcore.Field) { - if ce := log.check(PanicLevel, msg); ce != nil { - ce.Write(fields...) - } -} - -func (log *logger) Fatal(msg string, fields ...zapcore.Field) { - if ce := log.check(FatalLevel, msg); ce != nil { - ce.Write(fields...) - } -} - -// Facility returns the destination that logs entries are written to. -func (log *logger) Facility() zapcore.Facility { - return log.fac -} - -func (log *logger) clone() *logger { - copy := *log - return © -} diff --git a/logger_bench_test.go b/logger_bench_test.go index b5104f8ac..7aadee9e9 100644 --- a/logger_bench_test.go +++ b/logger_bench_test.go @@ -48,7 +48,7 @@ var _jane = &user{ CreatedAt: time.Date(1980, 1, 1, 12, 0, 0, 0, time.UTC), } -func withBenchedLogger(b *testing.B, f func(Logger)) { +func withBenchedLogger(b *testing.B, f func(*Logger)) { logger := New( zapcore.WriterFacility(zapcore.NewJSONEncoder(defaultEncoderConfig()), &testutils.Discarder{}, @@ -63,81 +63,81 @@ func withBenchedLogger(b *testing.B, f func(Logger)) { } func BenchmarkNoContext(b *testing.B) { - withBenchedLogger(b, func(log Logger) { + withBenchedLogger(b, func(log *Logger) { log.Info("No context.") }) } func BenchmarkBoolField(b *testing.B) { - withBenchedLogger(b, func(log Logger) { + withBenchedLogger(b, func(log *Logger) { log.Info("Boolean.", Bool("foo", true)) }) } func BenchmarkFloat64Field(b *testing.B) { - withBenchedLogger(b, func(log Logger) { + withBenchedLogger(b, func(log *Logger) { log.Info("Floating point.", Float64("foo", 3.14)) }) } func BenchmarkIntField(b *testing.B) { - withBenchedLogger(b, func(log Logger) { + withBenchedLogger(b, func(log *Logger) { log.Info("Integer.", Int("foo", 42)) }) } func BenchmarkInt64Field(b *testing.B) { - withBenchedLogger(b, func(log Logger) { + withBenchedLogger(b, func(log *Logger) { log.Info("64-bit integer.", Int64("foo", 42)) }) } func BenchmarkStringField(b *testing.B) { - withBenchedLogger(b, func(log Logger) { + withBenchedLogger(b, func(log *Logger) { log.Info("Strings.", String("foo", "bar")) }) } func BenchmarkStringerField(b *testing.B) { - withBenchedLogger(b, func(log Logger) { + withBenchedLogger(b, func(log *Logger) { log.Info("Level.", Stringer("foo", InfoLevel)) }) } func BenchmarkTimeField(b *testing.B) { t := time.Unix(0, 0) - withBenchedLogger(b, func(log Logger) { + withBenchedLogger(b, func(log *Logger) { log.Info("Time.", Time("foo", t)) }) } func BenchmarkDurationField(b *testing.B) { - withBenchedLogger(b, func(log Logger) { + withBenchedLogger(b, func(log *Logger) { log.Info("Duration", Duration("foo", time.Second)) }) } func BenchmarkErrorField(b *testing.B) { err := errors.New("egad") - withBenchedLogger(b, func(log Logger) { + withBenchedLogger(b, func(log *Logger) { log.Info("Error.", Error(err)) }) } func BenchmarkStackField(b *testing.B) { - withBenchedLogger(b, func(log Logger) { + withBenchedLogger(b, func(log *Logger) { log.Info("Error.", Stack()) }) } func BenchmarkObjectField(b *testing.B) { - withBenchedLogger(b, func(log Logger) { + withBenchedLogger(b, func(log *Logger) { log.Info("Arbitrary ObjectMarshaler.", Object("user", _jane)) }) } func BenchmarkReflectField(b *testing.B) { - withBenchedLogger(b, func(log Logger) { + withBenchedLogger(b, func(log *Logger) { log.Info("Reflection-based serialization.", Reflect("user", _jane)) }) } @@ -160,7 +160,7 @@ func BenchmarkAddCallerHook(b *testing.B) { } func Benchmark10Fields(b *testing.B) { - withBenchedLogger(b, func(log Logger) { + withBenchedLogger(b, func(log *Logger) { log.Info("Ten fields, passed at the log site.", Int("one", 1), Int("two", 2), diff --git a/logger_test.go b/logger_test.go index 1a3a45e40..de6cf7e78 100644 --- a/logger_test.go +++ b/logger_test.go @@ -37,7 +37,7 @@ func TestLoggerDynamicLevel(t *testing.T) { // Test that the DynamicLevel applys to all ancestors and descendants. dl := DynamicLevel() - withLogger(t, dl, nil, func(grandparent Logger, _ *observer.ObservedLogs) { + withLogger(t, dl, nil, func(grandparent *Logger, _ *observer.ObservedLogs) { parent := grandparent.With(Int("generation", 1)) child := parent.With(Int("generation", 2)) @@ -53,7 +53,7 @@ func TestLoggerDynamicLevel(t *testing.T) { for _, tt := range tests { dl.SetLevel(tt.setLevel) - for _, logger := range []Logger{grandparent, parent, child} { + for _, logger := range []*Logger{grandparent, parent, child} { if tt.enabled { assert.NotNil( t, @@ -74,7 +74,7 @@ func TestLoggerDynamicLevel(t *testing.T) { func TestLoggerInitialFields(t *testing.T) { fieldOpts := opts(Fields(Int("foo", 42), String("bar", "baz"))) - withLogger(t, DebugLevel, fieldOpts, func(logger Logger, logs *observer.ObservedLogs) { + withLogger(t, DebugLevel, fieldOpts, func(logger *Logger, logs *observer.ObservedLogs) { logger.Info("") assert.Equal( t, @@ -87,7 +87,7 @@ func TestLoggerInitialFields(t *testing.T) { func TestLoggerWith(t *testing.T) { fieldOpts := opts(Fields(Int("foo", 42))) - withLogger(t, DebugLevel, fieldOpts, func(logger Logger, logs *observer.ObservedLogs) { + withLogger(t, DebugLevel, fieldOpts, func(logger *Logger, logs *observer.ObservedLogs) { // Child loggers should have copy-on-write semantics, so two children // shouldn't stomp on each other's fields or affect the parent's fields. logger.With(String("one", "two")).Info("") @@ -104,14 +104,14 @@ func TestLoggerWith(t *testing.T) { func TestLoggerLogPanic(t *testing.T) { for _, tt := range []struct { - do func(Logger) + do func(*Logger) should bool expected string }{ - {func(logger Logger) { logger.Check(PanicLevel, "bar").Write() }, true, "bar"}, - {func(logger Logger) { logger.Panic("baz") }, true, "baz"}, + {func(logger *Logger) { logger.Check(PanicLevel, "bar").Write() }, true, "bar"}, + {func(logger *Logger) { logger.Panic("baz") }, true, "baz"}, } { - withLogger(t, DebugLevel, nil, func(logger Logger, logs *observer.ObservedLogs) { + withLogger(t, DebugLevel, nil, func(logger *Logger, logs *observer.ObservedLogs) { if tt.should { assert.Panics(t, func() { tt.do(logger) }, "Expected panic") } else { @@ -133,13 +133,13 @@ func TestLoggerLogPanic(t *testing.T) { func TestLoggerLogFatal(t *testing.T) { for _, tt := range []struct { - do func(Logger) + do func(*Logger) expected string }{ - {func(logger Logger) { logger.Check(FatalLevel, "bar").Write() }, "bar"}, - {func(logger Logger) { logger.Fatal("baz") }, "baz"}, + {func(logger *Logger) { logger.Check(FatalLevel, "bar").Write() }, "bar"}, + {func(logger *Logger) { logger.Fatal("baz") }, "baz"}, } { - withLogger(t, DebugLevel, nil, func(logger Logger, logs *observer.ObservedLogs) { + withLogger(t, DebugLevel, nil, func(logger *Logger, logs *observer.ObservedLogs) { stub := exit.WithStub(func() { tt.do(logger) }) @@ -158,7 +158,7 @@ func TestLoggerLogFatal(t *testing.T) { } func TestLoggerLeveledMethods(t *testing.T) { - withLogger(t, DebugLevel, nil, func(logger Logger, logs *observer.ObservedLogs) { + withLogger(t, DebugLevel, nil, func(logger *Logger, logs *observer.ObservedLogs) { tests := []struct { method func(string, ...zapcore.Field) expectedLevel zapcore.Level @@ -186,7 +186,7 @@ func TestLoggerLeveledMethods(t *testing.T) { func TestLoggerAlwaysPanics(t *testing.T) { // Users can disable writing out panic-level logs, but calls to logger.Panic() // should still call panic(). - withLogger(t, FatalLevel, nil, func(logger Logger, logs *observer.ObservedLogs) { + withLogger(t, FatalLevel, nil, func(logger *Logger, logs *observer.ObservedLogs) { msg := "Even if output is disabled, logger.Panic should always panic." assert.Panics(t, func() { logger.Panic("foo") }, msg) assert.Panics(t, func() { @@ -201,7 +201,7 @@ func TestLoggerAlwaysPanics(t *testing.T) { func TestLoggerAlwaysFatals(t *testing.T) { // Users can disable writing out fatal-level logs, but calls to logger.Fatal() // should still terminate the process. - withLogger(t, FatalLevel+1, nil, func(logger Logger, logs *observer.ObservedLogs) { + withLogger(t, FatalLevel+1, nil, func(logger *Logger, logs *observer.ObservedLogs) { stub := exit.WithStub(func() { logger.Fatal("") }) assert.True(t, stub.Exited, "Expected calls to logger.Fatal to terminate process.") @@ -217,7 +217,7 @@ func TestLoggerAlwaysFatals(t *testing.T) { } func TestLoggerDPanic(t *testing.T) { - withLogger(t, DebugLevel, nil, func(logger Logger, logs *observer.ObservedLogs) { + withLogger(t, DebugLevel, nil, func(logger *Logger, logs *observer.ObservedLogs) { assert.NotPanics(t, func() { logger.DPanic("") }) assert.Equal( t, @@ -226,7 +226,7 @@ func TestLoggerDPanic(t *testing.T) { "Unexpected log output from DPanic in production mode.", ) }) - withLogger(t, DebugLevel, opts(Development()), func(logger Logger, logs *observer.ObservedLogs) { + withLogger(t, DebugLevel, opts(Development()), func(logger *Logger, logs *observer.ObservedLogs) { assert.Panics(t, func() { logger.DPanic("") }) assert.Equal( t, @@ -238,7 +238,7 @@ func TestLoggerDPanic(t *testing.T) { } func TestLoggerNoOpsDisabledLevels(t *testing.T) { - withLogger(t, WarnLevel, nil, func(logger Logger, logs *observer.ObservedLogs) { + withLogger(t, WarnLevel, nil, func(logger *Logger, logs *observer.ObservedLogs) { logger.Info("silence!") assert.Equal( t, @@ -265,7 +265,7 @@ func TestLoggerNames(t *testing.T) { } for _, tt := range tests { - withLogger(t, DebugLevel, nil, func(log Logger, logs *observer.ObservedLogs) { + withLogger(t, DebugLevel, nil, func(log *Logger, logs *observer.ObservedLogs) { for _, n := range tt.names { log = log.Named(n) } @@ -312,7 +312,7 @@ func TestLoggerAddCaller(t *testing.T) { {opts(AddCaller(), AddCallerSkip(1), AddCallerSkip(3)), `.+/src/runtime/.*:[\d]+$`}, } for _, tt := range tests { - withLogger(t, DebugLevel, tt.options, func(logger Logger, logs *observer.ObservedLogs) { + withLogger(t, DebugLevel, tt.options, func(logger *Logger, logs *observer.ObservedLogs) { logger.Info("") output := logs.AllUntimed() assert.Equal(t, 1, len(output), "Unexpected number of logs written out.") @@ -328,11 +328,8 @@ func TestLoggerAddCaller(t *testing.T) { func TestLoggerAddCallerFail(t *testing.T) { errBuf := &testutils.Buffer{} - withLogger(t, DebugLevel, opts(AddCaller(), ErrorOutput(errBuf)), func(log Logger, logs *observer.ObservedLogs) { - // TODO: Use AddCallerSkip - logImpl := log.(*logger) - logImpl.callerSkip = 1e3 - + withLogger(t, DebugLevel, opts(AddCaller(), ErrorOutput(errBuf)), func(log *Logger, logs *observer.ObservedLogs) { + log.callerSkip = 1e3 log.Info("Failure.") assert.Regexp( t, @@ -353,7 +350,7 @@ func TestLoggerAddStacks(t *testing.T) { assert.Contains(t, obs.Entry.Stack, "zap.TestLoggerAddStacks", "Expected to find test function in stacktrace.") } - withLogger(t, DebugLevel, opts(AddStacks(InfoLevel)), func(logger Logger, logs *observer.ObservedLogs) { + withLogger(t, DebugLevel, opts(AddStacks(InfoLevel)), func(logger *Logger, logs *observer.ObservedLogs) { logger.Debug("") assert.Empty( t, @@ -370,7 +367,7 @@ func TestLoggerAddStacks(t *testing.T) { } func TestLoggerConcurrent(t *testing.T) { - withLogger(t, DebugLevel, nil, func(logger Logger, logs *observer.ObservedLogs) { + withLogger(t, DebugLevel, nil, func(logger *Logger, logs *observer.ObservedLogs) { child := logger.With(String("foo", "bar")) wg := &sync.WaitGroup{} diff --git a/options.go b/options.go index 4a8096ace..9e33a8f21 100644 --- a/options.go +++ b/options.go @@ -24,19 +24,19 @@ import "go.uber.org/zap/zapcore" // Option is used to set options for the logger. type Option interface { - apply(*logger) + apply(*Logger) } // optionFunc wraps a func so it satisfies the Option interface. -type optionFunc func(*logger) +type optionFunc func(*Logger) -func (f optionFunc) apply(log *logger) { +func (f optionFunc) apply(log *Logger) { f(log) } // Fields sets the initial fields for the logger. func Fields(fs ...zapcore.Field) Option { - return optionFunc(func(log *logger) { + return optionFunc(func(log *Logger) { log.fac = log.fac.With(fs) }) } @@ -45,7 +45,7 @@ func Fields(fs ...zapcore.Field) Option { // supplied WriteSyncer is automatically wrapped with a mutex, so it need not be // safe for concurrent use. func ErrorOutput(w zapcore.WriteSyncer) Option { - return optionFunc(func(log *logger) { + return optionFunc(func(log *Logger) { log.errorOutput = zapcore.Lock(w) }) } @@ -53,7 +53,7 @@ func ErrorOutput(w zapcore.WriteSyncer) Option { // Development puts the logger in development mode, which alters the behavior // of the DPanic method. func Development() Option { - return optionFunc(func(log *logger) { + return optionFunc(func(log *Logger) { log.development = true }) } @@ -61,7 +61,7 @@ func Development() Option { // AddCaller configures the Logger to annotate each message with the filename // and line number of zap's caller. func AddCaller() Option { - return optionFunc(func(log *logger) { + return optionFunc(func(log *Logger) { log.addCaller = true }) } @@ -69,7 +69,7 @@ func AddCaller() Option { // AddCallerSkip increases the number of callers skipped by caller annotation // (as enabled by the AddCaller option). func AddCallerSkip(skip int) Option { - return optionFunc(func(log *logger) { + return optionFunc(func(log *Logger) { log.callerSkip += skip }) } @@ -81,7 +81,7 @@ func AddCallerSkip(skip int) Option { // TODO: why is this called AddStacks rather than just AddStack or // AddStacktrace? func AddStacks(lvl zapcore.LevelEnabler) Option { - return optionFunc(func(log *logger) { + return optionFunc(func(log *Logger) { log.addStack = lvl }) } diff --git a/sugar.go b/sugar.go index 1de2ec9b1..f7e14bc1a 100644 --- a/sugar.go +++ b/sugar.go @@ -32,25 +32,21 @@ const ( _nonStringKeyErrMsg = "Ignored key-value pairs with non-string keys." ) -// A SugaredLogger wraps the core Logger functionality in a slower, but less -// verbose, API. +// A SugaredLogger wraps the base Logger functionality in a slower, but less +// verbose, API. Any Logger can be converted to a SugaredLogger with its Sugar +// method. type SugaredLogger struct { - core Logger -} - -// Sugar converts a Logger to a SugaredLogger. -func Sugar(core Logger) *SugaredLogger { - // TODO: increment caller skip. - return &SugaredLogger{core} + core *Logger } // Desugar unwraps a SugaredLogger, exposing the original Logger. -func Desugar(s *SugaredLogger) Logger { - // TODO: decrement caller skip. - return s.core +func (s *SugaredLogger) Desugar() *Logger { + base := s.core.clone() + base.callerSkip -= 2 + return base } -// Named adds a sub-scope to the logger's name. +// Named adds a sub-scope to the logger's name. See Logger.Named for details. func (s *SugaredLogger) Named(name string) *SugaredLogger { return &SugaredLogger{core: s.core.Named(name)} } diff --git a/sugar_bench_test.go b/sugar_bench_test.go index 24cd82d67..50c9cf45c 100644 --- a/sugar_bench_test.go +++ b/sugar_bench_test.go @@ -28,10 +28,10 @@ import ( ) func withBenchedSugar(b *testing.B, f func(*SugaredLogger)) { - logger := Sugar(New(zapcore.WriterFacility(zapcore.NewJSONEncoder(defaultEncoderConfig()), + logger := New(zapcore.WriterFacility(zapcore.NewJSONEncoder(defaultEncoderConfig()), &testutils.Discarder{}, DebugLevel, - ))) + )).Sugar() b.ResetTimer() b.RunParallel(func(pb *testing.PB) { for pb.Next() { diff --git a/sugar_test.go b/sugar_test.go index ffa7de256..4be956ac2 100644 --- a/sugar_test.go +++ b/sugar_test.go @@ -27,6 +27,7 @@ import ( "go.uber.org/zap/internal/exit" "go.uber.org/zap/internal/observer" + "go.uber.org/zap/testutils" "go.uber.org/zap/zapcore" ) @@ -307,3 +308,46 @@ func TestSugarFatalLogging(t *testing.T) { }) } } + +func TestSugarAddCaller(t *testing.T) { + tests := []struct { + options []Option + pat string + }{ + {opts(AddCaller()), `.+/sugar_test.go:[\d]+$`}, + {opts(AddCaller(), AddCallerSkip(1), AddCallerSkip(-1)), `.+/zap/sugar_test.go:[\d]+$`}, + {opts(AddCaller(), AddCallerSkip(1)), `.+/zap/common_test.go:[\d]+$`}, + {opts(AddCaller(), AddCallerSkip(1), AddCallerSkip(5)), `.+/src/runtime/.*:[\d]+$`}, + } + for _, tt := range tests { + withSugar(t, DebugLevel, tt.options, func(logger *SugaredLogger, logs *observer.ObservedLogs) { + logger.Info("") + output := logs.AllUntimed() + assert.Equal(t, 1, len(output), "Unexpected number of logs written out.") + assert.Regexp( + t, + tt.pat, + output[0].Entry.Caller, + "Expected to find package name and file name in output.", + ) + }) + } +} + +func TestSugarAddCallerFail(t *testing.T) { + errBuf := &testutils.Buffer{} + withSugar(t, DebugLevel, opts(AddCaller(), AddCallerSkip(1e3), ErrorOutput(errBuf)), func(log *SugaredLogger, logs *observer.ObservedLogs) { + log.Info("Failure.") + assert.Regexp( + t, + `addCaller error: failed to get caller`, + errBuf.String(), + "Didn't find expected failure message.", + ) + assert.Equal( + t, + logs.AllUntimed()[0].Entry.Message, + "Failure.", + "Expected original message to survive failures in runtime.Caller.") + }) +} diff --git a/zapcore/write_syncer.go b/zapcore/write_syncer.go index a536cd4df..924ff4920 100644 --- a/zapcore/write_syncer.go +++ b/zapcore/write_syncer.go @@ -35,9 +35,8 @@ type WriteSyncer interface { } // AddSync converts an io.Writer to a WriteSyncer. It attempts to be -// intelligent: if the concrete type of the io.Writer implements WriteSyncer or -// WriteFlusher, we'll use the existing Sync or Flush methods. If it doesn't, -// we'll add a no-op Sync method. +// intelligent: if the concrete type of the io.Writer implements WriteSyncer, +// we'll use the existing Sync method. If it doesn't, we'll add a no-op Sync. func AddSync(w io.Writer) WriteSyncer { switch w := w.(type) { case WriteSyncer: