Skip to content
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.

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

[pkg/ottl] Add support for profiles #36104

Open
dmathieu opened this issue Oct 31, 2024 · 21 comments · May be fixed by #37574
Open

[pkg/ottl] Add support for profiles #36104

dmathieu opened this issue Oct 31, 2024 · 21 comments · May be fixed by #37574
Labels
data:profiles Profiles related issues help wanted Extra attention is needed pkg/ottl priority:p2 Medium

Comments

@dmathieu
Copy link
Member

Component(s)

pkg/ottl

Describe the issue you're reporting

We should add support for profiles in the ottlp package, so components relying on it may start adding profiles support as well.

Related: #35982

@dmathieu dmathieu added the needs triage New item requiring triage label Oct 31, 2024
Copy link
Contributor

Pinging code owners:

See Adding Labels via Comments if you do not have permissions to add labels yourself.

@dmathieu
Copy link
Member Author

/label help-wanted data:profiles priority:p2

@github-actions github-actions bot added the help wanted Extra attention is needed label Oct 31, 2024
@thiha-min-thant
Copy link

Hi @dmathieu , could you please assign to me? Thanks!

@mx-psi mx-psi added priority:p2 Medium data:profiles Profiles related issues and removed needs triage New item requiring triage labels Oct 31, 2024
@dmathieu
Copy link
Member Author

I can't assign issues. But I'd say feel free to start working on this.

@TylerHelmuth
Copy link
Member

@dmathieu is there anything stateful in OTEL profiles?

@dmathieu
Copy link
Member Author

Not in the OTLP, nor the collector.
The agent could have some statefulness, but nothing that's kept across restarts or gets propagated over the wire.

@TylerHelmuth
Copy link
Member

Ok cool. In that case to make profiles compatible with OTTL all that is needed is a new ottlprofiles context in ottl/contexts.

Once that exists other components that use OTTL can be updated to use the new profiles context.

@TylerHelmuth TylerHelmuth changed the title [ottl] Add support for profiles [plg/ottl] Add support for profiles Nov 15, 2024
@TylerHelmuth TylerHelmuth changed the title [plg/ottl] Add support for profiles [pkg/ottl] Add support for profiles Nov 15, 2024
Copy link
Contributor

This issue has been inactive for 60 days. It will be closed in 60 days if there is no activity. To ping code owners by adding a component label, see Adding Labels via Comments, or if you are unsure of which component this issue relates to, please ping @open-telemetry/collector-contrib-triagers. If this issue is still relevant, please ping the code owners or leave a comment explaining why it is still relevant. Otherwise, please close it.

Pinging code owners:

See Adding Labels via Comments if you do not have permissions to add labels yourself.

@github-actions github-actions bot added the Stale label Jan 15, 2025
@rockdaboot rockdaboot linked a pull request Jan 29, 2025 that will close this issue
@TylerHelmuth
Copy link
Member

@rockdaboot i believe we are holding off on this until the pdata implementation is more stable. @dmathieu is the breaking change in the proto implemented yet?

@dmathieu
Copy link
Member Author

It is. @rockdaboot has a draft PR in the works for this.

@github-actions github-actions bot removed the Stale label Jan 30, 2025
@rockdaboot
Copy link
Contributor

Something I'd like to talk about is the strategy to make all sub-types of Profile accessible.

As an example, #37574 implements a TransformContext for Profile and Sample.

Should wecontinue with the same pattern for Link, Line, Function, Mapping, etc.?
Tbh, I am not sure what the alternative is.

@TylerHelmuth
Copy link
Member

Are those sub-types more similar to span and span event or datapoint and examplar?

@rockdaboot
Copy link
Contributor

Are those sub-types more similar to span and span event or datapoint and examplar?

What is the fundamental difference of e.g. span and datapoint? The code looks very similar to me.

@TylerHelmuth
Copy link
Member

I mean in the their concept in open telemetry. Span Events are used within a Span, but they end up being used a lot - people very quickly wanted access to a Span Event context so they could filter them/transform them.

The same has not been the same for Examplars. Examplars are a part of a datapoint, but we don't have any requests from users who want to modify them.

If it is unlikely users will want/need to transform or filter Link, Line, Function, or Mapping sub-types, then for now we should not make an ottl context for them.

@rockdaboot
Copy link
Contributor

rockdaboot commented Feb 10, 2025

If it is unlikely users will want/need to transform or filter Link, Line, Function, or Mapping sub-types, then for now we should not make an ottl context for them.

Good point. So if I understand correctly, filtering requires read/write (transform) of the profiles before they are passed further to exporters. Correct me if I am wrong.

We know that some filters are obviously useful, like filtering for sample type (e.g. on-cpu or off-cpu) or filtering for executable or process name (an attribute).
The result of filtering will mostly be removing samples.

Another use case might be collapsing/aggregating samples. The receiver code creates nanosecond precision timestamps. Reducing the resolution in a processor helps reducing storage costs and would be done by aggregating multiple identical samples and increasing the count (sample.value) appropriately. But for this, we need a new profilesprocessor, I guess.

Amending arbitrary fields is likely not a common use-case, e.g. why amending link->spanid values to something else?

Should we first go ahead with ottlprofile + ottlprofilesample? And add anything else when requested?

@TylerHelmuth
Copy link
Member

Would a user ever want to transform a Sample, Mapping, Line as an individual concept or is their value only every as a whole collection? I believe we should start with only ottlprofile as I suspect manipulation of any subtypes will need to be done with the whole collection in mind.

@rockdaboot
Copy link
Contributor

Would a user ever want to transform a Sample, Mapping, Line as an individual concept or is their value only every as a whole collection? I believe we should start with only ottlprofile as I suspect manipulation of any subtypes will need to be done with the whole collection in mind.

As mentioned above, Sample.Value will possible be something that needs to be amended. For on-cpu profiles, sample_type.unit == "count" will be used by the profiles receiver.

Otherwise I agree.

@rockdaboot
Copy link
Contributor

rockdaboot commented Feb 10, 2025

The reason I started with OTTL was because I wanted to add profiles support to the attributesprocessor. Profiles have attributes in several types (Profile, Sample, Mapping, Location). How are we going to expose this to the user?

@TylerHelmuth
Copy link
Member

Oh well in that case yes, we'd need ottl context for each of those eventually. But if they are like other OTLP signals we probably don't need them all immediately because users likely don't need the ability to change all those attributes immediately.

For example, the attributesprocessor doesn't like you set attributes on span events or span links. OTTL lets you do it on span events because users asked for that, but it still doesn't like you change attributes on span links, because there has not yet been enough desire.

I suspect the profile subtypes are similar, but I don't know which subtypes for which users will want to modify attributes and which they wont.

@TylerHelmuth
Copy link
Member

We'll need to implement it in separate PRs anyways, so I think the best way to move forward is to implement a ottlprofiles context and then follow it up with an ottlsamples context.

@rockdaboot
Copy link
Contributor

We'll need to implement it in separate PRs anyways, so I think the best way to move forward is to implement a ottlprofiles context and then follow it up with an ottlsamples context.

Removed the samples context and undrafted the PR.

One thing that is missing and would be helpful is a StandardGetSetter for Attributes. I'd like to use open-telemetry/opentelemetry-collector#12390 once it is merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
data:profiles Profiles related issues help wanted Extra attention is needed pkg/ottl priority:p2 Medium
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants