Skip to content

Commit

Permalink
raft: implement SafeFormatter on Progress
Browse files Browse the repository at this point in the history
This commit implements the `redact.SafeFormatter` interface on
`tracker.Progress` so that it will never be redacted when logged.

Epic: None
Release note: None
  • Loading branch information
nvanbenschoten committed Oct 25, 2024
1 parent 07624d1 commit d3745ff
Show file tree
Hide file tree
Showing 7 changed files with 30 additions and 13 deletions.
4 changes: 2 additions & 2 deletions pkg/base/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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:
Expand Down
2 changes: 2 additions & 0 deletions pkg/raft/tracker/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -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",
],
)
Expand All @@ -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",
],
Expand Down
23 changes: 14 additions & 9 deletions pkg/raft/tracker/progress.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import (
"strings"

pb "github.com/cockroachdb/cockroach/pkg/raft/raftpb"
"github.com/cockroachdb/redact"
"golang.org/x/exp/maps"
)

Expand Down Expand Up @@ -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.
Expand Down
8 changes: 7 additions & 1 deletion pkg/raft/tracker/progress_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand All @@ -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) {
Expand Down
1 change: 1 addition & 0 deletions pkg/raft/tracker/state.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,3 +40,4 @@ var prstmap = [...]string{
}

func (st StateType) String() string { return prstmap[st] }
func (st StateType) SafeValue() {}
2 changes: 1 addition & 1 deletion pkg/storage/enginepb/engine.go
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
3 changes: 3 additions & 0 deletions pkg/testutils/lint/passes/redactcheck/redactcheck.go
Original file line number Diff line number Diff line change
Expand Up @@ -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": {},
},
Expand Down

0 comments on commit d3745ff

Please sign in to comment.