Skip to content

Commit 0f9776a

Browse files
committed
pkg/kv/kvpb: Add SafeFormat method to BatchResponse struct
This patch helps to fix the overly redacted log line on instances where BatchResponse is logged. Implemented a SafeFormat function on BatchResponse struct which overrides the String() method when its object is logged. Epic: CRDB-37533 Part of: CRDB-44885 Release note: None
1 parent 8fed6e8 commit 0f9776a

File tree

2 files changed

+18
-12
lines changed

2 files changed

+18
-12
lines changed

pkg/kv/kvclient/kvcoord/dist_sender_test.go

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -4769,26 +4769,26 @@ func TestDistSenderSlowLogMessage(t *testing.T) {
47694769
br.Error = kvpb.NewError(errors.New("boom"))
47704770
desc := &roachpb.RangeDescriptor{RangeID: 9, StartKey: roachpb.RKey("x"), EndKey: roachpb.RKey("z")}
47714771
{
4772-
exp := `have been waiting 8.16s (120 attempts) for RPC Get(Shared,Unreplicated) ["a"] to` +
4773-
` r9:{x-z} [<no replicas>, next=0, gen=0]; resp: (err: boom)`
4772+
exp := `have been waiting 8.16s (120 attempts) for RPC Get(Shared,Unreplicated) ["a"] to` +
4773+
` r9:{x-z} [<no replicas>, next=0, gen=0]; resp: (err: boom)`
47744774
var s redact.StringBuilder
47754775
slowRangeRPCWarningStr(&s, ba, dur, attempts, desc, nil /* err */, br)
4776-
act := s.RedactableString()
4776+
act := s.RedactableString().StripMarkers()
47774777
require.EqualValues(t, exp, act)
47784778
}
47794779
{
47804780
exp := `slow RPC finished after 8.16s (120 attempts)`
47814781
var s redact.StringBuilder
47824782
slowRangeRPCReturnWarningStr(&s, dur, attempts)
4783-
act := s.RedactableString()
4783+
act := s.RedactableString().StripMarkers()
47844784
require.EqualValues(t, exp, act)
47854785
}
47864786
{
4787-
exp := `have been waiting 8.16s (120 attempts) for RPC Get(Shared,Unreplicated) ["a"] to` +
4788-
` replica (n2,s3):1; resp: (err: boom)`
4787+
exp := `have been waiting 8.16s (120 attempts) for RPC Get(Shared,Unreplicated) ["a"] to` +
4788+
` replica (n2,s3):1; resp: (err: boom)`
47894789
var s redact.StringBuilder
47904790
slowReplicaRPCWarningStr(&s, ba, dur, attempts, nil /* err */, br)
4791-
act := s.RedactableString()
4791+
act := s.RedactableString().StripMarkers()
47924792
require.EqualValues(t, exp, act)
47934793
}
47944794
}

pkg/kv/kvpb/batch.go

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -453,20 +453,26 @@ func (ba *BatchRequest) GetArg(method Method) (Request, bool) {
453453
}
454454

455455
func (br *BatchResponse) String() string {
456-
var str []string
457-
str = append(str, fmt.Sprintf("(err: %v)", br.Error))
456+
return redact.StringWithoutMarkers(br)
457+
}
458+
459+
// SafeFormat implements the redact.SafeFormatter interface.
460+
func (br *BatchResponse) SafeFormat(s redact.SafePrinter, verb rune) {
461+
// Marking Error of BatchResponse as safe as Outside of the RPC boundaries,
462+
// this field is nil and must neither be checked nor populated
463+
s.Printf("(err: %s)", redact.Safe(br.Error.String()))
464+
458465
for count, union := range br.Responses {
459466
// Limit the strings to provide just a summary. Without this limit a log
460467
// message with a BatchResponse can be very long.
461468
if count >= 20 && count < len(br.Responses)-5 {
462469
if count == 20 {
463-
str = append(str, fmt.Sprintf("... %d skipped ...", len(br.Responses)-25))
470+
s.Printf(", ... %d skipped ...", len(br.Responses)-25)
464471
}
465472
continue
466473
}
467-
str = append(str, fmt.Sprintf("%T", union.GetInner()))
474+
s.Printf(", %T", redact.Safe(union.GetInner()))
468475
}
469-
return strings.Join(str, ", ")
470476
}
471477

472478
// LockSpanIterate calls LockSpanIterate for each request in the batch.

0 commit comments

Comments
 (0)