diff --git a/.chloggen/profiles-PutAttributes.yaml b/.chloggen/profiles-PutAttributes.yaml new file mode 100644 index 00000000000..ccdedbbffd9 --- /dev/null +++ b/.chloggen/profiles-PutAttributes.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/profile + +# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`). +note: Replace AddAttribute with the PutAttribute helper method to modify the content of attributable records. + +# One or more tracking issues or pull requests related to the change +issues: [12798] + +# (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 index 34bb88cc2ef..232d7e4b68e 100644 --- a/pdata/pprofile/attributes.go +++ b/pdata/pprofile/attributes.go @@ -17,7 +17,7 @@ type attributable interface { // FromAttributeIndices builds a [pcommon.Map] containing the attributes of a // record. -// The record can by any struct that implements an `AttributeIndices` method. +// The record can be any struct that implements an `AttributeIndices` method. // Updates made to the return map will not be applied back to the record. func FromAttributeIndices(table AttributeTableSlice, record attributable) pcommon.Map { m := pcommon.NewMap() @@ -31,9 +31,7 @@ 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. +// Deprecated: [v0.126.0] Use PutAttribute instead. func AddAttribute(table AttributeTableSlice, record attributable, key string, value pcommon.Value) error { for i := range table.Len() { a := table.At(i) @@ -66,3 +64,80 @@ func AddAttribute(table AttributeTableSlice, record attributable, key string, va return nil } + +var errTooManyTableEntries = 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. +func PutAttribute(table AttributeTableSlice, record attributable, key string, value pcommon.Value) error { + for i := range record.AttributeIndices().Len() { + idx := int(record.AttributeIndices().At(i)) + if idx < 0 || idx >= table.Len() { + return fmt.Errorf("index value %d out of range in AttributeIndices[%d]", idx, i) + } + attr := table.At(idx) + if attr.Key() == key { + if attr.Value().Equal(value) { + // Attribute already exists, nothing to do. + return nil + } + + // If the attribute table already contains the key/value pair, just update the index. + for j := range table.Len() { + a := table.At(j) + if a.Key() == key && a.Value().Equal(value) { + if j > math.MaxInt32 { + return errTooManyTableEntries + } + record.AttributeIndices().SetAt(i, int32(j)) //nolint:gosec // overflow checked + return nil + } + } + + if table.Len() >= math.MaxInt32 { + return errTooManyTableEntries + } + + // Add the key/value pair as a new attribute to the table... + entry := table.AppendEmpty() + entry.SetKey(key) + value.CopyTo(entry.Value()) + + // ...and update the existing index. + record.AttributeIndices().SetAt(i, int32(table.Len()-1)) //nolint:gosec // overflow checked + return nil + } + } + + if record.AttributeIndices().Len() >= math.MaxInt32 { + return errors.New("too many entries in AttributeIndices") + } + + for j := range table.Len() { + a := table.At(j) + if a.Key() == key && a.Value().Equal(value) { + if j > math.MaxInt32 { + return errTooManyTableEntries + } + // Add the index of the existing attribute to the indices. + record.AttributeIndices().Append(int32(j)) //nolint:gosec // overflow checked + return nil + } + } + + if table.Len() >= math.MaxInt32 { + return errTooManyTableEntries + } + + // Add the key/value pair as a new attribute to the table... + entry := table.AppendEmpty() + entry.SetKey(key) + value.CopyTo(entry.Value()) + + // ...and add a new index to the indices. + 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 index 1524bac6ab0..fed4ff8d4d3 100644 --- a/pdata/pprofile/attributes_test.go +++ b/pdata/pprofile/attributes_test.go @@ -13,6 +13,36 @@ import ( "go.opentelemetry.io/collector/pdata/pcommon" ) +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", pcommon.NewValueStr("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", pcommon.NewValueStr("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", pcommon.NewValueStr("world")) + require.NoError(t, err) + + assert.Equal(t, 2, table.Len()) + assert.Equal(t, []int32{0}, mapp.AttributeIndices().AsRaw()) +} + func TestFromAttributeIndices(t *testing.T) { table := NewAttributeTableSlice() att := table.AppendEmpty() @@ -44,34 +74,85 @@ func TestFromAttributeIndices(t *testing.T) { assert.Equal(t, attrs.AsRaw(), m) } -func TestAddAttribute(t *testing.T) { +func testPutAttribute(t *testing.T, record attributable) { table := NewAttributeTableSlice() - att := table.AppendEmpty() - att.SetKey("hello") - att.Value().SetStr("world") - - // Add a brand new attribute - loc := NewLocation() - err := AddAttribute(table, loc, "bonjour", pcommon.NewValueStr("monde")) - require.NoError(t, err) + // Put a first attribute. + require.NoError(t, PutAttribute(table, record, "hello", pcommon.NewValueStr("world"))) + assert.Equal(t, 1, table.Len()) + assert.Equal(t, []int32{0}, record.AttributeIndices().AsRaw()) + + // Put an attribute, same key, same value. + // This should be a no-op. + require.NoError(t, PutAttribute(table, record, "hello", pcommon.NewValueStr("world"))) + assert.Equal(t, 1, table.Len()) + assert.Equal(t, []int32{0}, record.AttributeIndices().AsRaw()) + + // Special case: removing and adding again should not change the table as + // this can lead to multiple identical attributes in the table. + record.AttributeIndices().FromRaw([]int32{}) + require.NoError(t, PutAttribute(table, record, "hello", pcommon.NewValueStr("world"))) + assert.Equal(t, 1, table.Len()) + assert.Equal(t, []int32{0}, record.AttributeIndices().AsRaw()) + + // Put an attribute, same key, different value. + // This updates the index and adds to the table. + fmt.Printf("test\n") + require.NoError(t, PutAttribute(table, record, "hello", pcommon.NewValueStr("world2"))) 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", pcommon.NewValueStr("world")) - require.NoError(t, err) + assert.Equal(t, []int32{1}, record.AttributeIndices().AsRaw()) + // Put an attribute that already exists in the table. + // This updates the index and does not add to the table. + require.NoError(t, PutAttribute(table, record, "hello", pcommon.NewValueStr("world"))) assert.Equal(t, 2, table.Len()) - assert.Equal(t, []int32{0}, mapp.AttributeIndices().AsRaw()) + assert.Equal(t, []int32{0}, record.AttributeIndices().AsRaw()) + + // Put a new attribute. + // This adds an index and adds to the table. + require.NoError(t, PutAttribute(table, record, "good", pcommon.NewValueStr("day"))) + assert.Equal(t, 3, table.Len()) + assert.Equal(t, []int32{0, 2}, record.AttributeIndices().AsRaw()) + + // Add multiple distinct attributes. + for i := range 100 { + require.NoError(t, PutAttribute(table, record, fmt.Sprintf("key_%d", i), pcommon.NewValueStr("day"))) + assert.Equal(t, i+4, table.Len()) + assert.Equal(t, i+3, record.AttributeIndices().Len()) + } - // Add a duplicate attribute - err = AddAttribute(table, mapp, "hello", pcommon.NewValueStr("world")) - require.NoError(t, err) + // Add a negative index to the record. + record.AttributeIndices().Append(-1) + tableLen := table.Len() + indicesLen := record.AttributeIndices().Len() + // Try putting a new attribute, make sure it fails, and that table/indices didn't change. + require.Error(t, PutAttribute(table, record, "newKey", pcommon.NewValueStr("value"))) + require.Equal(t, tableLen, table.Len()) + require.Equal(t, indicesLen, record.AttributeIndices().Len()) + + // Set the last index to the table length, which is out of range. + record.AttributeIndices().SetAt(indicesLen-1, int32(tableLen)) //nolint:gosec + // Try putting a new attribute, make sure it fails, and that table/indices didn't change. + require.Error(t, PutAttribute(table, record, "newKey", pcommon.NewValueStr("value"))) + require.Equal(t, tableLen, table.Len()) + require.Equal(t, indicesLen, record.AttributeIndices().Len()) +} - assert.Equal(t, 2, table.Len()) - assert.Equal(t, []int32{0}, mapp.AttributeIndices().AsRaw()) +func TestPutAttribute(t *testing.T) { + // Test every existing record type. + for _, tt := range []struct { + name string + attr attributable + }{ + {"Profile", NewProfile()}, + {"Sample", NewSample()}, + {"Mapping", NewMapping()}, + {"Location", NewLocation()}, + } { + t.Run(tt.name, func(t *testing.T) { + testPutAttribute(t, tt.attr) + }) + } } func BenchmarkFromAttributeIndices(b *testing.B) { @@ -94,7 +175,7 @@ func BenchmarkFromAttributeIndices(b *testing.B) { } } -func BenchmarkAddAttribute(b *testing.B) { +func BenchmarkPutAttribute(b *testing.B) { for _, bb := range []struct { name string key string @@ -124,7 +205,7 @@ func BenchmarkAddAttribute(b *testing.B) { value: pcommon.NewValueStr("test"), runBefore: func(_ *testing.B, table AttributeTableSlice, obj attributable) { - require.NoError(b, AddAttribute(table, obj, "attribute", pcommon.NewValueStr("test"))) + require.NoError(b, PutAttribute(table, obj, "attribute", pcommon.NewValueStr("test"))) }, }, { @@ -157,7 +238,7 @@ func BenchmarkAddAttribute(b *testing.B) { b.ReportAllocs() for n := 0; n < b.N; n++ { - _ = AddAttribute(table, obj, bb.key, bb.value) + _ = PutAttribute(table, obj, bb.key, bb.value) } }) }