From 55a23674db166a6fcc75e964aaa23250b5de213f Mon Sep 17 00:00:00 2001 From: Abhinav Gupta Date: Wed, 23 Aug 2023 09:51:28 -0700 Subject: [PATCH] zapslog: Delete x/exp-based implementation (#1338) Deletes the implementation of slog.Handler based on golang.org/x/exp/slog. Maintaining two implementations doesn't serve much value here. Remaining files have been renamed to better match their contents: - handler.go holds the slog.Handler - handler_test.go tests that - example_test.go contains example tests Files that remain are still tagged with `go1.21` so they're only visible to Go 1.21 or newer. This way, `go test` in this directory with Go 1.20 will not fail: ``` % GOTOOLCHAIN=go1.20 go test -v ? go.uber.org/zap/exp/zapslog [no test files] ``` Resolves #1337 --- exp/go.mod | 1 - exp/go.sum | 2 - exp/zapslog/doc.go | 3 +- exp/zapslog/example_pre_go121_test.go | 79 -------- ...{example_go121_test.go => example_test.go} | 0 exp/zapslog/{slog_go121.go => handler.go} | 0 .../{slog_go121_test.go => handler_test.go} | 2 +- exp/zapslog/slog_pre_go121.go | 190 ------------------ exp/zapslog/slog_pre_go121_test.go | 50 ----- 9 files changed, 2 insertions(+), 325 deletions(-) delete mode 100644 exp/zapslog/example_pre_go121_test.go rename exp/zapslog/{example_go121_test.go => example_test.go} (100%) rename exp/zapslog/{slog_go121.go => handler.go} (100%) rename exp/zapslog/{slog_go121_test.go => handler_test.go} (98%) delete mode 100644 exp/zapslog/slog_pre_go121.go delete mode 100644 exp/zapslog/slog_pre_go121_test.go diff --git a/exp/go.mod b/exp/go.mod index 18e437164..162b00e27 100644 --- a/exp/go.mod +++ b/exp/go.mod @@ -5,7 +5,6 @@ go 1.19 require ( github.com/stretchr/testify v1.8.1 go.uber.org/zap v1.24.0 - golang.org/x/exp v0.0.0-20230811145659-89c5cff77bcb ) require ( diff --git a/exp/go.sum b/exp/go.sum index 2add8d8ea..f333d47c4 100644 --- a/exp/go.sum +++ b/exp/go.sum @@ -19,8 +19,6 @@ 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= -golang.org/x/exp v0.0.0-20230811145659-89c5cff77bcb h1:mIKbk8weKhSeLH2GmUTrvx8CjkyJmnU1wFmg59CUjFA= -golang.org/x/exp v0.0.0-20230811145659-89c5cff77bcb/go.mod h1:FXUEEKJgO7OQYeo8N01OfiKP8RXMtf6e8aTskBGqWdc= 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= diff --git a/exp/zapslog/doc.go b/exp/zapslog/doc.go index 8e935af87..d1b4564ad 100644 --- a/exp/zapslog/doc.go +++ b/exp/zapslog/doc.go @@ -21,6 +21,5 @@ // Package zapslog provides an implementation of slog.Handler which writes to // the supplied zapcore.Core. // -// For versions of Go before 1.21, this package uses golang.org/x/exp/slog. -// For Go 1.21 or newer, this package uses the standard log/slog package. +// Use of this package requires at least Go 1.21. package zapslog // import "go.uber.org/zap/exp/zapslog" diff --git a/exp/zapslog/example_pre_go121_test.go b/exp/zapslog/example_pre_go121_test.go deleted file mode 100644 index 666a7697c..000000000 --- a/exp/zapslog/example_pre_go121_test.go +++ /dev/null @@ -1,79 +0,0 @@ -// 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 -// 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. - -//go:build !go1.21 - -package zapslog_test - -import ( - "context" - "net" - "time" - - "go.uber.org/zap" - "go.uber.org/zap/exp/zapslog" - "golang.org/x/exp/slog" -) - -type Password string - -func (p Password) LogValue() slog.Value { - return slog.StringValue("REDACTED") -} - -func Example_slog() { - logger := zap.NewExample(zap.IncreaseLevel(zap.InfoLevel)) - defer logger.Sync() - - sl := slog.New(zapslog.NewHandler(logger.Core(), nil /* options */)) - ctx := context.Background() - - sl.Info("user", "name", "Al", "secret", Password("secret")) - sl.Error("oops", "err", net.ErrClosed, "status", 500) - sl.LogAttrs( - ctx, - slog.LevelError, - "oops", - slog.Any("err", net.ErrClosed), - slog.Int("status", 500), - ) - sl.Info("message", - slog.Group("group", - slog.Float64("pi", 3.14), - slog.Duration("1min", time.Minute), - ), - ) - sl.WithGroup("s").LogAttrs( - ctx, - slog.LevelWarn, - "warn msg", // message - slog.Uint64("u", 1), - slog.Any("m", map[string]any{ - "foo": "bar", - })) - sl.LogAttrs(ctx, slog.LevelDebug, "not show up") - - // Output: - // {"level":"info","msg":"user","name":"Al","secret":"REDACTED"} - // {"level":"error","msg":"oops","err":"use of closed network connection","status":500} - // {"level":"error","msg":"oops","err":"use of closed network connection","status":500} - // {"level":"info","msg":"message","group":{"pi":3.14,"1min":"1m0s"}} - // {"level":"warn","msg":"warn msg","s":{"u":1,"m":{"foo":"bar"}}} -} diff --git a/exp/zapslog/example_go121_test.go b/exp/zapslog/example_test.go similarity index 100% rename from exp/zapslog/example_go121_test.go rename to exp/zapslog/example_test.go diff --git a/exp/zapslog/slog_go121.go b/exp/zapslog/handler.go similarity index 100% rename from exp/zapslog/slog_go121.go rename to exp/zapslog/handler.go diff --git a/exp/zapslog/slog_go121_test.go b/exp/zapslog/handler_test.go similarity index 98% rename from exp/zapslog/slog_go121_test.go rename to exp/zapslog/handler_test.go index 8b033198a..c5c75d991 100644 --- a/exp/zapslog/slog_go121_test.go +++ b/exp/zapslog/handler_test.go @@ -43,7 +43,7 @@ func TestAddSource(t *testing.T) { entry := logs.AllUntimed()[0] assert.Equal(t, "msg", entry.Message, "Unexpected message") assert.Regexp(t, - `/slog_go121_test.go:\d+$`, + `/handler_test.go:\d+$`, entry.Caller.String(), "Unexpected caller annotation.", ) diff --git a/exp/zapslog/slog_pre_go121.go b/exp/zapslog/slog_pre_go121.go deleted file mode 100644 index 0a4f30db9..000000000 --- a/exp/zapslog/slog_pre_go121.go +++ /dev/null @@ -1,190 +0,0 @@ -// 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 -// 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. - -//go:build !go1.21 - -package zapslog - -import ( - "context" - "runtime" - - "go.uber.org/zap" - "go.uber.org/zap/zapcore" - "golang.org/x/exp/slog" -) - -// 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 -} - -// 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{} - } - return &Handler{ - core: core, - name: opts.LoggerName, - addSource: opts.AddSource, - } -} - -var _ slog.Handler = (*Handler)(nil) - -// groupObject holds all the Attrs saved in a slog.GroupValue. -type groupObject []slog.Attr - -func (gs groupObject) MarshalLogObject(enc zapcore.ObjectEncoder) error { - for _, attr := range gs { - convertAttrToField(attr).AddTo(enc) - } - return nil -} - -func convertAttrToField(attr slog.Attr) zapcore.Field { - switch attr.Value.Kind() { - case slog.KindBool: - return zap.Bool(attr.Key, attr.Value.Bool()) - case slog.KindDuration: - return zap.Duration(attr.Key, attr.Value.Duration()) - case slog.KindFloat64: - return zap.Float64(attr.Key, attr.Value.Float64()) - case slog.KindInt64: - return zap.Int64(attr.Key, attr.Value.Int64()) - case slog.KindString: - return zap.String(attr.Key, attr.Value.String()) - case slog.KindTime: - return zap.Time(attr.Key, attr.Value.Time()) - case slog.KindUint64: - return zap.Uint64(attr.Key, attr.Value.Uint64()) - case slog.KindGroup: - return zap.Object(attr.Key, groupObject(attr.Value.Group())) - case slog.KindLogValuer: - return convertAttrToField(slog.Attr{ - Key: attr.Key, - // TODO: resolve the value in a lazy way. - // This probably needs a new Zap field type - // that can be resolved lazily. - Value: attr.Value.Resolve(), - }) - default: - return zap.Any(attr.Key, attr.Value.Any()) - } -} - -// convertSlogLevel maps slog Levels to zap Levels. -// Note that there is some room between slog levels while zap levels are continuous, so we can't 1:1 map them. -// See also https://go.googlesource.com/proposal/+/master/design/56345-structured-logging.md?pli=1#levels -func convertSlogLevel(l slog.Level) zapcore.Level { - switch { - case l >= slog.LevelError: - return zapcore.ErrorLevel - case l >= slog.LevelWarn: - return zapcore.WarnLevel - case l >= slog.LevelInfo: - return zapcore.InfoLevel - default: - return zapcore.DebugLevel - } -} - -// Enabled reports whether the handler handles records at the given level. -func (h *Handler) Enabled(ctx context.Context, level slog.Level) bool { - return h.core.Enabled(convertSlogLevel(level)) -} - -// Handle handles the Record. -func (h *Handler) Handle(ctx context.Context, record slog.Record) error { - ent := zapcore.Entry{ - Level: convertSlogLevel(record.Level), - 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 { - frame, _ := runtime.CallersFrames([]uintptr{record.PC}).Next() - if frame.PC != 0 { - ce.Caller = zapcore.EntryCaller{ - Defined: true, - PC: frame.PC, - File: frame.File, - Line: frame.Line, - Function: frame.Function, - } - } - } - - fields := make([]zapcore.Field, 0, record.NumAttrs()) - record.Attrs(func(attr slog.Attr) bool { - fields = append(fields, convertAttrToField(attr)) - return true - }) - ce.Write(fields...) - return nil -} - -// WithAttrs returns a new Handler whose attributes consist of -// both the receiver's attributes and the arguments. -func (h *Handler) WithAttrs(attrs []slog.Attr) slog.Handler { - fields := make([]zapcore.Field, len(attrs)) - for i, attr := range attrs { - fields[i] = convertAttrToField(attr) - } - return h.withFields(fields...) -} - -// WithGroup returns a new Handler with the given group appended to -// the receiver's existing groups. -func (h *Handler) WithGroup(group string) slog.Handler { - return h.withFields(zap.Namespace(group)) -} - -// withFields returns a cloned Handler with the given fields. -func (h *Handler) withFields(fields ...zapcore.Field) *Handler { - cloned := *h - cloned.core = h.core.With(fields) - return &cloned -} diff --git a/exp/zapslog/slog_pre_go121_test.go b/exp/zapslog/slog_pre_go121_test.go deleted file mode 100644 index 78fd94a4b..000000000 --- a/exp/zapslog/slog_pre_go121_test.go +++ /dev/null @@ -1,50 +0,0 @@ -// 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 -// 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. - -//go:build !go1.21 - -package zapslog - -import ( - "testing" - - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" - "go.uber.org/zap/zapcore" - "go.uber.org/zap/zaptest/observer" - "golang.org/x/exp/slog" -) - -func TestAddSource(t *testing.T) { - fac, logs := observer.New(zapcore.DebugLevel) - sl := slog.New(NewHandler(fac, &HandlerOptions{ - AddSource: true, - })) - sl.Info("msg") - - require.Len(t, logs.AllUntimed(), 1, "Expected exactly one entry to be logged") - entry := logs.AllUntimed()[0] - assert.Equal(t, "msg", entry.Message, "Unexpected message") - assert.Regexp(t, - `/slog_pre_go121_test.go:\d+$`, - entry.Caller.String(), - "Unexpected caller annotation.", - ) -}