Skip to content

Commit

Permalink
sdk/trace: use slices.Grow() to avoid excessive runtime.growslice() (#…
Browse files Browse the repository at this point in the history
…4818)

* sdk/trace: use slices.Grow() to avoid excessive runtime.growslice()

* go1.20 compat

* update go.mod/sum

* Update CHANGELOG.md

* Revert "update go.mod/sum"

This reverts commit 1e4fcfa.

* inline slices.Grow as ensureAttributesCapacity

* fix comment

* add bench

* fix inlining

* Apply suggestions from code review

Co-authored-by: Robert Pająk <[email protected]>
Co-authored-by: Damien Mathieu <[email protected]>

* add ReportAllocs

* add benchmark variations with lower limit

* bugfix: we always want to set cap, just the value is a min()

* lint

---------

Co-authored-by: Robert Pająk <[email protected]>
Co-authored-by: Damien Mathieu <[email protected]>
  • Loading branch information
3 people authored Jan 12, 2024
1 parent 19622d3 commit 4491b39
Show file tree
Hide file tree
Showing 3 changed files with 47 additions and 0 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
- Improve `go.opentelemetry.io/otel/baggage` performance. (#4743)
- Improve performance of the `(*Set).Filter` method in `go.opentelemetry.io/otel/attribute` when the passed filter does not filter out any attributes from the set. (#4774)
- `Member.String` in `go.opentelemetry.io/otel/baggage` percent-encodes only when necessary. (#4775)
- Improve `go.opentelemetry.io/otel/trace.Span`'s performance when adding multiple attributes. (#4818)
- `Property.Value` in `go.opentelemetry.io/otel/baggage` now returns a raw string instead of a percent-encoded value. (#4804)

### Fixed
Expand Down
17 changes: 17 additions & 0 deletions sdk/trace/span.go
Original file line number Diff line number Diff line change
Expand Up @@ -208,6 +208,16 @@ func (s *recordingSpan) SetStatus(code codes.Code, description string) {
s.status = status
}

// ensureAttributesCapacity inlines functionality from slices.Grow
// so that we can avoid needing to import golang.org/x/exp for go1.20.
// Once support for go1.20 is dropped, we can use slices.Grow available since go1.21 instead.
// Tracking issue: https://github.com/open-telemetry/opentelemetry-go/issues/4819.
func (s *recordingSpan) ensureAttributesCapacity(minCapacity int) {
if n := minCapacity - cap(s.attributes); n > 0 {
s.attributes = append(s.attributes[:cap(s.attributes)], make([]attribute.KeyValue, n)...)[:len(s.attributes)]
}
}

// SetAttributes sets attributes of this span.
//
// If a key from attributes already exists the value associated with that key
Expand Down Expand Up @@ -242,6 +252,7 @@ func (s *recordingSpan) SetAttributes(attributes ...attribute.KeyValue) {

// Otherwise, add without deduplication. When attributes are read they
// will be deduplicated, optimizing the operation.
s.ensureAttributesCapacity(len(s.attributes) + len(attributes))
for _, a := range attributes {
if !a.Valid() {
// Drop all invalid attributes.
Expand Down Expand Up @@ -277,6 +288,12 @@ func (s *recordingSpan) addOverCapAttrs(limit int, attrs []attribute.KeyValue) {

// Now that s.attributes is deduplicated, adding unique attributes up to
// the capacity of s will not over allocate s.attributes.
if sum := len(attrs) + len(s.attributes); sum < limit {
// After support for go1.20 is dropped, simplify if-else to min(sum, limit).
s.ensureAttributesCapacity(sum)
} else {
s.ensureAttributesCapacity(limit)
}
for _, a := range attrs {
if !a.Valid() {
// Drop all invalid attributes.
Expand Down
29 changes: 29 additions & 0 deletions sdk/trace/span_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ package trace

import (
"bytes"
"context"
"fmt"
"testing"

Expand Down Expand Up @@ -243,3 +244,31 @@ func TestTruncateAttr(t *testing.T) {
})
}
}

func BenchmarkRecordingSpanSetAttributes(b *testing.B) {
var attrs []attribute.KeyValue
for i := 0; i < 100; i++ {
attr := attribute.String(fmt.Sprintf("hello.attrib%d", i), fmt.Sprintf("goodbye.attrib%d", i))
attrs = append(attrs, attr)
}

ctx := context.Background()
for _, limit := range []bool{false, true} {
b.Run(fmt.Sprintf("WithLimit/%t", limit), func(b *testing.B) {
b.ReportAllocs()
sl := NewSpanLimits()
if limit {
sl.AttributeCountLimit = 50
}
tp := NewTracerProvider(WithSampler(AlwaysSample()), WithSpanLimits(sl))
tracer := tp.Tracer("tracer")

b.ResetTimer()
for n := 0; n < b.N; n++ {
_, span := tracer.Start(ctx, "span")
span.SetAttributes(attrs...)
span.End()
}
})
}
}

0 comments on commit 4491b39

Please sign in to comment.