Skip to content

Commit

Permalink
metrics: revert changes to MultiLabelMap's String method
Browse files Browse the repository at this point in the history
This breaks its ability to be used as an expvar and is blocking a trunkd
deploy. Revert for now, and add a test to ensure that we don't break it
in a future change.

Updates tailscale#13550

Signed-off-by: Andrew Dunham <andrew@du.nham.ca>
Change-Id: I1f1221c257c1de47b4bff0597c12f8530736116d
  • Loading branch information
andrew-d committed Sep 25, 2024
1 parent 65c2635 commit 717d589
Show file tree
Hide file tree
Showing 2 changed files with 21 additions and 6 deletions.
8 changes: 2 additions & 6 deletions metrics/multilabelmap.go
Original file line number Diff line number Diff line change
Expand Up @@ -97,12 +97,8 @@ type KeyValue[T comparable] struct {
}

func (v *MultiLabelMap[T]) String() string {
var sb strings.Builder
sb.WriteString("MultiLabelMap:\n")
v.Do(func(kv KeyValue[T]) {
fmt.Fprintf(&sb, "\t%v: %v\n", kv.Key, kv.Value)
})
return sb.String()
// NOTE: This has to be valid JSON because it's used by expvar.
return `"MultiLabelMap"`
}

// WritePrometheus writes v to w in Prometheus exposition format.
Expand Down
19 changes: 19 additions & 0 deletions metrics/multilabelmap_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ package metrics

import (
"bytes"
"encoding/json"
"expvar"
"fmt"
"io"
Expand Down Expand Up @@ -129,3 +130,21 @@ func BenchmarkMultiLabelWriteAllocs(b *testing.B) {
m.WritePrometheus(w, "test")
}
}

func TestMultiLabelMapExpvar(t *testing.T) {
m := new(MultiLabelMap[L2])
m.Add(L2{"a", "b"}, 2)
m.Add(L2{"b", "c"}, 4)

em := new(expvar.Map)
em.Set("multi", m)

// Ensure that the String method is valid JSON to ensure that it can be
// used by expvar.
encoded := []byte(em.String())
if !json.Valid(encoded) {
t.Fatalf("invalid JSON: %s", encoded)
}

t.Logf("em = %+v", em)
}

0 comments on commit 717d589

Please sign in to comment.