diff --git a/pkg/util/tracing/crdbspan.go b/pkg/util/tracing/crdbspan.go index 87a8c689be0..deaa06c9101 100644 --- a/pkg/util/tracing/crdbspan.go +++ b/pkg/util/tracing/crdbspan.go @@ -90,16 +90,16 @@ type crdbSpanMu struct { remoteSpans []tracingpb.RecordedSpan } + // The Span's associated baggage. + baggage map[string]string + // tags are only captured when recording. These are tags that have been // added to this Span, and will be appended to the tags in logTags when // someone needs to actually observe the total set of tags that is a part of // this Span. // TODO(radu): perhaps we want a recording to capture all the tags (even // those that were set before recording started)? - tags map[string]attribute.Value - - // The Span's associated baggage. - baggage map[string]string + tags []attribute.KeyValue } type childSpanRefs struct { @@ -274,15 +274,14 @@ func (s *crdbSpan) importRemoteSpans(remoteSpans []tracingpb.RecordedSpan) { } func (s *crdbSpan) setTagLocked(key string, value attribute.Value) { - if s.recordingType() != RecordingVerbose { - // Don't bother storing tags if we're unlikely to retrieve them. - return - } - - if s.mu.tags == nil { - s.mu.tags = make(map[string]attribute.Value) + k := attribute.Key(key) + for i := range s.mu.tags { + if s.mu.tags[i].Key == k { + s.mu.tags[i].Value = value + return + } } - s.mu.tags[key] = value + s.mu.tags = append(s.mu.tags, attribute.KeyValue{Key: k, Value: value}) } func (s *crdbSpan) record(msg redact.RedactableString) { @@ -459,11 +458,9 @@ func (s *crdbSpan) getRecordingLocked(wantTags bool) tracingpb.RecordedSpan { addTag(remappedKey, tag.ValueStr()) }) } - if len(s.mu.tags) > 0 { - for k, v := range s.mu.tags { - // We encode the tag values as strings. - addTag(k, v.Emit()) - } + for _, kv := range s.mu.tags { + // We encode the tag values as strings. + addTag(string(kv.Key), kv.Value.Emit()) } } diff --git a/pkg/util/tracing/span_test.go b/pkg/util/tracing/span_test.go index d442f5d31be..ab02661ca39 100644 --- a/pkg/util/tracing/span_test.go +++ b/pkg/util/tracing/span_test.go @@ -504,8 +504,8 @@ func (i *countingStringer) String() string { return fmt.Sprint(*i) } -// TestSpanTagsInRecordings verifies that tags are dropped if the span is -// not verbose. +// TestSpanTagsInRecordings verifies that tags added before a recording started +// are part of the recording. func TestSpanTagsInRecordings(t *testing.T) { tr := NewTracer() var counter countingStringer @@ -525,20 +525,19 @@ func TestSpanTagsInRecordings(t *testing.T) { // We didn't stringify the log tag. require.Zero(t, int(counter)) - // Verify that we didn't hold onto anything underneath. sp.SetVerbose(true) rec = sp.GetRecording() require.Len(t, rec, 1) - require.Len(t, rec[0].Tags, 4) // _unfinished:1 _verbose:1 tagfoo:tagbar foo1:1 + require.Len(t, rec[0].Tags, 5) // _unfinished:1 _verbose:1 tagfoo:tagbar foo1:1 foor2:bar2 _, ok := rec[0].Tags["foo2"] - require.False(t, ok) + require.True(t, ok) require.Equal(t, 1, int(counter)) - // Verify that subsequent tags are captured. + // Verify that subsequent tags are also captured. sp.SetTag("foo3", attribute.StringValue("bar3")) rec = sp.GetRecording() require.Len(t, rec, 1) - require.Len(t, rec[0].Tags, 5) + require.Len(t, rec[0].Tags, 6) _, ok = rec[0].Tags["foo3"] require.True(t, ok) require.Equal(t, 2, int(counter)) diff --git a/pkg/util/tracing/tracer.go b/pkg/util/tracing/tracer.go index c1d737ea5d7..5d2411896e0 100644 --- a/pkg/util/tracing/tracer.go +++ b/pkg/util/tracing/tracer.go @@ -509,6 +509,8 @@ func (t *Tracer) startSpanGeneric( span Span crdbSpan crdbSpan octx optimizedContext + // tagsAlloc preallocates space for crdbSpan.mu.tags. + tagsAlloc [3]attribute.KeyValue }{} helper.crdbSpan = crdbSpan{ @@ -526,6 +528,10 @@ func (t *Tracer) startSpanGeneric( helper.crdbSpan.mu.operation = opName helper.crdbSpan.mu.recording.logs = newSizeLimitedBuffer(maxLogBytesPerSpan) helper.crdbSpan.mu.recording.structured = newSizeLimitedBuffer(maxStructuredBytesPerSpan) + helper.crdbSpan.mu.tags = helper.tagsAlloc[:0] + if opts.SpanKind != oteltrace.SpanKindUnspecified { + helper.crdbSpan.setTagLocked(spanKindTagKey, attribute.StringValue(opts.SpanKind.String())) + } helper.span.i = spanInner{ tracer: t, crdb: &helper.crdbSpan, @@ -561,12 +567,6 @@ func (t *Tracer) startSpanGeneric( s.i.crdb.enableRecording(opts.recordingType()) } - // Deal with opts.SpanKind. This needs to be done after we enable recording - // above because tags are dropped on the floor before recording is enabled. - if opts.SpanKind != oteltrace.SpanKindUnspecified { - helper.crdbSpan.setTagLocked(spanKindTagKey, attribute.StringValue(opts.SpanKind.String())) - } - // Copy baggage from parent. This similarly fans out over the various // spans contained in Span. //