Skip to content

Commit 72a8471

Browse files
dmathieurockdabootjmacd
authored
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]>
1 parent e43c36c commit 72a8471

File tree

3 files changed

+167
-0
lines changed

3 files changed

+167
-0
lines changed

Diff for: .chloggen/pprofile-add-attribute.yaml

+25
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
# Use this changelog template to create an entry for release notes.
2+
3+
# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
4+
change_type: enhancement
5+
6+
# The name of the component, or a single word describing the area of concern, (e.g. otlpreceiver)
7+
component: pdata/pprofile
8+
9+
# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
10+
note: Introduce AddAttribute helper method to modify the content of attributable records
11+
12+
# One or more tracking issues or pull requests related to the change
13+
issues: [12206]
14+
15+
# (Optional) One or more lines of additional information to render under the primary note.
16+
# These lines will be padded with 2 spaces and then inserted directly into the document.
17+
# Use pipe (|) for multiline entries.
18+
subtext:
19+
20+
# Optional: The change log or logs in which this entry should be included.
21+
# e.g. '[user]' or '[user, api]'
22+
# Include 'user' if the change is relevant to end users.
23+
# Include 'api' if there is a change to a library API.
24+
# Default: '[user]'
25+
change_logs: [api]

Diff for: pdata/pprofile/attributes.go

+42
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,10 @@
44
package pprofile // import "go.opentelemetry.io/collector/pdata/pprofile"
55

66
import (
7+
"errors"
8+
"fmt"
9+
"math"
10+
711
"go.opentelemetry.io/collector/pdata/pcommon"
812
)
913

@@ -26,3 +30,41 @@ func FromAttributeIndices(table AttributeTableSlice, record attributable) pcommo
2630

2731
return m
2832
}
33+
34+
// AddAttribute updates an AttributeTable and a record's AttributeIndices to
35+
// add a new attribute.
36+
// The record can by any struct that implements an `AttributeIndices` method.
37+
func AddAttribute(table AttributeTableSlice, record attributable, key string, value any) error {
38+
for i := range table.Len() {
39+
a := table.At(i)
40+
41+
if a.Key() == key && value == a.Value().AsRaw() {
42+
if i >= math.MaxInt32 {
43+
return fmt.Errorf("Attribute %s=%#v has too high an index to be added to AttributeIndices", key, value)
44+
}
45+
46+
for j := range record.AttributeIndices().Len() {
47+
v := record.AttributeIndices().At(j)
48+
if v == int32(i) { //nolint:gosec // overflow checked
49+
return nil
50+
}
51+
}
52+
53+
record.AttributeIndices().Append(int32(i)) //nolint:gosec // overflow checked
54+
return nil
55+
}
56+
}
57+
58+
if table.Len() >= math.MaxInt32 {
59+
return errors.New("AttributeTable can't take more attributes")
60+
}
61+
table.EnsureCapacity(table.Len() + 1)
62+
entry := table.AppendEmpty()
63+
entry.SetKey(key)
64+
if err := entry.Value().FromRaw(value); err != nil {
65+
return err
66+
}
67+
record.AttributeIndices().Append(int32(table.Len()) - 1) //nolint:gosec // overflow checked
68+
69+
return nil
70+
}

Diff for: pdata/pprofile/attributes_test.go

+100
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import (
88
"testing"
99

1010
"github.com/stretchr/testify/assert"
11+
"github.com/stretchr/testify/require"
1112

1213
"go.opentelemetry.io/collector/pdata/pcommon"
1314
)
@@ -43,6 +44,36 @@ func TestFromAttributeIndices(t *testing.T) {
4344
assert.Equal(t, attrs.AsRaw(), m)
4445
}
4546

47+
func TestAddAttribute(t *testing.T) {
48+
table := NewAttributeTableSlice()
49+
att := table.AppendEmpty()
50+
att.SetKey("hello")
51+
att.Value().SetStr("world")
52+
53+
// Add a brand new attribute
54+
loc := NewLocation()
55+
err := AddAttribute(table, loc, "bonjour", "monde")
56+
require.NoError(t, err)
57+
58+
assert.Equal(t, 2, table.Len())
59+
assert.Equal(t, []int32{1}, loc.AttributeIndices().AsRaw())
60+
61+
// Add an already existing attribute
62+
mapp := NewMapping()
63+
err = AddAttribute(table, mapp, "hello", "world")
64+
require.NoError(t, err)
65+
66+
assert.Equal(t, 2, table.Len())
67+
assert.Equal(t, []int32{0}, mapp.AttributeIndices().AsRaw())
68+
69+
// Add a duplicate attribute
70+
err = AddAttribute(table, mapp, "hello", "world")
71+
require.NoError(t, err)
72+
73+
assert.Equal(t, 2, table.Len())
74+
assert.Equal(t, []int32{0}, mapp.AttributeIndices().AsRaw())
75+
}
76+
4677
func BenchmarkFromAttributeIndices(b *testing.B) {
4778
table := NewAttributeTableSlice()
4879

@@ -62,3 +93,72 @@ func BenchmarkFromAttributeIndices(b *testing.B) {
6293
_ = FromAttributeIndices(table, obj)
6394
}
6495
}
96+
97+
func BenchmarkAddAttribute(b *testing.B) {
98+
for _, bb := range []struct {
99+
name string
100+
key string
101+
value any
102+
103+
runBefore func(*testing.B, AttributeTableSlice, attributable)
104+
}{
105+
{
106+
name: "with a new string attribute",
107+
key: "attribute",
108+
value: "test",
109+
},
110+
{
111+
name: "with an existing attribute",
112+
key: "attribute",
113+
value: "test",
114+
115+
runBefore: func(_ *testing.B, table AttributeTableSlice, _ attributable) {
116+
entry := table.AppendEmpty()
117+
entry.SetKey("attribute")
118+
entry.Value().SetStr("test")
119+
},
120+
},
121+
{
122+
name: "with a duplicate attribute",
123+
key: "attribute",
124+
value: "test",
125+
126+
runBefore: func(_ *testing.B, table AttributeTableSlice, obj attributable) {
127+
require.NoError(b, AddAttribute(table, obj, "attribute", "test"))
128+
},
129+
},
130+
{
131+
name: "with a hundred attributes to loop through",
132+
key: "attribute",
133+
value: "test",
134+
135+
runBefore: func(_ *testing.B, table AttributeTableSlice, _ attributable) {
136+
for i := range 100 {
137+
entry := table.AppendEmpty()
138+
entry.SetKey(fmt.Sprintf("attr_%d", i))
139+
entry.Value().SetStr("test")
140+
}
141+
142+
entry := table.AppendEmpty()
143+
entry.SetKey("attribute")
144+
entry.Value().SetStr("test")
145+
},
146+
},
147+
} {
148+
b.Run(bb.name, func(b *testing.B) {
149+
table := NewAttributeTableSlice()
150+
obj := NewLocation()
151+
152+
if bb.runBefore != nil {
153+
bb.runBefore(b, table, obj)
154+
}
155+
156+
b.ResetTimer()
157+
b.ReportAllocs()
158+
159+
for n := 0; n < b.N; n++ {
160+
_ = AddAttribute(table, obj, bb.key, bb.value)
161+
}
162+
})
163+
}
164+
}

0 commit comments

Comments
 (0)