Merge #133469

133469: raft: implement SafeFormatter on Progress r=nvanbenschoten a=nvanbenschoten

This commit implements the `redact.SafeFormatter` interface on `tracker.Progress` so that it will never be redacted when logged.

Epic: None
Release note: None

Co-authored-by: Nathan VanBenschoten <[email protected]>

pkg/base/config.go

@@ -852,7 +852,7 @@ func (m *WALFailoverMode) String() string {
 	return redact.StringWithoutMarkers(m)
 }
 
-// SafeFormat implements the refact.SafeFormatter interface.
+// SafeFormat implements the redact.SafeFormatter interface.
 func (m *WALFailoverMode) SafeFormat(p redact.SafePrinter, _ rune) {
 	switch *m {
 	case WALFailoverDefault:
@@ -905,7 +905,7 @@ func (c *WALFailoverConfig) String() string {
 	return redact.StringWithoutMarkers(c)
 }
 
-// SafeFormat implements the refact.SafeFormatter interface.
+// SafeFormat implements the redact.SafeFormatter interface.
 func (c *WALFailoverConfig) SafeFormat(p redact.SafePrinter, _ rune) {
 	switch c.Mode {
 	case WALFailoverDefault:

pkg/raft/tracker/BUILD.bazel

@@ -19,6 +19,7 @@ go_library(
         "//pkg/raft/raftstoreliveness",
         "//pkg/util/hlc",
         "//pkg/util/syncutil",
+        "@com_github_cockroachdb_redact//:redact",
         "@org_golang_x_exp//maps",
     ],
 )
@@ -39,6 +40,7 @@ go_test(
         "//pkg/util/hlc",
         "//pkg/util/leaktest",
         "//pkg/util/log",
+        "@com_github_cockroachdb_redact//:redact",
         "@com_github_stretchr_testify//assert",
         "@com_github_stretchr_testify//require",
     ],

pkg/raft/tracker/progress.go

@@ -23,6 +23,7 @@ import (
 	"strings"
 
 	pb "github.com/cockroachdb/cockroach/pkg/raft/raftpb"
+	"github.com/cockroachdb/redact"
 	"golang.org/x/exp/maps"
 )
 
@@ -390,29 +391,33 @@ func (pr *Progress) ShouldSendProbe(last, commit uint64, advanceCommit bool) boo
 	}
 }
 
+// String implements the fmt.Stringer interface.
 func (pr *Progress) String() string {
-	var buf strings.Builder
-	fmt.Fprintf(&buf, "%s match=%d next=%d sentCommit=%d matchCommit=%d", pr.State, pr.Match,
+	return redact.StringWithoutMarkers(pr)
+}
+
+// SafeFormat implements the redact.SafeFormatter interface.
+func (pr *Progress) SafeFormat(p redact.SafePrinter, _ rune) {
+	p.Printf("%s match=%d next=%d sentCommit=%d matchCommit=%d", pr.State, pr.Match,
 		pr.Next, pr.SentCommit, pr.MatchCommit)
 	if pr.IsLearner {
-		fmt.Fprint(&buf, " learner")
+		p.SafeString(" learner")
 	}
 	if pr.IsPaused() {
-		fmt.Fprint(&buf, " paused")
+		p.SafeString(" paused")
 	}
 	if pr.PendingSnapshot > 0 {
-		fmt.Fprintf(&buf, " pendingSnap=%d", pr.PendingSnapshot)
+		p.Printf(" pendingSnap=%d", pr.PendingSnapshot)
 	}
 	if !pr.RecentActive {
-		fmt.Fprint(&buf, " inactive")
+		p.SafeString(" inactive")
 	}
 	if n := pr.Inflights.Count(); n > 0 {
-		fmt.Fprintf(&buf, " inflight=%d", n)
+		p.Printf(" inflight=%d", n)
 		if pr.Inflights.Full() {
-			fmt.Fprint(&buf, "[full]")
+			p.SafeString("[full]")
 		}
 	}
-	return buf.String()
 }
 
 // ProgressMap is a map of *Progress.

pkg/raft/tracker/progress_test.go

@@ -20,10 +20,11 @@ package tracker
 import (
 	"testing"
 
+	"github.com/cockroachdb/redact"
 	"github.com/stretchr/testify/assert"
 )
 
-func TestProgressString(t *testing.T) {
+func TestProgressStringAndSafeFormat(t *testing.T) {
 	ins := NewInflights(1, 0)
 	ins.Add(123, 1)
 	pr := &Progress{
@@ -40,7 +41,12 @@ func TestProgressString(t *testing.T) {
 	}
 	const exp = "StateSnapshot match=3 next=4 sentCommit=2 matchCommit=1 learner paused " +
 		"pendingSnap=123 inactive inflight=1[full]"
+	// String.
 	assert.Equal(t, exp, pr.String())
+	// Redactable string.
+	assert.EqualValues(t, exp, redact.Sprint(pr))
+	// Redacted string.
+	assert.EqualValues(t, exp, redact.Sprint(pr).Redact())
 }
 
 func TestProgressIsPaused(t *testing.T) {

pkg/raft/tracker/state.go

@@ -1,3 +1,6 @@
+// This code has been modified from its original form by The Cockroach Authors.
+// All modifications are Copyright 2024 The Cockroach Authors.
+//
 // Copyright 2019 The etcd Authors
 //
 // Licensed under the Apache License, Version 2.0 (the "License");
@@ -40,3 +43,4 @@ var prstmap = [...]string{
 }
 
 func (st StateType) String() string { return prstmap[st] }
+func (st StateType) SafeValue()     {}

pkg/storage/enginepb/engine.go

@@ -17,7 +17,7 @@ func (e *EngineType) Type() string { return "string" }
 // String implements the pflag.Value interface.
 func (e *EngineType) String() string { return redact.StringWithoutMarkers(e) }
 
-// SafeFormat implements the refact.SafeFormatter interface.
+// SafeFormat implements the redact.SafeFormatter interface.
 func (e *EngineType) SafeFormat(p redact.SafePrinter, _ rune) {
 	switch *e {
 	case EngineTypeDefault:

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

@@ -145,6 +145,9 @@ func runAnalyzer(pass *analysis.Pass) (interface{}, error) {
 						"ConfChangeType":       {},
 						"ConfChangeTransition": {},
 					},
+					"github.com/cockroachdb/cockroach/pkg/raft/tracker": {
+						"StateType": {},
+					},
 					"github.com/cockroachdb/cockroach/pkg/repstream/streampb": {
 						"StreamID": {},
 					},