Merge #134617

134617: kvprober: unredact leaseholder information in kvprober logs r=dhartunian a=VishalJaishankar Previously, this is the log line seeing in datadog `...r=393210 having likely leaseholder=‹×› returned success...` The leaseholder info is not a PII and does not serve any purpose if redacted This PR makes it redact safe. Epic: none Release note: None Co-authored-by: Vishal Jaishankar <[email protected]>
cockroachdb · Dec 23, 2024 · bea204f · bea204f
2 parents b63a736 + 17ae278
commit bea204f
Show file tree

Hide file tree

Showing 3 changed files with 6 additions and 4 deletions.
diff --git a/pkg/kv/kvprober/BUILD.bazel b/pkg/kv/kvprober/BUILD.bazel
@@ -28,6 +28,7 @@ go_library(
         "//pkg/util/tracing/tracingpb",
         "@com_github_cockroachdb_errors//:errors",
         "@com_github_cockroachdb_logtags//:logtags",
+        "@com_github_cockroachdb_redact//:redact",
     ],
 )
 

diff --git a/pkg/kv/kvprober/kvprober.go b/pkg/kv/kvprober/kvprober.go
@@ -34,6 +34,7 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/util/tracing/tracingpb"
 	"github.com/cockroachdb/errors"
 	"github.com/cockroachdb/logtags"
+	"github.com/cockroachdb/redact"
 )
 
 const putValue = "thekvproberwrotethis"
@@ -533,7 +534,7 @@ func (p *Prober) quarantineProbe(ctx context.Context, pl planner) {
 // log messages indicating leaseholder information, extracts leaseholder
 // node ID, and returns this information. Returns an empty string if
 // leaseholder information is not found.
-func (p *Prober) returnLeaseholderInfo(recording tracingpb.Recording) string {
+func (p *Prober) returnLeaseholderInfo(recording tracingpb.Recording) redact.SafeString {
 	// The leaseholder is determined by the kvclient in dist_sender.go,
 	// which decides the node to handle the request and sends it. The log
 	// entry with "node received request" shows the leaseholder acknowledging
@@ -551,7 +552,7 @@ func (p *Prober) returnLeaseholderInfo(recording tracingpb.Recording) string {
 		leaseholder := leaseRegex.FindStringSubmatch(informationLog)
 		// Return leaseholder node ID if found.
 		if len(leaseholder) == 2 {
-			return leaseholder[1]
+			return redact.SafeString(leaseholder[1])
 		}
 	}
 	return ""

diff --git a/pkg/kv/kvprober/kvprober_test.go b/pkg/kv/kvprober/kvprober_test.go
@@ -303,7 +303,7 @@ func TestReturnLeaseholderInfo(t *testing.T) {
 		m := &mock{t: t, read: true}
 		p := initTestProber(ctx, m)
 		// Expected leaseholder information is node 1.
-		require.Equal(t, "1", p.returnLeaseholderInfo(mockRecording))
+		require.Equal(t, "1", string(p.returnLeaseholderInfo(mockRecording)))
 	})
 
 	t.Run("traces do not contain leaseholder information", func(t *testing.T) {
@@ -333,7 +333,7 @@ func TestReturnLeaseholderInfo(t *testing.T) {
 		m := &mock{t: t, read: true}
 		p := initTestProber(ctx, m)
 		// Since no leaseholder information is present, the function is expected to return an empty string.
-		require.Equal(t, "", p.returnLeaseholderInfo(mockRecording))
+		require.Equal(t, "", string(p.returnLeaseholderInfo(mockRecording)))
 	})
 }