[pkg/ottl] Accessors for profile attributes #39681
Conversation
rockdaboot
requested review from
TylerHelmuth,
bogdandrutu,
evan-bradley,
edmocosta and
a team
as code owners
rockdaboot
force-pushed
the
profiles-attribute-accessors
branch
from
3d6044b to
ecc282f
Compare
rockdaboot
changed the title
rockdaboot
force-pushed
the
profiles-attribute-accessors
branch
from
ecc282f to
6745566
Compare
|
Please mark it as ready for review when open-telemetry/opentelemetry-collector#12798 gets merged. Thanks. |
rockdaboot
force-pushed
the
profiles-attribute-accessors
branch
3 times, most recently
from
df8e707 to
3b1d852
Compare
|
This PR was marked stale due to lack of activity. It will be closed in 14 days. |
andrzej-stencel
added
pkg/ottl
and removed
exporter/honeycombmarker
connector/failover
confmap/provider/secretsmanagerprovider
connector/grafanacloud
connector/roundrobin
connector/sum
connector/otlpjson
processor/logdedup
confmap/provider/aesprovider
connector/signaltometrics
cmd/golden
confmap/provider/googlesecretmanagerprovider
processor/transform
There was a problem hiding this comment.
It looks good to me, thanks for working on that and I'm sorry for the reviewing delay.
Regarding the profile dictionary, whenever I set an attributes it creates another entry on the attributes tables and updates the profile's attributes indices, which I guess is expected as we might have the same key with different values used by different profiles. My only concern is how the transform processor, for example, will impact the size of that table, considering that it happens even when I set the same profile's attribute key. For example, the following statements would generate two profile attributes table entries:
- set(attributes["newKey"], 1)
- set(attributes["newKey"], 2)
I guess it's okay, but I'm wondering if future lower contexts, that will run it in a loop, could become an issue, as this table would grow very quickly depending on the users statements. Just a thought, I understand you cannot tackle it on this PR, but just wanted to double check this behavior.
Yes, that is expected atm. The table is a set of (unique) k/v pairs. But nothing we can tackle in this PR. |
<!--Ex. Fixing a bug - Describe the bug and how this fixes the issue. Ex. Adding a feature - Explain what this achieves.--> #### Description Access to these lookup tables isn't really useful, as they only provide a slice of values, and we can't check which one is being used by the current profile. Also, with open-telemetry/opentelemetry-collector#13075, the lookup tables are moving out of the profile and into a new dictionary object. So as a first step to the proto migration, this removes access to the lookup tables for a profile. The replacement for this is open-telemetry#39681, which will give acces to the profile attributes, as we do with other signals and abstract away the lookup tables. --------- Co-authored-by: Edmo Vamerlatti Costa <[email protected]>
…yName()` internal functions (open-telemetry#39952) #### Description These functions have been requested [here](open-telemetry#39681 (comment)) and [here](open-telemetry#39681 (comment)) as prerequisite for OTTL attributes accessors for the profiles signal. Also adds tests for `GetMap()`, that were missing. #### Testing Unit tests --------- Co-authored-by: Edmo Vamerlatti Costa <[email protected]>
#### Description PR for profiles support in the transform processor. It's a split-out from #39036 and contains only basic functionality, that makes this PR independent from #39681, #39416 and open-telemetry/opentelemetry-collector#12798. For this, several tests were commented out. The reason for this PR is the hope to get it merged so that users and developers can start experimenting with profiles functionality in the transformprocessor. Attributes can't currently can't be used with profiles (due to the above mentioned unmerged PRs). #### Link to tracking issue Fixes #39009 --------- Co-authored-by: Edmo Vamerlatti Costa <[email protected]> Co-authored-by: Tyler Helmuth <[email protected]>
Co-authored-by: Evan Bradley <[email protected]>
#### Description PR for profiles support in the transform processor. It's a split-out from open-telemetry#39036 and contains only basic functionality, that makes this PR independent from open-telemetry#39681, open-telemetry#39416 and open-telemetry/opentelemetry-collector#12798. For this, several tests were commented out. The reason for this PR is the hope to get it merged so that users and developers can start experimenting with profiles functionality in the transformprocessor. Attributes can't currently can't be used with profiles (due to the above mentioned unmerged PRs). #### Link to tracking issue Fixes open-telemetry#39009 --------- Co-authored-by: Edmo Vamerlatti Costa <[email protected]> Co-authored-by: Tyler Helmuth <[email protected]>
Description
Adds support for
profile.attributesto OTTL.This allows users to use profile.attributes without deeper knowledge of the protocol internals.
Currently, users have to deal with attribute_table and attribute_indices, which is error-prone and fragile when the experimental profiling signal changes internal representation.
This change depends on open-telemetry/opentelemetry-collector#12798 and thus includes that PR. It will be removed as soon as possible.
The profiles support for the transformprocessor currently relies/waits on this change.