From 28b05cb63c318c9e27c80a22fb78efd64c6d2e2b Mon Sep 17 00:00:00 2001
From: junya koyama <arukiidou@yahoo.co.jp>
Date: Sat, 3 Feb 2024 08:14:16 +0900
Subject: [PATCH] zapslog: Ignore groups if empty Record #1401

Signed-off-by: junya koyama <arukiidou@yahoo.co.jp>
---
 exp/zapslog/handler.go      | 26 ++++++++++++++++++++++----
 exp/zapslog/handler_test.go | 30 ++++++++++++++++++++++++++++++
 2 files changed, 52 insertions(+), 4 deletions(-)

diff --git a/exp/zapslog/handler.go b/exp/zapslog/handler.go
index 5abef1c0d..591744423 100644
--- a/exp/zapslog/handler.go
+++ b/exp/zapslog/handler.go
@@ -26,6 +26,7 @@ import (
 	"context"
 	"log/slog"
 	"runtime"
+	"slices"
 
 	"go.uber.org/zap"
 	"go.uber.org/zap/internal/stacktrace"
@@ -39,6 +40,7 @@ type Handler struct {
 	addCaller  bool
 	addStackAt slog.Level
 	callerSkip int
+	holdGroup  string // holds latest group.
 }
 
 // NewHandler builds a [Handler] that writes to the supplied [zapcore.Core]
@@ -161,11 +163,17 @@ func (h *Handler) Handle(ctx context.Context, record slog.Record) error {
 		ce.Stack = stacktrace.Take(3 + h.callerSkip)
 	}
 
-	fields := make([]zapcore.Field, 0, record.NumAttrs())
+	fields := make([]zapcore.Field, 0, record.NumAttrs()+1)
 	record.Attrs(func(attr slog.Attr) bool {
-		fields = append(fields, convertAttrToField(attr))
+		f := convertAttrToField(attr)
+		if h.holdGroup != "" && f != zap.Skip() {
+			fields = slices.Insert(fields, 0, zap.Namespace(h.holdGroup))
+			h.holdGroup = ""
+		}
+		fields = append(fields, f)
 		return true
 	})
+
 	ce.Write(fields...)
 	return nil
 }
@@ -175,7 +183,12 @@ func (h *Handler) Handle(ctx context.Context, record slog.Record) error {
 func (h *Handler) WithAttrs(attrs []slog.Attr) slog.Handler {
 	fields := make([]zapcore.Field, 0, len(attrs))
 	for _, attr := range attrs {
-		fields = append(fields, convertAttrToField(attr))
+		f := convertAttrToField(attr)
+		if h.holdGroup != "" && f != zap.Skip() {
+			fields = slices.Insert(fields, 0, zap.Namespace(h.holdGroup))
+			h.holdGroup = ""
+		}
+		fields = append(fields, f)
 	}
 	return h.withFields(fields...)
 }
@@ -183,7 +196,12 @@ func (h *Handler) WithAttrs(attrs []slog.Attr) slog.Handler {
 // 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))
+	cloned := *h
+	if h.holdGroup != "" {
+		cloned.core = h.core.With([]zapcore.Field{zap.Namespace(group)})
+	}
+	cloned.holdGroup = group
+	return &cloned
 }
 
 // withFields returns a cloned Handler with the given fields.
diff --git a/exp/zapslog/handler_test.go b/exp/zapslog/handler_test.go
index 4011e3ae2..2a02e7f34 100644
--- a/exp/zapslog/handler_test.go
+++ b/exp/zapslog/handler_test.go
@@ -183,6 +183,36 @@ func TestInlineGroup(t *testing.T) {
 	})
 }
 
+func TestWithGroup(t *testing.T) {
+	t.Parallel()
+	fac, observedLogs := observer.New(zapcore.DebugLevel)
+	t.Run("empty-group-record", func(t *testing.T) {
+		sl := slog.New(NewHandler(fac))
+		sl.With("a", "b").WithGroup("G").With("c", "d").WithGroup("H").Info("msg")
+
+		logs := observedLogs.TakeAll()
+		require.Len(t, logs, 1, "Expected exactly one entry to be logged")
+		entry := logs[0]
+		assert.Equal(t, "", entry.LoggerName, "Unexpected logger name")
+		assert.Equal(t, map[string]any{
+			"G": map[string]any{
+				"c": "d",
+			},
+			"a": "b",
+		}, logs[0].ContextMap(), "Unexpected context")
+	})
+	t.Run("only-records-to-ignore", func(t *testing.T) {
+		sl := slog.New(NewHandler(fac))
+		sl.WithGroup("H").With(slog.Attr{}).Info("msg")
+
+		logs := observedLogs.TakeAll()
+		require.Len(t, logs, 1, "Expected exactly one entry to be logged")
+		entry := logs[0]
+		assert.Equal(t, "", entry.LoggerName, "Unexpected logger name")
+		assert.Equal(t, map[string]any{}, logs[0].ContextMap(), "Unexpected context")
+	})
+}
+
 type Token string
 
 func (Token) LogValue() slog.Value {