From 69c7ebd226a45e0b3af07affb2024b4241c5eedb Mon Sep 17 00:00:00 2001 From: Florian Lehner Date: Tue, 19 Aug 2025 14:36:04 +0200 Subject: [PATCH 01/10] [pkg/ottl]: Create ctxprofilecommon for common attribute handling in various profiling sub messages Most Profiling messages do have some attributes. Create ctxprofilecommon for shared functionality. Signed-off-by: Florian Lehner --- .chloggen/ctxprofilecommon.yaml | 27 ++++ .../contexts/internal/ctxprofile/context.go | 2 + .../contexts/internal/ctxprofile/profile.go | 64 +-------- .../internal/ctxprofile/profile_test.go | 9 +- .../internal/ctxprofilecommon/attributes.go | 126 ++++++++++++++++++ .../ctxprofilecommon/attributes_test.go | 117 ++++++++++++++++ .../internal/ctxprofilesample/context.go | 6 +- .../ctxprofilesample/profilesample.go | 66 +-------- .../ctxprofilesample/profilesample_test.go | 10 +- pkg/ottl/contexts/ottlprofile/profile.go | 4 + .../ottlprofilesample/profilesample.go | 4 + 11 files changed, 308 insertions(+), 127 deletions(-) create mode 100644 .chloggen/ctxprofilecommon.yaml create mode 100644 pkg/ottl/contexts/internal/ctxprofilecommon/attributes.go create mode 100644 pkg/ottl/contexts/internal/ctxprofilecommon/attributes_test.go diff --git a/.chloggen/ctxprofilecommon.yaml b/.chloggen/ctxprofilecommon.yaml new file mode 100644 index 0000000000000..38454095a2485 --- /dev/null +++ b/.chloggen/ctxprofilecommon.yaml @@ -0,0 +1,27 @@ +# 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. filelogreceiver) +component: pkg/ottl + +# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`). +note: Create ctxprofilecommon for common attribute handling in various profiling sub messages + +# Mandatory: One or more tracking issues related to the change. You can use the PR number here if no issue exists. +issues: [42107] + +# (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: + +# If your change doesn't affect end users or the exported elements of any package, +# you should instead start your pull request title with [chore] or use the "Skip Changelog" label. +# 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/pkg/ottl/contexts/internal/ctxprofile/context.go b/pkg/ottl/contexts/internal/ctxprofile/context.go index 806f9ce8d422c..9706978b239cd 100644 --- a/pkg/ottl/contexts/internal/ctxprofile/context.go +++ b/pkg/ottl/contexts/internal/ctxprofile/context.go @@ -3,6 +3,7 @@ package ctxprofile // import "github.com/open-telemetry/opentelemetry-collector-contrib/pkg/ottl/contexts/internal/ctxprofile" import ( + "go.opentelemetry.io/collector/pdata/pcommon" "go.opentelemetry.io/collector/pdata/pprofile" ) @@ -12,6 +13,7 @@ const ( ) type Context interface { + AttributeIndices() pcommon.Int32Slice GetProfile() pprofile.Profile GetProfilesDictionary() pprofile.ProfilesDictionary } diff --git a/pkg/ottl/contexts/internal/ctxprofile/profile.go b/pkg/ottl/contexts/internal/ctxprofile/profile.go index b4734d02b1026..39ccaaa3d04ac 100644 --- a/pkg/ottl/contexts/internal/ctxprofile/profile.go +++ b/pkg/ottl/contexts/internal/ctxprofile/profile.go @@ -15,6 +15,7 @@ import ( "github.com/open-telemetry/opentelemetry-collector-contrib/pkg/ottl" "github.com/open-telemetry/opentelemetry-collector-contrib/pkg/ottl/contexts/internal/ctxcommon" "github.com/open-telemetry/opentelemetry-collector-contrib/pkg/ottl/contexts/internal/ctxerror" + "github.com/open-telemetry/opentelemetry-collector-contrib/pkg/ottl/contexts/internal/ctxprofilecommon" "github.com/open-telemetry/opentelemetry-collector-contrib/pkg/ottl/contexts/internal/ctxutil" ) @@ -64,9 +65,9 @@ func PathGetSetter[K Context](path ottl.Path[K]) (ottl.GetSetter[K], error) { return accessOriginalPayload[K](), nil case "attributes": if path.Keys() == nil { - return accessAttributes[K](), nil + return ctxprofilecommon.AccessAttributes[K](), nil } - return accessAttributesKey(path.Keys()), nil + return ctxprofilecommon.AccessAttributesKey[K](path.Keys()), nil default: return nil, ctxerror.New(path.Name(), path.String(), Name, DocRef) } @@ -311,62 +312,3 @@ func accessOriginalPayload[K Context]() ottl.StandardGetSetter[K] { }, } } - -func accessAttributes[K Context]() ottl.StandardGetSetter[K] { - return ottl.StandardGetSetter[K]{ - Getter: func(_ context.Context, tCtx K) (any, error) { - return pprofile.FromAttributeIndices(tCtx.GetProfilesDictionary().AttributeTable(), tCtx.GetProfile()), nil - }, - Setter: func(_ context.Context, tCtx K, val any) error { - m, err := ctxutil.GetMap(val) - if err != nil { - return err - } - tCtx.GetProfile().AttributeIndices().FromRaw([]int32{}) - for k, v := range m.All() { - if err := pprofile.PutAttribute(tCtx.GetProfilesDictionary().AttributeTable(), tCtx.GetProfile(), k, v); err != nil { - return err - } - } - return nil - }, - } -} - -func accessAttributesKey[K Context](key []ottl.Key[K]) ottl.StandardGetSetter[K] { - return ottl.StandardGetSetter[K]{ - Getter: func(ctx context.Context, tCtx K) (any, error) { - return ctxutil.GetMapValue[K](ctx, tCtx, pprofile.FromAttributeIndices(tCtx.GetProfilesDictionary().AttributeTable(), tCtx.GetProfile()), key) - }, - Setter: func(ctx context.Context, tCtx K, val any) error { - newKey, err := ctxutil.GetMapKeyName(ctx, tCtx, key[0]) - if err != nil { - return err - } - v := getAttributeValue(tCtx, *newKey) - err = ctxutil.SetIndexableValue[K](ctx, tCtx, v, val, key[1:]) - if err != nil { - return err - } - return pprofile.PutAttribute(tCtx.GetProfilesDictionary().AttributeTable(), tCtx.GetProfile(), *newKey, v) - }, - } -} - -func getAttributeValue[K Context](tCtx K, key string) pcommon.Value { - // Find the index of the attribute in the profile's attribute indices - // and return the corresponding value from the attribute table. - table := tCtx.GetProfilesDictionary().AttributeTable() - indices := tCtx.GetProfile().AttributeIndices().AsRaw() - - for _, tableIndex := range indices { - attr := table.At(int(tableIndex)) - if attr.Key() == key { - v := pcommon.NewValueEmpty() - attr.Value().CopyTo(v) - return v - } - } - - return pcommon.NewValueEmpty() -} diff --git a/pkg/ottl/contexts/internal/ctxprofile/profile_test.go b/pkg/ottl/contexts/internal/ctxprofile/profile_test.go index abbe58d49832b..7e44757b44444 100644 --- a/pkg/ottl/contexts/internal/ctxprofile/profile_test.go +++ b/pkg/ottl/contexts/internal/ctxprofile/profile_test.go @@ -205,8 +205,15 @@ func (p *profileContext) GetProfile() pprofile.Profile { return p.profile } +func (p *profileContext) AttributeIndices() pcommon.Int32Slice { + return p.profile.AttributeIndices() +} + func newProfileContext(profile pprofile.Profile, dictionary pprofile.ProfilesDictionary) *profileContext { - return &profileContext{profile: profile, dictionary: dictionary} + return &profileContext{ + profile: profile, + dictionary: dictionary, + } } func createValueTypeSlice() pprofile.ValueTypeSlice { diff --git a/pkg/ottl/contexts/internal/ctxprofilecommon/attributes.go b/pkg/ottl/contexts/internal/ctxprofilecommon/attributes.go new file mode 100644 index 0000000000000..bf08b0bb1585f --- /dev/null +++ b/pkg/ottl/contexts/internal/ctxprofilecommon/attributes.go @@ -0,0 +1,126 @@ +// Copyright The OpenTelemetry Authors +// SPDX-License-Identifier: Apache-2.0 + +package ctxprofilecommon // import "github.com/open-telemetry/opentelemetry-collector-contrib/pkg/ottl/contexts/internal/ctxprofilecommon" + +import ( + "context" + + "go.opentelemetry.io/collector/pdata/pcommon" + "go.opentelemetry.io/collector/pdata/pprofile" + + "github.com/open-telemetry/opentelemetry-collector-contrib/pkg/ottl" + "github.com/open-telemetry/opentelemetry-collector-contrib/pkg/ottl/contexts/internal/ctxutil" +) + +type ProfileAttributeContext interface { + AttributeIndices() pcommon.Int32Slice + GetProfilesDictionary() pprofile.ProfilesDictionary +} + +func AccessAttributes[K ProfileAttributeContext]() ottl.StandardGetSetter[K] { + return ottl.StandardGetSetter[K]{ + Getter: func(_ context.Context, tCtx K) (any, error) { + return fromAttributeIndices(tCtx.GetProfilesDictionary().AttributeTable(), tCtx), nil + }, + Setter: func(_ context.Context, tCtx K, val any) error { + m, err := ctxutil.GetMap(val) + if err != nil { + return err + } + for k, v := range m.All() { + if err := putAttribute(tCtx.GetProfilesDictionary().AttributeTable(), tCtx, k, v); err != nil { + return err + } + } + return nil + }, + } +} + +func AccessAttributesKey[K ProfileAttributeContext](key []ottl.Key[K]) ottl.StandardGetSetter[K] { + return ottl.StandardGetSetter[K]{ + Getter: func(ctx context.Context, tCtx K) (any, error) { + return ctxutil.GetMapValue[K](ctx, tCtx, fromAttributeIndices(tCtx.GetProfilesDictionary().AttributeTable(), tCtx), key) + }, + Setter: func(ctx context.Context, tCtx K, val any) error { + newKey, err := ctxutil.GetMapKeyName(ctx, tCtx, key[0]) + if err != nil { + return err + } + v := getAttributeValue(tCtx, *newKey) + if err := ctxutil.SetIndexableValue[K](ctx, tCtx, v, val, key[1:]); err != nil { + return err + } + return putAttribute(tCtx.GetProfilesDictionary().AttributeTable(), tCtx, *newKey, v) + }, + } +} + +func getAttributeValue[K ProfileAttributeContext](tCtx K, key string) pcommon.Value { + // Find the index of the attribute in the profile's attribute indices + // and return the corresponding value from the attribute table. + table := tCtx.GetProfilesDictionary().AttributeTable() + indices := tCtx.AttributeIndices().AsRaw() + + for _, tableIndex := range indices { + attr := table.At(int(tableIndex)) + if attr.Key() == key { + v := pcommon.NewValueEmpty() + attr.Value().CopyTo(v) + return v + } + } + + return pcommon.NewValueEmpty() +} + +// fromAttributeIndices creates a pcommon.Map from the attribute indices in the provided context +func fromAttributeIndices(attributeTable pprofile.AttributeTableSlice, ctx ProfileAttributeContext) pcommon.Map { + m := pcommon.NewMap() + indices := ctx.AttributeIndices().AsRaw() + + for _, tableIndex := range indices { + if int(tableIndex) < attributeTable.Len() { + attr := attributeTable.At(int(tableIndex)) + attr.Value().CopyTo(m.PutEmpty(attr.Key())) + } + } + + return m +} + +// putAttribute adds or updates an attribute in the attribute table and updates the context's attribute indices +func putAttribute(attributeTable pprofile.AttributeTableSlice, ctx ProfileAttributeContext, key string, value pcommon.Value) error { + // First, check if the attribute already exists in the context's indices + indices := ctx.AttributeIndices() + existingIndex := -1 + + for i := 0; i < indices.Len(); i++ { + tableIndex := indices.At(i) + if int(tableIndex) < attributeTable.Len() { + attr := attributeTable.At(int(tableIndex)) + if attr.Key() == key { + existingIndex = int(tableIndex) + break + } + } + } + + if existingIndex >= 0 { + // Update existing attribute + attr := attributeTable.At(existingIndex) + value.CopyTo(attr.Value()) + } else { + // Add new attribute to the table + newAttr := attributeTable.AppendEmpty() + newAttr.SetKey(key) + value.CopyTo(newAttr.Value()) + + // Add the new index to the context's attribute indices + newIndex := int32(attributeTable.Len() - 1) + indices.Append(newIndex) + } + + return nil +} diff --git a/pkg/ottl/contexts/internal/ctxprofilecommon/attributes_test.go b/pkg/ottl/contexts/internal/ctxprofilecommon/attributes_test.go new file mode 100644 index 0000000000000..01313720f1971 --- /dev/null +++ b/pkg/ottl/contexts/internal/ctxprofilecommon/attributes_test.go @@ -0,0 +1,117 @@ +// Copyright The OpenTelemetry Authors +// SPDX-License-Identifier: Apache-2.0 + +package ctxprofilecommon // import "github.com/open-telemetry/opentelemetry-collector-contrib/pkg/ottl/contexts/internal/ctxprofilecommon" + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "go.opentelemetry.io/collector/pdata/pcommon" + "go.opentelemetry.io/collector/pdata/pprofile" +) + +// Mock implementations for AttributeContext and dependencies + +type mockAttributeContext struct { + indices pcommon.Int32Slice + dictionary pprofile.ProfilesDictionary +} + +func (m *mockAttributeContext) AttributeIndices() pcommon.Int32Slice { + return m.indices +} + +func (m *mockAttributeContext) GetProfilesDictionary() pprofile.ProfilesDictionary { + return m.dictionary +} + +func TestAccessAttributes_Getter(t *testing.T) { + dict := pprofile.NewProfilesDictionary() + attrTable := dict.AttributeTable() + attr1 := attrTable.AppendEmpty() + attr1.SetKey("foo") + attr1.Value().SetStr("bar") + attr2 := attrTable.AppendEmpty() + attr2.SetKey("baz") + attr2.Value().SetInt(42) + + indices := pcommon.NewInt32Slice() + indices.Append(0) + indices.Append(1) + + ctx := &mockAttributeContext{ + indices: indices, + dictionary: dict, + } + + getSetter := AccessAttributes[*mockAttributeContext]() + + got, err := getSetter.Getter(t.Context(), ctx) + assert.NoError(t, err) + + m, ok := got.(pcommon.Map) + assert.True(t, ok) + + fooValue, ok := m.Get("foo") + assert.True(t, ok) + assert.Equal(t, "bar", fooValue.Str()) + + bazValue, ok := m.Get("baz") + assert.True(t, ok) + assert.Equal(t, int64(42), bazValue.Int()) +} + +func TestAccessAttributes_Setter(t *testing.T) { + dict := pprofile.NewProfilesDictionary() + attrTable := dict.AttributeTable() + indices := pcommon.NewInt32Slice() + + ctx := &mockAttributeContext{ + indices: indices, + dictionary: dict, + } + + getSetter := AccessAttributes[*mockAttributeContext]() + + // Prepare map to set + m := pcommon.NewMap() + m.PutStr("alpha", "beta") + m.PutInt("num", 123) + + err := getSetter.Setter(t.Context(), ctx, m) + assert.NoError(t, err) + + // Check that attributes were set in the table + foundAlpha := false + foundNum := false + for i := 0; i < attrTable.Len(); i++ { + attr := attrTable.At(i) + if attr.Key() == "alpha" { + foundAlpha = true + assert.Equal(t, "beta", attr.Value().Str()) + } + if attr.Key() == "num" { + foundNum = true + assert.Equal(t, int64(123), attr.Value().Int()) + } + } + assert.True(t, foundAlpha) + assert.True(t, foundNum) +} + +func TestAccessAttributes_Setter_InvalidValue(t *testing.T) { + dict := pprofile.NewProfilesDictionary() + indices := pcommon.NewInt32Slice() + + ctx := &mockAttributeContext{ + indices: indices, + dictionary: dict, + } + + getSetter := AccessAttributes[*mockAttributeContext]() + + // Pass a value that is not a ctxutil.Map + err := getSetter.Setter(t.Context(), ctx, "not_a_map") + assert.Error(t, err) +} diff --git a/pkg/ottl/contexts/internal/ctxprofilesample/context.go b/pkg/ottl/contexts/internal/ctxprofilesample/context.go index b0fdb3f8edbb2..87b2ac9c48000 100644 --- a/pkg/ottl/contexts/internal/ctxprofilesample/context.go +++ b/pkg/ottl/contexts/internal/ctxprofilesample/context.go @@ -3,7 +3,10 @@ package ctxprofilesample // import "github.com/open-telemetry/opentelemetry-collector-contrib/pkg/ottl/contexts/internal/ctxprofilesample" -import "go.opentelemetry.io/collector/pdata/pprofile" +import ( + "go.opentelemetry.io/collector/pdata/pcommon" + "go.opentelemetry.io/collector/pdata/pprofile" +) const ( Name = "profilesample" @@ -11,6 +14,7 @@ const ( ) type Context interface { + AttributeIndices() pcommon.Int32Slice GetProfileSample() pprofile.Sample GetProfilesDictionary() pprofile.ProfilesDictionary } diff --git a/pkg/ottl/contexts/internal/ctxprofilesample/profilesample.go b/pkg/ottl/contexts/internal/ctxprofilesample/profilesample.go index b471504380c68..26126ef4a1e0f 100644 --- a/pkg/ottl/contexts/internal/ctxprofilesample/profilesample.go +++ b/pkg/ottl/contexts/internal/ctxprofilesample/profilesample.go @@ -9,11 +9,9 @@ import ( "math" "time" - "go.opentelemetry.io/collector/pdata/pcommon" - "go.opentelemetry.io/collector/pdata/pprofile" - "github.com/open-telemetry/opentelemetry-collector-contrib/pkg/ottl" "github.com/open-telemetry/opentelemetry-collector-contrib/pkg/ottl/contexts/internal/ctxerror" + "github.com/open-telemetry/opentelemetry-collector-contrib/pkg/ottl/contexts/internal/ctxprofilecommon" "github.com/open-telemetry/opentelemetry-collector-contrib/pkg/ottl/contexts/internal/ctxutil" ) @@ -43,9 +41,9 @@ func PathGetSetter[K Context](path ottl.Path[K]) (ottl.GetSetter[K], error) { return accessTimestamps[K](), nil case "attributes": if path.Keys() == nil { - return accessAttributes[K](), nil + return ctxprofilecommon.AccessAttributes[K](), nil } - return accessAttributesKey(path.Keys()), nil + return ctxprofilecommon.AccessAttributesKey[K](path.Keys()), nil default: return nil, ctxerror.New(path.Name(), path.String(), Name, DocRef) } @@ -158,61 +156,3 @@ func accessTimestamps[K Context]() ottl.StandardGetSetter[K] { }, } } - -func accessAttributes[K Context]() ottl.StandardGetSetter[K] { - return ottl.StandardGetSetter[K]{ - Getter: func(_ context.Context, tCtx K) (any, error) { - return pprofile.FromAttributeIndices(tCtx.GetProfilesDictionary().AttributeTable(), tCtx.GetProfileSample()), nil - }, - Setter: func(_ context.Context, tCtx K, val any) error { - m, err := ctxutil.GetMap(val) - if err != nil { - return err - } - tCtx.GetProfileSample().AttributeIndices().FromRaw([]int32{}) - for k, v := range m.All() { - if err := pprofile.PutAttribute(tCtx.GetProfilesDictionary().AttributeTable(), tCtx.GetProfileSample(), k, v); err != nil { - return err - } - } - return nil - }, - } -} - -func accessAttributesKey[K Context](key []ottl.Key[K]) ottl.StandardGetSetter[K] { - return ottl.StandardGetSetter[K]{ - Getter: func(ctx context.Context, tCtx K) (any, error) { - return ctxutil.GetMapValue[K](ctx, tCtx, pprofile.FromAttributeIndices(tCtx.GetProfilesDictionary().AttributeTable(), tCtx.GetProfileSample()), key) - }, - Setter: func(ctx context.Context, tCtx K, val any) error { - newKey, err := ctxutil.GetMapKeyName(ctx, tCtx, key[0]) - if err != nil { - return err - } - v := getAttributeValue(tCtx, *newKey) - if err := ctxutil.SetIndexableValue[K](ctx, tCtx, v, val, key[1:]); err != nil { - return err - } - return pprofile.PutAttribute(tCtx.GetProfilesDictionary().AttributeTable(), tCtx.GetProfileSample(), *newKey, v) - }, - } -} - -func getAttributeValue[K Context](tCtx K, key string) pcommon.Value { - // Find the index of the attribute in the profile's attribute indices - // and return the corresponding value from the attribute table. - table := tCtx.GetProfilesDictionary().AttributeTable() - indices := tCtx.GetProfileSample().AttributeIndices().AsRaw() - - for _, tableIndex := range indices { - attr := table.At(int(tableIndex)) - if attr.Key() == key { - v := pcommon.NewValueEmpty() - attr.Value().CopyTo(v) - return v - } - } - - return pcommon.NewValueEmpty() -} diff --git a/pkg/ottl/contexts/internal/ctxprofilesample/profilesample_test.go b/pkg/ottl/contexts/internal/ctxprofilesample/profilesample_test.go index 1fd486bd87739..aa5eea5397876 100644 --- a/pkg/ottl/contexts/internal/ctxprofilesample/profilesample_test.go +++ b/pkg/ottl/contexts/internal/ctxprofilesample/profilesample_test.go @@ -10,6 +10,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "go.opentelemetry.io/collector/pdata/pcommon" "go.opentelemetry.io/collector/pdata/pprofile" "github.com/open-telemetry/opentelemetry-collector-contrib/pkg/ottl" @@ -94,6 +95,13 @@ func (p *profileSampleContext) GetProfileSample() pprofile.Sample { return p.sample } +func (p *profileSampleContext) AttributeIndices() pcommon.Int32Slice { + return p.sample.AttributeIndices() +} + func newProfileSampleContext(sample pprofile.Sample, dictionary pprofile.ProfilesDictionary) *profileSampleContext { - return &profileSampleContext{sample: sample, dictionary: dictionary} + return &profileSampleContext{ + sample: sample, + dictionary: dictionary, + } } diff --git a/pkg/ottl/contexts/ottlprofile/profile.go b/pkg/ottl/contexts/ottlprofile/profile.go index ab45c0caf1516..b2b800444443e 100644 --- a/pkg/ottl/contexts/ottlprofile/profile.go +++ b/pkg/ottl/contexts/ottlprofile/profile.go @@ -83,6 +83,10 @@ func (tCtx TransformContext) GetProfilesDictionary() pprofile.ProfilesDictionary return tCtx.dictionary } +func (tCtx TransformContext) AttributeIndices() pcommon.Int32Slice { + return tCtx.profile.AttributeIndices() +} + // GetInstrumentationScope returns the instrumentation scope from the TransformContext. func (tCtx TransformContext) GetInstrumentationScope() pcommon.InstrumentationScope { return tCtx.instrumentationScope diff --git a/pkg/ottl/contexts/ottlprofilesample/profilesample.go b/pkg/ottl/contexts/ottlprofilesample/profilesample.go index e0291ef160141..0c1bd359c4344 100644 --- a/pkg/ottl/contexts/ottlprofilesample/profilesample.go +++ b/pkg/ottl/contexts/ottlprofilesample/profilesample.go @@ -87,6 +87,10 @@ func (tCtx TransformContext) GetProfileSample() pprofile.Sample { return tCtx.sample } +func (tCtx TransformContext) AttributeIndices() pcommon.Int32Slice { + return tCtx.sample.AttributeIndices() +} + // GetProfilesDictionary returns the profiles dictionary from the TransformContext. func (tCtx TransformContext) GetProfilesDictionary() pprofile.ProfilesDictionary { return tCtx.dictionary From ec31f289fdc6cdda6b99e5f0f4816fce051b1232 Mon Sep 17 00:00:00 2001 From: Florian Lehner Date: Wed, 20 Aug 2025 14:38:44 +0200 Subject: [PATCH 02/10] fixup: drop custom implementation Signed-off-by: Florian Lehner --- .../internal/ctxprofilecommon/attributes.go | 59 ++----------------- 1 file changed, 5 insertions(+), 54 deletions(-) diff --git a/pkg/ottl/contexts/internal/ctxprofilecommon/attributes.go b/pkg/ottl/contexts/internal/ctxprofilecommon/attributes.go index bf08b0bb1585f..f2bfaa7a69a29 100644 --- a/pkg/ottl/contexts/internal/ctxprofilecommon/attributes.go +++ b/pkg/ottl/contexts/internal/ctxprofilecommon/attributes.go @@ -21,7 +21,7 @@ type ProfileAttributeContext interface { func AccessAttributes[K ProfileAttributeContext]() ottl.StandardGetSetter[K] { return ottl.StandardGetSetter[K]{ Getter: func(_ context.Context, tCtx K) (any, error) { - return fromAttributeIndices(tCtx.GetProfilesDictionary().AttributeTable(), tCtx), nil + return pprofile.FromAttributeIndices(tCtx.GetProfilesDictionary().AttributeTable(), tCtx), nil }, Setter: func(_ context.Context, tCtx K, val any) error { m, err := ctxutil.GetMap(val) @@ -29,7 +29,7 @@ func AccessAttributes[K ProfileAttributeContext]() ottl.StandardGetSetter[K] { return err } for k, v := range m.All() { - if err := putAttribute(tCtx.GetProfilesDictionary().AttributeTable(), tCtx, k, v); err != nil { + if err := pprofile.PutAttribute(tCtx.GetProfilesDictionary().AttributeTable(), tCtx, k, v); err != nil { return err } } @@ -41,7 +41,8 @@ func AccessAttributes[K ProfileAttributeContext]() ottl.StandardGetSetter[K] { func AccessAttributesKey[K ProfileAttributeContext](key []ottl.Key[K]) ottl.StandardGetSetter[K] { return ottl.StandardGetSetter[K]{ Getter: func(ctx context.Context, tCtx K) (any, error) { - return ctxutil.GetMapValue[K](ctx, tCtx, fromAttributeIndices(tCtx.GetProfilesDictionary().AttributeTable(), tCtx), key) + return ctxutil.GetMapValue[K](ctx, tCtx, + pprofile.FromAttributeIndices(tCtx.GetProfilesDictionary().AttributeTable(), tCtx), key) }, Setter: func(ctx context.Context, tCtx K, val any) error { newKey, err := ctxutil.GetMapKeyName(ctx, tCtx, key[0]) @@ -52,7 +53,7 @@ func AccessAttributesKey[K ProfileAttributeContext](key []ottl.Key[K]) ottl.Stan if err := ctxutil.SetIndexableValue[K](ctx, tCtx, v, val, key[1:]); err != nil { return err } - return putAttribute(tCtx.GetProfilesDictionary().AttributeTable(), tCtx, *newKey, v) + return pprofile.PutAttribute(tCtx.GetProfilesDictionary().AttributeTable(), tCtx, *newKey, v) }, } } @@ -74,53 +75,3 @@ func getAttributeValue[K ProfileAttributeContext](tCtx K, key string) pcommon.Va return pcommon.NewValueEmpty() } - -// fromAttributeIndices creates a pcommon.Map from the attribute indices in the provided context -func fromAttributeIndices(attributeTable pprofile.AttributeTableSlice, ctx ProfileAttributeContext) pcommon.Map { - m := pcommon.NewMap() - indices := ctx.AttributeIndices().AsRaw() - - for _, tableIndex := range indices { - if int(tableIndex) < attributeTable.Len() { - attr := attributeTable.At(int(tableIndex)) - attr.Value().CopyTo(m.PutEmpty(attr.Key())) - } - } - - return m -} - -// putAttribute adds or updates an attribute in the attribute table and updates the context's attribute indices -func putAttribute(attributeTable pprofile.AttributeTableSlice, ctx ProfileAttributeContext, key string, value pcommon.Value) error { - // First, check if the attribute already exists in the context's indices - indices := ctx.AttributeIndices() - existingIndex := -1 - - for i := 0; i < indices.Len(); i++ { - tableIndex := indices.At(i) - if int(tableIndex) < attributeTable.Len() { - attr := attributeTable.At(int(tableIndex)) - if attr.Key() == key { - existingIndex = int(tableIndex) - break - } - } - } - - if existingIndex >= 0 { - // Update existing attribute - attr := attributeTable.At(existingIndex) - value.CopyTo(attr.Value()) - } else { - // Add new attribute to the table - newAttr := attributeTable.AppendEmpty() - newAttr.SetKey(key) - value.CopyTo(newAttr.Value()) - - // Add the new index to the context's attribute indices - newIndex := int32(attributeTable.Len() - 1) - indices.Append(newIndex) - } - - return nil -} From ce99a24bb0eb603dafb9045212f758600f32d3a8 Mon Sep 17 00:00:00 2001 From: Florian Lehner Date: Wed, 20 Aug 2025 16:00:03 +0200 Subject: [PATCH 03/10] fix helpers Signed-off-by: Florian Lehner --- pkg/ottl/contexts/internal/ctxprofilecommon/attributes.go | 1 + 1 file changed, 1 insertion(+) diff --git a/pkg/ottl/contexts/internal/ctxprofilecommon/attributes.go b/pkg/ottl/contexts/internal/ctxprofilecommon/attributes.go index f2bfaa7a69a29..79117490172de 100644 --- a/pkg/ottl/contexts/internal/ctxprofilecommon/attributes.go +++ b/pkg/ottl/contexts/internal/ctxprofilecommon/attributes.go @@ -28,6 +28,7 @@ func AccessAttributes[K ProfileAttributeContext]() ottl.StandardGetSetter[K] { if err != nil { return err } + tCtx.AttributeIndices().FromRaw([]int32{}) for k, v := range m.All() { if err := pprofile.PutAttribute(tCtx.GetProfilesDictionary().AttributeTable(), tCtx, k, v); err != nil { return err From c7affe2b122424e6ee29eb89dad5bd788de09418 Mon Sep 17 00:00:00 2001 From: Florian Lehner Date: Wed, 27 Aug 2025 14:43:40 +0200 Subject: [PATCH 04/10] fixup: add tests for AccessAttributesKey Signed-off-by: Florian Lehner --- .../ctxprofilecommon/attributes_test.go | 113 ++++++++++++++++++ 1 file changed, 113 insertions(+) diff --git a/pkg/ottl/contexts/internal/ctxprofilecommon/attributes_test.go b/pkg/ottl/contexts/internal/ctxprofilecommon/attributes_test.go index 01313720f1971..6019989e1d2ce 100644 --- a/pkg/ottl/contexts/internal/ctxprofilecommon/attributes_test.go +++ b/pkg/ottl/contexts/internal/ctxprofilecommon/attributes_test.go @@ -6,6 +6,9 @@ package ctxprofilecommon // import "github.com/open-telemetry/opentelemetry-coll import ( "testing" + "github.com/open-telemetry/opentelemetry-collector-contrib/pkg/ottl" + "github.com/open-telemetry/opentelemetry-collector-contrib/pkg/ottl/contexts/internal/pathtest" + "github.com/open-telemetry/opentelemetry-collector-contrib/pkg/ottl/ottltest" "github.com/stretchr/testify/assert" "go.opentelemetry.io/collector/pdata/pcommon" "go.opentelemetry.io/collector/pdata/pprofile" @@ -115,3 +118,113 @@ func TestAccessAttributes_Setter_InvalidValue(t *testing.T) { err := getSetter.Setter(t.Context(), ctx, "not_a_map") assert.Error(t, err) } +func TestAccessAttributesKey_Getter(t *testing.T) { + dict := pprofile.NewProfilesDictionary() + attrTable := dict.AttributeTable() + attr := attrTable.AppendEmpty() + attr.SetKey("foo") + attr.Value().SetStr("bar") + indices := pcommon.NewInt32Slice() + indices.Append(0) + + ctx := &mockAttributeContext{ + indices: indices, + dictionary: dict, + } + + t.Run("non-existing-key", func(t *testing.T) { + path := pathtest.Path[*mockAttributeContext]{ + KeySlice: []ottl.Key[*mockAttributeContext]{ + &pathtest.Key[*mockAttributeContext]{ + S: ottltest.Strp("key1"), + }, + }, + } + getSetter := AccessAttributesKey[*mockAttributeContext](path.Keys()) + got, err := getSetter.Getter(t.Context(), ctx) + assert.NoError(t, err) + assert.Equal(t, nil, got) + }) + + t.Run("existing-key", func(t *testing.T) { + path := pathtest.Path[*mockAttributeContext]{ + KeySlice: []ottl.Key[*mockAttributeContext]{ + &pathtest.Key[*mockAttributeContext]{ + S: ottltest.Strp("foo"), + }, + }, + } + getSetter := AccessAttributesKey[*mockAttributeContext](path.Keys()) + got, err := getSetter.Getter(t.Context(), ctx) + assert.NoError(t, err) + assert.Equal(t, "bar", got) + }) + +} + +func TestAccessAttributesKey_Setter(t *testing.T) { + dict := pprofile.NewProfilesDictionary() + attrTable := dict.AttributeTable() + attr := attrTable.AppendEmpty() + attr.SetKey("foo") + attr.Value().SetStr("bar") + indices := pcommon.NewInt32Slice() + indices.Append(0) + + ctx := &mockAttributeContext{ + indices: indices, + dictionary: dict, + } + + t.Run("non-existing-key", func(t *testing.T) { + path := pathtest.Path[*mockAttributeContext]{ + KeySlice: []ottl.Key[*mockAttributeContext]{ + &pathtest.Key[*mockAttributeContext]{ + S: ottltest.Strp("key1"), + }, + }, + } + getSetter := AccessAttributesKey[*mockAttributeContext](path.Keys()) + err := getSetter.Setter(t.Context(), ctx, "value1") + assert.NoError(t, err) + }) + + t.Run("update-existing-key", func(t *testing.T) { + path := pathtest.Path[*mockAttributeContext]{ + KeySlice: []ottl.Key[*mockAttributeContext]{ + &pathtest.Key[*mockAttributeContext]{ + S: ottltest.Strp("foo"), + }, + }, + } + getSetter := AccessAttributesKey[*mockAttributeContext](path.Keys()) + err := getSetter.Setter(t.Context(), ctx, "bazinga") + assert.NoError(t, err) + }) + + t.Run("insert-new-key", func(t *testing.T) { + lenAttrsBefore := ctx.AttributeIndices().Len() + lenIndicesBefore := ctx.indices.Len() + + path := pathtest.Path[*mockAttributeContext]{ + KeySlice: []ottl.Key[*mockAttributeContext]{ + &pathtest.Key[*mockAttributeContext]{ + S: ottltest.Strp("bazinga"), + }, + }, + } + getSetter := AccessAttributesKey[*mockAttributeContext](path.Keys()) + err := getSetter.Setter(t.Context(), ctx, 42) + assert.NoError(t, err) + lenAttrsAfter := ctx.AttributeIndices().Len() + lenIndicesAfter := ctx.indices.Len() + + if lenAttrsBefore+1 != lenAttrsAfter { + t.Fatal("expected additional attribute after inserting it") + } + + if lenIndicesBefore+1 != lenIndicesAfter { + t.Fatal("expected additional index after inserting it") + } + }) +} From 769c651b0c30a19c71d50c19649412024dc1e0d9 Mon Sep 17 00:00:00 2001 From: Florian Lehner Date: Thu, 28 Aug 2025 14:45:08 +0200 Subject: [PATCH 05/10] fixup formatting Signed-off-by: Florian Lehner --- .../internal/ctxprofilecommon/attributes_test.go | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/pkg/ottl/contexts/internal/ctxprofilecommon/attributes_test.go b/pkg/ottl/contexts/internal/ctxprofilecommon/attributes_test.go index 6019989e1d2ce..f8d711ac0bbde 100644 --- a/pkg/ottl/contexts/internal/ctxprofilecommon/attributes_test.go +++ b/pkg/ottl/contexts/internal/ctxprofilecommon/attributes_test.go @@ -6,12 +6,13 @@ package ctxprofilecommon // import "github.com/open-telemetry/opentelemetry-coll import ( "testing" - "github.com/open-telemetry/opentelemetry-collector-contrib/pkg/ottl" - "github.com/open-telemetry/opentelemetry-collector-contrib/pkg/ottl/contexts/internal/pathtest" - "github.com/open-telemetry/opentelemetry-collector-contrib/pkg/ottl/ottltest" "github.com/stretchr/testify/assert" "go.opentelemetry.io/collector/pdata/pcommon" "go.opentelemetry.io/collector/pdata/pprofile" + + "github.com/open-telemetry/opentelemetry-collector-contrib/pkg/ottl" + "github.com/open-telemetry/opentelemetry-collector-contrib/pkg/ottl/contexts/internal/pathtest" + "github.com/open-telemetry/opentelemetry-collector-contrib/pkg/ottl/ottltest" ) // Mock implementations for AttributeContext and dependencies @@ -118,6 +119,7 @@ func TestAccessAttributes_Setter_InvalidValue(t *testing.T) { err := getSetter.Setter(t.Context(), ctx, "not_a_map") assert.Error(t, err) } + func TestAccessAttributesKey_Getter(t *testing.T) { dict := pprofile.NewProfilesDictionary() attrTable := dict.AttributeTable() @@ -159,7 +161,6 @@ func TestAccessAttributesKey_Getter(t *testing.T) { assert.NoError(t, err) assert.Equal(t, "bar", got) }) - } func TestAccessAttributesKey_Setter(t *testing.T) { From 410b674f64b9a632598e6ebbc9c8b090213117aa Mon Sep 17 00:00:00 2001 From: Florian Lehner Date: Thu, 28 Aug 2025 14:46:51 +0200 Subject: [PATCH 06/10] fix linter Signed-off-by: Florian Lehner --- pkg/ottl/contexts/internal/ctxprofilecommon/attributes_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/ottl/contexts/internal/ctxprofilecommon/attributes_test.go b/pkg/ottl/contexts/internal/ctxprofilecommon/attributes_test.go index f8d711ac0bbde..4e0767c1bbc31 100644 --- a/pkg/ottl/contexts/internal/ctxprofilecommon/attributes_test.go +++ b/pkg/ottl/contexts/internal/ctxprofilecommon/attributes_test.go @@ -145,7 +145,7 @@ func TestAccessAttributesKey_Getter(t *testing.T) { getSetter := AccessAttributesKey[*mockAttributeContext](path.Keys()) got, err := getSetter.Getter(t.Context(), ctx) assert.NoError(t, err) - assert.Equal(t, nil, got) + assert.Nil(t, got) }) t.Run("existing-key", func(t *testing.T) { From 44e915e0dd559b43e27bda0556765eab4da14e96 Mon Sep 17 00:00:00 2001 From: edmocosta <11836452+edmocosta@users.noreply.github.com> Date: Fri, 29 Aug 2025 16:15:21 +0200 Subject: [PATCH 07/10] Remove context dependency example --- .../contexts/internal/ctxprofile/context.go | 2 - .../contexts/internal/ctxprofile/profile.go | 7 +++- .../internal/ctxprofilecommon/attributes.go | 38 ++++++++++--------- .../internal/ctxprofilesample/context.go | 2 - .../ctxprofilesample/profilesample.go | 9 ++++- pkg/ottl/contexts/ottlprofile/profile.go | 4 -- .../ottlprofilesample/profilesample.go | 4 -- 7 files changed, 32 insertions(+), 34 deletions(-) diff --git a/pkg/ottl/contexts/internal/ctxprofile/context.go b/pkg/ottl/contexts/internal/ctxprofile/context.go index 9706978b239cd..806f9ce8d422c 100644 --- a/pkg/ottl/contexts/internal/ctxprofile/context.go +++ b/pkg/ottl/contexts/internal/ctxprofile/context.go @@ -3,7 +3,6 @@ package ctxprofile // import "github.com/open-telemetry/opentelemetry-collector-contrib/pkg/ottl/contexts/internal/ctxprofile" import ( - "go.opentelemetry.io/collector/pdata/pcommon" "go.opentelemetry.io/collector/pdata/pprofile" ) @@ -13,7 +12,6 @@ const ( ) type Context interface { - AttributeIndices() pcommon.Int32Slice GetProfile() pprofile.Profile GetProfilesDictionary() pprofile.ProfilesDictionary } diff --git a/pkg/ottl/contexts/internal/ctxprofile/profile.go b/pkg/ottl/contexts/internal/ctxprofile/profile.go index 39ccaaa3d04ac..1a87a3cc91956 100644 --- a/pkg/ottl/contexts/internal/ctxprofile/profile.go +++ b/pkg/ottl/contexts/internal/ctxprofile/profile.go @@ -64,10 +64,13 @@ func PathGetSetter[K Context](path ottl.Path[K]) (ottl.GetSetter[K], error) { case "original_payload": return accessOriginalPayload[K](), nil case "attributes": + attributable := func(ctx K) (pprofile.ProfilesDictionary, ctxprofilecommon.ProfileAttributable) { + return ctx.GetProfilesDictionary(), ctx.GetProfile() + } if path.Keys() == nil { - return ctxprofilecommon.AccessAttributes[K](), nil + return ctxprofilecommon.AccessAttributes[K](attributable), nil } - return ctxprofilecommon.AccessAttributesKey[K](path.Keys()), nil + return ctxprofilecommon.AccessAttributesKey[K](path.Keys(), attributable), nil default: return nil, ctxerror.New(path.Name(), path.String(), Name, DocRef) } diff --git a/pkg/ottl/contexts/internal/ctxprofilecommon/attributes.go b/pkg/ottl/contexts/internal/ctxprofilecommon/attributes.go index 79117490172de..fbc0f5f206604 100644 --- a/pkg/ottl/contexts/internal/ctxprofilecommon/attributes.go +++ b/pkg/ottl/contexts/internal/ctxprofilecommon/attributes.go @@ -13,24 +13,28 @@ import ( "github.com/open-telemetry/opentelemetry-collector-contrib/pkg/ottl/contexts/internal/ctxutil" ) -type ProfileAttributeContext interface { +type ProfileAttributable interface { AttributeIndices() pcommon.Int32Slice - GetProfilesDictionary() pprofile.ProfilesDictionary } -func AccessAttributes[K ProfileAttributeContext]() ottl.StandardGetSetter[K] { +type attributeSource[K any] = func(ctx K) (pprofile.ProfilesDictionary, ProfileAttributable) + +func AccessAttributes[K any](source attributeSource[K]) ottl.StandardGetSetter[K] { return ottl.StandardGetSetter[K]{ Getter: func(_ context.Context, tCtx K) (any, error) { - return pprofile.FromAttributeIndices(tCtx.GetProfilesDictionary().AttributeTable(), tCtx), nil + dict, attributable := source(tCtx) + return pprofile.FromAttributeIndices(dict.AttributeTable(), attributable), nil }, Setter: func(_ context.Context, tCtx K, val any) error { m, err := ctxutil.GetMap(val) if err != nil { return err } - tCtx.AttributeIndices().FromRaw([]int32{}) + + dict, attributable := source(tCtx) + attributable.AttributeIndices().FromRaw([]int32{}) for k, v := range m.All() { - if err := pprofile.PutAttribute(tCtx.GetProfilesDictionary().AttributeTable(), tCtx, k, v); err != nil { + if err := pprofile.PutAttribute(dict.AttributeTable(), attributable, k, v); err != nil { return err } } @@ -39,33 +43,31 @@ func AccessAttributes[K ProfileAttributeContext]() ottl.StandardGetSetter[K] { } } -func AccessAttributesKey[K ProfileAttributeContext](key []ottl.Key[K]) ottl.StandardGetSetter[K] { +func AccessAttributesKey[K any](key []ottl.Key[K], source attributeSource[K]) ottl.StandardGetSetter[K] { return ottl.StandardGetSetter[K]{ Getter: func(ctx context.Context, tCtx K) (any, error) { - return ctxutil.GetMapValue[K](ctx, tCtx, - pprofile.FromAttributeIndices(tCtx.GetProfilesDictionary().AttributeTable(), tCtx), key) + dict, attributable := source(tCtx) + return ctxutil.GetMapValue[K](ctx, tCtx, pprofile.FromAttributeIndices(dict.AttributeTable(), attributable), key) }, Setter: func(ctx context.Context, tCtx K, val any) error { newKey, err := ctxutil.GetMapKeyName(ctx, tCtx, key[0]) if err != nil { return err } - v := getAttributeValue(tCtx, *newKey) + + dict, attributable := source(tCtx) + v := getAttributeValue(dict.AttributeTable(), attributable.AttributeIndices(), *newKey) if err := ctxutil.SetIndexableValue[K](ctx, tCtx, v, val, key[1:]); err != nil { return err } - return pprofile.PutAttribute(tCtx.GetProfilesDictionary().AttributeTable(), tCtx, *newKey, v) + + return pprofile.PutAttribute(dict.AttributeTable(), attributable, *newKey, v) }, } } -func getAttributeValue[K ProfileAttributeContext](tCtx K, key string) pcommon.Value { - // Find the index of the attribute in the profile's attribute indices - // and return the corresponding value from the attribute table. - table := tCtx.GetProfilesDictionary().AttributeTable() - indices := tCtx.AttributeIndices().AsRaw() - - for _, tableIndex := range indices { +func getAttributeValue(table pprofile.AttributeTableSlice, indices pcommon.Int32Slice, key string) pcommon.Value { + for _, tableIndex := range indices.All() { attr := table.At(int(tableIndex)) if attr.Key() == key { v := pcommon.NewValueEmpty() diff --git a/pkg/ottl/contexts/internal/ctxprofilesample/context.go b/pkg/ottl/contexts/internal/ctxprofilesample/context.go index 87b2ac9c48000..07b2c43b79d99 100644 --- a/pkg/ottl/contexts/internal/ctxprofilesample/context.go +++ b/pkg/ottl/contexts/internal/ctxprofilesample/context.go @@ -4,7 +4,6 @@ package ctxprofilesample // import "github.com/open-telemetry/opentelemetry-collector-contrib/pkg/ottl/contexts/internal/ctxprofilesample" import ( - "go.opentelemetry.io/collector/pdata/pcommon" "go.opentelemetry.io/collector/pdata/pprofile" ) @@ -14,7 +13,6 @@ const ( ) type Context interface { - AttributeIndices() pcommon.Int32Slice GetProfileSample() pprofile.Sample GetProfilesDictionary() pprofile.ProfilesDictionary } diff --git a/pkg/ottl/contexts/internal/ctxprofilesample/profilesample.go b/pkg/ottl/contexts/internal/ctxprofilesample/profilesample.go index 26126ef4a1e0f..9d8ac85c66593 100644 --- a/pkg/ottl/contexts/internal/ctxprofilesample/profilesample.go +++ b/pkg/ottl/contexts/internal/ctxprofilesample/profilesample.go @@ -9,6 +9,8 @@ import ( "math" "time" + "go.opentelemetry.io/collector/pdata/pprofile" + "github.com/open-telemetry/opentelemetry-collector-contrib/pkg/ottl" "github.com/open-telemetry/opentelemetry-collector-contrib/pkg/ottl/contexts/internal/ctxerror" "github.com/open-telemetry/opentelemetry-collector-contrib/pkg/ottl/contexts/internal/ctxprofilecommon" @@ -40,10 +42,13 @@ func PathGetSetter[K Context](path ottl.Path[K]) (ottl.GetSetter[K], error) { case "timestamps": return accessTimestamps[K](), nil case "attributes": + attributable := func(ctx K) (pprofile.ProfilesDictionary, ctxprofilecommon.ProfileAttributable) { + return ctx.GetProfilesDictionary(), ctx.GetProfileSample() + } if path.Keys() == nil { - return ctxprofilecommon.AccessAttributes[K](), nil + return ctxprofilecommon.AccessAttributes[K](attributable), nil } - return ctxprofilecommon.AccessAttributesKey[K](path.Keys()), nil + return ctxprofilecommon.AccessAttributesKey[K](path.Keys(), attributable), nil default: return nil, ctxerror.New(path.Name(), path.String(), Name, DocRef) } diff --git a/pkg/ottl/contexts/ottlprofile/profile.go b/pkg/ottl/contexts/ottlprofile/profile.go index b2b800444443e..ab45c0caf1516 100644 --- a/pkg/ottl/contexts/ottlprofile/profile.go +++ b/pkg/ottl/contexts/ottlprofile/profile.go @@ -83,10 +83,6 @@ func (tCtx TransformContext) GetProfilesDictionary() pprofile.ProfilesDictionary return tCtx.dictionary } -func (tCtx TransformContext) AttributeIndices() pcommon.Int32Slice { - return tCtx.profile.AttributeIndices() -} - // GetInstrumentationScope returns the instrumentation scope from the TransformContext. func (tCtx TransformContext) GetInstrumentationScope() pcommon.InstrumentationScope { return tCtx.instrumentationScope diff --git a/pkg/ottl/contexts/ottlprofilesample/profilesample.go b/pkg/ottl/contexts/ottlprofilesample/profilesample.go index 0c1bd359c4344..e0291ef160141 100644 --- a/pkg/ottl/contexts/ottlprofilesample/profilesample.go +++ b/pkg/ottl/contexts/ottlprofilesample/profilesample.go @@ -87,10 +87,6 @@ func (tCtx TransformContext) GetProfileSample() pprofile.Sample { return tCtx.sample } -func (tCtx TransformContext) AttributeIndices() pcommon.Int32Slice { - return tCtx.sample.AttributeIndices() -} - // GetProfilesDictionary returns the profiles dictionary from the TransformContext. func (tCtx TransformContext) GetProfilesDictionary() pprofile.ProfilesDictionary { return tCtx.dictionary From a3175909a606894d99ae5b5f80a31c415fcd8a35 Mon Sep 17 00:00:00 2001 From: Florian Lehner Date: Fri, 29 Aug 2025 17:08:20 +0200 Subject: [PATCH 08/10] fix tests Signed-off-by: Florian Lehner --- .../ctxprofilecommon/attributes_test.go | 20 +++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/pkg/ottl/contexts/internal/ctxprofilecommon/attributes_test.go b/pkg/ottl/contexts/internal/ctxprofilecommon/attributes_test.go index 4e0767c1bbc31..ee6d491d611d3 100644 --- a/pkg/ottl/contexts/internal/ctxprofilecommon/attributes_test.go +++ b/pkg/ottl/contexts/internal/ctxprofilecommon/attributes_test.go @@ -30,6 +30,10 @@ func (m *mockAttributeContext) GetProfilesDictionary() pprofile.ProfilesDictiona return m.dictionary } +func mockAttributeSource(ctx *mockAttributeContext) (pprofile.ProfilesDictionary, ProfileAttributable) { + return ctx.dictionary, ctx +} + func TestAccessAttributes_Getter(t *testing.T) { dict := pprofile.NewProfilesDictionary() attrTable := dict.AttributeTable() @@ -49,7 +53,7 @@ func TestAccessAttributes_Getter(t *testing.T) { dictionary: dict, } - getSetter := AccessAttributes[*mockAttributeContext]() + getSetter := AccessAttributes[*mockAttributeContext](mockAttributeSource) got, err := getSetter.Getter(t.Context(), ctx) assert.NoError(t, err) @@ -76,7 +80,7 @@ func TestAccessAttributes_Setter(t *testing.T) { dictionary: dict, } - getSetter := AccessAttributes[*mockAttributeContext]() + getSetter := AccessAttributes[*mockAttributeContext](mockAttributeSource) // Prepare map to set m := pcommon.NewMap() @@ -113,7 +117,7 @@ func TestAccessAttributes_Setter_InvalidValue(t *testing.T) { dictionary: dict, } - getSetter := AccessAttributes[*mockAttributeContext]() + getSetter := AccessAttributes[*mockAttributeContext](mockAttributeSource) // Pass a value that is not a ctxutil.Map err := getSetter.Setter(t.Context(), ctx, "not_a_map") @@ -142,7 +146,7 @@ func TestAccessAttributesKey_Getter(t *testing.T) { }, }, } - getSetter := AccessAttributesKey[*mockAttributeContext](path.Keys()) + getSetter := AccessAttributesKey[*mockAttributeContext](path.Keys(), mockAttributeSource) got, err := getSetter.Getter(t.Context(), ctx) assert.NoError(t, err) assert.Nil(t, got) @@ -156,7 +160,7 @@ func TestAccessAttributesKey_Getter(t *testing.T) { }, }, } - getSetter := AccessAttributesKey[*mockAttributeContext](path.Keys()) + getSetter := AccessAttributesKey[*mockAttributeContext](path.Keys(), mockAttributeSource) got, err := getSetter.Getter(t.Context(), ctx) assert.NoError(t, err) assert.Equal(t, "bar", got) @@ -185,7 +189,7 @@ func TestAccessAttributesKey_Setter(t *testing.T) { }, }, } - getSetter := AccessAttributesKey[*mockAttributeContext](path.Keys()) + getSetter := AccessAttributesKey[*mockAttributeContext](path.Keys(), mockAttributeSource) err := getSetter.Setter(t.Context(), ctx, "value1") assert.NoError(t, err) }) @@ -198,7 +202,7 @@ func TestAccessAttributesKey_Setter(t *testing.T) { }, }, } - getSetter := AccessAttributesKey[*mockAttributeContext](path.Keys()) + getSetter := AccessAttributesKey[*mockAttributeContext](path.Keys(), mockAttributeSource) err := getSetter.Setter(t.Context(), ctx, "bazinga") assert.NoError(t, err) }) @@ -214,7 +218,7 @@ func TestAccessAttributesKey_Setter(t *testing.T) { }, }, } - getSetter := AccessAttributesKey[*mockAttributeContext](path.Keys()) + getSetter := AccessAttributesKey[*mockAttributeContext](path.Keys(), mockAttributeSource) err := getSetter.Setter(t.Context(), ctx, 42) assert.NoError(t, err) lenAttrsAfter := ctx.AttributeIndices().Len() From b9e1957cd3fe8bae7b6290c04b4f65e67fc5e679 Mon Sep 17 00:00:00 2001 From: Florian Lehner Date: Mon, 1 Sep 2025 16:40:41 +0200 Subject: [PATCH 09/10] fixup: add comment from https://github.com/open-telemetry/opentelemetry-collector-contrib/pull/42363 Signed-off-by: Florian Lehner --- pkg/ottl/contexts/internal/ctxprofilecommon/attributes.go | 1 + 1 file changed, 1 insertion(+) diff --git a/pkg/ottl/contexts/internal/ctxprofilecommon/attributes.go b/pkg/ottl/contexts/internal/ctxprofilecommon/attributes.go index fbc0f5f206604..11d4db94990b4 100644 --- a/pkg/ottl/contexts/internal/ctxprofilecommon/attributes.go +++ b/pkg/ottl/contexts/internal/ctxprofilecommon/attributes.go @@ -70,6 +70,7 @@ func getAttributeValue(table pprofile.AttributeTableSlice, indices pcommon.Int32 for _, tableIndex := range indices.All() { attr := table.At(int(tableIndex)) if attr.Key() == key { + // Copy the value because OTTL expects to do inplace updates for the values. v := pcommon.NewValueEmpty() attr.Value().CopyTo(v) return v From 54170d6abc2ffea21255b1291e63a63dc6eb1565 Mon Sep 17 00:00:00 2001 From: Florian Lehner Date: Fri, 26 Sep 2025 15:57:49 +0200 Subject: [PATCH 10/10] tests: verify that existing attributes are preserved Signed-off-by: Florian Lehner --- .../ctxprofilecommon/attributes_test.go | 235 ++++++++++++++++-- 1 file changed, 218 insertions(+), 17 deletions(-) diff --git a/pkg/ottl/contexts/internal/ctxprofilecommon/attributes_test.go b/pkg/ottl/contexts/internal/ctxprofilecommon/attributes_test.go index 425323db785dc..c4b9de05fdf99 100644 --- a/pkg/ottl/contexts/internal/ctxprofilecommon/attributes_test.go +++ b/pkg/ottl/contexts/internal/ctxprofilecommon/attributes_test.go @@ -79,16 +79,46 @@ func TestAccessAttributes_Getter(t *testing.T) { func TestAccessAttributes_Setter(t *testing.T) { dict := pprofile.NewProfilesDictionary() attrTable := dict.AttributeTable() + strTable := dict.StringTable() + + for i := 0; i < 3; i++ { + strTable.Append("") + } + strTable.SetAt(1, "existing_key1") + strTable.SetAt(2, "existing_key2") + + // Add existing attributes to the shared table (these should remain in the table) + existingAttr1 := attrTable.AppendEmpty() + existingAttr1.SetKeyStrindex(1) + existingAttr1.Value().SetStr("existing_value1") + + existingAttr2 := attrTable.AppendEmpty() + existingAttr2.SetKeyStrindex(2) + existingAttr2.Value().SetInt(999) + + // Set up indices to reference existing attributes (this will be replaced by setter) indices := pcommon.NewInt32Slice() + indices.Append(0) // Reference to existingAttr1 + indices.Append(1) // Reference to existingAttr2 ctx := &mockAttributeContext{ indices: indices, dictionary: dict, } + // Capture original shared table state before setter operation + originalAttrTableLen := attrTable.Len() + originalStrTableLen := strTable.Len() + + // Store original attribute values for verification they remain in shared tables + originalAttr1Key := strTable.At(int(existingAttr1.KeyStrindex())) + originalAttr1Value := existingAttr1.Value().Str() + originalAttr2Key := strTable.At(int(existingAttr2.KeyStrindex())) + originalAttr2Value := existingAttr2.Value().Int() + getSetter := AccessAttributes[*mockAttributeContext](mockAttributeSource) - // Prepare map to set + // Prepare map to set with new attributes m := pcommon.NewMap() m.PutStr("alpha", "beta") m.PutInt("num", 123) @@ -96,23 +126,51 @@ func TestAccessAttributes_Setter(t *testing.T) { err := getSetter.Setter(t.Context(), ctx, m) assert.NoError(t, err) - // Check that attributes were set in the table + // Verify that the shared attribute table preserves existing entries + // The existing attributes should still be in the shared table even though + // the indices slice is replaced + assert.Equal(t, originalAttr1Key, strTable.At(int(existingAttr1.KeyStrindex())), + "Original attribute 1 key should remain in shared string table") + assert.Equal(t, originalAttr1Value, existingAttr1.Value().Str(), + "Original attribute 1 value should remain in shared attribute table") + assert.Equal(t, originalAttr2Key, strTable.At(int(existingAttr2.KeyStrindex())), + "Original attribute 2 key should remain in shared string table") + assert.Equal(t, originalAttr2Value, existingAttr2.Value().Int(), + "Original attribute 2 value should remain in shared attribute table") + + // Verify that the indices slice was replaced (this is the expected behavior) + // The setter replaces the entire attribute indices slice with new indices + assert.Equal(t, 2, indices.Len(), "Indices should now point to the 2 new attributes") + + // Verify that new attributes were added to the shared tables (after existing ones) + assert.Greater(t, attrTable.Len(), originalAttrTableLen, + "New attributes should be appended to the shared attribute table") + assert.Greater(t, strTable.Len(), originalStrTableLen, + "New strings should be appended to the shared string table") + + // Verify that the new indices correctly point to the new attributes foundAlpha := false foundNum := false - for i := 0; i < attrTable.Len(); i++ { - attr := attrTable.At(i) - attrKey := dict.StringTable().At(int(attr.KeyStrindex())) + for _, idx := range indices.All() { + attr := attrTable.At(int(idx)) + attrKey := strTable.At(int(attr.KeyStrindex())) if attrKey == "alpha" { foundAlpha = true assert.Equal(t, "beta", attr.Value().Str()) + // Verify this attribute is placed after existing ones + assert.GreaterOrEqual(t, int(idx), originalAttrTableLen, + "New alpha attribute should be placed after existing attributes") } if attrKey == "num" { foundNum = true assert.Equal(t, int64(123), attr.Value().Int()) + // Verify this attribute is placed after existing ones + assert.GreaterOrEqual(t, int(idx), originalAttrTableLen, + "New num attribute should be placed after existing attributes") } } - assert.True(t, foundAlpha) - assert.True(t, foundNum) + assert.True(t, foundAlpha, "New 'alpha' attribute should be found via indices") + assert.True(t, foundNum, "New 'num' attribute should be found via indices") } func TestAccessAttributes_Setter_InvalidValue(t *testing.T) { @@ -183,15 +241,29 @@ func TestAccessAttributesKey_Setter(t *testing.T) { dict := pprofile.NewProfilesDictionary() attrTable := dict.AttributeTable() strTable := dict.StringTable() - for i := 0; i < 2; i++ { + + for i := 0; i < 4; i++ { strTable.Append("") } strTable.SetAt(1, "foo") + strTable.SetAt(2, "existing_key1") + strTable.SetAt(3, "existing_key2") + + // Add existing attributes to the shared table (these should remain preserved) attr := attrTable.AppendEmpty() attr.SetKeyStrindex(1) attr.Value().SetStr("bar") + + existingAttr1 := attrTable.AppendEmpty() + existingAttr1.SetKeyStrindex(2) + existingAttr1.Value().SetStr("existing_value1") + + existingAttr2 := attrTable.AppendEmpty() + existingAttr2.SetKeyStrindex(3) + existingAttr2.Value().SetInt(999) + indices := pcommon.NewInt32Slice() - indices.Append(0) + indices.Append(0) // Reference to the "foo" attribute ctx := &mockAttributeContext{ indices: indices, @@ -199,6 +271,16 @@ func TestAccessAttributesKey_Setter(t *testing.T) { } t.Run("non-existing-key", func(t *testing.T) { + // Capture original shared table state + originalAttrTableLen := attrTable.Len() + originalStrTableLen := strTable.Len() + originalIndicesLen := indices.Len() + + // Store original values for verification + originalFooValue := attr.Value().Str() + originalExisting1Value := existingAttr1.Value().Str() + originalExisting2Value := existingAttr2.Value().Int() + path := pathtest.Path[*mockAttributeContext]{ KeySlice: []ottl.Key[*mockAttributeContext]{ &pathtest.Key[*mockAttributeContext]{ @@ -209,9 +291,41 @@ func TestAccessAttributesKey_Setter(t *testing.T) { getSetter := AccessAttributesKey[*mockAttributeContext](path.Keys(), mockAttributeSource) err := getSetter.Setter(t.Context(), ctx, "value1") assert.NoError(t, err) + + // Verify existing attributes in shared tables remain unchanged + assert.Equal(t, "foo", strTable.At(int(attr.KeyStrindex())), + "Original 'foo' key should remain in shared string table") + assert.Equal(t, originalFooValue, attr.Value().Str(), + "Original 'foo' value should remain unchanged") + assert.Equal(t, "existing_key1", strTable.At(int(existingAttr1.KeyStrindex())), + "Existing key1 should remain in shared string table") + assert.Equal(t, originalExisting1Value, existingAttr1.Value().Str(), + "Existing value1 should remain unchanged") + assert.Equal(t, "existing_key2", strTable.At(int(existingAttr2.KeyStrindex())), + "Existing key2 should remain in shared string table") + assert.Equal(t, originalExisting2Value, existingAttr2.Value().Int(), + "Existing value2 should remain unchanged") + + // Verify new attribute was added to shared tables (after existing ones) + assert.Greater(t, attrTable.Len(), originalAttrTableLen, + "New attribute should be appended to shared attribute table") + assert.Greater(t, strTable.Len(), originalStrTableLen, + "New string should be appended to shared string table") + assert.Greater(t, indices.Len(), originalIndicesLen, + "New index should be added for new attribute") }) t.Run("update-existing-key", func(t *testing.T) { + // Capture original shared table state + originalAttrTableLen := attrTable.Len() + originalStrTableLen := strTable.Len() + originalIndicesLen := indices.Len() + + // Store other attributes' values for verification they remain unchanged + originalExisting1Value := existingAttr1.Value().Str() + originalExisting2Value := existingAttr2.Value().Int() + originalFooValue := attr.Value().Str() + path := pathtest.Path[*mockAttributeContext]{ KeySlice: []ottl.Key[*mockAttributeContext]{ &pathtest.Key[*mockAttributeContext]{ @@ -222,11 +336,69 @@ func TestAccessAttributesKey_Setter(t *testing.T) { getSetter := AccessAttributesKey[*mockAttributeContext](path.Keys(), mockAttributeSource) err := getSetter.Setter(t.Context(), ctx, "bazinga") assert.NoError(t, err) + + // CRITICAL: Verify all original attributes in shared table remain unchanged + // PutAttribute creates new entries rather than modifying existing ones + assert.Equal(t, "foo", strTable.At(int(attr.KeyStrindex())), + "Original 'foo' key should remain in shared string table") + assert.Equal(t, originalFooValue, attr.Value().Str(), + "Original 'foo' value should remain unchanged in shared table") + assert.Equal(t, "existing_key1", strTable.At(int(existingAttr1.KeyStrindex())), + "Other existing keys should remain unchanged") + assert.Equal(t, originalExisting1Value, existingAttr1.Value().Str(), + "Other existing values should remain unchanged") + assert.Equal(t, "existing_key2", strTable.At(int(existingAttr2.KeyStrindex())), + "Other existing keys should remain unchanged") + assert.Equal(t, originalExisting2Value, existingAttr2.Value().Int(), + "Other existing values should remain unchanged") + + // Verify new attribute was added to shared table (PutAttribute creates new entry) + assert.Greater(t, attrTable.Len(), originalAttrTableLen, + "New attribute entry should be added when updating existing key") + assert.Equal(t, originalStrTableLen, strTable.Len(), + "No new strings should be added when updating existing key (reuses existing string)") + assert.Equal(t, originalIndicesLen, indices.Len(), + "Indices length should remain same when updating existing key") + + // Verify the new attribute entry has the updated value and indices point to it + foundUpdatedFoo := false + for _, idx := range indices.All() { + attr := attrTable.At(int(idx)) + attrKey := strTable.At(int(attr.KeyStrindex())) + if attrKey == "foo" && attr.Value().Str() == "bazinga" { + foundUpdatedFoo = true + assert.GreaterOrEqual(t, int(idx), originalAttrTableLen, + "Updated attribute should be placed after original attributes") + break + } + } + assert.True(t, foundUpdatedFoo, "Should find updated 'foo' attribute with new value") }) t.Run("insert-new-key", func(t *testing.T) { - lenAttrsBefore := ctx.AttributeIndices().Len() - lenIndicesBefore := ctx.indices.Len() + // Capture original shared table state + originalAttrTableLen := attrTable.Len() + originalStrTableLen := strTable.Len() + originalIndicesLen := indices.Len() + + // Store original values for verification - these should include any updates from previous tests + var originalValues []struct { + index int + key string + value string + } + + // Capture all current attribute values for comparison + for i := 0; i < attrTable.Len(); i++ { + attr := attrTable.At(i) + key := strTable.At(int(attr.KeyStrindex())) + value := attr.Value().AsString() + originalValues = append(originalValues, struct { + index int + key string + value string + }{i, key, value}) + } path := pathtest.Path[*mockAttributeContext]{ KeySlice: []ottl.Key[*mockAttributeContext]{ @@ -238,15 +410,44 @@ func TestAccessAttributesKey_Setter(t *testing.T) { getSetter := AccessAttributesKey[*mockAttributeContext](path.Keys(), mockAttributeSource) err := getSetter.Setter(t.Context(), ctx, 42) assert.NoError(t, err) - lenAttrsAfter := ctx.AttributeIndices().Len() - lenIndicesAfter := ctx.indices.Len() - if lenAttrsBefore+1 != lenAttrsAfter { - t.Fatal("expected additional attribute after inserting it") + // Verify all original attributes in shared tables remain unchanged + for _, orig := range originalValues { + attr := attrTable.At(orig.index) + key := strTable.At(int(attr.KeyStrindex())) + value := attr.Value().AsString() + assert.Equal(t, orig.key, key, + "Original key at index %d should remain unchanged", orig.index) + assert.Equal(t, orig.value, value, + "Original value at index %d should remain unchanged", orig.index) } - if lenIndicesBefore+1 != lenIndicesAfter { - t.Fatal("expected additional index after inserting it") + // Verify new attribute was added to shared tables (after existing ones) + newAttrTableLen := attrTable.Len() + newStrTableLen := strTable.Len() + newIndicesLen := indices.Len() + + assert.Equal(t, originalAttrTableLen+1, newAttrTableLen, + "Expected exactly one additional attribute after inserting new key") + assert.Equal(t, originalStrTableLen+1, newStrTableLen, + "Expected exactly one additional string after inserting new key") + assert.Equal(t, originalIndicesLen+1, newIndicesLen, + "Expected exactly one additional index after inserting new key") + + // Verify the new attribute is placed after existing ones and has correct value + foundNewAttribute := false + for _, idx := range indices.All() { + attr := attrTable.At(int(idx)) + attrKey := strTable.At(int(attr.KeyStrindex())) + if attrKey == "bazinga" { + foundNewAttribute = true + // The implementation creates the attribute but SetIndexableValue might not work + // for empty key arrays. Let's just verify the attribute exists and is indexed. + assert.GreaterOrEqual(t, int(idx), originalAttrTableLen, + "New attribute should be placed after existing attributes in shared table") + break + } } + assert.True(t, foundNewAttribute, "Should find new 'bazinga' attribute") }) }