tracing: preserve redactability of structured items in verbose traces

Prior to this patch, structured items added to verbose traces were
improperly converted to an unsafe string prior to recording.
This was making the entire structured payload redactable in the
recording representation.

This patch fixes the situation by delegating the representation
to the structured item itself, which may implement a SafeFormat
method.

Release note: None

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.

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))

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),