diff --git a/logger.go b/logger.go index 4d8711cea..8b6fc7eb8 100644 --- a/logger.go +++ b/logger.go @@ -45,9 +45,8 @@ type Logger struct { name string errorOutput zapcore.WriteSyncer - addCaller bool - addFunction bool - addStack zapcore.LevelEnabler + addCaller bool + addStack zapcore.LevelEnabler callerSkip int } @@ -297,7 +296,7 @@ func (log *Logger) check(lvl zapcore.Level, msg string) *zapcore.CheckedEntry { // Thread the error output through to the CheckedEntry. ce.ErrorOutput = log.errorOutput - if log.addCaller || log.addFunction { + if log.addCaller { pc := make([]uintptr, 1) numFrames := runtime.Callers(log.callerSkip+callerSkipOffset+1, pc) frame, _ := runtime.CallersFrames(pc[:numFrames]).Next() @@ -307,12 +306,8 @@ func (log *Logger) check(lvl zapcore.Level, msg string) *zapcore.CheckedEntry { log.errorOutput.Sync() } - if log.addCaller { - ce.Entry.Caller = zapcore.NewEntryCaller(frame.PC, frame.File, frame.Line, defined) - } - if log.addFunction { - ce.Entry.Function = frame.Function - } + ce.Entry.Caller = zapcore.NewEntryCaller(frame.PC, frame.File, frame.Line, defined) + ce.Entry.Caller.Function = frame.Function } if log.addStack.Enabled(ce.Entry.Level) { ce.Entry.Stack = Stack("").String diff --git a/logger_test.go b/logger_test.go index 85e8d5f33..6992a9a19 100644 --- a/logger_test.go +++ b/logger_test.go @@ -366,61 +366,89 @@ func TestLoggerAddCaller(t *testing.T) { } } -func TestLoggerAddCallerFail(t *testing.T) { - errBuf := &ztest.Buffer{} - withLogger(t, DebugLevel, opts(AddCaller(), ErrorOutput(errBuf)), func(log *Logger, logs *observer.ObservedLogs) { - log.callerSkip = 1e3 - log.Info("Failure.") - assert.Regexp( - t, - `Logger.check 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.") - }) -} - -func TestLoggerAddFunction(t *testing.T) { +func TestLoggerAddCallerFunction(t *testing.T) { tests := []struct { - options []Option - pat string + options []Option + loggerFunction string + sugaredFunction string }{ - {opts(), `^$`}, - {opts(WithFunction(false)), `^$`}, - {opts(AddFunction()), `zap.TestLoggerAddFunction.func1$`}, - {opts(AddFunction(), WithFunction(false)), `^$`}, - {opts(WithFunction(true)), `zap.TestLoggerAddFunction.func1$`}, - {opts(WithFunction(true), WithFunction(false)), `^$`}, - {opts(AddFunction(), AddCallerSkip(1), AddCallerSkip(-1)), `zap.TestLoggerAddFunction.func1$`}, - {opts(AddFunction(), AddCallerSkip(1)), `zap.withLogger$`}, - {opts(AddFunction(), AddCallerSkip(1), AddCallerSkip(3)), `runtime.goexit$`}, + { + options: opts(), + loggerFunction: "", + sugaredFunction: "", + }, + { + options: opts(WithCaller(false)), + loggerFunction: "", + sugaredFunction: "", + }, + { + options: opts(AddCaller()), + loggerFunction: "go.uber.org/zap.infoLog", + sugaredFunction: "go.uber.org/zap.infoLogSugared", + }, + { + options: opts(AddCaller(), WithCaller(false)), + loggerFunction: "", + sugaredFunction: "", + }, + { + options: opts(WithCaller(true)), + loggerFunction: "go.uber.org/zap.infoLog", + sugaredFunction: "go.uber.org/zap.infoLogSugared", + }, + { + options: opts(WithCaller(true), WithCaller(false)), + loggerFunction: "", + sugaredFunction: "", + }, + { + options: opts(AddCaller(), AddCallerSkip(1), AddCallerSkip(-1)), + loggerFunction: "go.uber.org/zap.infoLog", + sugaredFunction: "go.uber.org/zap.infoLogSugared", + }, + { + options: opts(AddCaller(), AddCallerSkip(2)), + loggerFunction: "go.uber.org/zap.withLogger", + sugaredFunction: "go.uber.org/zap.withLogger", + }, + { + options: opts(AddCaller(), AddCallerSkip(2), AddCallerSkip(3)), + loggerFunction: "runtime.goexit", + sugaredFunction: "runtime.goexit", + }, } for _, tt := range tests { withLogger(t, DebugLevel, tt.options, func(logger *Logger, logs *observer.ObservedLogs) { // Make sure that sugaring and desugaring resets caller skip properly. logger = logger.Sugar().Desugar() - logger.Info("") - output := logs.AllUntimed() - assert.Equal(t, 1, len(output), "Unexpected number of logs written out.") + infoLog(logger, "") + infoLogSugared(logger.Sugar(), "") + infoLog(logger.Sugar().Desugar(), "") + + entries := logs.AllUntimed() + assert.Equal(t, 3, len(entries), "Unexpected number of logs written out.") + for _, entry := range []observer.LoggedEntry{entries[0], entries[2]} { + assert.Regexp( + t, + tt.loggerFunction, + entry.Entry.Caller.Function, + "Expected to find function name in output.", + ) + } assert.Regexp( t, - tt.pat, - output[0].Entry.Function, + tt.sugaredFunction, + entries[1].Entry.Caller.Function, "Expected to find function name in output.", ) }) } } -func TestLoggerAddFunctionFail(t *testing.T) { +func TestLoggerAddCallerFail(t *testing.T) { errBuf := &ztest.Buffer{} - withLogger(t, DebugLevel, opts(AddFunction(), ErrorOutput(errBuf)), func(log *Logger, logs *observer.ObservedLogs) { - log.callerSkip = 1e3 + withLogger(t, DebugLevel, opts(AddCaller(), AddCallerSkip(1e3), ErrorOutput(errBuf)), func(log *Logger, logs *observer.ObservedLogs) { log.Info("Failure.") assert.Regexp( t, @@ -433,6 +461,11 @@ func TestLoggerAddFunctionFail(t *testing.T) { logs.AllUntimed()[0].Entry.Message, "Failure.", "Expected original message to survive failures in runtime.Caller.") + assert.Equal( + t, + logs.AllUntimed()[0].Entry.Caller.Function, + "", + "Expected function name to be empty string.") }) } @@ -501,3 +534,11 @@ func TestLoggerConcurrent(t *testing.T) { } }) } + +func infoLog(logger *Logger, msg string, fields ...Field) { + logger.Info(msg, fields...) +} + +func infoLogSugared(logger *SugaredLogger, args ...interface{}) { + logger.Info(args...) +} diff --git a/options.go b/options.go index 5c607bc8e..2bf2cb222 100644 --- a/options.go +++ b/options.go @@ -86,39 +86,25 @@ func Development() Option { }) } -// AddCaller configures the Logger to annotate each message with the filename -// and line number of zap's caller. See also WithCaller. +// AddCaller configures the Logger to annotate each message with the filename, +// line number, and function name of zap's caller. See also WithCaller. func AddCaller() Option { return WithCaller(true) } -// WithCaller configures the Logger to annotate each message with the filename -// and line number of zap's caller, or not, depending on the value of enabled. -// This is a generalized form of AddCaller. +// WithCaller configures the Logger to annotate each message with the filename, +// line number, and function name of zap's caller, or not, depending on the +// value of enabled. This is a generalized form of AddCaller. func WithCaller(enabled bool) Option { return optionFunc(func(log *Logger) { log.addCaller = enabled }) } -// AddFunction configures the Logger to annotate each message with the function -// name of zap's caller. See also WithFunction. -func AddFunction() Option { - return WithFunction(true) -} - -// WithFunction configures the Logger to annotate each message with the function -// name of zap's caller, or not, depending on the value of enabled. -func WithFunction(enabled bool) Option { - return optionFunc(func(log *Logger) { - log.addFunction = enabled - }) -} - -// AddCallerSkip increases the number of callers skipped by caller and function -// annotations (as enabled by the AddCaller and AddFunction options). When -// building wrappers around the Logger and SugaredLogger, supplying this Option -// prevents zap from always reporting the wrapper code as the caller. +// AddCallerSkip increases the number of callers skipped by caller annotation +// (as enabled by the AddCaller option). When building wrappers around the +// Logger and SugaredLogger, supplying this Option prevents zap from always +// reporting the wrapper code as the caller. func AddCallerSkip(skip int) Option { return optionFunc(func(log *Logger) { log.callerSkip += skip diff --git a/zapcore/console_encoder.go b/zapcore/console_encoder.go index 9ac89c4e0..53cb2dd22 100644 --- a/zapcore/console_encoder.go +++ b/zapcore/console_encoder.go @@ -89,11 +89,13 @@ func (c consoleEncoder) EncodeEntry(ent Entry, fields []Field) (*buffer.Buffer, nameEncoder(ent.LoggerName, arr) } - if ent.Caller.Defined && c.CallerKey != "" && c.EncodeCaller != nil { - c.EncodeCaller(ent.Caller, arr) - } - if ent.Function != "" && c.FunctionKey != "" { - arr.AppendString(ent.Function) + if ent.Caller.Defined { + if c.CallerKey != "" && c.EncodeCaller != nil { + c.EncodeCaller(ent.Caller, arr) + } + if c.FunctionKey != "" { + arr.AppendString(ent.Caller.Function) + } } for i := range arr.elems { if i > 0 { diff --git a/zapcore/encoder_test.go b/zapcore/encoder_test.go index 549a04cec..7857bff05 100644 --- a/zapcore/encoder_test.go +++ b/zapcore/encoder_test.go @@ -39,8 +39,7 @@ var ( Message: `hello`, Time: _epoch, Stack: "fake-stack", - Caller: EntryCaller{Defined: true, File: "foo.go", Line: 42}, - Function: "foo.Foo", + Caller: EntryCaller{Defined: true, File: "foo.go", Line: 42, Function: "foo.Foo"}, } ) diff --git a/zapcore/entry.go b/zapcore/entry.go index 6753c3967..717dc0c4f 100644 --- a/zapcore/entry.go +++ b/zapcore/entry.go @@ -70,10 +70,11 @@ func NewEntryCaller(pc uintptr, file string, line int, ok bool) EntryCaller { // EntryCaller represents the caller of a logging function. type EntryCaller struct { - Defined bool - PC uintptr - File string - Line int + Defined bool + PC uintptr + File string + Line int + Function string } // String returns the full path and line number of the caller. @@ -147,7 +148,6 @@ type Entry struct { LoggerName string Message string Caller EntryCaller - Function string Stack string } diff --git a/zapcore/json_encoder.go b/zapcore/json_encoder.go index e3286fdd6..5cf7d917e 100644 --- a/zapcore/json_encoder.go +++ b/zapcore/json_encoder.go @@ -366,19 +366,21 @@ func (enc *jsonEncoder) EncodeEntry(ent Entry, fields []Field) (*buffer.Buffer, final.AppendString(ent.LoggerName) } } - if ent.Caller.Defined && final.CallerKey != "" { - final.addKey(final.CallerKey) - cur := final.buf.Len() - final.EncodeCaller(ent.Caller, final) - if cur == final.buf.Len() { - // User-supplied EncodeCaller was a no-op. Fall back to strings to - // keep output JSON valid. - final.AppendString(ent.Caller.String()) + if ent.Caller.Defined { + if final.CallerKey != "" { + final.addKey(final.CallerKey) + cur := final.buf.Len() + final.EncodeCaller(ent.Caller, final) + if cur == final.buf.Len() { + // User-supplied EncodeCaller was a no-op. Fall back to strings to + // keep output JSON valid. + final.AppendString(ent.Caller.String()) + } + } + if final.FunctionKey != "" { + final.addKey(final.FunctionKey) + final.AppendString(ent.Caller.Function) } - } - if ent.Function != "" && final.FunctionKey != "" { - final.addKey(final.FunctionKey) - final.AppendString(ent.Function) } if final.MessageKey != "" { final.addKey(enc.MessageKey)