Deprecate PutAttribute and introduce SetAttribute (#14041)

Part of #14016.

Author: dmathieu

.chloggen/deprecate-put-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: deprecation
+
+# 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: Deprecated `PutAttribute` helper method
+
+# One or more tracking issues or pull requests related to the change
+issues: [14016, 14041]
+
+# (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]

.chloggen/keyvalueandunit-equal.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 `Equal` method on the `KeyValueAndUnit` type
+
+# One or more tracking issues or pull requests related to the change
+issues: [14041]
+
+# (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]

.chloggen/pprofile-set-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 `SetAttribute` helper method
+
+# One or more tracking issues or pull requests related to the change
+issues: [14016, 14041]
+
+# (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]

pdata/pprofile/attributes.go

@@ -32,13 +32,15 @@ func FromAttributeIndices(table KeyValueAndUnitSlice, record attributable, dic P
 	return m
 }
 
-var errTooManyTableEntries = errors.New("too many entries in AttributeTable")
+var errTooManyAttributeTableEntries = errors.New("too many entries in AttributeTable")
 
 // PutAttribute updates an AttributeTable and a record's AttributeIndices to
 // add or update an attribute.
 // The assumption is that attributes are a map as for other signals (metrics, logs, etc.), thus
 // the same key must not appear twice in a list of attributes / attribute indices.
 // The record can be any struct that implements an `AttributeIndices` method.
+//
+// Deprecated: [v0.138.0] use SetAttribute instead.
 func PutAttribute(table KeyValueAndUnitSlice, record attributable, dic ProfilesDictionary, key string, value pcommon.Value) error {
 	for i := range record.AttributeIndices().Len() {
 		idx := int(record.AttributeIndices().At(i))
@@ -58,15 +60,15 @@ func PutAttribute(table KeyValueAndUnitSlice, record attributable, dic ProfilesD
 				a := table.At(j)
 				if dic.StringTable().At(int(a.KeyStrindex())) == key && a.Value().Equal(value) {
 					if j > math.MaxInt32 {
-						return errTooManyTableEntries
+						return errTooManyAttributeTableEntries
 					}
 					record.AttributeIndices().SetAt(i, int32(j)) //nolint:gosec // overflow checked
 					return nil
 				}
 			}
 
 			if table.Len() >= math.MaxInt32 {
-				return errTooManyTableEntries
+				return errTooManyAttributeTableEntries
 			}
 
 			// Find the key in the StringTable, or add it
@@ -94,7 +96,7 @@ func PutAttribute(table KeyValueAndUnitSlice, record attributable, dic ProfilesD
 		a := table.At(j)
 		if dic.StringTable().At(int(a.KeyStrindex())) == key && a.Value().Equal(value) {
 			if j > math.MaxInt32 {
-				return errTooManyTableEntries
+				return errTooManyAttributeTableEntries
 			}
 			// Add the index of the existing attribute to the indices.
 			record.AttributeIndices().Append(int32(j)) //nolint:gosec // overflow checked
@@ -103,7 +105,7 @@ func PutAttribute(table KeyValueAndUnitSlice, record attributable, dic ProfilesD
 	}
 
 	if table.Len() >= math.MaxInt32 {
-		return errTooManyTableEntries
+		return errTooManyAttributeTableEntries
 	}
 
 	// Find the key in the StringTable, or add it
@@ -121,3 +123,23 @@ func PutAttribute(table KeyValueAndUnitSlice, record attributable, dic ProfilesD
 	record.AttributeIndices().Append(int32(table.Len() - 1)) //nolint:gosec // overflow checked
 	return nil
 }
+
+// SetAttribute updates an AttributeTable, adding or providing a value and
+// returns its index.
+func SetAttribute(table KeyValueAndUnitSlice, attr KeyValueAndUnit) (int32, error) {
+	for j, a := range table.All() {
+		if a.Equal(attr) {
+			if j > math.MaxInt32 {
+				return 0, errTooManyAttributeTableEntries
+			}
+			return int32(j), nil //nolint:gosec // G115 overflow checked
+		}
+	}
+
+	if table.Len() >= math.MaxInt32 {
+		return 0, errTooManyAttributeTableEntries
+	}
+
+	attr.CopyTo(table.AppendEmpty())
+	return int32(table.Len() - 1), nil //nolint:gosec // G115 overflow checked
+}

pdata/pprofile/attributes_test.go

@@ -226,3 +226,112 @@ func BenchmarkPutAttribute(b *testing.B) {
 		})
 	}
 }
+
+func TestSetAttribute(t *testing.T) {
+	table := NewKeyValueAndUnitSlice()
+	attr := NewKeyValueAndUnit()
+	attr.SetKeyStrindex(1)
+	attr.SetUnitStrindex(2)
+	require.NoError(t, attr.Value().FromRaw("test"))
+	attr2 := NewKeyValueAndUnit()
+	attr2.SetKeyStrindex(3)
+	attr2.SetUnitStrindex(4)
+	require.NoError(t, attr.Value().FromRaw("test2"))
+
+	// Put a first value
+	idx, err := SetAttribute(table, attr)
+	require.NoError(t, err)
+	assert.Equal(t, 1, table.Len())
+	assert.Equal(t, int32(0), idx)
+
+	// Put the same attribute
+	// This should be a no-op.
+	idx, err = SetAttribute(table, attr)
+	require.NoError(t, err)
+	assert.Equal(t, 1, table.Len())
+	assert.Equal(t, int32(0), idx)
+
+	// Set a new value
+	// This sets the index and adds to the table.
+	idx, err = SetAttribute(table, attr2)
+	require.NoError(t, err)
+	assert.Equal(t, 2, table.Len())
+	assert.Equal(t, int32(table.Len()-1), idx) //nolint:gosec // G115
+
+	// Set an existing value
+	idx, err = SetAttribute(table, attr)
+	require.NoError(t, err)
+	assert.Equal(t, 2, table.Len())
+	assert.Equal(t, int32(0), idx)
+	// Set another existing value
+	idx, err = SetAttribute(table, attr2)
+	require.NoError(t, err)
+	assert.Equal(t, 2, table.Len())
+	assert.Equal(t, int32(table.Len()-1), idx) //nolint:gosec // G115
+}
+
+func BenchmarkSetAttribute(b *testing.B) {
+	for _, bb := range []struct {
+		name string
+		attr KeyValueAndUnit
+
+		runBefore func(*testing.B, KeyValueAndUnitSlice)
+	}{
+		{
+			name: "with a new attribute",
+			attr: NewKeyValueAndUnit(),
+		},
+		{
+			name: "with an existing attribute",
+			attr: func() KeyValueAndUnit {
+				a := NewKeyValueAndUnit()
+				a.SetKeyStrindex(1)
+				return a
+			}(),
+
+			runBefore: func(_ *testing.B, table KeyValueAndUnitSlice) {
+				a := table.AppendEmpty()
+				a.SetKeyStrindex(1)
+			},
+		},
+		{
+			name: "with a duplicate attribute",
+			attr: NewKeyValueAndUnit(),
+
+			runBefore: func(_ *testing.B, table KeyValueAndUnitSlice) {
+				_, err := SetAttribute(table, NewKeyValueAndUnit())
+				require.NoError(b, err)
+			},
+		},
+		{
+			name: "with a hundred locations to loop through",
+			attr: func() KeyValueAndUnit {
+				a := NewKeyValueAndUnit()
+				a.SetKeyStrindex(1)
+				return a
+			}(),
+
+			runBefore: func(_ *testing.B, table KeyValueAndUnitSlice) {
+				for i := range 100 {
+					l := table.AppendEmpty()
+					l.SetKeyStrindex(int32(i)) //nolint:gosec // overflow checked
+				}
+			},
+		},
+	} {
+		b.Run(bb.name, func(b *testing.B) {
+			table := NewKeyValueAndUnitSlice()
+
+			if bb.runBefore != nil {
+				bb.runBefore(b, table)
+			}
+
+			b.ResetTimer()
+			b.ReportAllocs()
+
+			for b.Loop() {
+				_, _ = SetAttribute(table, bb.attr)
+			}
+		})
+	}
+}

pdata/pprofile/keyvalueandunit.go

@@ -0,0 +1,12 @@
+// Copyright The OpenTelemetry Authors
+// SPDX-License-Identifier: Apache-2.0
+
+package pprofile // import "go.opentelemetry.io/collector/pdata/pprofile"
+
+// Equal checks equality with another KeyValueAndUnit
+// It assumes both structs refer to the same dictionary.
+func (ms KeyValueAndUnit) Equal(val KeyValueAndUnit) bool {
+	return ms.KeyStrindex() == val.KeyStrindex() &&
+		ms.UnitStrindex() == val.UnitStrindex() &&
+		ms.Value().Equal(val.Value())
+}

pdata/pprofile/keyvalueandunit_test.go

@@ -0,0 +1,68 @@
+// Copyright The OpenTelemetry Authors
+// SPDX-License-Identifier: Apache-2.0
+
+package pprofile
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+
+	"go.opentelemetry.io/collector/pdata/pcommon"
+)
+
+func TestKeyValueAndUnitEqual(t *testing.T) {
+	for _, tt := range []struct {
+		name string
+		orig KeyValueAndUnit
+		dest KeyValueAndUnit
+		want bool
+	}{
+		{
+			name: "empty keyvalueandunit",
+			orig: NewKeyValueAndUnit(),
+			dest: NewKeyValueAndUnit(),
+			want: true,
+		},
+		{
+			name: "non-empty identical keyvalueandunit",
+			orig: buildKeyValueAndUnit(1, 2, pcommon.NewValueStr("test")),
+			dest: buildKeyValueAndUnit(1, 2, pcommon.NewValueStr("test")),
+			want: true,
+		},
+		{
+			name: "with different key index",
+			orig: buildKeyValueAndUnit(1, 2, pcommon.NewValueStr("test")),
+			dest: buildKeyValueAndUnit(2, 2, pcommon.NewValueStr("test")),
+			want: false,
+		},
+		{
+			name: "with different unit index",
+			orig: buildKeyValueAndUnit(1, 2, pcommon.NewValueStr("test")),
+			dest: buildKeyValueAndUnit(1, 3, pcommon.NewValueStr("test")),
+			want: false,
+		},
+		{
+			name: "with different value",
+			orig: buildKeyValueAndUnit(1, 2, pcommon.NewValueStr("test")),
+			dest: buildKeyValueAndUnit(1, 2, pcommon.NewValueStr("hello")),
+			want: false,
+		},
+	} {
+		t.Run(tt.name, func(t *testing.T) {
+			if tt.want {
+				assert.True(t, tt.orig.Equal(tt.dest))
+			} else {
+				assert.False(t, tt.orig.Equal(tt.dest))
+			}
+		})
+	}
+}
+
+func buildKeyValueAndUnit(keyIdx, unitIdx int32, val pcommon.Value) KeyValueAndUnit {
+	kvu := NewKeyValueAndUnit()
+	kvu.SetKeyStrindex(keyIdx)
+	kvu.SetUnitStrindex(unitIdx)
+	val.CopyTo(kvu.Value())
+	return kvu
+}