From 590c693b96756ed1636c2c919bfe2ff23507d813 Mon Sep 17 00:00:00 2001 From: Brad Fitzpatrick Date: Mon, 28 Aug 2023 15:01:38 -0700 Subject: [PATCH] types/logger: add AsJSON Printing out JSON representation things in log output is pretty common. Updates #cleanup Change-Id: Ife2d2e321a18e6e1185efa8b699a23061ac5e5a4 Signed-off-by: Brad Fitzpatrick --- cmd/tailscale/cli/cli_test.go | 8 ++------ cmd/tailscale/cli/serve_test.go | 5 +++-- control/controlclient/map_test.go | 14 ++++++-------- ipn/ipnlocal/profiles_test.go | 5 ----- types/logger/logger.go | 20 ++++++++++++++++++++ types/logger/logger_test.go | 30 ++++++++++++++++++++++++++++++ 6 files changed, 61 insertions(+), 21 deletions(-) diff --git a/cmd/tailscale/cli/cli_test.go b/cmd/tailscale/cli/cli_test.go index 30165c295..12a1b0638 100644 --- a/cmd/tailscale/cli/cli_test.go +++ b/cmd/tailscale/cli/cli_test.go @@ -21,6 +21,7 @@ import ( "tailscale.com/tailcfg" "tailscale.com/tka" "tailscale.com/tstest" + "tailscale.com/types/logger" "tailscale.com/types/persist" "tailscale.com/types/preftype" "tailscale.com/util/cmpx" @@ -1151,18 +1152,13 @@ func TestUpdatePrefs(t *testing.T) { justEditMP.Prefs = ipn.Prefs{} // uninteresting } if !reflect.DeepEqual(justEditMP, tt.wantJustEditMP) { - t.Logf("justEditMP != wantJustEditMP; following diff omits the Prefs field, which was \n%v", asJSON(oldEditPrefs)) + t.Logf("justEditMP != wantJustEditMP; following diff omits the Prefs field, which was \n%v", logger.AsJSON(oldEditPrefs)) t.Fatalf("justEditMP: %v\n\n: ", cmp.Diff(justEditMP, tt.wantJustEditMP, cmpIP)) } }) } } -func asJSON(v any) string { - b, _ := json.MarshalIndent(v, "", "\t") - return string(b) -} - var cmpIP = cmp.Comparer(func(a, b netip.Addr) bool { return a == b }) diff --git a/cmd/tailscale/cli/serve_test.go b/cmd/tailscale/cli/serve_test.go index 642cce5ce..18d223930 100644 --- a/cmd/tailscale/cli/serve_test.go +++ b/cmd/tailscale/cli/serve_test.go @@ -22,6 +22,7 @@ import ( "tailscale.com/ipn" "tailscale.com/ipn/ipnstate" "tailscale.com/tailcfg" + "tailscale.com/types/logger" ) func TestCleanMountPoint(t *testing.T) { @@ -737,8 +738,8 @@ func TestServeConfigMutations(t *testing.T) { got = lc.config } if !reflect.DeepEqual(got, st.want) { - t.Fatalf("[%d] %v: bad state. got:\n%s\n\nwant:\n%s\n", - i, st.command, asJSON(got), asJSON(st.want)) + t.Fatalf("[%d] %v: bad state. got:\n%v\n\nwant:\n%v\n", + i, st.command, logger.AsJSON(got), logger.AsJSON(st.want)) // NOTE: asJSON will omit empty fields, which might make // result in bad state got/want diffs being the same, even // though the actual state is different. Use below to debug: diff --git a/control/controlclient/map_test.go b/control/controlclient/map_test.go index 1b54aecb7..1cfd511ff 100644 --- a/control/controlclient/map_test.go +++ b/control/controlclient/map_test.go @@ -4,7 +4,6 @@ package controlclient import ( - "bytes" "context" "encoding/json" "fmt" @@ -21,6 +20,7 @@ import ( "tailscale.com/tstest" "tailscale.com/tstime" "tailscale.com/types/key" + "tailscale.com/types/logger" "tailscale.com/types/netmap" "tailscale.com/types/ptr" "tailscale.com/util/mak" @@ -637,7 +637,7 @@ func TestDeltaDERPMap(t *testing.T) { for stepi, s := range tt.steps { nm := ms.netmapForResponse(&tailcfg.MapResponse{DERPMap: s.got}) if !reflect.DeepEqual(nm.DERPMap, s.want) { - t.Errorf("unexpected result at step index %v; got: %s", stepi, must.Get(json.Marshal(nm.DERPMap))) + t.Errorf("unexpected result at step index %v; got: %s", stepi, logger.AsJSON(nm.DERPMap)) } } }) @@ -740,17 +740,15 @@ func TestPeerChangeDiff(t *testing.T) { pc, ok := peerChangeDiff(tt.a.View(), tt.b) if tt.wantEqual { if !ok || pc != nil { - t.Errorf("got (%p, %v); want (nil, true); pc=%v", pc, ok, must.Get(json.Marshal(pc))) + t.Errorf("got (%p, %v); want (nil, true); pc=%v", pc, ok, logger.AsJSON(pc)) } return } if (pc != nil) != ok { t.Fatalf("inconsistent ok=%v, pc=%p", ok, pc) } - gotj := must.Get(json.Marshal(pc)) - wantj := must.Get(json.Marshal(tt.want)) - if !bytes.Equal(gotj, wantj) { - t.Errorf("mismatch\n got: %s\nwant: %s\n", gotj, wantj) + if !reflect.DeepEqual(pc, tt.want) { + t.Errorf("mismatch\n got: %v\nwant: %v\n", logger.AsJSON(pc), logger.AsJSON(tt.want)) } }) } @@ -762,7 +760,7 @@ func TestPeerChangeDiffAllocs(t *testing.T) { n := testing.AllocsPerRun(10000, func() { diff, ok := peerChangeDiff(a.View(), b) if !ok || diff != nil { - t.Fatalf("unexpected result: (%s, %v)", must.Get(json.Marshal(diff)), ok) + t.Fatalf("unexpected result: (%s, %v)", logger.AsJSON(diff), ok) } }) if n != 0 { diff --git a/ipn/ipnlocal/profiles_test.go b/ipn/ipnlocal/profiles_test.go index 8d881376e..a75571fd4 100644 --- a/ipn/ipnlocal/profiles_test.go +++ b/ipn/ipnlocal/profiles_test.go @@ -4,7 +4,6 @@ package ipnlocal import ( - "encoding/json" "fmt" "os/user" "strconv" @@ -312,10 +311,6 @@ func TestProfileDupe(t *testing.T) { } } -func asJSON(v any) []byte { - return must.Get(json.MarshalIndent(v, "", " ")) -} - // TestProfileManagement tests creating, loading, and switching profiles. func TestProfileManagement(t *testing.T) { store := new(mem.Store) diff --git a/types/logger/logger.go b/types/logger/logger.go index 1df273b3e..70f956ded 100644 --- a/types/logger/logger.go +++ b/types/logger/logger.go @@ -353,3 +353,23 @@ func LogfCloser(logf Logf) (newLogf Logf, close func()) { } return newLogf, close } + +// AsJSON returns a formatter that formats v as JSON. The value is suitable to +// passing to a regular %v printf argument. (%s is not required) +// +// If json.Marshal returns an error, the output is "%%!JSON-ERROR:" followed by +// the error string. +func AsJSON(v any) fmt.Formatter { + return asJSONResult{v} +} + +type asJSONResult struct{ v any } + +func (a asJSONResult) Format(s fmt.State, verb rune) { + v, err := json.Marshal(a.v) + if err != nil { + fmt.Fprintf(s, "%%!JSON-ERROR:%v", err) + return + } + s.Write(v) +} diff --git a/types/logger/logger_test.go b/types/logger/logger_test.go index e3f56dce6..510207027 100644 --- a/types/logger/logger_test.go +++ b/types/logger/logger_test.go @@ -221,3 +221,33 @@ func TestJSON(t *testing.T) { t.Errorf("mismatch\n got: %q\nwant: %q\n", got, want) } } + +func TestAsJSON(t *testing.T) { + got := fmt.Sprintf("got %v", AsJSON(struct { + Foo string + Bar int + }{"hi", 123})) + const want = `got {"Foo":"hi","Bar":123}` + if got != want { + t.Errorf("got %#q; want %#q", got, want) + } + + got = fmt.Sprintf("got %v", AsJSON(func() {})) + const wantErr = `got %!JSON-ERROR:json: unsupported type: func()` + if got != wantErr { + t.Errorf("for marshal error, got %#q; want %#q", got, wantErr) + } + + var buf bytes.Buffer + n := int(testing.AllocsPerRun(1000, func() { + buf.Reset() + fmt.Fprintf(&buf, "got %v", AsJSON("hi")) + })) + if n > 2 { + // the JSON AsMarshal itself + boxing + // the asJSONResult into an interface (which needs + // to happen at some point to get to fmt, so might + // as well return an interface from AsJSON)) + t.Errorf("allocs = %v; want max 2", n) + } +}