From 4491b39db28770af8c1b2bec183cfaa426ef4b1f Mon Sep 17 00:00:00 2001
From: Liz Fong-Jones <lizf@honeycomb.io>
Date: Fri, 12 Jan 2024 13:42:36 -0800
Subject: [PATCH] sdk/trace: use slices.Grow() to avoid excessive
 runtime.growslice() (#4818)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

* 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 1e4fcfa86cb3c42a3ff113df419e16f9a02b3687.

* inline slices.Grow as ensureAttributesCapacity

* fix comment

* add bench

* fix inlining

* Apply suggestions from code review

Co-authored-by: Robert Pająk <pellared@hotmail.com>
Co-authored-by: Damien Mathieu <42@dmathieu.com>

* 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 <pellared@hotmail.com>
Co-authored-by: Damien Mathieu <42@dmathieu.com>
---
 CHANGELOG.md           |  1 +
 sdk/trace/span.go      | 17 +++++++++++++++++
 sdk/trace/span_test.go | 29 +++++++++++++++++++++++++++++
 3 files changed, 47 insertions(+)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 236b8a467a9..1603a98c621 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -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
diff --git a/sdk/trace/span.go b/sdk/trace/span.go
index 8a4a355ca81..85bc702a019 100644
--- a/sdk/trace/span.go
+++ b/sdk/trace/span.go
@@ -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
@@ -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.
@@ -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.
diff --git a/sdk/trace/span_test.go b/sdk/trace/span_test.go
index 20148a78702..3288b32b087 100644
--- a/sdk/trace/span_test.go
+++ b/sdk/trace/span_test.go
@@ -16,6 +16,7 @@ package trace
 
 import (
 	"bytes"
+	"context"
 	"fmt"
 	"testing"
 
@@ -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()
+			}
+		})
+	}
+}