diff --git a/CHANGELOG.md b/CHANGELOG.md index 89df8a8c5a0..b65fd81c2de 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -31,6 +31,10 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm - Specify explicit buckets in Prometheus example. (#2493) - W3C baggage will now decode urlescaped values. (#2529) - Baggage members are now only validated once, when calling `NewMember` and not also when adding it to the baggage itself. (#2522) +- The order attributes are dropped from spans in the `go.opentelemetry.io/otel/sdk/trace` package when capacity is reached is fixed to be in compliance with the OpenTelemetry specification. + Instead of dropping the least-recently-used attribute, the last added attribute is dropped. + This drop order still only applies to attributes with unique keys not already contained in the span. + If an attribute is added with a key already contained in the span, that attribute is updated to the new value being added. (#2576) ### Removed diff --git a/sdk/trace/attributesmap.go b/sdk/trace/attributesmap.go deleted file mode 100644 index b891c8178b7..00000000000 --- a/sdk/trace/attributesmap.go +++ /dev/null @@ -1,91 +0,0 @@ -// Copyright The OpenTelemetry Authors -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package trace // import "go.opentelemetry.io/otel/sdk/trace" - -import ( - "container/list" - - "go.opentelemetry.io/otel/attribute" -) - -// attributesMap is a capped map of attributes, holding the most recent attributes. -// Eviction is done via a LRU method, the oldest entry is removed to create room for a new entry. -// Updates are allowed and they refresh the usage of the key. -// -// This is based from https://github.com/hashicorp/golang-lru/blob/master/simplelru/lru.go -// With a subset of the its operations and specific for holding attribute.KeyValue -type attributesMap struct { - attributes map[attribute.Key]*list.Element - evictList *list.List - droppedCount int - capacity int -} - -func newAttributesMap(capacity int) *attributesMap { - lm := &attributesMap{ - attributes: make(map[attribute.Key]*list.Element), - evictList: list.New(), - capacity: capacity, - } - return lm -} - -func (am *attributesMap) add(kv attribute.KeyValue) { - // Check for existing item - if ent, ok := am.attributes[kv.Key]; ok { - am.evictList.MoveToFront(ent) - ent.Value = &kv - return - } - - // Add new item - entry := am.evictList.PushFront(&kv) - am.attributes[kv.Key] = entry - - // Verify size not exceeded - if am.evictList.Len() > am.capacity { - am.removeOldest() - am.droppedCount++ - } -} - -// toKeyValue copies the attributesMap into a slice of attribute.KeyValue and -// returns it. If the map is empty, a nil is returned. -// TODO: Is it more efficient to return a pointer to the slice? -func (am *attributesMap) toKeyValue() []attribute.KeyValue { - len := am.evictList.Len() - if len == 0 { - return nil - } - - attributes := make([]attribute.KeyValue, 0, len) - for ent := am.evictList.Back(); ent != nil; ent = ent.Prev() { - if value, ok := ent.Value.(*attribute.KeyValue); ok { - attributes = append(attributes, *value) - } - } - - return attributes -} - -// removeOldest removes the oldest item from the cache. -func (am *attributesMap) removeOldest() { - ent := am.evictList.Back() - if ent != nil { - am.evictList.Remove(ent) - kv := ent.Value.(*attribute.KeyValue) - delete(am.attributes, kv.Key) - } -} diff --git a/sdk/trace/attributesmap_test.go b/sdk/trace/attributesmap_test.go deleted file mode 100644 index 90a201e2195..00000000000 --- a/sdk/trace/attributesmap_test.go +++ /dev/null @@ -1,103 +0,0 @@ -// Copyright The OpenTelemetry Authors -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package trace - -import ( - "fmt" - "testing" - - "go.opentelemetry.io/otel/attribute" -) - -const testKeyFmt = "test-key-%d" - -func TestAttributesMap(t *testing.T) { - wantCapacity := 128 - attrMap := newAttributesMap(wantCapacity) - - for i := 0; i < 256; i++ { - attrMap.add(attribute.Int(fmt.Sprintf(testKeyFmt, i), i)) - } - if attrMap.capacity != wantCapacity { - t.Errorf("attrMap.capacity: got '%d'; want '%d'", attrMap.capacity, wantCapacity) - } - - if attrMap.droppedCount != wantCapacity { - t.Errorf("attrMap.droppedCount: got '%d'; want '%d'", attrMap.droppedCount, wantCapacity) - } - - for i := 0; i < wantCapacity; i++ { - key := attribute.Key(fmt.Sprintf(testKeyFmt, i)) - _, ok := attrMap.attributes[key] - if ok { - t.Errorf("key %q should be dropped", testKeyFmt) - } - } - for i := wantCapacity; i < 256; i++ { - key := attribute.Key(fmt.Sprintf(testKeyFmt, i)) - _, ok := attrMap.attributes[key] - if !ok { - t.Errorf("key %q should not be dropped", key) - } - } -} - -func TestAttributesMapGetOldestRemoveOldest(t *testing.T) { - attrMap := newAttributesMap(128) - - for i := 0; i < 128; i++ { - attrMap.add(attribute.Int(fmt.Sprintf(testKeyFmt, i), i)) - } - - attrMap.removeOldest() - attrMap.removeOldest() - attrMap.removeOldest() - - for i := 0; i < 3; i++ { - key := attribute.Key(fmt.Sprintf(testKeyFmt, i)) - _, ok := attrMap.attributes[key] - if ok { - t.Errorf("key %q should be removed", key) - } - } -} - -func TestAttributesMapToKeyValue(t *testing.T) { - attrMap := newAttributesMap(128) - - for i := 0; i < 128; i++ { - attrMap.add(attribute.Int(fmt.Sprintf(testKeyFmt, i), i)) - } - - kv := attrMap.toKeyValue() - - gotAttrLen := len(kv) - wantAttrLen := 128 - if gotAttrLen != wantAttrLen { - t.Errorf("len(attrMap.attributes): got '%d'; want '%d'", gotAttrLen, wantAttrLen) - } -} - -func BenchmarkAttributesMapToKeyValue(b *testing.B) { - attrMap := newAttributesMap(128) - - for i := 0; i < 128; i++ { - attrMap.add(attribute.Int(fmt.Sprintf(testKeyFmt, i), i)) - } - - for n := 0; n < b.N; n++ { - attrMap.toKeyValue() - } -} diff --git a/sdk/trace/benchmark_test.go b/sdk/trace/benchmark_test.go index 335deaf4fc3..81a06e2f8f9 100644 --- a/sdk/trace/benchmark_test.go +++ b/sdk/trace/benchmark_test.go @@ -16,6 +16,7 @@ package trace_test import ( "context" + "fmt" "testing" "time" @@ -24,6 +25,28 @@ import ( "go.opentelemetry.io/otel/trace" ) +func BenchmarkSpanSetAttributesOverCapacity(b *testing.B) { + tp := sdktrace.NewTracerProvider( + sdktrace.WithSpanLimits(sdktrace.SpanLimits{AttributeCountLimit: 1}), + ) + tracer := tp.Tracer("BenchmarkSpanSetAttributesOverCapacity") + ctx := context.Background() + attrs := make([]attribute.KeyValue, 128) + for i := range attrs { + key := fmt.Sprintf("key-%d", i) + attrs[i] = attribute.Bool(key, true) + } + + b.ReportAllocs() + b.ResetTimer() + + for i := 0; i < b.N; i++ { + _, span := tracer.Start(ctx, "/foo") + span.SetAttributes(attrs...) + span.End() + } +} + func BenchmarkStartEndSpan(b *testing.B) { traceBenchmark(b, "Benchmark StartEndSpan", func(b *testing.B, t trace.Tracer) { ctx := context.Background() diff --git a/sdk/trace/span.go b/sdk/trace/span.go index bf0c41c1112..779cde691dd 100644 --- a/sdk/trace/span.go +++ b/sdk/trace/span.go @@ -54,6 +54,7 @@ type ReadOnlySpan interface { // the span has not ended. EndTime() time.Time // Attributes returns the defining attributes of the span. + // The order of the returned attributes is not guaranteed to be stable across invocations. Attributes() []attribute.KeyValue // Links returns all the links the span has to other spans. Links() []Link @@ -129,9 +130,14 @@ type recordingSpan struct { // spanContext holds the SpanContext of this span. spanContext trace.SpanContext - // attributes are capped at configured limit. When the capacity is reached - // an oldest entry is removed to create room for a new entry. - attributes *attributesMap + // attributes is a collection of user provided key/values. The collection + // is constrained by a configurable maximum held by the parent + // TracerProvider. When additional attributes are added after this maximum + // is reached these attributes the user is attempting to add are dropped. + // This dropped number of attributes is tracked and reported in the + // ReadOnlySpan exported when the span ends. + attributes []attribute.KeyValue + droppedAttributes int // events are stored in FIFO queue capped by configured limit. events evictedQueue @@ -194,11 +200,80 @@ func (s *recordingSpan) SetStatus(code codes.Code, description string) { // will be overwritten with the value contained in attributes. // // If this span is not being recorded than this method does nothing. +// +// If adding attributes to the span would exceed the maximum amount of +// attributes the span is configured to have, the last added attributes will +// be dropped. func (s *recordingSpan) SetAttributes(attributes ...attribute.KeyValue) { if !s.IsRecording() { return } - s.copyToCappedAttributes(attributes...) + + s.mu.Lock() + defer s.mu.Unlock() + + // If adding these attributes could exceed the capacity of s perform a + // de-duplication and truncation while adding to avoid over allocation. + if len(s.attributes)+len(attributes) > s.tracer.provider.spanLimits.AttributeCountLimit { + s.addOverCapAttrs(attributes) + return + } + + // Otherwise, add without deduplication. When attributes are read they + // will be deduplicated, optimizing the operation. + for _, a := range attributes { + if !a.Valid() { + // Drop all invalid attributes. + s.droppedAttributes++ + continue + } + s.attributes = append(s.attributes, a) + } +} + +// addOverCapAttrs adds the attributes attrs to the span s while +// de-duplicating the attributes of s and attrs and dropping attributes that +// exceed the capacity of s. +// +// This method assumes s.mu.Lock is held by the caller. +// +// This method should only be called when there is a possibility that adding +// attrs to s will exceed the capacity of s. Otherwise, attrs should be added +// to s without checking for duplicates and all retrieval methods of the +// attributes for s will de-duplicate as needed. +func (s *recordingSpan) addOverCapAttrs(attrs []attribute.KeyValue) { + // In order to not allocate more capacity to s.attributes than needed, + // prune and truncate this addition of attributes while adding. + + // Do not set a capacity when creating this map. Benchmark testing has + // showed this to only add unused memory allocations in general use. + exists := make(map[attribute.Key]int) + s.dedupeAttrsFromRecord(&exists) + + // Now that s.attributes is deduplicated, adding unique attributes up to + // the capacity of s will not over allocate s.attributes. + for _, a := range attrs { + if !a.Valid() { + // Drop all invalid attributes. + s.droppedAttributes++ + continue + } + + if idx, ok := exists[a.Key]; ok { + // Perform all updates before dropping, even when at capacity. + s.attributes[idx] = a + continue + } + + if len(s.attributes) >= s.tracer.provider.spanLimits.AttributeCountLimit { + // Do not just drop all of the remaining attributes, make sure + // updates are checked and performed. + s.droppedAttributes++ + } else { + s.attributes = append(s.attributes, a) + exists[a.Key] = len(s.attributes) - 1 + } + } } // End ends the span. This method does nothing if the span is already ended or @@ -388,13 +463,45 @@ func (s *recordingSpan) EndTime() time.Time { } // Attributes returns the attributes of this span. +// +// The order of the returned attributes is not guaranteed to be stable. func (s *recordingSpan) Attributes() []attribute.KeyValue { s.mu.Lock() defer s.mu.Unlock() - if s.attributes.evictList.Len() == 0 { - return []attribute.KeyValue{} + s.dedupeAttrs() + return s.attributes +} + +// dedupeAttrs deduplicates the attributes of s to fit capacity. +// +// This method assumes s.mu.Lock is held by the caller. +func (s *recordingSpan) dedupeAttrs() { + // Do not set a capacity when creating this map. Benchmark testing has + // showed this to only add unused memory allocations in general use. + exists := make(map[attribute.Key]int) + s.dedupeAttrsFromRecord(&exists) +} + +// dedupeAttrsFromRecord deduplicates the attributes of s to fit capacity +// using record as the record of unique attribute keys to their index. +// +// This method assumes s.mu.Lock is held by the caller. +func (s *recordingSpan) dedupeAttrsFromRecord(record *map[attribute.Key]int) { + // Use the fact that slices share the same backing array. + unique := s.attributes[:0] + for _, a := range s.attributes { + if idx, ok := (*record)[a.Key]; ok { + unique[idx] = a + } else { + unique = append(unique, a) + (*record)[a.Key] = len(unique) - 1 + } } - return s.attributes.toKeyValue() + // s.attributes have element types of attribute.KeyValue. These types are + // not pointers and they themselves do not contain pointer fields, + // therefore the duplicate values do not need to be zeroed for them to be + // garbage collected. + s.attributes = unique } // Links returns the links of this span. @@ -463,7 +570,7 @@ func (s *recordingSpan) addLink(link trace.Link) { func (s *recordingSpan) DroppedAttributes() int { s.mu.Lock() defer s.mu.Unlock() - return s.attributes.droppedCount + return s.droppedAttributes } // DroppedLinks returns the number of links dropped by the span due to limits @@ -513,10 +620,11 @@ func (s *recordingSpan) snapshot() ReadOnlySpan { sd.status = s.status sd.childSpanCount = s.childSpanCount - if s.attributes.evictList.Len() > 0 { - sd.attributes = s.attributes.toKeyValue() - sd.droppedAttributeCount = s.attributes.droppedCount + if len(s.attributes) > 0 { + s.dedupeAttrs() + sd.attributes = s.attributes } + sd.droppedAttributeCount = s.droppedAttributes if len(s.events.queue) > 0 { sd.events = s.interfaceArrayToEventArray() sd.droppedEventCount = s.events.droppedCount @@ -544,18 +652,6 @@ func (s *recordingSpan) interfaceArrayToEventArray() []Event { return eventArr } -func (s *recordingSpan) copyToCappedAttributes(attributes ...attribute.KeyValue) { - s.mu.Lock() - defer s.mu.Unlock() - for _, a := range attributes { - // Ensure attributes conform to the specification: - // https://github.com/open-telemetry/opentelemetry-specification/blob/v1.0.1/specification/common/common.md#attributes - if a.Valid() { - s.attributes.add(a) - } - } -} - func (s *recordingSpan) addChild() { if !s.IsRecording() { return diff --git a/sdk/trace/trace_test.go b/sdk/trace/trace_test.go index be27ed62041..5ba0015584a 100644 --- a/sdk/trace/trace_test.go +++ b/sdk/trace/trace_test.go @@ -419,34 +419,6 @@ func TestSetSpanAttributesOnStart(t *testing.T) { } } -func TestSetSpanAttributes(t *testing.T) { - te := NewTestExporter() - tp := NewTracerProvider(WithSyncer(te), WithResource(resource.Empty())) - span := startSpan(tp, "SpanAttribute") - span.SetAttributes(attribute.String("key1", "value1")) - got, err := endSpan(te, span) - if err != nil { - t.Fatal(err) - } - - want := &snapshot{ - spanContext: trace.NewSpanContext(trace.SpanContextConfig{ - TraceID: tid, - TraceFlags: 0x1, - }), - parent: sc.WithRemote(true), - name: "span0", - attributes: []attribute.KeyValue{ - attribute.String("key1", "value1"), - }, - spanKind: trace.SpanKindInternal, - instrumentationLibrary: instrumentation.Library{Name: "SpanAttribute"}, - } - if diff := cmpDiff(got, want); diff != "" { - t.Errorf("SetSpanAttributes: -got +want %s", diff) - } -} - func TestSamplerAttributesLocalChildSpan(t *testing.T) { sampler := &testSampler{prefix: "span", t: t} te := NewTestExporter() @@ -469,72 +441,193 @@ func TestSamplerAttributesLocalChildSpan(t *testing.T) { assert.Equal(t, []attribute.KeyValue{attribute.Int("callCount", 1)}, gotSpan1.Attributes()) } -func TestSetSpanAttributesOverLimit(t *testing.T) { - te := NewTestExporter() - tp := NewTracerProvider(WithSpanLimits(SpanLimits{AttributeCountLimit: 2}), WithSyncer(te), WithResource(resource.Empty())) - - span := startSpan(tp, "SpanAttributesOverLimit") - span.SetAttributes( - attribute.Bool("key1", true), +func TestSpanSetAttributes(t *testing.T) { + attrs := [...]attribute.KeyValue{ + attribute.String("key1", "value1"), attribute.String("key2", "value2"), - attribute.Bool("key1", false), // Replace key1. - attribute.Int64("key4", 4), // Remove key2 and add key4 - ) - got, err := endSpan(te, span) - if err != nil { - t.Fatal(err) + attribute.String("key3", "value3"), + attribute.String("key4", "value4"), + attribute.String("key1", "value5"), + attribute.String("key2", "value6"), + attribute.String("key3", "value7"), } + invalid := attribute.KeyValue{} - want := &snapshot{ - spanContext: trace.NewSpanContext(trace.SpanContextConfig{ - TraceID: tid, - TraceFlags: 0x1, - }), - parent: sc.WithRemote(true), - name: "span0", - attributes: []attribute.KeyValue{ - attribute.Bool("key1", false), - attribute.Int64("key4", 4), + tests := []struct { + name string + input [][]attribute.KeyValue + wantAttrs []attribute.KeyValue + wantDropped int + }{ + { + name: "array", + input: [][]attribute.KeyValue{attrs[:3]}, + wantAttrs: attrs[:3], + }, + { + name: "single_value:array", + input: [][]attribute.KeyValue{attrs[:1], attrs[1:3]}, + wantAttrs: attrs[:3], + }, + { + name: "array:single_value", + input: [][]attribute.KeyValue{attrs[:2], attrs[2:3]}, + wantAttrs: attrs[:3], + }, + { + name: "single_values", + input: [][]attribute.KeyValue{attrs[:1], attrs[1:2], attrs[2:3]}, + wantAttrs: attrs[:3], }, - spanKind: trace.SpanKindInternal, - droppedAttributeCount: 1, - instrumentationLibrary: instrumentation.Library{Name: "SpanAttributesOverLimit"}, - } - if diff := cmpDiff(got, want); diff != "" { - t.Errorf("SetSpanAttributesOverLimit: -got +want %s", diff) - } -} -func TestSetSpanAttributesWithInvalidKey(t *testing.T) { - te := NewTestExporter() - tp := NewTracerProvider(WithSpanLimits(SpanLimits{}), WithSyncer(te), WithResource(resource.Empty())) + // The tracing specification states: + // + // For each unique attribute key, addition of which would result in + // exceeding the limit, SDK MUST discard that key/value pair + // + // Therefore, adding attributes after the capacity is reached should + // result in those attributes being dropped. - span := startSpan(tp, "SpanToSetInvalidKeyOrValue") - span.SetAttributes( - attribute.Bool("", true), - attribute.Bool("key1", false), - ) - got, err := endSpan(te, span) - if err != nil { - t.Fatal(err) - } + { + name: "drop_last_added", + input: [][]attribute.KeyValue{attrs[:3], attrs[3:4], attrs[3:4]}, + wantAttrs: attrs[:3], + wantDropped: 2, + }, - want := &snapshot{ - spanContext: trace.NewSpanContext(trace.SpanContextConfig{ - TraceID: tid, - TraceFlags: 0x1, - }), - parent: sc.WithRemote(true), - name: "span0", - attributes: []attribute.KeyValue{ - attribute.Bool("key1", false), + // The tracing specification states: + // + // Setting an attribute with the same key as an existing attribute + // SHOULD overwrite the existing attribute's value. + // + // Therefore, attributes are updated regardless of capacity state. + + { + name: "single_value_update", + input: [][]attribute.KeyValue{attrs[:1], attrs[:3]}, + wantAttrs: attrs[:3], + }, + { + name: "all_update", + input: [][]attribute.KeyValue{attrs[:3], attrs[4:7]}, + wantAttrs: attrs[4:7], + }, + { + name: "all_update/multi", + input: [][]attribute.KeyValue{attrs[:3], attrs[4:7], attrs[:3]}, + wantAttrs: attrs[:3], + }, + { + name: "deduplicate/under_capacity", + input: [][]attribute.KeyValue{attrs[:1], attrs[:1], attrs[:1]}, + wantAttrs: attrs[:1], + }, + { + name: "deduplicate/over_capacity", + input: [][]attribute.KeyValue{attrs[:1], attrs[:1], attrs[:1], attrs[:3]}, + wantAttrs: attrs[:3], + }, + { + name: "deduplicate/added", + input: [][]attribute.KeyValue{ + attrs[:2], + {attrs[2], attrs[2], attrs[2]}, + }, + wantAttrs: attrs[:3], + }, + { + name: "deduplicate/added_at_cappacity", + input: [][]attribute.KeyValue{ + attrs[:3], + {attrs[2], attrs[2], attrs[2]}, + }, + wantAttrs: attrs[:3], + }, + { + name: "invalid", + input: [][]attribute.KeyValue{ + {invalid}, + }, + wantDropped: 1, + }, + { + name: "invalid_with_valid", + input: [][]attribute.KeyValue{ + {invalid, attrs[0]}, + }, + wantAttrs: attrs[:1], + wantDropped: 1, + }, + { + name: "invalid_over_capacity", + input: [][]attribute.KeyValue{ + {invalid, invalid, invalid, invalid, attrs[0]}, + }, + wantAttrs: attrs[:1], + wantDropped: 4, + }, + { + name: "valid:invalid/under_capacity", + input: [][]attribute.KeyValue{ + attrs[:1], + {invalid}, + }, + wantAttrs: attrs[:1], + wantDropped: 1, + }, + { + name: "valid:invalid/over_capacity", + input: [][]attribute.KeyValue{ + attrs[:1], + {invalid, invalid, invalid, invalid}, + }, + wantAttrs: attrs[:1], + wantDropped: 4, + }, + { + name: "valid_at_capacity:invalid", + input: [][]attribute.KeyValue{ + attrs[:3], + {invalid, invalid, invalid, invalid}, + }, + wantAttrs: attrs[:3], + wantDropped: 4, }, - spanKind: trace.SpanKindInternal, - droppedAttributeCount: 0, - instrumentationLibrary: instrumentation.Library{Name: "SpanToSetInvalidKeyOrValue"}, } - if diff := cmpDiff(got, want); diff != "" { - t.Errorf("SetSpanAttributesWithInvalidKey: -got +want %s", diff) + + const ( + capacity = 3 + instName = "TestSpanAttributeCapacity" + spanName = "test span" + ) + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + te := NewTestExporter() + tp := NewTracerProvider( + WithSyncer(te), + WithSpanLimits(SpanLimits{AttributeCountLimit: capacity}), + ) + _, span := tp.Tracer(instName).Start(context.Background(), spanName) + for _, a := range test.input { + span.SetAttributes(a...) + } + span.End() + + require.Implements(t, (*ReadOnlySpan)(nil), span) + roSpan := span.(ReadOnlySpan) + + // Ensure the span itself is valid. + assert.ElementsMatch(t, test.wantAttrs, roSpan.Attributes(), "exected attributes") + assert.Equal(t, test.wantDropped, roSpan.DroppedAttributes(), "dropped attributes") + + snap, ok := te.GetSpan(spanName) + require.Truef(t, ok, "span %s not exported", spanName) + + // Ensure the exported span snapshot is valid. + assert.ElementsMatch(t, test.wantAttrs, snap.Attributes(), "exected attributes") + assert.Equal(t, test.wantDropped, snap.DroppedAttributes(), "dropped attributes") + }) } } @@ -1852,3 +1945,11 @@ func TestWithIDGenerator(t *testing.T) { require.NoError(t, err) } } + +func TestEmptyRecordingSpanAttributes(t *testing.T) { + assert.Nil(t, (&recordingSpan{}).Attributes()) +} + +func TestEmptyRecordingSpanDroppedAttributes(t *testing.T) { + assert.Equal(t, 0, (&recordingSpan{}).DroppedAttributes()) +} diff --git a/sdk/trace/tracer.go b/sdk/trace/tracer.go index b63b4196516..5b8ab43be3d 100644 --- a/sdk/trace/tracer.go +++ b/sdk/trace/tracer.go @@ -122,12 +122,19 @@ func (tr *tracer) newRecordingSpan(psc, sc trace.SpanContext, name string, sr Sa } s := &recordingSpan{ + // Do not pre-allocate the attributes slice here! Doing so will + // allocate memory that is likely never going to be used, or if used, + // will be over-sized. The default Go compiler has been tested to + // dynamically allocate needed space very well. Benchmarking has shown + // it to be more performant than what we can predetermine here, + // especially for the common use case of few to no added + // attributes. + parent: psc, spanContext: sc, spanKind: trace.ValidateSpanKind(config.SpanKind()), name: name, startTime: startTime, - attributes: newAttributesMap(tr.provider.spanLimits.AttributeCountLimit), events: newEvictedQueue(tr.provider.spanLimits.EventCountLimit), links: newEvictedQueue(tr.provider.spanLimits.LinkCountLimit), tracer: tr,