ddtrace/tracer/slog: fix slog handler (#3131)

ddtrace/tracer/slog.go

@@ -13,11 +13,16 @@ import (
 	"gopkg.in/DataDog/dd-trace-go.v1/internal/log"
 )
 
+// groupOrAttrs holds either a group name or a list of slog.Attrs.
+type groupOrAttrs struct {
+	group string      // group name if non-empty
+	attrs []slog.Attr // attrs if non-empty
+}
+
 // slogHandler implements the slog.Handler interface to dispatch messages to our
 // internal logger.
 type slogHandler struct {
-	attrs  []string
-	groups []string
+	goas []groupOrAttrs
 }
 
 func (h slogHandler) Enabled(ctx context.Context, lvl slog.Level) bool {
@@ -30,10 +35,30 @@ func (h slogHandler) Enabled(ctx context.Context, lvl slog.Level) bool {
 }
 
 func (h slogHandler) Handle(ctx context.Context, r slog.Record) error {
-	parts := make([]string, 0, len(h.attrs)+r.NumAttrs())
-	parts = append(parts, h.attrs...)
+	goas := h.goas
+
+	if r.NumAttrs() == 0 {
+		// If the record has no Attrs, remove groups at the end of the list; they are empty.
+		for len(goas) > 0 && goas[len(goas)-1].group != "" {
+			goas = goas[:len(goas)-1]
+		}
+	}
+
+	parts := make([]string, 0, len(goas)+r.NumAttrs())
+	formatGroup := ""
+
+	for _, goa := range goas {
+		if goa.group != "" {
+			formatGroup += goa.group + "."
+		} else {
+			for _, a := range goa.attrs {
+				parts = append(parts, formatGroup+a.String())
+			}
+		}
+	}
+
 	r.Attrs(func(a slog.Attr) bool {
-		parts = append(parts, formatAttr(a, h.groups))
+		parts = append(parts, formatGroup+a.String())
 		return true
 	})
 
@@ -51,18 +76,25 @@ func (h slogHandler) Handle(ctx context.Context, r slog.Record) error {
 	return nil
 }
 
-func (h slogHandler) WithAttrs(attrs []slog.Attr) slog.Handler {
-	for _, a := range attrs {
-		h.attrs = append(h.attrs, formatAttr(a, h.groups))
-	}
+func (h slogHandler) withGroupOrAttrs(goa groupOrAttrs) slogHandler {
+	h.goas = append(h.goas, goa)
 	return h
 }
 
+// WithGroup returns a new Handler whose group consist of
+// both the receiver's groups and the arguments.
 func (h slogHandler) WithGroup(name string) slog.Handler {
-	h.groups = append(h.groups, name)
-	return h
+	if name == "" {
+		return h
+	}
+	return h.withGroupOrAttrs(groupOrAttrs{group: name})
 }
 
-func formatAttr(a slog.Attr, groups []string) string {
-	return strings.Join(append(groups, a.String()), ".")
+// WithAttrs returns a new Handler whose attributes consist of
+// both the receiver's attributes and the arguments.
+func (h slogHandler) WithAttrs(attrs []slog.Attr) slog.Handler {
+	if len(attrs) == 0 {
+		return h
+	}
+	return h.withGroupOrAttrs(groupOrAttrs{attrs: attrs})
 }

ddtrace/tracer/slog_test.go

@@ -37,9 +37,17 @@ func Test_slogHandler(t *testing.T) {
 	l.Error("error test", "n", 3)
 	log.Flush() // needed to get the error log flushed
 
+	// Check that chaining works as expected.
+	l = l.With("baz", "qux")
+	l = l.WithGroup("c").WithGroup("d")
+	l.Info("info test", "n", 1)
+
+	log.Flush()
+
 	// Check that the logs were written correctly.
-	require.Len(t, rl.Logs(), 3)
+	require.Len(t, rl.Logs(), 4)
 	require.Contains(t, rl.Logs()[0], "info test foo=bar a.b.n=1")
 	require.Contains(t, rl.Logs()[1], "warn test foo=bar a.b.n=2")
 	require.Contains(t, rl.Logs()[2], "error test foo=bar a.b.n=3")
+	require.Contains(t, rl.Logs()[3], "info test foo=bar a.b.baz=qux a.b.c.d.n=1")
 }