fix: support slog.Group in the zap bridge#183
Conversation
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.
This comment was marked as off-topic.
This comment was marked as off-topic.
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 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
There was a problem hiding this comment.
lgtm, tested with kubo and no regressions (afaik, for what we use go-log for).
note: pushed three follow-ups (see below), feel free to tag a release or lmk if you want me to make one
fix: resolve slog LogValuer in zap bridge
slog.Handler must call Value.Resolve() at the leaf to honor LogValuer. Without it, a LogValuer attr falls through every switch case to ReflectType and zap renders the wrapper struct instead of what LogValue() returns. This also blocks the new Group path: a LogValuer returning a Group never reaches KindGroup, and value.Group() on an unresolved value panics. Matches log/slog's own commonHandler resolution points.
| input | v2.9.1 | this branch |
|---|---|---|
slog.Any("user", LogValuer→Group) |
"user":{} |
"user":{"id":"u123","email":"alice@example.com"} |
fix: inline top-level empty-key slog.Group
slog inlines a Group with an empty key into its parent. The nested encoder did this; the record-level path did not. Fixed via zap.Inline(slogGroup(g)).
| input | v2.9.1 | this branch |
|---|---|---|
slog.Group("", ...) at top level |
"":[{"Key":"inlined","Value":{}},...] |
"inlined":"yes","count":7 |
chore: flag WithGroup as a known no-op
A TODO for humans and LLMs. Inline slog.Group(...) attrs nest correctly via the new path, but attrs added after Handler.WithGroup(name) are not prefixed with the group name. Full support needs deferred attr conversion plus a group-frame stack walked at Handle time. No caller in kubo/boxo needs it today; ship as-is, implement when someone hits it.
2 participants
slog.Groupattrs falls through toReflectTypeinslogAttrToZapFieldand is rendered[]slog.Attrvia reflection as[{"Key":"0","Value":{}},...]. This fix renderGroups via azapcore.ObjectMarshaleradapter.