From 97ce357c700da331728e8d8a50ab9c865c6bd54c Mon Sep 17 00:00:00 2001
From: Abhinav Gupta <mail@abhinavg.net>
Date: Thu, 3 Aug 2023 08:31:13 -0700
Subject: [PATCH] zapslog: Drop HandlerOptions.New, use NewHandler

The pattern of constructors for zapslog.Handler was previously:

    type HandlerOptions struct{ ... }

    func (*HandlerOptions) New(zapcore.Core) *Handler
    func NewHandler(zapcore.Core) *Handler

This was modeled after similar constructors in slog for JSON and Text
handlers.

slog has since dropped those constructors in favor of simple:

    func NewJSONHandler(io.Writer, *HandlerOptions) *JSONHandler
    func NewTextHandler(io.Writer, *HandlerOptions) *TextHandler

This change similarly drops the options.New method and default
no-argument constructor in favor of:

    func NewHandler(zapcore.Core, *HandlerOptions) *Handler

As with slog's JSON or Text handlers, the first argument is the
destination: an io.Writer for JSON and Text, a Zap core for us.

Refs https://github.com/golang/go/issues/59339
---
 exp/zapslog/example_test.go |  2 +-
 exp/zapslog/slog.go         | 20 +++++++++-----------
 exp/zapslog/slog_test.go    |  6 +++---
 3 files changed, 13 insertions(+), 15 deletions(-)

diff --git a/exp/zapslog/example_test.go b/exp/zapslog/example_test.go
index d6f8c9c87..e19adb853 100644
--- a/exp/zapslog/example_test.go
+++ b/exp/zapslog/example_test.go
@@ -40,7 +40,7 @@ func Example_slog() {
 	logger := zap.NewExample(zap.IncreaseLevel(zap.InfoLevel))
 	defer logger.Sync()
 
-	sl := slog.New(zapslog.NewHandler(logger.Core()))
+	sl := slog.New(zapslog.NewHandler(logger.Core(), nil /* options */))
 	ctx := context.Background()
 
 	sl.Info("user", "name", "Al", "secret", Password("secret"))
diff --git a/exp/zapslog/slog.go b/exp/zapslog/slog.go
index 39f89549a..b999f17f6 100644
--- a/exp/zapslog/slog.go
+++ b/exp/zapslog/slog.go
@@ -50,9 +50,12 @@ type HandlerOptions struct {
 	AddSource bool
 }
 
-// New builds a [Handler] that writes to the supplied [zapcore.Core].
-// This handler may be supplied to [slog.New] to create a new [slog.Logger].
-func (opts HandlerOptions) New(core zapcore.Core) *Handler {
+// 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,
@@ -60,13 +63,6 @@ func (opts HandlerOptions) New(core zapcore.Core) *Handler {
 	}
 }
 
-// NewHandler builds a [Handler] that writes to the supplied [zapcore.Core]
-// with the default options.
-func NewHandler(core zapcore.Core) *Handler {
-	var opts HandlerOptions
-	return opts.New(core)
-}
-
 var _ slog.Handler = (*Handler)(nil)
 
 // groupObject holds all the Attrs saved in a slog.GroupValue.
@@ -100,7 +96,9 @@ func convertAttrToField(attr slog.Attr) zapcore.Field {
 	case slog.KindLogValuer:
 		return convertAttrToField(slog.Attr{
 			Key: attr.Key,
-			// TODO: resolve the value in a lazy way
+			// 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:
diff --git a/exp/zapslog/slog_test.go b/exp/zapslog/slog_test.go
index 2ef3a0d81..f49c52562 100644
--- a/exp/zapslog/slog_test.go
+++ b/exp/zapslog/slog_test.go
@@ -32,14 +32,14 @@ import (
 func TestAddSource(t *testing.T) {
 	r := require.New(t)
 	fac, logs := observer.New(zapcore.DebugLevel)
-	sl := slog.New(HandlerOptions{
+	sl := slog.New(NewHandler(fac, &HandlerOptions{
 		AddSource: true,
-	}.New(fac))
+	}))
 	sl.Info("msg")
 
 	r.Len(logs.AllUntimed(), 1, "Expected exactly one entry to be logged")
 	entry := logs.AllUntimed()[0]
-	r.Equal("msg", entry.Entry.Message, "Unexpected message")
+	r.Equal("msg", entry.Message, "Unexpected message")
 	r.Regexp(
 		`/slog_test.go:\d+$`,
 		entry.Caller.String(),