From 4c54bf49e8192dce82d84db2f2363e0aa3efb1b0 Mon Sep 17 00:00:00 2001 From: Rod Vagg Date: Wed, 6 May 2026 10:19:43 +1000 Subject: [PATCH 1/4] fix: support slog.Group in the zap bridge slog.Group attrs falls through to ReflectType in slogAttrToZapField and is rendered []slog.Attr via reflection as [{"Key":"0","Value":{}},...]. This fix render Groups via a zapcore.ObjectMarshaler adapter. --- slog_bridge.go | 54 ++++++++++++++++++++++++++++ slog_bridge_test.go | 87 +++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 141 insertions(+) diff --git a/slog_bridge.go b/slog_bridge.go index a30dc7e..bd16090 100644 --- a/slog_bridge.go +++ b/slog_bridge.go @@ -235,6 +235,13 @@ func slogAttrToZapField(attr slog.Attr) zapcore.Field { return zapcore.Field{Key: key, Type: zapcore.DurationType, Integer: value.Duration().Nanoseconds()} case slog.KindTime: return zapcore.Field{Key: key, Type: zapcore.TimeType, Integer: value.Time().UnixNano(), Interface: value.Time().Location()} + case slog.KindGroup: + g := value.Group() + if len(g) == 0 { + // slog: a Group with no Attrs is ignored. + return zap.Skip() + } + return zap.Object(key, slogGroup(g)) case slog.KindAny: return zapcore.Field{Key: key, Type: zapcore.ReflectType, Interface: value.Any()} default: @@ -243,6 +250,53 @@ func slogAttrToZapField(attr slog.Attr) zapcore.Field { } } +// slogGroup adapts a slog group's attrs as a zapcore.ObjectMarshaler so nested +// fields render as a structured object rather than reflected []slog.Attr. +type slogGroup []slog.Attr + +func (g slogGroup) MarshalLogObject(enc zapcore.ObjectEncoder) error { + for _, attr := range g { + // slog inlines a Group whose key is empty into the enclosing object. + if attr.Key == "" && attr.Value.Kind() == slog.KindGroup { + if err := slogGroup(attr.Value.Group()).MarshalLogObject(enc); err != nil { + return err + } + continue + } + addSlogAttrToObjectEncoder(enc, attr) + } + return nil +} + +func addSlogAttrToObjectEncoder(enc zapcore.ObjectEncoder, attr slog.Attr) { + key := attr.Key + value := attr.Value + switch value.Kind() { + case slog.KindString: + enc.AddString(key, value.String()) + case slog.KindInt64: + enc.AddInt64(key, value.Int64()) + case slog.KindUint64: + enc.AddUint64(key, value.Uint64()) + case slog.KindFloat64: + enc.AddFloat64(key, value.Float64()) + case slog.KindBool: + enc.AddBool(key, value.Bool()) + case slog.KindDuration: + enc.AddDuration(key, value.Duration()) + case slog.KindTime: + enc.AddTime(key, value.Time()) + case slog.KindGroup: + g := value.Group() + if len(g) == 0 { + return + } + _ = enc.AddObject(key, slogGroup(g)) + default: + _ = enc.AddReflected(key, value.Any()) + } +} + func boolToInt64(b bool) int64 { if b { return 1 diff --git a/slog_bridge_test.go b/slog_bridge_test.go index f2922da..4b065dc 100644 --- a/slog_bridge_test.go +++ b/slog_bridge_test.go @@ -3,6 +3,7 @@ package log import ( "bufio" "bytes" + "encoding/json" "log/slog" "os" "strings" @@ -168,6 +169,92 @@ func TestSlogAttrFieldConversions(t *testing.T) { } } +func TestSlogGroupConversion(t *testing.T) { + var buf bytes.Buffer + ws := zapcore.AddSync(&buf) + testCore := newCore(JSONOutput, ws, LevelDebug) + logger := slog.New(newZapToSlogBridge(testCore)) + + decode := func(t *testing.T) map[string]any { + t.Helper() + var m map[string]any + if err := json.Unmarshal(buf.Bytes(), &m); err != nil { + t.Fatalf("invalid JSON output: %v\n%s", err, buf.String()) + } + return m + } + + t.Run("nested groups with mixed types", func(t *testing.T) { + buf.Reset() + logger.Info("msg", slog.Group("outer", + slog.String("s", "hello"), + slog.Int("n", 42), + slog.Bool("b", true), + slog.Group("inner", slog.String("k", "v")), + )) + got := decode(t) + outer, ok := got["outer"].(map[string]any) + if !ok { + t.Fatalf("expected outer to be an object, got %T: %v", got["outer"], got["outer"]) + } + if outer["s"] != "hello" || outer["n"] != float64(42) || outer["b"] != true { + t.Errorf("unexpected scalar contents: %v", outer) + } + inner, ok := outer["inner"].(map[string]any) + if !ok { + t.Fatalf("expected inner to be an object, got %T", outer["inner"]) + } + if inner["k"] != "v" { + t.Errorf("unexpected inner contents: %v", inner) + } + }) + + t.Run("fxevent-style synthetic-array group", func(t *testing.T) { + // fxevent.slogStrings packs []string as a Group with numeric-string keys. + // Without group support this rendered as [{"Key":"0","Value":{}},...]. + buf.Reset() + logger.Info("msg", slog.Group("stacktrace", + slog.String("0", "frame0"), + slog.String("1", "frame1"), + )) + got := decode(t) + st, ok := got["stacktrace"].(map[string]any) + if !ok { + t.Fatalf("expected stacktrace to be an object, got %T: %v", got["stacktrace"], got["stacktrace"]) + } + if st["0"] != "frame0" || st["1"] != "frame1" { + t.Errorf("unexpected stacktrace contents: %v", st) + } + }) + + t.Run("empty-key group is inlined", func(t *testing.T) { + buf.Reset() + logger.Info("msg", slog.Group("wrap", + slog.Group("", slog.String("inlined", "yes")), + )) + got := decode(t) + wrap, ok := got["wrap"].(map[string]any) + if !ok { + t.Fatalf("expected wrap to be an object, got %T", got["wrap"]) + } + if wrap["inlined"] != "yes" { + t.Errorf("expected inlined attr at parent level, got: %v", wrap) + } + }) + + t.Run("empty group is skipped", func(t *testing.T) { + buf.Reset() + logger.Info("msg", slog.Group("ignored"), slog.String("kept", "v")) + got := decode(t) + if _, present := got["ignored"]; present { + t.Errorf("empty group should be omitted, got: %v", got) + } + if got["kept"] != "v" { + t.Errorf("expected sibling attr to survive, got: %v", got) + } + }) +} + func TestSubsystemAwareLevelControl(t *testing.T) { // Save and restore global state originalDefault := slog.Default() From 3ea0df43ce66cad034689c73f3988391b81b463f Mon Sep 17 00:00:00 2001 From: Marcin Rataj Date: Wed, 6 May 2026 12:30:59 +0200 Subject: [PATCH 2/4] fix: inline top-level empty-key slog.Group slog inlines a Group with an empty key into its parent, but the record-level path emitted an empty-key field instead. Mirror the nested encoder via zap.Inline and broaden test coverage. - slog_bridge.go: inline path for empty-key group at record level - slog_bridge.go: cross-ref comment between paired switches - slog_bridge_test.go: cover time/duration/float/uint inside group - slog_bridge_test.go: cover top-level empty-key inlining --- slog_bridge.go | 6 ++++++ slog_bridge_test.go | 46 +++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 52 insertions(+) diff --git a/slog_bridge.go b/slog_bridge.go index bd16090..f5b8d49 100644 --- a/slog_bridge.go +++ b/slog_bridge.go @@ -241,6 +241,10 @@ func slogAttrToZapField(attr slog.Attr) zapcore.Field { // slog: a Group with no Attrs is ignored. return zap.Skip() } + if key == "" { + // slog: a Group with an empty key is inlined into its parent. + return zap.Inline(slogGroup(g)) + } return zap.Object(key, slogGroup(g)) case slog.KindAny: return zapcore.Field{Key: key, Type: zapcore.ReflectType, Interface: value.Any()} @@ -268,6 +272,8 @@ func (g slogGroup) MarshalLogObject(enc zapcore.ObjectEncoder) error { return nil } +// addSlogAttrToObjectEncoder mirrors slogAttrToZapField for nested object +// encoding; keep the two switches in sync when adding new slog.Kind cases. func addSlogAttrToObjectEncoder(enc zapcore.ObjectEncoder, attr slog.Attr) { key := attr.Key value := attr.Value diff --git a/slog_bridge_test.go b/slog_bridge_test.go index 4b065dc..2ba1f58 100644 --- a/slog_bridge_test.go +++ b/slog_bridge_test.go @@ -253,6 +253,52 @@ func TestSlogGroupConversion(t *testing.T) { t.Errorf("expected sibling attr to survive, got: %v", got) } }) + + t.Run("group with time, duration, float, uint", func(t *testing.T) { + buf.Reset() + when := time.Date(2025, 1, 2, 3, 4, 5, 0, time.UTC) + logger.Info("msg", slog.Group("g", + slog.Time("t", when), + slog.Duration("d", 1500*time.Millisecond), + slog.Float64("f", 2.5), + slog.Uint64("u", uint64(1)<<33), + )) + got := decode(t) + g, ok := got["g"].(map[string]any) + if !ok { + t.Fatalf("expected g to be an object, got %T: %v", got["g"], got["g"]) + } + if g["t"] != "2025-01-02T03:04:05.000Z" { + t.Errorf("unexpected time: %v", g["t"]) + } + if g["d"] != 1.5 { + t.Errorf("unexpected duration: %v", g["d"]) + } + if g["f"] != 2.5 { + t.Errorf("unexpected float: %v", g["f"]) + } + if g["u"] != float64(uint64(1)<<33) { + t.Errorf("unexpected uint: %v", g["u"]) + } + }) + + t.Run("top-level empty-key group inlines into record", func(t *testing.T) { + buf.Reset() + logger.Info("msg", slog.Group("", + slog.String("inlined", "yes"), + slog.Int("n", 7), + )) + got := decode(t) + if got["inlined"] != "yes" { + t.Errorf("expected inlined string at record level: %v", got) + } + if got["n"] != float64(7) { + t.Errorf("expected inlined int at record level: %v", got) + } + if _, present := got[""]; present { + t.Errorf("empty-key group should not produce an empty-key field, got: %v", got) + } + }) } func TestSubsystemAwareLevelControl(t *testing.T) { From de806a7ea24bb67d50efd3072842364472736628 Mon Sep 17 00:00:00 2001 From: Marcin Rataj Date: Wed, 6 May 2026 16:30:43 +0200 Subject: [PATCH 3/4] fix: resolve slog LogValuer in zap bridge slog handlers must call Value.Resolve() at the leaf to honor the LogValuer contract. Without it, an attr whose Value is a LogValuer arrives with Kind() == KindLogValuer and falls through every switch case to ReflectType, handing zap the raw wrapper struct instead of the value LogValue() would produce. Concretely, for a User type whose LogValue() returns a Group of {id, email}, the bridge would emit the reflected wrapper rather than {"user":{"id":"u123","email":"***@example.com"}}. The output is usually unhelpful and sometimes empty, depending on which fields the wrapper happens to export. It also hides the new Group path, since a LogValuer returning a slog.Group never reaches the KindGroup case without resolution; calling value.Group() on an unresolved LogValuer would also panic for the same reason. Resolve() is idempotent and effectively free for non-LogValuer values (a single Kind check), and is called on paths gated by slog's Enabled() check, so filtered logs do not pay the cost. This matches log/slog's own commonHandler resolution points. - slog_bridge.go: resolve in both attr conversion paths - slog_bridge.go: resolve in slogGroup.MarshalLogObject so the empty-key inline check sees LogValuer-of-Group - slog_bridge_test.go: cover LogValuer returning a Group --- slog_bridge.go | 13 ++++++++----- slog_bridge_test.go | 25 +++++++++++++++++++++++++ 2 files changed, 33 insertions(+), 5 deletions(-) diff --git a/slog_bridge.go b/slog_bridge.go index f5b8d49..a66f88e 100644 --- a/slog_bridge.go +++ b/slog_bridge.go @@ -218,7 +218,8 @@ func slogLevelToZap(level slog.Level) zapcore.Level { // slogAttrToZapField converts slog.Attr to zapcore.Field. func slogAttrToZapField(attr slog.Attr) zapcore.Field { key := attr.Key - value := attr.Value + // slog: handlers must resolve LogValuer at the leaf. + value := attr.Value.Resolve() switch value.Kind() { case slog.KindString: @@ -260,14 +261,15 @@ type slogGroup []slog.Attr func (g slogGroup) MarshalLogObject(enc zapcore.ObjectEncoder) error { for _, attr := range g { + v := attr.Value.Resolve() // slog inlines a Group whose key is empty into the enclosing object. - if attr.Key == "" && attr.Value.Kind() == slog.KindGroup { - if err := slogGroup(attr.Value.Group()).MarshalLogObject(enc); err != nil { + if attr.Key == "" && v.Kind() == slog.KindGroup { + if err := slogGroup(v.Group()).MarshalLogObject(enc); err != nil { return err } continue } - addSlogAttrToObjectEncoder(enc, attr) + addSlogAttrToObjectEncoder(enc, slog.Attr{Key: attr.Key, Value: v}) } return nil } @@ -276,7 +278,8 @@ func (g slogGroup) MarshalLogObject(enc zapcore.ObjectEncoder) error { // encoding; keep the two switches in sync when adding new slog.Kind cases. func addSlogAttrToObjectEncoder(enc zapcore.ObjectEncoder, attr slog.Attr) { key := attr.Key - value := attr.Value + // slog: handlers must resolve LogValuer at the leaf. + value := attr.Value.Resolve() switch value.Kind() { case slog.KindString: enc.AddString(key, value.String()) diff --git a/slog_bridge_test.go b/slog_bridge_test.go index 2ba1f58..a6063dc 100644 --- a/slog_bridge_test.go +++ b/slog_bridge_test.go @@ -19,6 +19,11 @@ type goLogBridge interface { GoLogBridge() } +// logValuerFunc adapts a function to slog.LogValuer for tests. +type logValuerFunc func() slog.Value + +func (f logValuerFunc) LogValue() slog.Value { return f() } + func TestSlogInterop(t *testing.T) { // Save initial state originalDefault := slog.Default() @@ -282,6 +287,26 @@ func TestSlogGroupConversion(t *testing.T) { } }) + t.Run("LogValuer returning group is resolved and rendered", func(t *testing.T) { + buf.Reset() + logger.Info("msg", + slog.Any("user", logValuerFunc(func() slog.Value { + return slog.GroupValue( + slog.String("name", "alice"), + slog.Int("age", 30), + ) + })), + ) + got := decode(t) + user, ok := got["user"].(map[string]any) + if !ok { + t.Fatalf("expected user to be an object, got %T: %v", got["user"], got["user"]) + } + if user["name"] != "alice" || user["age"] != float64(30) { + t.Errorf("unexpected user contents: %v", user) + } + }) + t.Run("top-level empty-key group inlines into record", func(t *testing.T) { buf.Reset() logger.Info("msg", slog.Group("", From 80a03d3e9f8cfc731080907e8692ebf6905086d7 Mon Sep 17 00:00:00 2001 From: Marcin Rataj Date: Wed, 6 May 2026 17:55:17 +0200 Subject: [PATCH 4/4] chore: flag WithGroup as a known no-op --- slog_bridge.go | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/slog_bridge.go b/slog_bridge.go index a66f88e..7933b16 100644 --- a/slog_bridge.go +++ b/slog_bridge.go @@ -76,6 +76,7 @@ func (h *subsystemAwareHandler) WithAttrs(attrs []slog.Attr) slog.Handler { } } +// TODO: same no-op as zapToSlogBridge.WithGroup; see TODO there. func (h *subsystemAwareHandler) WithGroup(name string) slog.Handler { return &subsystemAwareHandler{ bridge: &zapToSlogBridge{ @@ -191,9 +192,13 @@ func (h *zapToSlogBridge) WithAttrs(attrs []slog.Attr) slog.Handler { } // WithGroup implements slog.Handler. +// +// TODO: Handler.WithGroup is a no-op. Inline slog.Group(...) attrs render as +// nested objects via slogGroup, but attrs added after a Handler.WithGroup +// call are not nested under the group name. Full support needs deferred +// attr conversion plus a group-frame stack walked at Handle time, with the +// subsystem-key filter applied only at depth 0. func (h *zapToSlogBridge) WithGroup(name string) slog.Handler { - // Groups are currently not implemented - just return a handler preserving the subsystem. - // A more sophisticated implementation would nest fields under the group name. return &zapToSlogBridge{ core: h.core, subsystemName: h.subsystemName, // Preserve subsystem