slog: add Level.MarshalJSON
Add a MarshalJSON method to Level so that it
marshals as a string.
This lets us remove special cases for Level in the handlers.
It also ensures that others who write handlers that output JSON will
get strings instead of numbers; the numbers are obscure,
uninformative, and could be confused with the values of other
level-numbering schemes.
Also, improve handler tests to check replacement.
Change-Id: Iedc7aa797469922fb9cfcc3b5d804806f3deb5d8
Reviewed-on: https://go-review.googlesource.com/c/exp/+/432183
Run-TryBot: Jonathan Amsterdam <jba@google.com>
Reviewed-by: Alan Donovan <adonovan@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
diff --git a/slog/handler.go b/slog/handler.go
index a180e47..0cee928 100644
--- a/slog/handler.go
+++ b/slog/handler.go
@@ -138,7 +138,9 @@
a = rep(a)
if a.Key() != "" {
app.appendKey(a.Key())
- app.appendAttrValue(a)
+ if err := app.appendAttrValue(a); err != nil {
+ app.appendString(fmt.Sprintf("!ERROR:%v", err))
+ }
}
}
@@ -163,17 +165,7 @@
app.appendKey(key)
app.appendString(val.String())
} else {
- // Don't use replace: we want to output Levels as strings
- // to match the above.
- a := rep(Any(key, val))
- if a.Key() != "" {
- app.appendKey(a.Key())
- if l, ok := a.any.(Level); ok {
- app.appendString(l.String())
- } else {
- app.appendAttrValue(a)
- }
- }
+ replace(Any(key, val))
}
app.appendSep()
}
diff --git a/slog/handler_test.go b/slog/handler_test.go
index a3e3ec7..9800ef7 100644
--- a/slog/handler_test.go
+++ b/slog/handler_test.go
@@ -51,11 +51,7 @@
{
name: "cap keys",
h: &commonHandler{
- opts: HandlerOptions{
- ReplaceAttr: func(a Attr) Attr {
- return a.WithKey(strings.ToUpper(a.Key()))
- },
- },
+ opts: HandlerOptions{ReplaceAttr: upperCaseKey},
},
want: "(TIME=2022-09-18T08:26:33.000Z;LEVEL=INFO;MSG=message;A=one;B=2)",
},
@@ -87,6 +83,10 @@
}
}
+func upperCaseKey(a Attr) Attr {
+ return a.WithKey(strings.ToUpper(a.Key()))
+}
+
type testAppender struct {
buf *buffer.Buffer
}
diff --git a/slog/json_handler.go b/slog/json_handler.go
index 4abe4a4..b473011 100644
--- a/slog/json_handler.go
+++ b/slog/json_handler.go
@@ -149,10 +149,7 @@
return err
}
case AnyKind:
- v := a.Value()
- if l, ok := v.(Level); ok {
- ap.appendString(l.String())
- } else if err := ap.appendJSONMarshal(v); err != nil {
+ if err := ap.appendJSONMarshal(a.Value()); err != nil {
return err
}
default:
diff --git a/slog/json_handler_test.go b/slog/json_handler_test.go
index 48e82c0..2d47b66 100644
--- a/slog/json_handler_test.go
+++ b/slog/json_handler_test.go
@@ -12,6 +12,7 @@
"io"
"math"
"os"
+ "strings"
"testing"
"time"
@@ -19,18 +20,35 @@
)
func TestJSONHandler(t *testing.T) {
- var buf bytes.Buffer
- h := NewJSONHandler(&buf)
- r := NewRecord(testTime, InfoLevel, "m", 0)
- r.AddAttrs(Int("a", 1))
- if err := h.Handle(r); err != nil {
- t.Fatal(err)
- }
- got := buf.String()
- got = got[:len(got)-1] // Remove final newline.
- want := `{"time":"2000-01-02T03:04:05Z","level":"INFO","msg":"m","a":1}`
- if got != want {
- t.Errorf("\ngot %s\nwant %s", got, want)
+ for _, test := range []struct {
+ name string
+ opts HandlerOptions
+ want string
+ }{
+ {
+ "none",
+ HandlerOptions{},
+ `{"time":"2000-01-02T03:04:05Z","level":"INFO","msg":"m","a":1,"m":{"b":2}}`,
+ },
+ {
+ "replace",
+ HandlerOptions{ReplaceAttr: upperCaseKey},
+ `{"TIME":"2000-01-02T03:04:05Z","LEVEL":"INFO","MSG":"m","A":1,"M":{"b":2}}`,
+ },
+ } {
+ t.Run(test.name, func(t *testing.T) {
+ var buf bytes.Buffer
+ h := test.opts.NewJSONHandler(&buf)
+ r := NewRecord(testTime, InfoLevel, "m", 0)
+ r.AddAttrs(Int("a", 1), Any("m", map[string]int{"b": 2}))
+ if err := h.Handle(r); err != nil {
+ t.Fatal(err)
+ }
+ got := strings.TrimSuffix(buf.String(), "\n")
+ if got != test.want {
+ t.Errorf("\ngot %s\nwant %s", got, test.want)
+ }
+ })
}
}
diff --git a/slog/level.go b/slog/level.go
index 1664e55..5ac3efa 100644
--- a/slog/level.go
+++ b/slog/level.go
@@ -7,6 +7,7 @@
import (
"fmt"
"math"
+ "strconv"
"sync/atomic"
)
@@ -78,6 +79,13 @@
}
}
+func (l Level) MarshalJSON() ([]byte, error) {
+ // AppendQuote is sufficient for JSON-encoding all Level strings.
+ // They don't contain any runes that would produce invalid JSON
+ // when escaped.
+ return strconv.AppendQuote(nil, l.String()), nil
+}
+
// An AtomicLevel is Level that can be read and written safely by multiple
// goroutines.
// Use NewAtomicLevel to create one.
diff --git a/slog/text_handler_test.go b/slog/text_handler_test.go
index 055652b..f9f4f30 100644
--- a/slog/text_handler_test.go
+++ b/slog/text_handler_test.go
@@ -21,50 +21,72 @@
func TestTextHandler(t *testing.T) {
for _, test := range []struct {
- name string
- attr Attr
- want string
+ name string
+ attr Attr
+ wantKey, wantVal string
}{
{
"unquoted",
Int("a", 1),
- "a=1",
+ "a", "1",
},
{
"quoted",
String("x = y", `qu"o`),
- `"x = y"="qu\"o"`,
+ `"x = y"`, `"qu\"o"`,
},
{
"Sprint",
Any("name", name{"Ren", "Hoek"}),
- `name="Hoek, Ren"`,
+ `name`, `"Hoek, Ren"`,
},
{
"TextMarshaler",
Any("t", text{"abc"}),
- `t="text{\"abc\"}"`,
+ `t`, `"text{\"abc\"}"`,
},
{
"TextMarshaler error",
Any("t", text{""}),
- `t="!ERROR:text: empty string"`,
+ `t`, `"!ERROR:text: empty string"`,
},
} {
t.Run(test.name, func(t *testing.T) {
- var buf bytes.Buffer
- h := NewTextHandler(&buf)
- r := NewRecord(testTime, InfoLevel, "a message", 0)
- r.AddAttrs(test.attr)
- if err := h.Handle(r); err != nil {
- t.Fatal(err)
- }
- got := buf.String()
- // Remove final newline.
- got = got[:len(got)-1]
- want := `time=2000-01-02T03:04:05.000Z level=INFO msg="a message" ` + test.want
- if got != want {
- t.Errorf("\ngot %s\nwant %s", got, want)
+ for _, opts := range []struct {
+ name string
+ opts HandlerOptions
+ wantPrefix string
+ modKey func(string) string
+ }{
+ {
+ "none",
+ HandlerOptions{},
+ `time=2000-01-02T03:04:05.000Z level=INFO msg="a message"`,
+ func(s string) string { return s },
+ },
+ {
+ "replace",
+ HandlerOptions{ReplaceAttr: upperCaseKey},
+ `TIME=2000-01-02T03:04:05.000Z LEVEL=INFO MSG="a message"`,
+ strings.ToUpper,
+ },
+ } {
+ t.Run(opts.name, func(t *testing.T) {
+ var buf bytes.Buffer
+ h := opts.opts.NewTextHandler(&buf)
+ r := NewRecord(testTime, InfoLevel, "a message", 0)
+ r.AddAttrs(test.attr)
+ if err := h.Handle(r); err != nil {
+ t.Fatal(err)
+ }
+ got := buf.String()
+ // Remove final newline.
+ got = got[:len(got)-1]
+ want := opts.wantPrefix + " " + opts.modKey(test.wantKey) + "=" + test.wantVal
+ if got != want {
+ t.Errorf("\ngot %s\nwant %s", got, want)
+ }
+ })
}
})
}
