storepool: implement safe formatting for store pool(list)

Implement `SafeFormat` for `StorePool` and `StoreList` to unredact log
messages.

Release note: None
Part of: cockroachdb#102948

pkg/kv/kvserver/allocator/storepool/BUILD.bazel

@@ -27,6 +27,7 @@ go_library(
         "//pkg/util/syncutil",
         "//pkg/util/timeutil",
         "@com_github_cockroachdb_errors//:errors",
+        "@com_github_cockroachdb_redact//:redact",
     ],
 )
 

pkg/kv/kvserver/allocator/storepool/override_store_pool.go

@@ -18,6 +18,7 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/kv/kvserver/liveness/livenesspb"
 	"github.com/cockroachdb/cockroach/pkg/roachpb"
 	"github.com/cockroachdb/cockroach/pkg/util/hlc"
+	"github.com/cockroachdb/redact"
 )
 
 // OverrideStorePool is an implementation of AllocatorStorePool that allows
@@ -92,7 +93,12 @@ func NewOverrideStorePool(
 }
 
 func (o *OverrideStorePool) String() string {
-	return o.sp.statusString(o.overrideNodeLivenessFn)
+	return redact.StringWithoutMarkers(o)
+}
+
+// SafeFormat implements the redact.SafeFormatter interface.
+func (o *OverrideStorePool) SafeFormat(w redact.SafePrinter, _ rune) {
+	w.Print(o.sp.statusString(o.overrideNodeLivenessFn))
 }
 
 // IsStoreReadyForRoutineReplicaTransfer implements the AllocatorStorePool interface.

pkg/kv/kvserver/allocator/storepool/store_pool.go

@@ -11,7 +11,6 @@
 package storepool
 
 import (
-	"bytes"
 	"context"
 	"fmt"
 	"sort"
@@ -31,6 +30,7 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/util/shuffle"
 	"github.com/cockroachdb/cockroach/pkg/util/syncutil"
 	"github.com/cockroachdb/errors"
+	"github.com/cockroachdb/redact"
 )
 
 // FailedReservationsTimeout specifies a duration during which the local
@@ -120,6 +120,9 @@ func (ss storeStatus) String() string {
 		"decommissioning", "suspect", "draining"}[ss]
 }
 
+// SafeValue implements the redact.SafeValue interface.
+func (ss storeStatus) SafeValue() {}
+
 func (sd *StoreDetail) status(
 	now hlc.Timestamp,
 	deadThreshold time.Duration,
@@ -220,6 +223,7 @@ type CapacityChangeFn func(
 // of all known stores in the cluster and information on their health.
 type AllocatorStorePool interface {
 	fmt.Stringer
+	redact.SafeFormatter
 
 	// ClusterNodeCount returns the number of nodes that are possible allocation
 	// targets.
@@ -395,10 +399,15 @@ func NewStorePool(
 }
 
 func (sp *StorePool) String() string {
-	return sp.statusString(sp.NodeLivenessFn)
+	return redact.StringWithoutMarkers(sp)
+}
+
+// SafeFormat implements the redact.SafeFormatter interface.
+func (sp *StorePool) SafeFormat(w redact.SafePrinter, _ rune) {
+	w.Print(sp.statusString(sp.NodeLivenessFn))
 }
 
-func (sp *StorePool) statusString(nl NodeLivenessFunc) string {
+func (sp *StorePool) statusString(nl NodeLivenessFunc) redact.RedactableString {
 	sp.DetailsMu.RLock()
 	defer sp.DetailsMu.RUnlock()
 
@@ -408,28 +417,30 @@ func (sp *StorePool) statusString(nl NodeLivenessFunc) string {
 	}
 	sort.Sort(ids)
 
-	var buf bytes.Buffer
+	var buf redact.StringBuilder
 	now := sp.clock.Now()
 	timeUntilNodeDead := liveness.TimeUntilNodeDead.Get(&sp.st.SV)
 	timeAfterNodeSuspect := liveness.TimeAfterNodeSuspect.Get(&sp.st.SV)
 
 	for _, id := range ids {
 		detail := sp.DetailsMu.StoreDetails[id]
-		fmt.Fprintf(&buf, "%d", id)
+		buf.Print(id)
 		status := detail.status(now, timeUntilNodeDead, nl, timeAfterNodeSuspect)
 		if status != storeStatusAvailable {
-			fmt.Fprintf(&buf, " (status=%s)", status)
+			buf.Printf(" (status=%s)", status)
 		}
 		if detail.Desc != nil {
-			fmt.Fprintf(&buf, ": range-count=%d fraction-used=%.2f",
-				detail.Desc.Capacity.RangeCount, detail.Desc.Capacity.FractionUsed())
+			buf.Printf(": range-count=%d fraction-used=%.2f",
+				detail.Desc.Capacity.RangeCount,
+				detail.Desc.Capacity.FractionUsed())
 		}
 		if detail.ThrottledUntil.After(now) {
-			fmt.Fprintf(&buf, " [throttled=%.1fs]", detail.ThrottledUntil.GoTime().Sub(now.GoTime()).Seconds())
+			buf.Printf(" [throttled=%v]", humanizeutil.Duration(
+				detail.ThrottledUntil.GoTime().Sub(now.GoTime())))
 		}
-		_, _ = buf.WriteString("\n")
+		buf.SafeRune('\n')
 	}
-	return buf.String()
+	return buf.RedactableString()
 }
 
 // storeGossipUpdate is the Gossip callback used to keep the StorePool up to date.
@@ -970,8 +981,13 @@ func MakeStoreList(descriptors []roachpb.StoreDescriptor) StoreList {
 }
 
 func (sl StoreList) String() string {
-	var buf bytes.Buffer
-	fmt.Fprintf(&buf,
+	return redact.StringWithoutMarkers(sl)
+}
+
+// SafeFormat implements the redact.SafeFormatter interface.
+func (sl StoreList) SafeFormat(w redact.SafePrinter, _ rune) {
+	var buf redact.StringBuilder
+	buf.Printf(
 		"  candidate: avg-ranges=%.2f avg-leases=%.2f avg-disk-usage=%s avg-queries-per-second=%.2f avg-store-cpu-per-second=%s",
 		sl.CandidateRanges.Mean,
 		sl.CandidateLeases.Mean,
@@ -980,21 +996,21 @@ func (sl StoreList) String() string {
 		humanizeutil.Duration(time.Duration(int64(sl.CandidateCPU.Mean))),
 	)
 	if len(sl.Stores) > 0 {
-		fmt.Fprintf(&buf, "\n")
+		buf.Printf("\n")
 	} else {
-		fmt.Fprintf(&buf, " <no candidates>")
+		buf.Printf(" <no candidates>")
 	}
 	for _, desc := range sl.Stores {
 		ioScore, _ := desc.Capacity.IOThreshold.Score()
-		fmt.Fprintf(&buf, "  %d: ranges=%d leases=%d disk-usage=%s queries-per-second=%.2f store-cpu-per-second=%s io-overload=%.2f\n",
+		buf.Printf("  %v: ranges=%d leases=%d disk-usage=%s queries-per-second=%.2f store-cpu-per-second=%s io-overload=%.2f\n",
 			desc.StoreID, desc.Capacity.RangeCount,
 			desc.Capacity.LeaseCount, humanizeutil.IBytes(desc.Capacity.LogicalBytes),
 			desc.Capacity.QueriesPerSecond,
 			humanizeutil.Duration(time.Duration(int64(desc.Capacity.CPUPerSecond))),
 			ioScore,
 		)
 	}
-	return buf.String()
+	w.Print(buf)
 }
 
 // ExcludeInvalid takes a store list and removes Stores that would be explicitly invalid

pkg/kv/kvserver/allocator/storepool/store_pool_test.go

@@ -915,7 +915,7 @@ func TestStorePoolString(t *testing.T) {
 		"6 (status=dead): range-count=60 fraction-used=0.60\n"+
 		"7 (status=draining): range-count=70 fraction-used=0.70\n"+
 		"8 (status=suspect): range-count=80 fraction-used=0.80\n"+
-		"9 (status=throttled): range-count=90 fraction-used=0.90 [throttled=1.0s]\n",
+		"9 (status=throttled): range-count=90 fraction-used=0.90 [throttled=1s]\n",
 		sp.String())
 }
 

pkg/testutils/lint/passes/redactcheck/redactcheck.go

@@ -100,6 +100,9 @@ func runAnalyzer(pass *analysis.Pass) (interface{}, error) {
 					"github.com/cockroachdb/cockroach/pkg/kv/kvserver/allocator/load": {
 						"Dimension": {},
 					},
+					"github.com/cockroachdb/cockroach/pkg/kv/kvserver/allocator/storepool": {
+						"storeStatus": {},
+					},
 					"github.com/cockroachdb/cockroach/pkg/kv/kvserver/closedts/ctpb": {
 						"SeqNum": {},
 					},