introduce pprofile.AddAttribute helper (#12390)

#### Description This introduces the `pprofile.AddAttribute` helper method so profile extensions can modify attributes. Closes #12206 ``` BenchmarkAddAttribute BenchmarkAddAttribute/with_a_new_string_attribute BenchmarkAddAttribute/with_a_new_string_attribute-10 30461164 39.55 ns/op 32 B/op 2 allocs/op BenchmarkAddAttribute/with_an_existing_attribute BenchmarkAddAttribute/with_an_existing_attribute-10 30714298 39.44 ns/op 32 B/op 2 allocs/op BenchmarkAddAttribute/with_a_duplicate_attribute BenchmarkAddAttribute/with_a_duplicate_attribute-10 29132547 39.47 ns/op 32 B/op 2 allocs/op BenchmarkAddAttribute/with_a_hundred_attributes_to_loop_through BenchmarkAddAttribute/with_a_hundred_attributes_to_loop_through-10 6762793 176.8 ns/op 32 B/op 2 allocs/op ``` Those benchmarks kind of have an N+1 issue, where the method will take longer the more attributes there are. However, since the attribute table is a map, we can't go around with looping through it. I also had to use `FromRaw` and `AsRaw` in there, to properly handle `any` values. This currently doesn't support removing entries. But doing so would also have a non-trivial performance impact, since we'd have to loop through the `AttributeIndices` slice. --------- Co-authored-by: Tim Rühsen <[email protected]> Co-authored-by: Joshua MacDonald <[email protected]>
open-telemetry · Mar 5, 2025 · 72a8471 · 72a8471
1 parent e43c36c
commit 72a8471
Show file tree

Hide file tree

Showing 3 changed files with 167 additions and 0 deletions.
diff --git a/.chloggen/pprofile-add-attribute.yaml b/.chloggen/pprofile-add-attribute.yaml
@@ -0,0 +1,25 @@
+# Use this changelog template to create an entry for release notes.
+
+# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
+change_type: enhancement
+
+# The name of the component, or a single word describing the area of concern, (e.g. otlpreceiver)
+component: pdata/pprofile
+
+# A brief description of the change.  Surround your text with quotes ("") if it needs to start with a backtick (`).
+note: Introduce AddAttribute helper method to modify the content of attributable records
+
+# One or more tracking issues or pull requests related to the change
+issues: [12206]
+
+# (Optional) One or more lines of additional information to render under the primary note.
+# These lines will be padded with 2 spaces and then inserted directly into the document.
+# Use pipe (|) for multiline entries.
+subtext:
+
+# Optional: The change log or logs in which this entry should be included.
+# e.g. '[user]' or '[user, api]'
+# Include 'user' if the change is relevant to end users.
+# Include 'api' if there is a change to a library API.
+# Default: '[user]'
+change_logs: [api]
diff --git a/pdata/pprofile/attributes.go b/pdata/pprofile/attributes.go
@@ -4,6 +4,10 @@
 package pprofile // import "go.opentelemetry.io/collector/pdata/pprofile"
 
 import (
+	"errors"
+	"fmt"
+	"math"
+
 	"go.opentelemetry.io/collector/pdata/pcommon"
 )
 
@@ -26,3 +30,41 @@ func FromAttributeIndices(table AttributeTableSlice, record attributable) pcommo
 
 	return m
 }
+
+// AddAttribute updates an AttributeTable and a record's AttributeIndices to
+// add a new attribute.
+// The record can by any struct that implements an `AttributeIndices` method.
+func AddAttribute(table AttributeTableSlice, record attributable, key string, value any) error {
+	for i := range table.Len() {
+		a := table.At(i)
+
+		if a.Key() == key && value == a.Value().AsRaw() {
+			if i >= math.MaxInt32 {
+				return fmt.Errorf("Attribute %s=%#v has too high an index to be added to AttributeIndices", key, value)
+			}
+
+			for j := range record.AttributeIndices().Len() {
+				v := record.AttributeIndices().At(j)
+				if v == int32(i) { //nolint:gosec // overflow checked
+					return nil
+				}
+			}
+
+			record.AttributeIndices().Append(int32(i)) //nolint:gosec // overflow checked
+			return nil
+		}
+	}
+
+	if table.Len() >= math.MaxInt32 {
+		return errors.New("AttributeTable can't take more attributes")
+	}
+	table.EnsureCapacity(table.Len() + 1)
+	entry := table.AppendEmpty()
+	entry.SetKey(key)
+	if err := entry.Value().FromRaw(value); err != nil {
+		return err
+	}
+	record.AttributeIndices().Append(int32(table.Len()) - 1) //nolint:gosec // overflow checked
+
+	return nil
+}
diff --git a/pdata/pprofile/attributes_test.go b/pdata/pprofile/attributes_test.go
@@ -8,6 +8,7 @@ import (
 	"testing"
 
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 
 	"go.opentelemetry.io/collector/pdata/pcommon"
 )
@@ -43,6 +44,36 @@ func TestFromAttributeIndices(t *testing.T) {
 	assert.Equal(t, attrs.AsRaw(), m)
 }
 
+func TestAddAttribute(t *testing.T) {
+	table := NewAttributeTableSlice()
+	att := table.AppendEmpty()
+	att.SetKey("hello")
+	att.Value().SetStr("world")
+
+	// Add a brand new attribute
+	loc := NewLocation()
+	err := AddAttribute(table, loc, "bonjour", "monde")
+	require.NoError(t, err)
+
+	assert.Equal(t, 2, table.Len())
+	assert.Equal(t, []int32{1}, loc.AttributeIndices().AsRaw())
+
+	// Add an already existing attribute
+	mapp := NewMapping()
+	err = AddAttribute(table, mapp, "hello", "world")
+	require.NoError(t, err)
+
+	assert.Equal(t, 2, table.Len())
+	assert.Equal(t, []int32{0}, mapp.AttributeIndices().AsRaw())
+
+	// Add a duplicate attribute
+	err = AddAttribute(table, mapp, "hello", "world")
+	require.NoError(t, err)
+
+	assert.Equal(t, 2, table.Len())
+	assert.Equal(t, []int32{0}, mapp.AttributeIndices().AsRaw())
+}
+
 func BenchmarkFromAttributeIndices(b *testing.B) {
 	table := NewAttributeTableSlice()
 
@@ -62,3 +93,72 @@ func BenchmarkFromAttributeIndices(b *testing.B) {
 		_ = FromAttributeIndices(table, obj)
 	}
 }
+
+func BenchmarkAddAttribute(b *testing.B) {
+	for _, bb := range []struct {
+		name  string
+		key   string
+		value any
+
+		runBefore func(*testing.B, AttributeTableSlice, attributable)
+	}{
+		{
+			name:  "with a new string attribute",
+			key:   "attribute",
+			value: "test",
+		},
+		{
+			name:  "with an existing attribute",
+			key:   "attribute",
+			value: "test",
+
+			runBefore: func(_ *testing.B, table AttributeTableSlice, _ attributable) {
+				entry := table.AppendEmpty()
+				entry.SetKey("attribute")
+				entry.Value().SetStr("test")
+			},
+		},
+		{
+			name:  "with a duplicate attribute",
+			key:   "attribute",
+			value: "test",
+
+			runBefore: func(_ *testing.B, table AttributeTableSlice, obj attributable) {
+				require.NoError(b, AddAttribute(table, obj, "attribute", "test"))
+			},
+		},
+		{
+			name:  "with a hundred attributes to loop through",
+			key:   "attribute",
+			value: "test",
+
+			runBefore: func(_ *testing.B, table AttributeTableSlice, _ attributable) {
+				for i := range 100 {
+					entry := table.AppendEmpty()
+					entry.SetKey(fmt.Sprintf("attr_%d", i))
+					entry.Value().SetStr("test")
+				}
+
+				entry := table.AppendEmpty()
+				entry.SetKey("attribute")
+				entry.Value().SetStr("test")
+			},
+		},
+	} {
+		b.Run(bb.name, func(b *testing.B) {
+			table := NewAttributeTableSlice()
+			obj := NewLocation()
+
+			if bb.runBefore != nil {
+				bb.runBefore(b, table, obj)
+			}
+
+			b.ResetTimer()
+			b.ReportAllocs()
+
+			for n := 0; n < b.N; n++ {
+				_ = AddAttribute(table, obj, bb.key, bb.value)
+			}
+		})
+	}
+}