Skip to content

Remove lookup table access from profiles in ottl #40227

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
May 27, 2025
Merged

Remove lookup table access from profiles in ottl #40227

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
938b6ea
remove lookup table access from profiles in ottl
dmathieu May 22, 2025
546c2d8
add changelog entry
dmathieu May 22, 2025
bf96c14
Update .chloggen/remove-ottl-profiles-lookup-tables.yaml
dmathieu May 26, 2025
f6e24ce
specify each removed field
dmathieu May 26, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 27 additions & 0 deletions .chloggen/remove-ottl-profiles-lookup-tables.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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: breaking

# 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: Remove access to the profile lookup tables

# Mandatory: One or more tracking issues related to the change. You can use the PR number here if no issue exists.
issues: [40227]

# (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: the mapping_table, location_table, function_table, attribute_table, attribute_units, link_table, string_stable have been moved to a root dictionary attribute and are not part of profile anymore.

# 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: [user]
109 changes: 0 additions & 109 deletions pkg/ottl/contexts/internal/ctxprofile/profile.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,22 +31,8 @@ func PathGetSetter[K ProfileContext](path ottl.Path[K]) (ottl.GetSetter[K], erro
return accessSampleType[K](), nil
case "sample":
return accessSample[K](), nil
case "mapping_table":
return accessMappingTable[K](), nil
case "location_table":
return accessLocationTable[K](), nil
case "location_indices":
return accessLocationIndices[K](), nil
case "function_table":
return accessFunctionTable[K](), nil
case "attribute_table":
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we also remove the attribute_indices? Not sure if those indices would be useful without this removed table. Same question applies to other *indices that the lookup table is being removed: location_indices, comment_string_indices, default_sample_type_string_index (seems to use the string_table).

If the plan is to abstract the internal format, I guess we wouldn't want them to be set manually, and some mechanism similar to the attributes would be provided. Is that correct?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The plan is to make the pdata upgrade possible.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree removing the _indices fields makes sense as well, and I'm happy to do it. But I'd rather do that in another PR in order to avoid stalling #40285

return accessAttributeTable[K](), nil
case "attribute_units":
return accessAttributeUnits[K](), nil
case "link_table":
return accessLinkTable[K](), nil
case "string_table":
return accessStringTable[K](), nil
case "time_unix_nano":
return accessTimeUnixNano[K](), nil
case "time":
Expand Down Expand Up @@ -113,34 +99,6 @@ func accessSample[K ProfileContext]() ottl.StandardGetSetter[K] {
}
}

func accessMappingTable[K ProfileContext]() ottl.StandardGetSetter[K] {
return ottl.StandardGetSetter[K]{
Getter: func(_ context.Context, tCtx K) (any, error) {
return tCtx.GetProfile().MappingTable(), nil
},
Setter: func(_ context.Context, tCtx K, val any) error {
if v, ok := val.(pprofile.MappingSlice); ok {
v.CopyTo(tCtx.GetProfile().MappingTable())
}
return nil
},
}
}

func accessLocationTable[K ProfileContext]() ottl.StandardGetSetter[K] {
return ottl.StandardGetSetter[K]{
Getter: func(_ context.Context, tCtx K) (any, error) {
return tCtx.GetProfile().LocationTable(), nil
},
Setter: func(_ context.Context, tCtx K, val any) error {
if v, ok := val.(pprofile.LocationSlice); ok {
v.CopyTo(tCtx.GetProfile().LocationTable())
}
return nil
},
}
}

func accessLocationIndices[K ProfileContext]() ottl.StandardGetSetter[K] {
return ottl.StandardGetSetter[K]{
Getter: func(_ context.Context, tCtx K) (any, error) {
Expand All @@ -152,73 +110,6 @@ func accessLocationIndices[K ProfileContext]() ottl.StandardGetSetter[K] {
}
}

func accessFunctionTable[K ProfileContext]() ottl.StandardGetSetter[K] {
return ottl.StandardGetSetter[K]{
Getter: func(_ context.Context, tCtx K) (any, error) {
return tCtx.GetProfile().FunctionTable(), nil
},
Setter: func(_ context.Context, tCtx K, val any) error {
if v, ok := val.(pprofile.FunctionSlice); ok {
v.CopyTo(tCtx.GetProfile().FunctionTable())
}
return nil
},
}
}

func accessAttributeTable[K ProfileContext]() ottl.StandardGetSetter[K] {
return ottl.StandardGetSetter[K]{
Getter: func(_ context.Context, tCtx K) (any, error) {
return tCtx.GetProfile().AttributeTable(), nil
},
Setter: func(_ context.Context, tCtx K, val any) error {
if v, ok := val.(pprofile.AttributeTableSlice); ok {
v.CopyTo(tCtx.GetProfile().AttributeTable())
}
return nil
},
}
}

func accessAttributeUnits[K ProfileContext]() ottl.StandardGetSetter[K] {
return ottl.StandardGetSetter[K]{
Getter: func(_ context.Context, tCtx K) (any, error) {
return tCtx.GetProfile().AttributeUnits(), nil
},
Setter: func(_ context.Context, tCtx K, val any) error {
if v, ok := val.(pprofile.AttributeUnitSlice); ok {
v.CopyTo(tCtx.GetProfile().AttributeUnits())
}
return nil
},
}
}

func accessLinkTable[K ProfileContext]() ottl.StandardGetSetter[K] {
return ottl.StandardGetSetter[K]{
Getter: func(_ context.Context, tCtx K) (any, error) {
return tCtx.GetProfile().LinkTable(), nil
},
Setter: func(_ context.Context, tCtx K, val any) error {
if v, ok := val.(pprofile.LinkSlice); ok {
v.CopyTo(tCtx.GetProfile().LinkTable())
}
return nil
},
}
}

func accessStringTable[K ProfileContext]() ottl.StandardGetSetter[K] {
return ottl.StandardGetSetter[K]{
Getter: func(_ context.Context, tCtx K) (any, error) {
return tCtx.GetProfile().StringTable().AsRaw(), nil
},
Setter: func(_ context.Context, tCtx K, val any) error {
return ctxutil.SetCommonTypedSliceValues[string](tCtx.GetProfile().StringTable(), val)
},
}
}

func accessTimeUnixNano[K ProfileContext]() ottl.StandardGetSetter[K] {
return ottl.StandardGetSetter[K]{
Getter: func(_ context.Context, tCtx K) (any, error) {
Expand Down
122 changes: 0 additions & 122 deletions pkg/ottl/contexts/internal/ctxprofile/profile_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ 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/contexts/internal/pathtest"
Expand All @@ -32,14 +31,6 @@ func TestPathGetSetter(t *testing.T) {
path: "sample",
val: createSampleSlice(),
},
{
path: "mapping_table",
val: createMappingSlice(),
},
{
path: "location_table",
val: createLocationSlice(),
},
{
path: "location_indices",
val: []int64{5},
Expand All @@ -49,26 +40,6 @@ func TestPathGetSetter(t *testing.T) {
val: []string{"x"},
setFails: true,
},
{
path: "function_table",
val: createFunctionSlice(),
},
{
path: "attribute_table",
val: createAttributeTableSlice(),
},
{
path: "attribute_units",
val: createAttributeUnitSlice(),
},
{
path: "link_table",
val: createLinkSlice(),
},
{
path: "string_table",
val: []string{"", "string"},
},
{
path: "time_unix_nano",
val: int64(123),
Expand Down Expand Up @@ -202,99 +173,6 @@ func createSampleSlice() pprofile.SampleSlice {
return sl
}

func createMappingSlice() pprofile.MappingSlice {
sl := pprofile.NewMappingSlice()
mapping := sl.AppendEmpty()
mapping.CopyTo(createMapping())
return sl
}

func createMapping() pprofile.Mapping {
mapping := pprofile.NewMapping()
mapping.SetFilenameStrindex(2)
mapping.SetFileOffset(1)
mapping.SetHasFilenames(true)
mapping.SetHasFunctions(true)
mapping.SetHasInlineFrames(true)
mapping.SetHasLineNumbers(true)
mapping.SetMemoryLimit(3)
mapping.SetMemoryStart(4)
return mapping
}

func createLocationSlice() pprofile.LocationSlice {
sl := pprofile.NewLocationSlice()
location := sl.AppendEmpty()
location.CopyTo(createLocation())
return sl
}

func createLocation() pprofile.Location {
location := pprofile.NewLocation()
location.SetAddress(1)
location.SetIsFolded(true)
location.SetMappingIndex(2)
return location
}

func createFunctionSlice() pprofile.FunctionSlice {
sl := pprofile.NewFunctionSlice()
function := sl.AppendEmpty()
function.CopyTo(createFunction())
return sl
}

func createFunction() pprofile.Function {
function := pprofile.NewFunction()
function.SetFilenameStrindex(1)
function.SetNameStrindex(2)
function.SetStartLine(3)
function.SetSystemNameStrindex(4)
return function
}

func createAttributeTableSlice() pprofile.AttributeTableSlice {
sl := pprofile.NewAttributeTableSlice()
attribute := sl.AppendEmpty()
attribute.CopyTo(createAttributeTable())
return sl
}

func createAttributeTable() pprofile.Attribute {
attribute := pprofile.NewAttribute()
attribute.SetKey("key")
attribute.Value().SetStr("value")
return attribute
}

func createAttributeUnitSlice() pprofile.AttributeUnitSlice {
sl := pprofile.NewAttributeUnitSlice()
attributeUnit := sl.AppendEmpty()
attributeUnit.CopyTo(createAttributeUnit())
return sl
}

func createAttributeUnit() pprofile.AttributeUnit {
attributeUnit := pprofile.NewAttributeUnit()
attributeUnit.SetUnitStrindex(1)
attributeUnit.SetAttributeKeyStrindex(2)
return attributeUnit
}

func createLinkSlice() pprofile.LinkSlice {
sl := pprofile.NewLinkSlice()
link := sl.AppendEmpty()
link.CopyTo(createLink())
return sl
}

func createLink() pprofile.Link {
link := pprofile.NewLink()
link.SetSpanID(pcommon.SpanID([]byte{1, 2, 3, 4, 5, 6, 7, 8}))
link.SetTraceID(pcommon.TraceID([]byte{16, 15, 14, 13, 12, 11, 10, 9, 8, 7, 6, 5, 4, 3, 2, 1}))
return link
}

func createProfileID() pprofile.ProfileID {
return [16]byte{1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16}
}
Expand Down
7 changes: 0 additions & 7 deletions pkg/ottl/contexts/ottlprofile/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,14 +24,7 @@ The following paths are supported.
| instrumentation_scope.attributes\[""\] | the value of the instrumentation scope attribute of the data point being processed. Supports multiple indexes to access nested fields. | string, bool, int64, float64, pcommon.Map, pcommon.Slice, []byte or nil |
| profile.sample_type | the sample types of the profile being processed | pprofile.ValueTypeSlice |
| profile.sample | the samples of the profile being processed | pprofile.SampleSlice |
| profile.mapping_table | the mapping table of the profile being processed | pprofile.MappingSlice |
| profile.location_table | the location table of the profile being processed | pprofile.LocationSlice |
| profile.location_indices | the location indices of the profile being processed | []int64 |
| profile.function_table | the function table of the profile being processed | pprofile.FunctionSlice |
| profile.attribute_table | the attribute table of the profile being processed | pprofile.AttributeTableSlice |
| profile.attribute_units | the attribute units of the profile being processed | pprofile.AttributeUnitSlice |
| profile.link_table | the link table of the profile being processed | pprofile.LinkSlice |
| profile.string_table | the string table of the profile being processed | []string |
| profile.time_unix_nano | the time in unix nano of the profile being processed | int64 |
| profile.time | the time in `time.Time` of the profile being processed | time.Time |
| profile.duration_unix_nano | the duration in unix nano of the profile being processed | int64 |
Expand Down
Loading