From 3e34ef1b97fee98e77919ad9a2491ea31a28d9ab Mon Sep 17 00:00:00 2001 From: Vincent Le Goff Date: Wed, 23 Aug 2023 15:24:45 +0200 Subject: [PATCH 01/17] feat(zapslog): implement stack trace handling --- exp/zapslog/handler.go | 49 ++++++++++-------------- exp/zapslog/handler_test.go | 27 ++++++++++++-- exp/zapslog/options.go | 74 +++++++++++++++++++++++++++++++++++++ 3 files changed, 117 insertions(+), 33 deletions(-) create mode 100644 exp/zapslog/options.go diff --git a/exp/zapslog/handler.go b/exp/zapslog/handler.go index a24449390..08b609d51 100644 --- a/exp/zapslog/handler.go +++ b/exp/zapslog/handler.go @@ -33,36 +33,24 @@ import ( // Handler implements the slog.Handler by writing to a zap Core. type Handler struct { - core zapcore.Core - name string // logger name - addSource bool -} - -// HandlerOptions are options for a Zap-based [slog.Handler]. -type HandlerOptions struct { - // LoggerName is used for log entries received from slog. - // - // Defaults to empty. - LoggerName string - - // AddSource configures the handler to annotate each message with the filename, - // line number, and function name. - // AddSource is false by default to skip the cost of computing - // this information. - AddSource bool + core zapcore.Core + name string // logger name + addCaller bool + addStack zapcore.LevelEnabler + callerSkip int } // NewHandler builds a [Handler] that writes to the supplied [zapcore.Core] -// with the default options. -func NewHandler(core zapcore.Core, opts *HandlerOptions) *Handler { - if opts == nil { - opts = &HandlerOptions{} +// with options. +func NewHandler(core zapcore.Core, opts ...Option) *Handler { + h := &Handler{ + core: core, + addStack: zapcore.ErrorLevel, } - return &Handler{ - core: core, - name: opts.LoggerName, - addSource: opts.AddSource, + for _, v := range opts { + v.apply(h) } + return h } var _ slog.Handler = (*Handler)(nil) @@ -131,20 +119,19 @@ func (h *Handler) Enabled(ctx context.Context, level slog.Level) bool { // Handle handles the Record. func (h *Handler) Handle(ctx context.Context, record slog.Record) error { + zapLevel := convertSlogLevel(record.Level) ent := zapcore.Entry{ - Level: convertSlogLevel(record.Level), + Level: zapLevel, Time: record.Time, Message: record.Message, LoggerName: h.name, - // TODO: do we need to set the following fields? - // Stack: } ce := h.core.Check(ent, nil) if ce == nil { return nil } - if h.addSource && record.PC != 0 { + if h.addCaller && record.PC != 0 { frame, _ := runtime.CallersFrames([]uintptr{record.PC}).Next() if frame.PC != 0 { ce.Caller = zapcore.EntryCaller{ @@ -157,6 +144,10 @@ func (h *Handler) Handle(ctx context.Context, record slog.Record) error { } } + if h.addStack.Enabled(zapLevel) { + ce.Stack = zap.TakeStacktrace(4 + h.callerSkip) + } + fields := make([]zapcore.Field, 0, record.NumAttrs()) record.Attrs(func(attr slog.Attr) bool { fields = append(fields, convertAttrToField(attr)) diff --git a/exp/zapslog/handler_test.go b/exp/zapslog/handler_test.go index c5c75d991..edcf73bbb 100644 --- a/exp/zapslog/handler_test.go +++ b/exp/zapslog/handler_test.go @@ -32,11 +32,9 @@ import ( "go.uber.org/zap/zaptest/observer" ) -func TestAddSource(t *testing.T) { +func TestAddCaller(t *testing.T) { fac, logs := observer.New(zapcore.DebugLevel) - sl := slog.New(NewHandler(fac, &HandlerOptions{ - AddSource: true, - })) + sl := slog.New(NewHandler(fac, AddCaller())) sl.Info("msg") require.Len(t, logs.AllUntimed(), 1, "Expected exactly one entry to be logged") @@ -48,3 +46,24 @@ func TestAddSource(t *testing.T) { "Unexpected caller annotation.", ) } + +func TestAddStack(t *testing.T) { + r := require.New(t) + fac, logs := observer.New(zapcore.DebugLevel) + sl := slog.New(NewHandler(fac, AddStacktrace(zapcore.DebugLevel))) + sl.Info("msg") + + r.Len(logs.AllUntimed(), 1, "Expected exactly one entry to be logged") + entry := logs.AllUntimed()[0] + r.Equal("msg", entry.Message, "Unexpected message") + assert.Regexp(t, + `^go.uber.org/zap/exp/zapslog.TestAddStack`, + entry.Stack, + "Unexpected stack trace annotation.", + ) + assert.Regexp(t, + `/zapslog/slog_go121_test.go:\d+`, + entry.Stack, + "Unexpected stack trace annotation.", + ) +} diff --git a/exp/zapslog/options.go b/exp/zapslog/options.go new file mode 100644 index 000000000..604fc4ef5 --- /dev/null +++ b/exp/zapslog/options.go @@ -0,0 +1,74 @@ +// Copyright (c) 2016 Uber Technologies, Inc. +// +// Permission is hereby granted, free of charge, to any person obtaining a copy +// of this software and associated documentation files (the "Software"), to deal +// in the Software without restriction, including without limitation the rights +// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +// copies of the Software, and to permit persons to whom the Software is +// furnished to do so, subject to the following conditions: +// +// The above copyright notice and this permission notice shall be included in +// all copies or substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN +// THE SOFTWARE. + +package zapslog + +import "go.uber.org/zap/zapcore" + +type Option interface { + apply(*Handler) +} + +// optionFunc wraps a func so it satisfies the Option interface. +type optionFunc func(*Handler) + +func (f optionFunc) apply(handler *Handler) { + f(handler) +} + +// WithName configures the Logger to annotate each message with the logger name. +func WithName(name string) Option { + return optionFunc(func(h *Handler) { + h.name = name + }) +} + +// 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, +// 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(handler *Handler) { + handler.addCaller = enabled + }) +} + +// 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 *Handler) { + log.callerSkip += skip + }) +} + +// AddStacktrace configures the Logger to record a stack trace for all messages at +// or above a given level. +func AddStacktrace(lvl zapcore.LevelEnabler) Option { + return optionFunc(func(log *Handler) { + log.addStack = lvl + }) +} From 855ea4be53576a7d640e4b55687012785dbbec23 Mon Sep 17 00:00:00 2001 From: Abhinav Gupta Date: Fri, 25 Aug 2023 05:25:55 -0700 Subject: [PATCH 02/17] options.go: Fix year in copyright header --- exp/zapslog/options.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/exp/zapslog/options.go b/exp/zapslog/options.go index 604fc4ef5..6558c596e 100644 --- a/exp/zapslog/options.go +++ b/exp/zapslog/options.go @@ -1,4 +1,4 @@ -// Copyright (c) 2016 Uber Technologies, Inc. +// Copyright (c) 2023 Uber Technologies, Inc. // // Permission is hereby granted, free of charge, to any person obtaining a copy // of this software and associated documentation files (the "Software"), to deal From 46f52288816e76c1f9d749ada5700769c7158201 Mon Sep 17 00:00:00 2001 From: Abhinav Gupta Date: Fri, 25 Aug 2023 05:26:24 -0700 Subject: [PATCH 03/17] option: Drop AddCaller We don't need AddCaller and WithCaller both. WithCaller was added because AddCaller doesn't take a parameter. --- exp/zapslog/handler_test.go | 2 +- exp/zapslog/options.go | 10 ++-------- 2 files changed, 3 insertions(+), 9 deletions(-) diff --git a/exp/zapslog/handler_test.go b/exp/zapslog/handler_test.go index edcf73bbb..7deaf4e62 100644 --- a/exp/zapslog/handler_test.go +++ b/exp/zapslog/handler_test.go @@ -34,7 +34,7 @@ import ( func TestAddCaller(t *testing.T) { fac, logs := observer.New(zapcore.DebugLevel) - sl := slog.New(NewHandler(fac, AddCaller())) + sl := slog.New(NewHandler(fac, WithCaller(true))) sl.Info("msg") require.Len(t, logs.AllUntimed(), 1, "Expected exactly one entry to be logged") diff --git a/exp/zapslog/options.go b/exp/zapslog/options.go index 6558c596e..07895121a 100644 --- a/exp/zapslog/options.go +++ b/exp/zapslog/options.go @@ -40,15 +40,9 @@ func WithName(name string) Option { }) } -// 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, -// line number, and function name of zap's caller, or not, depending on the -// value of enabled. This is a generalized form of AddCaller. +// line number, and function name of Zap's caller, or not, depending on the +// value of enabled. func WithCaller(enabled bool) Option { return optionFunc(func(handler *Handler) { handler.addCaller = enabled From f6bf5073650c791bc05c742cd2c183d9f264f336 Mon Sep 17 00:00:00 2001 From: Abhinav Gupta Date: Fri, 25 Aug 2023 06:00:19 -0700 Subject: [PATCH 04/17] fix(example): NewHandler shouldn't be called with nil --- exp/zapslog/example_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/exp/zapslog/example_test.go b/exp/zapslog/example_test.go index 7a18fc84b..da3c63458 100644 --- a/exp/zapslog/example_test.go +++ b/exp/zapslog/example_test.go @@ -42,7 +42,7 @@ func Example_slog() { logger := zap.NewExample(zap.IncreaseLevel(zap.InfoLevel)) defer logger.Sync() - sl := slog.New(zapslog.NewHandler(logger.Core(), nil /* options */)) + sl := slog.New(zapslog.NewHandler(logger.Core())) ctx := context.Background() sl.Info("user", "name", "Al", "secret", Password("secret")) From b8c01322a84b5a0215521a5a3897a935d084da0b Mon Sep 17 00:00:00 2001 From: Abhinav Gupta Date: Fri, 25 Aug 2023 06:06:31 -0700 Subject: [PATCH 05/17] go.mod: Add replace zap => ../ --- exp/go.mod | 3 ++- exp/go.sum | 9 ++------- 2 files changed, 4 insertions(+), 8 deletions(-) diff --git a/exp/go.mod b/exp/go.mod index 162b00e27..28c5a95a7 100644 --- a/exp/go.mod +++ b/exp/go.mod @@ -10,7 +10,8 @@ require ( require ( github.com/davecgh/go-spew v1.1.1 // indirect github.com/pmezard/go-difflib v1.0.0 // indirect - go.uber.org/atomic v1.10.0 // indirect go.uber.org/multierr v1.10.0 // indirect gopkg.in/yaml.v3 v3.0.1 // indirect ) + +replace go.uber.org/zap => ../ diff --git a/exp/go.sum b/exp/go.sum index f333d47c4..96489348a 100644 --- a/exp/go.sum +++ b/exp/go.sum @@ -1,8 +1,7 @@ -github.com/benbjohnson/clock v1.1.0 h1:Q92kusRqC1XV2MjkWETPvjJVqKetz1OzxZB7mHJLju8= +github.com/benbjohnson/clock v1.3.0 h1:ip6w0uFQkncKQ979AypyG0ER7mqUSBdKLOgAle/AT8A= github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c= github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= -github.com/pkg/errors v0.8.1 h1:iURUrRGxPUNPdy5/HRSm+Yj6okJ6UtLINN0Q9M4+h3I= github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= @@ -12,13 +11,9 @@ github.com/stretchr/testify v1.7.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/ github.com/stretchr/testify v1.8.0/go.mod h1:yNjHg4UonilssWZ8iaSj1OCr/vHnekPRkoO+kdMU+MU= github.com/stretchr/testify v1.8.1 h1:w7B6lhMri9wdJUVmEZPGGhZzrYTPvgJArz7wNPgYKsk= github.com/stretchr/testify v1.8.1/go.mod h1:w2LPCIKwWwSfY2zedu0+kehJoqGctiVI29o6fzry7u4= -go.uber.org/atomic v1.10.0 h1:9qC72Qh0+3MqyJbAn8YU5xVq1frD8bn3JtD2oXtafVQ= -go.uber.org/atomic v1.10.0/go.mod h1:LUxbIzbOniOlMKjJjyPfpl4v+PKK2cNJn91OQbhoJI0= -go.uber.org/goleak v1.1.11 h1:wy28qYRKZgnJTxGxvye5/wgWr1EKjmUDGYox5mGlRlI= +go.uber.org/goleak v1.2.0 h1:xqgm/S+aQvhWFTtR0XK3Jvg7z8kGV8P4X14IzwN3Eqk= go.uber.org/multierr v1.10.0 h1:S0h4aNzvfcFsC3dRF1jLoaov7oRaKqRGC/pUEJ2yvPQ= go.uber.org/multierr v1.10.0/go.mod h1:20+QtiLqy0Nd6FdQB9TLXag12DsQkrbs3htMFfDN80Y= -go.uber.org/zap v1.24.0 h1:FiJd5l1UOLj0wCgbSE0rwwXHzEdAZS6hiiSnxJN/D60= -go.uber.org/zap v1.24.0/go.mod h1:2kMP+WWQ8aoFoedH3T2sq6iJ2yDWpHbP0f6MQbS9Gkg= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+SDo693bJlVdllGtEeKM= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= From 397c305a1f6bcf923323eb65a1781f393e83dc05 Mon Sep 17 00:00:00 2001 From: Vincent Le Goff Date: Fri, 25 Aug 2023 18:16:18 +0200 Subject: [PATCH 06/17] feat(slog): use internal stacktrace package --- exp/zapslog/handler.go | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/exp/zapslog/handler.go b/exp/zapslog/handler.go index 08b609d51..f1171fc76 100644 --- a/exp/zapslog/handler.go +++ b/exp/zapslog/handler.go @@ -28,6 +28,7 @@ import ( "runtime" "go.uber.org/zap" + "go.uber.org/zap/internal/stacktrace" "go.uber.org/zap/zapcore" ) @@ -145,7 +146,11 @@ func (h *Handler) Handle(ctx context.Context, record slog.Record) error { } if h.addStack.Enabled(zapLevel) { - ce.Stack = zap.TakeStacktrace(4 + h.callerSkip) + // Skipping 3: + // zapslog/handler log/slog.(*Logger).log + // slog/logger log/slog.(*Logger).log + // slog/logger log/slog.(*Logger). + ce.Stack = stacktrace.Take(3 + h.callerSkip) } fields := make([]zapcore.Field, 0, record.NumAttrs()) From b66a3d395a47cf4e360ee331e21861009671e9cd Mon Sep 17 00:00:00 2001 From: Vincent Le Goff Date: Fri, 25 Aug 2023 18:16:45 +0200 Subject: [PATCH 07/17] feat(slog): replace to latest --- exp/go.mod | 3 ++- exp/go.sum | 2 ++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/exp/go.mod b/exp/go.mod index 28c5a95a7..cc6f83cda 100644 --- a/exp/go.mod +++ b/exp/go.mod @@ -14,4 +14,5 @@ require ( gopkg.in/yaml.v3 v3.0.1 // indirect ) -replace go.uber.org/zap => ../ +// replace to commit: https://github.com/uber-go/zap/commit/98e9c4fe632cc00c99033d8d616f1318b7063eee +replace go.uber.org/zap => go.uber.org/zap v1.24.1-0.20230825131617-98e9c4fe632c diff --git a/exp/go.sum b/exp/go.sum index 96489348a..8e41f6164 100644 --- a/exp/go.sum +++ b/exp/go.sum @@ -14,6 +14,8 @@ github.com/stretchr/testify v1.8.1/go.mod h1:w2LPCIKwWwSfY2zedu0+kehJoqGctiVI29o go.uber.org/goleak v1.2.0 h1:xqgm/S+aQvhWFTtR0XK3Jvg7z8kGV8P4X14IzwN3Eqk= go.uber.org/multierr v1.10.0 h1:S0h4aNzvfcFsC3dRF1jLoaov7oRaKqRGC/pUEJ2yvPQ= go.uber.org/multierr v1.10.0/go.mod h1:20+QtiLqy0Nd6FdQB9TLXag12DsQkrbs3htMFfDN80Y= +go.uber.org/zap v1.24.1-0.20230825131617-98e9c4fe632c h1:qCcNiJbqgoZUdgWoa1xo+N7Fv82mJ5upF8x/T/1FK2Y= +go.uber.org/zap v1.24.1-0.20230825131617-98e9c4fe632c/go.mod h1:JIAUzQIH94IC4fOJQm7gMmBJP5k7wQfdcnYdPoEXJYk= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+SDo693bJlVdllGtEeKM= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= From b0bb45f8c5fcb0f9a0113361a5bf40ae27983b0b Mon Sep 17 00:00:00 2001 From: Vincent Le Goff Date: Fri, 25 Aug 2023 18:21:06 +0200 Subject: [PATCH 08/17] fix(slog): test --- exp/zapslog/handler_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/exp/zapslog/handler_test.go b/exp/zapslog/handler_test.go index 7deaf4e62..9460fd9db 100644 --- a/exp/zapslog/handler_test.go +++ b/exp/zapslog/handler_test.go @@ -62,7 +62,7 @@ func TestAddStack(t *testing.T) { "Unexpected stack trace annotation.", ) assert.Regexp(t, - `/zapslog/slog_go121_test.go:\d+`, + `/zapslog/handler_test.go:\d+`, entry.Stack, "Unexpected stack trace annotation.", ) From 34f37b7c312ada9ce4c14365f6fb72871a1a4c4e Mon Sep 17 00:00:00 2001 From: Vincent Le Goff Date: Fri, 25 Aug 2023 18:47:18 +0200 Subject: [PATCH 09/17] fix build tag --- exp/zapslog/options.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/exp/zapslog/options.go b/exp/zapslog/options.go index 07895121a..7a6b36006 100644 --- a/exp/zapslog/options.go +++ b/exp/zapslog/options.go @@ -18,6 +18,8 @@ // OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN // THE SOFTWARE. +//go:build go1.21 + package zapslog import "go.uber.org/zap/zapcore" From 3a38323135a2e89cd4feb53955ba1bf8f3eda745 Mon Sep 17 00:00:00 2001 From: Vincent Le Goff Date: Fri, 25 Aug 2023 18:48:14 +0200 Subject: [PATCH 10/17] lint --- exp/zapslog/options.go | 1 + 1 file changed, 1 insertion(+) diff --git a/exp/zapslog/options.go b/exp/zapslog/options.go index 7a6b36006..cac76cc73 100644 --- a/exp/zapslog/options.go +++ b/exp/zapslog/options.go @@ -24,6 +24,7 @@ package zapslog import "go.uber.org/zap/zapcore" +// An Option configures a slog Handler. type Option interface { apply(*Handler) } From 6fe8c05c1d3bb4278bb441e74461417b2138aa5f Mon Sep 17 00:00:00 2001 From: Abhinav Gupta Date: Sat, 26 Aug 2023 21:16:44 -0700 Subject: [PATCH 11/17] go.mod: replace Zap to ../ --- exp/go.mod | 3 +-- exp/go.sum | 2 -- 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/exp/go.mod b/exp/go.mod index cc6f83cda..28c5a95a7 100644 --- a/exp/go.mod +++ b/exp/go.mod @@ -14,5 +14,4 @@ require ( gopkg.in/yaml.v3 v3.0.1 // indirect ) -// replace to commit: https://github.com/uber-go/zap/commit/98e9c4fe632cc00c99033d8d616f1318b7063eee -replace go.uber.org/zap => go.uber.org/zap v1.24.1-0.20230825131617-98e9c4fe632c +replace go.uber.org/zap => ../ diff --git a/exp/go.sum b/exp/go.sum index 8e41f6164..96489348a 100644 --- a/exp/go.sum +++ b/exp/go.sum @@ -14,8 +14,6 @@ github.com/stretchr/testify v1.8.1/go.mod h1:w2LPCIKwWwSfY2zedu0+kehJoqGctiVI29o go.uber.org/goleak v1.2.0 h1:xqgm/S+aQvhWFTtR0XK3Jvg7z8kGV8P4X14IzwN3Eqk= go.uber.org/multierr v1.10.0 h1:S0h4aNzvfcFsC3dRF1jLoaov7oRaKqRGC/pUEJ2yvPQ= go.uber.org/multierr v1.10.0/go.mod h1:20+QtiLqy0Nd6FdQB9TLXag12DsQkrbs3htMFfDN80Y= -go.uber.org/zap v1.24.1-0.20230825131617-98e9c4fe632c h1:qCcNiJbqgoZUdgWoa1xo+N7Fv82mJ5upF8x/T/1FK2Y= -go.uber.org/zap v1.24.1-0.20230825131617-98e9c4fe632c/go.mod h1:JIAUzQIH94IC4fOJQm7gMmBJP5k7wQfdcnYdPoEXJYk= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+SDo693bJlVdllGtEeKM= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= From e63605e37815034d891ced5d06b6cb810b15e673 Mon Sep 17 00:00:00 2001 From: Abhinav Gupta Date: Sat, 26 Aug 2023 21:17:53 -0700 Subject: [PATCH 12/17] Rename AddCallerSkip to WithCallerSkip And update documentation. --- exp/zapslog/options.go | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/exp/zapslog/options.go b/exp/zapslog/options.go index cac76cc73..4bea891b7 100644 --- a/exp/zapslog/options.go +++ b/exp/zapslog/options.go @@ -52,11 +52,13 @@ func WithCaller(enabled bool) Option { }) } -// 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 { +// WithCallerSkip increases the number of callers skipped by caller annotation +// (as enabled by the [WithCaller] option). +// +// When building wrappers around the Logger, +// supplying this Option prevents Zap from always reporting +// the wrapper code as the caller. +func WithCallerSkip(skip int) Option { return optionFunc(func(log *Handler) { log.callerSkip += skip }) From d5ebf656e75c981e14c1b9ce40df277f652472b2 Mon Sep 17 00:00:00 2001 From: Abhinav Gupta Date: Sat, 26 Aug 2023 21:22:24 -0700 Subject: [PATCH 13/17] AddStacktrace: Use slog.Level, rename to AddStacktraceAt Changes AddStacktrace to use a slog.Level instead of a Zap Level, and renames it to AddStacktraceAt. This leaves room for a LevelEnabler (as suggested in the PR) to be specified with an AddStacktrace in the future. --- exp/zapslog/handler.go | 8 ++++---- exp/zapslog/handler_test.go | 2 +- exp/zapslog/options.go | 10 +++++----- 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/exp/zapslog/handler.go b/exp/zapslog/handler.go index f1171fc76..b8842984e 100644 --- a/exp/zapslog/handler.go +++ b/exp/zapslog/handler.go @@ -37,7 +37,7 @@ type Handler struct { core zapcore.Core name string // logger name addCaller bool - addStack zapcore.LevelEnabler + addStackAt slog.Level callerSkip int } @@ -45,8 +45,8 @@ type Handler struct { // with options. func NewHandler(core zapcore.Core, opts ...Option) *Handler { h := &Handler{ - core: core, - addStack: zapcore.ErrorLevel, + core: core, + addStackAt: slog.LevelError, } for _, v := range opts { v.apply(h) @@ -145,7 +145,7 @@ func (h *Handler) Handle(ctx context.Context, record slog.Record) error { } } - if h.addStack.Enabled(zapLevel) { + if record.Level >= h.addStackAt { // Skipping 3: // zapslog/handler log/slog.(*Logger).log // slog/logger log/slog.(*Logger).log diff --git a/exp/zapslog/handler_test.go b/exp/zapslog/handler_test.go index 9460fd9db..e7cc0c069 100644 --- a/exp/zapslog/handler_test.go +++ b/exp/zapslog/handler_test.go @@ -50,7 +50,7 @@ func TestAddCaller(t *testing.T) { func TestAddStack(t *testing.T) { r := require.New(t) fac, logs := observer.New(zapcore.DebugLevel) - sl := slog.New(NewHandler(fac, AddStacktrace(zapcore.DebugLevel))) + sl := slog.New(NewHandler(fac, AddStacktrace(slog.LevelDebug))) sl.Info("msg") r.Len(logs.AllUntimed(), 1, "Expected exactly one entry to be logged") diff --git a/exp/zapslog/options.go b/exp/zapslog/options.go index 4bea891b7..9ae294f20 100644 --- a/exp/zapslog/options.go +++ b/exp/zapslog/options.go @@ -22,7 +22,7 @@ package zapslog -import "go.uber.org/zap/zapcore" +import "log/slog" // An Option configures a slog Handler. type Option interface { @@ -64,10 +64,10 @@ func WithCallerSkip(skip int) Option { }) } -// AddStacktrace configures the Logger to record a stack trace for all messages at -// or above a given level. -func AddStacktrace(lvl zapcore.LevelEnabler) Option { +// AddStacktraceAt configures the Logger to record a stack trace +// for all messages at or above a given level. +func AddStacktraceAt(lvl slog.Level) Option { return optionFunc(func(log *Handler) { - log.addStack = lvl + log.addStackAt = lvl }) } From b68ffcf9e92c6f75763090dd03e1ad71ae7e3b28 Mon Sep 17 00:00:00 2001 From: Abhinav Gupta Date: Sat, 26 Aug 2023 21:23:39 -0700 Subject: [PATCH 14/17] AddStacktraceAt: Fix test --- exp/zapslog/handler_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/exp/zapslog/handler_test.go b/exp/zapslog/handler_test.go index e7cc0c069..fe20b45a1 100644 --- a/exp/zapslog/handler_test.go +++ b/exp/zapslog/handler_test.go @@ -50,7 +50,7 @@ func TestAddCaller(t *testing.T) { func TestAddStack(t *testing.T) { r := require.New(t) fac, logs := observer.New(zapcore.DebugLevel) - sl := slog.New(NewHandler(fac, AddStacktrace(slog.LevelDebug))) + sl := slog.New(NewHandler(fac, AddStacktraceAt(slog.LevelDebug))) sl.Info("msg") r.Len(logs.AllUntimed(), 1, "Expected exactly one entry to be logged") From 41948fb84d1644a7aef59af534b21ccab4284f0a Mon Sep 17 00:00:00 2001 From: Abhinav Gupta Date: Sat, 26 Aug 2023 21:27:51 -0700 Subject: [PATCH 15/17] test: Don't use require.New We avoid use of assert.New and require.New in our tests. --- exp/zapslog/handler_test.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/exp/zapslog/handler_test.go b/exp/zapslog/handler_test.go index fe20b45a1..7792a737c 100644 --- a/exp/zapslog/handler_test.go +++ b/exp/zapslog/handler_test.go @@ -48,14 +48,13 @@ func TestAddCaller(t *testing.T) { } func TestAddStack(t *testing.T) { - r := require.New(t) fac, logs := observer.New(zapcore.DebugLevel) sl := slog.New(NewHandler(fac, AddStacktraceAt(slog.LevelDebug))) sl.Info("msg") - r.Len(logs.AllUntimed(), 1, "Expected exactly one entry to be logged") + require.Len(t, logs.AllUntimed(), 1, "Expected exactly one entry to be logged") entry := logs.AllUntimed()[0] - r.Equal("msg", entry.Message, "Unexpected message") + require.Equal(t, "msg", entry.Message, "Unexpected message") assert.Regexp(t, `^go.uber.org/zap/exp/zapslog.TestAddStack`, entry.Stack, From 31b7f037b1b6727364046de760878519ee8b8608 Mon Sep 17 00:00:00 2001 From: Abhinav Gupta Date: Sat, 26 Aug 2023 21:29:09 -0700 Subject: [PATCH 16/17] Handler.Handle: Inline zapLevel back Now that zaplevel isn't used to determine if we need to capture the stack trace, it can be inlined back into the entry construction. --- exp/zapslog/handler.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/exp/zapslog/handler.go b/exp/zapslog/handler.go index b8842984e..a5ddfec8f 100644 --- a/exp/zapslog/handler.go +++ b/exp/zapslog/handler.go @@ -120,9 +120,8 @@ func (h *Handler) Enabled(ctx context.Context, level slog.Level) bool { // Handle handles the Record. func (h *Handler) Handle(ctx context.Context, record slog.Record) error { - zapLevel := convertSlogLevel(record.Level) ent := zapcore.Entry{ - Level: zapLevel, + Level: convertSlogLevel(record.Level), Time: record.Time, Message: record.Message, LoggerName: h.name, From fd6cc64e9099e93394fa88b40bc51f027eac42ea Mon Sep 17 00:00:00 2001 From: Abhinav Gupta Date: Sat, 26 Aug 2023 21:32:08 -0700 Subject: [PATCH 17/17] doc(WithCaller): remove Zap logger reference --- exp/zapslog/options.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/exp/zapslog/options.go b/exp/zapslog/options.go index 9ae294f20..0eb5c8c0e 100644 --- a/exp/zapslog/options.go +++ b/exp/zapslog/options.go @@ -43,9 +43,8 @@ func WithName(name string) Option { }) } -// 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. +// WithCaller configures the Logger to include the filename and line number +// of the caller in log messages--if available. func WithCaller(enabled bool) Option { return optionFunc(func(handler *Handler) { handler.addCaller = enabled