diff --git a/pkg/util/tracing/span_inner.go b/pkg/util/tracing/span_inner.go index 34f4a427b227..2c3cae8750fe 100644 --- a/pkg/util/tracing/span_inner.go +++ b/pkg/util/tracing/span_inner.go @@ -265,7 +265,10 @@ func (s *spanInner) RecordStructured(item Structured) { } s.crdb.recordStructured(item) if s.hasVerboseSink() { - s.Record(item.String()) + // Do not call .String() on the item, so that non-redactable bits + // in its representation are properly preserved by Recordf() in + // verbose recordings. + s.Recordf("%v", item) } } @@ -283,10 +286,16 @@ func (s *spanInner) Recordf(format string, args ...interface{}) { } else { // `fmt.Sprintf` when called on a logEntry will use the faster // `logEntry.String` method instead of `logEntry.SafeFormat`. - // The additional use of `redact.Sprintf("%s",...)` is necessary + // The additional use of `redact.Sprint(...)` is necessary // to wrap the result in redaction markers. - str = redact.Sprintf("%s", fmt.Sprintf(format, args...)) + str = redact.Sprint(fmt.Sprintf(format, args...)) } + s.recordRedactable(format, args, str) +} + +func (s *spanInner) recordRedactable( + format string, args []interface{}, str redact.RedactableString, +) { if s.otelSpan != nil { // TODO(obs-inf): depending on the situation it may be more appropriate to // redact the string here. diff --git a/pkg/util/tracing/span_test.go b/pkg/util/tracing/span_test.go index 7c721d6a8b82..8ed491d8f9dc 100644 --- a/pkg/util/tracing/span_test.go +++ b/pkg/util/tracing/span_test.go @@ -247,6 +247,24 @@ func TestImportRemoteRecordingMaintainsRightByteSize(t *testing.T) { rootTrace.check(t) } +func TestSpanRecordStructuredRedactable(t *testing.T) { + tr := NewTracer() + tr.SetRedactable(true) + sp := tr.StartSpan("root", WithRecording(tracingpb.RecordingVerbose)) + defer sp.Finish() + + payload := &tracingpb.OperationMetadata{Count: 123} + sp.RecordStructured(payload) + rec := sp.GetRecording(tracingpb.RecordingStructured) + + // The following check fails if RecordStructured incorrectly + // serializes the event to a string. + checkRecordingWithRedact(t, rec, ` + === operation:root +event:{count: 123, duration 0µs} +structured:‹×›`, true) +} + func TestSpanRecordStructured(t *testing.T) { tr := NewTracer() sp := tr.StartSpan("root", WithRecording(tracingpb.RecordingStructured)) diff --git a/pkg/util/tracing/test_utils.go b/pkg/util/tracing/test_utils.go index 519d2e78ff9b..a464b25f71ad 100644 --- a/pkg/util/tracing/test_utils.go +++ b/pkg/util/tracing/test_utils.go @@ -178,6 +178,10 @@ type T interface { // event:remote child 1 // `) func checkRecording(t T, rec tracingpb.Recording, expected string) { + checkRecordingWithRedact(t, rec, expected, false) +} + +func checkRecordingWithRedact(t T, rec tracingpb.Recording, expected string, redactValues bool) { normalize := func(rec string) string { // normalize the string form of a recording for ease of comparison. // @@ -235,8 +239,12 @@ func checkRecording(t T, rec tracingpb.Recording, expected string) { sortChildrenMetadataByName(rec[i].ChildrenMetadata) } + actual := redact.Sprint(rec) + if redactValues { + actual = actual.Redact() + } exp := normalize(expected) - got := normalize(string(redact.Sprint(rec))) + got := normalize(string(actual)) if got != exp { diff := difflib.UnifiedDiff{ A: difflib.SplitLines(exp),