From 02ebf0f4a6dbf4df0743791f22443ffea3dfedb3 Mon Sep 17 00:00:00 2001 From: Jeremy Quirke Date: Fri, 18 Aug 2023 16:43:05 -0700 Subject: [PATCH] Clarify argument capturing behaviour of With (#1325) Make it clear that the contract with the With and logging methods is such that any arguments that could change are captured at the time of the With and logging method, and that any other behaviour introduced in future is intentionally contrary to this contract. There are proposed (https://github.com/uber-go/zap/pull/1319) future opt-in, changes that will explicitly relax this contract. --------- Co-authored-by: Abhinav Gupta --- logger.go | 5 ++++- logger_test.go | 37 +++++++++++++++++++++++++++++++++++++ 2 files changed, 41 insertions(+), 1 deletion(-) diff --git a/logger.go b/logger.go index 2bebb87b3..9b45b07b0 100644 --- a/logger.go +++ b/logger.go @@ -173,7 +173,8 @@ func (log *Logger) WithOptions(opts ...Option) *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. +// to the child don't affect the parent, and vice versa. Any fields that +// require evaluation (such as Objects) are evaluated upon invocation of With. func (log *Logger) With(fields ...Field) *Logger { if len(fields) == 0 { return log @@ -199,6 +200,8 @@ func (log *Logger) Check(lvl zapcore.Level, msg string) *zapcore.CheckedEntry { // Log logs a message at the specified level. The message includes any fields // passed at the log site, as well as any fields accumulated on the logger. +// Any Fields that require evaluation (such as Objects) are evaluated upon +// invocation of Log. func (log *Logger) Log(lvl zapcore.Level, msg string, fields ...Field) { if ce := log.check(lvl, msg); ce != nil { ce.Write(fields...) diff --git a/logger_test.go b/logger_test.go index d4af57512..8f0b01bb5 100644 --- a/logger_test.go +++ b/logger_test.go @@ -140,6 +140,43 @@ func TestLoggerWith(t *testing.T) { }) } +func TestLoggerWithCaptures(t *testing.T) { + enc := zapcore.NewJSONEncoder(zapcore.EncoderConfig{ + MessageKey: "m", + }) + + var bs ztest.Buffer + logger := New(zapcore.NewCore(enc, &bs, DebugLevel)) + + x := 0 + arr := zapcore.ArrayMarshalerFunc(func(enc zapcore.ArrayEncoder) error { + enc.AppendInt(x) + return nil + }) + + // Demonstrate the arguments are captured when With() and Info() are invoked. + logger = logger.With(Array("a", arr)) + x = 1 + logger.Info("hello", Array("b", arr)) + x = 2 + logger = logger.With(Array("c", arr)) + logger.Info("world") + + if lines := bs.Lines(); assert.Len(t, lines, 2) { + assert.JSONEq(t, `{ + "m": "hello", + "a": [0], + "b": [1] + }`, lines[0], "Unexpected output from first log.") + + assert.JSONEq(t, `{ + "m": "world", + "a": [0], + "c": [2] + }`, lines[1], "Unexpected output from second log.") + } +} + func TestLoggerLogPanic(t *testing.T) { for _, tt := range []struct { do func(*Logger)