Skip to content

Commit

Permalink
zapslog: Handler should ignore an empty Attr
Browse files Browse the repository at this point in the history
Per the slog.Handler contract,
handlers should not log attributes that are equal to the zero value.
This is equivalent to Zap's `zap.Skip()`.

Discovered by uber-go#1335
Refs uber-go#1334
  • Loading branch information
abhinav committed Sep 1, 2023
1 parent 98e9c4f commit 910c4cd
Show file tree
Hide file tree
Showing 2 changed files with 43 additions and 3 deletions.
13 changes: 10 additions & 3 deletions exp/zapslog/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,10 @@ func (h *Handler) Handle(ctx context.Context, record slog.Record) error {

fields := make([]zapcore.Field, 0, record.NumAttrs())
record.Attrs(func(attr slog.Attr) bool {
if attr.Equal(slog.Attr{}) {
return true // ignore empty attributes
}

fields = append(fields, convertAttrToField(attr))
return true
})
Expand All @@ -169,9 +173,12 @@ func (h *Handler) Handle(ctx context.Context, record slog.Record) error {
// 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)
fields := make([]zapcore.Field, 0, len(attrs))
for _, attr := range attrs {
if attr.Equal(slog.Attr{}) {
continue // ignore empty attributes
}
fields = append(fields, convertAttrToField(attr))
}
return h.withFields(fields...)
}
Expand Down
33 changes: 33 additions & 0 deletions exp/zapslog/handler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,8 @@ import (
)

func TestAddSource(t *testing.T) {
t.Parallel()

fac, logs := observer.New(zapcore.DebugLevel)
sl := slog.New(NewHandler(fac, &HandlerOptions{
AddSource: true,
Expand All @@ -48,3 +50,34 @@ func TestAddSource(t *testing.T) {
"Unexpected caller annotation.",
)
}

func TestEmptyAttr(t *testing.T) {
t.Parallel()

fac, observedLogs := observer.New(zapcore.DebugLevel)
sl := slog.New(NewHandler(fac, nil))

t.Run("Handle", func(t *testing.T) {
sl.Info(
"msg",
slog.String("foo", "bar"),
slog.Attr{},
)

logs := observedLogs.TakeAll()
require.Len(t, logs, 1, "Expected exactly one entry to be logged")
assert.Equal(t, map[string]any{
"foo": "bar",
}, logs[0].ContextMap(), "Unexpected context")
})

t.Run("WithAttrs", func(t *testing.T) {
sl.With(slog.String("foo", "bar"), slog.Attr{}).Info("msg")

logs := observedLogs.TakeAll()
require.Len(t, logs, 1, "Expected exactly one entry to be logged")
assert.Equal(t, map[string]any{
"foo": "bar",
}, logs[0].ContextMap(), "Unexpected context")
})
}

0 comments on commit 910c4cd

Please sign in to comment.