From f131dcd7dbdc55b5822c4fc4e9cf586cacf393a5 Mon Sep 17 00:00:00 2001 From: Raphael 'kena' Poss Date: Wed, 10 May 2023 18:16:36 +0200 Subject: [PATCH] tracingpb: more fine grained redactability for tracing events Prior to this patch, `RecordedSpan` and `Recording` only had a `String` method to dump a representation, that would remove all redaction markers and turn the result in an unsafe string. This was making it hard to export recordings into logs while preserving redactability of individual trace spans. This patch improves the situation by introducing a `SafeFormat` method which exposes the redactability of trace spans into a mixed string that represents the recording. Release note: None --- pkg/util/tracing/tracingpb/recorded_span.go | 13 +++++++----- pkg/util/tracing/tracingpb/recording.go | 23 ++++++++++++--------- 2 files changed, 21 insertions(+), 15 deletions(-) diff --git a/pkg/util/tracing/tracingpb/recorded_span.go b/pkg/util/tracing/tracingpb/recorded_span.go index 40474b5637ab..ccfeec1184ed 100644 --- a/pkg/util/tracing/tracingpb/recorded_span.go +++ b/pkg/util/tracing/tracingpb/recorded_span.go @@ -12,7 +12,6 @@ package tracingpb import ( "fmt" - "strings" "time" "github.com/cockroachdb/cockroach/pkg/util/humanizeutil" @@ -53,13 +52,17 @@ func (r Recording) Len() int { // LogMessageField is the field name used for the log message in a LogRecord. const LogMessageField = "event" +// String implements fmt.Stringer. func (s *RecordedSpan) String() string { - sb := strings.Builder{} - sb.WriteString(fmt.Sprintf("=== %s (id: %d parent: %d)\n", s.Operation, s.SpanID, s.ParentSpanID)) + return redact.Sprint(s).StripMarkers() +} + +// SafeFormat implements redact.SafeFormatter. +func (s *RecordedSpan) SafeFormat(w redact.SafePrinter, _ rune) { + w.Printf("=== %s (id: %d parent: %d)\n", redact.Safe(s.Operation), s.SpanID, s.ParentSpanID) for _, ev := range s.Logs { - sb.WriteString(fmt.Sprintf("%s %s\n", ev.Time.UTC().Format(time.RFC3339Nano), ev.Msg())) + w.Printf("%s %s\n", redact.Safe(ev.Time.UTC().Format(time.RFC3339Nano)), ev.Msg()) } - return sb.String() } // Structured visits the data passed to RecordStructured for the Span from which diff --git a/pkg/util/tracing/tracingpb/recording.go b/pkg/util/tracing/tracingpb/recording.go index 39abab6763f7..d03a35226718 100644 --- a/pkg/util/tracing/tracingpb/recording.go +++ b/pkg/util/tracing/tracingpb/recording.go @@ -16,7 +16,6 @@ import ( "regexp" "sort" "strconv" - "strings" "time" "github.com/cockroachdb/cockroach/pkg/util/protoutil" @@ -140,20 +139,25 @@ type logRecord struct { // TODO(andrei): this should be unified with // SessionTracing.generateSessionTraceVTable(). func (r Recording) String() string { + return redact.Sprint(r).StripMarkers() +} + +// SafeFormat implements the redact.SafeFormatter interface. +func (r Recording) SafeFormat(w redact.SafePrinter, _ rune) { if len(r) == 0 { - return "" + w.SafeString("") } - var buf strings.Builder start := r[0].StartTime writeLogs := func(logs []traceLogData) { for _, entry := range logs { - fmt.Fprintf(&buf, "% 10.3fms % 10.3fms%s", + w.Printf("% 10.3fms % 10.3fms", 1000*entry.Timestamp.Sub(start).Seconds(), - 1000*entry.timeSincePrev.Seconds(), - strings.Repeat(" ", entry.depth+1)) - fmt.Fprint(&buf, entry.Msg.StripMarkers()) - buf.WriteByte('\n') + 1000*entry.timeSincePrev.Seconds()) + for i := 0; i < entry.depth+1; i++ { + w.SafeString(" ") + } + w.Printf("%s\n", entry.Msg) } } @@ -167,13 +171,12 @@ func (r Recording) String() string { orphans := r.OrphanSpans() if len(orphans) > 0 { // This shouldn't happen. - buf.WriteString("orphan spans (trace is missing spans):\n") + w.SafeString("orphan spans (trace is missing spans):\n") for _, o := range orphans { logs := r.visitSpan(o, 0 /* depth */) writeLogs(logs) } } - return buf.String() } // OrphanSpans returns the spans with parents missing from the recording.