From 614a24763b014981e9e7f42e8f331246995e5665 Mon Sep 17 00:00:00 2001
From: Brad Fitzpatrick <bradfitz@tailscale.com>
Date: Mon, 10 Oct 2022 19:52:07 -0700
Subject: [PATCH] tsweb: sort top-level expvars after removing type prefixes

Fixes #5778

Change-Id: I56c367338fa5686da288cc6545209ef4d6b88549
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
---
 tsweb/tsweb.go      | 24 +++++++++++++++++++++++-
 tsweb/tsweb_test.go | 28 ++++++++++++++++++++++++++++
 2 files changed, 51 insertions(+), 1 deletion(-)

diff --git a/tsweb/tsweb.go b/tsweb/tsweb.go
index fcd907033..8ba1a52a7 100644
--- a/tsweb/tsweb.go
+++ b/tsweb/tsweb.go
@@ -591,6 +591,18 @@ func writePromExpVar(w io.Writer, prefix string, kv expvar.KeyValue) {
 	}
 }
 
+var sortedKVsPool = &sync.Pool{New: func() any { return new(sortedKVs) }}
+
+// sortedKV is a KeyValue with a sort key.
+type sortedKV struct {
+	expvar.KeyValue
+	sortKey string // KeyValue.Key with type prefix removed
+}
+
+type sortedKVs struct {
+	kvs []sortedKV
+}
+
 // VarzHandler is an HTTP handler to write expvar values into the
 // prometheus export format:
 //
@@ -610,9 +622,19 @@ func writePromExpVar(w io.Writer, prefix string, kv expvar.KeyValue) {
 // This will evolve over time, or perhaps be replaced.
 func VarzHandler(w http.ResponseWriter, r *http.Request) {
 	w.Header().Set("Content-Type", "text/plain; version=0.0.4")
+
+	s := sortedKVsPool.Get().(*sortedKVs)
+	defer sortedKVsPool.Put(s)
+	s.kvs = s.kvs[:0]
 	expvarDo(func(kv expvar.KeyValue) {
-		writePromExpVar(w, "", kv)
+		s.kvs = append(s.kvs, sortedKV{kv, removeTypePrefixes(kv.Key)})
 	})
+	sort.Slice(s.kvs, func(i, j int) bool {
+		return s.kvs[i].sortKey < s.kvs[j].sortKey
+	})
+	for _, e := range s.kvs {
+		writePromExpVar(w, "", e.KeyValue)
+	}
 }
 
 // PrometheusMetricsReflectRooter is an optional interface that expvar.Var implementations
diff --git a/tsweb/tsweb_test.go b/tsweb/tsweb_test.go
index 42bef16d3..a38b671c9 100644
--- a/tsweb/tsweb_test.go
+++ b/tsweb/tsweb_test.go
@@ -22,6 +22,7 @@ import (
 	"github.com/google/go-cmp/cmp"
 	"tailscale.com/metrics"
 	"tailscale.com/tstest"
+	"tailscale.com/version"
 )
 
 type noopHijacker struct {
@@ -743,3 +744,30 @@ func TestSortedStructAllocs(t *testing.T) {
 		t.Errorf("allocs = %v; want 0", n)
 	}
 }
+
+func TestVarzHandlerSorting(t *testing.T) {
+	defer func() { expvarDo = expvar.Do }()
+	expvarDo = func(f func(expvar.KeyValue)) {
+		f(expvar.KeyValue{Key: "counter_zz", Value: new(expvar.Int)})
+		f(expvar.KeyValue{Key: "gauge_aa", Value: new(expvar.Int)})
+	}
+	rec := httptest.NewRecorder()
+	req := httptest.NewRequest("GET", "/", nil)
+	VarzHandler(rec, req)
+	got := rec.Body.Bytes()
+	const want = "# TYPE aa gauge\naa 0\n# TYPE zz counter\nzz 0\n"
+	if string(got) != want {
+		t.Errorf("got %q; want %q", got, want)
+	}
+	rec = new(httptest.ResponseRecorder) // without a body
+
+	// Lock in the current number of allocs, to prevent it from growing.
+	if !version.IsRace() {
+		allocs := int(testing.AllocsPerRun(1000, func() {
+			VarzHandler(rec, req)
+		}))
+		if max := 13; allocs > max {
+			t.Errorf("allocs = %v; want max %v", allocs, max)
+		}
+	}
+}