Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

zapslog: fix all with slogtest, support inline group, ignore empty group. #1408

Merged
merged 9 commits into from
Feb 5, 2024
31 changes: 26 additions & 5 deletions exp/zapslog/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@
addCaller bool
addStackAt slog.Level
callerSkip int
holdGroup string // holds latest group.
}

// NewHandler builds a [Handler] that writes to the supplied [zapcore.Core]
Expand Down Expand Up @@ -88,6 +89,10 @@
case slog.KindUint64:
return zap.Uint64(attr.Key, attr.Value.Uint64())
case slog.KindGroup:
if attr.Key == "" {
// Inlines recursively.
return zap.Inline(groupObject(attr.Value.Group()))
}
return zap.Object(attr.Key, groupObject(attr.Value.Group()))
case slog.KindLogValuer:
return convertAttrToField(slog.Attr{
Expand Down Expand Up @@ -157,29 +162,45 @@
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 = append(fields, zap.Namespace(h.holdGroup))
h.holdGroup = ""
}
fields = append(fields, f)
abhinav marked this conversation as resolved.
Show resolved Hide resolved
return true
})

ce.Write(fields...)
return nil
}

// 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, 0, len(attrs))
fields := make([]zapcore.Field, 0, len(attrs)+1)
for _, attr := range attrs {
fields = append(fields, convertAttrToField(attr))
f := convertAttrToField(attr)
if h.holdGroup != "" && f != zap.Skip() {
fields = append(fields, zap.Namespace(h.holdGroup))
h.holdGroup = ""
abhinav marked this conversation as resolved.
Show resolved Hide resolved
}
fields = append(fields, f)
}
return h.withFields(fields...)
}

// 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)})
}

Check warning on line 201 in exp/zapslog/handler.go

View check run for this annotation

Codecov / codecov/patch

exp/zapslog/handler.go#L200-L201

Added lines #L200 - L201 were not covered by tests
cloned.holdGroup = group
return &cloned
}

// withFields returns a cloned Handler with the given fields.
Expand Down
105 changes: 105 additions & 0 deletions exp/zapslog/handler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,13 +23,17 @@
package zapslog

import (
"bytes"
"encoding/json"
"log/slog"
"testing"
"testing/slogtest"
"time"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"go.uber.org/zap/zapcore"
"go.uber.org/zap/zaptest"
"go.uber.org/zap/zaptest/observer"
)

Expand Down Expand Up @@ -150,6 +154,69 @@ func TestWithName(t *testing.T) {
})
}

func TestInlineGroup(t *testing.T) {
t.Parallel()
fac, observedLogs := observer.New(zapcore.DebugLevel)
t.Run("inline-group", func(t *testing.T) {
sl := slog.New(NewHandler(fac))
sl.Info("msg", "a", "b", slog.Group("", slog.String("c", "d")), "e", "f")

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{
"a": "b",
"c": "d",
"e": "f",
}, logs[0].ContextMap(), "Unexpected context")
})
t.Run("inline-group-recursive", func(t *testing.T) {
sl := slog.New(NewHandler(fac))
sl.Info("msg", "a", "b", slog.Group("", slog.Group("", slog.Group("", slog.String("c", "d"))), slog.Group("", "e", "f")))

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{
"a": "b",
"c": "d",
"e": "f",
}, logs[0].ContextMap(), "Unexpected context")
})
}

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 {
Expand Down Expand Up @@ -189,3 +256,41 @@ func TestAttrKinds(t *testing.T) {
},
entry.ContextMap())
}

func TestSlogtest(t *testing.T) {
var buff bytes.Buffer
core := zapcore.NewCore(
zapcore.NewJSONEncoder(zapcore.EncoderConfig{
TimeKey: slog.TimeKey,
MessageKey: slog.MessageKey,
LevelKey: slog.LevelKey,
EncodeLevel: zapcore.CapitalLevelEncoder,
EncodeTime: zapcore.RFC3339TimeEncoder,
}),
zapcore.AddSync(&buff),
zapcore.DebugLevel,
)

// zaptest doesn't expose the underlying core,
// so we'll extract it from the logger.
testCore := zaptest.NewLogger(t).Core()

handler := NewHandler(zapcore.NewTee(core, testCore))
err := slogtest.TestHandler(
handler,
func() []map[string]any {
// Parse the newline-delimted JSON in buff.
var entries []map[string]any

dec := json.NewDecoder(bytes.NewReader(buff.Bytes()))
for dec.More() {
var ent map[string]any
require.NoError(t, dec.Decode(&ent), "Error decoding log message")
entries = append(entries, ent)
}

return entries
},
)
require.NoError(t, err, "Unexpected error from slogtest.TestHandler")
}
Loading