[pkg/ottl] add profilelocation

[florianl]

Description

This is the next subtype as listed in #40489.

Link to tracking issue

Fixes #40163.

Testing

Documentation

[florianl]

Friendly ping @TylerHelmuth, @evan-bradley and @edmocosta as the respective code owners.

[florianl]

Friendly ping for feedback @mx-psi

[mx-psi]

@florianl Can you post in #otel-collector-dev (if you get no reply after a reasonable time feel free to ping the codeowners there as well)

[edmocosta]

Please use int64 for getting and setting integer values. OTTL cannot produce uint64 values unless it's coming from non-standardized paths, which should be avoided.

[edmocosta]

It seems we're missing handling the profilesample context (all places), which is a higher location's context.

// ....
// ┌──────────────────┐
// │      Profile     │
// └──────────────────┘
//   │                                n-1
//   │ 1-n         ┌───────────────────────────────────────┐
//   ▼             │                                       ▽
// ┌──────────────────┐   1-n   ┌──────────────┐      ┌──────────┐
// │      Sample      │ ──────▷ │   KeyValue   │      │   Link   │
// └──────────────────┘         └──────────────┘      └──────────┘
//   │                    1-n       △      △
//   │ 1-n        ┌─────────────────┘      │ 1-n
//   ▽            │                        │
// ┌──────────────────┐   n-1   ┌──────────────┐
// │     Location     │ ──────▷ │   Mapping    │
// └──────────────────┘         └──────────────┘
// ....

[florianl]

While the diagram suggests a direct relation between message Sample and message Location this is actually not the case.

https://github.com/open-telemetry/opentelemetry-proto/blob/6836a416404eb8ff73636dd552dcbd59bd7a5a20/opentelemetry/proto/profiles/v1development/profiles.proto#L416-L434

message Sample holds indices to message Location, that are hold in ProfilesDictionary. Therefore there is no direct connection between Sample and Location.

[edmocosta]

If sample holds locations_start_index and locations_length, doesn't it mean it's a logical 1-N relation between both types, even though location is shared? If not, which type owns the sample? I got confused with https://github.com/open-telemetry/opentelemetry-proto/blob/main/opentelemetry/proto/profiles/v1development/profiles.proto#L222-L230 and the diagram, and I'm trying to understand the access pattern here, for example, would we need to access any profilesample field from a profilelocation statement/condition? Like we currently do with spanevent and span: spanevent.attributes["foo"] == "bar" and span.kind != SPAN_KIND_UNSPECIFIED?

[florianl]

would we need to access any profilesample field from a profilelocation statement/condition?

This is hardly possible with reasonable resources. In theory it could be possible by following these steps:

• Figure out the index of profileLocation in ProfilesDictionary
• Walk all profileSample and check for every sample, if the location index is in the scope of locations_start_index to locations_length

One thing, that is no depicted in the shown diagram of the protocol, are the intermediate steps via ProfilesDictionary.
If you look at profileSample, it just holds indices that point to something in ProfilesDictionary.

[edmocosta]

I guess it doesn't make it different from other types then. Abstracting the proto details for a moment, if the logical hierarchy is profile -> sample -> location (indepently of where location is stored), then we're dealing with the same span and spanevent pattern, but with a deeper level.

For example, the transform processor currently supports the profile data, so it goes down into the pdata until it finds the profiles (see). Then it builds the transformation context for that type, including all needed data, and runs it.

Once we add support for the profile's sample, it will require another function, that does almost the same, but going a bit deeper, profiles -> sample.

Finally, to support locations, we would need to do exactly the same, but again, going a level deeper, and also including on the NewTransformContext all data dependencies, so all higher contexts data (up on the proto definition) can be accessed by the child context, something like the following:

func (l profileStatements) ConsumeProfiles(ctx context.Context, ld pprofile.Profiles) error {
	for _, rprofile := range ld.ResourceProfiles().All() {
		for _, sprofile := range rprofile.ScopeProfiles().All() {
			for _, profile := range sprofile.Profiles().All() {
				for _, sample := range profile.Sample().All() {
					for i := sample.LocationsStartIndex(); i < sample.LocationsLength(); i++ {
						location := tCtx.GetProfilesDictionary().LocationTable().At(int(i))
						tCtx := ottlprofilelocation.NewTransformContext(
							profile,
							sample,
							location,
							tCtx.GetProfilesDictionary(),
							sprofile.Scope(),
							rprofile.Resource(),
							sprofile,
							rprofile,
						)
						...
					}
				}
			}
		}
	}
}

Another question we need to answer is: should the transform processor change the pprofile.ProfilesDictionary location entry? and consequently apply the changes to all other data that's referring that same location? or should it work similarly to the profile's attributes, which a copy of the key is created, and indexes are updated to reference the new value?

[florianl]

Thanks for the feedback 🙏 I appreciate that!

Abstracting the proto details for a moment, if the logical hierarchy is profile -> sample -> location (indepently of where location is stored), then we're dealing with the same span and spanevent pattern, but with a deeper level.

Thinking further about the rest of the submessages, this will become a bit of a burden - like profile -> sample -> location -> line -> function. And I'm a bit worried, how this is supposed to scale on the TransformContext side.

should the transform processor change the pprofile.ProfilesDictionary location entry? and consequently apply the changes to all other data that's referring that same location? or should it work similarly to the profile's attributes, which a copy of the key is created, and indexes are updated to reference the new value?

For consistency, I suggest, for all lookup tables in pprofile.ProfilesDictionary like mappings, locations, functions, links and strings, to keep the same mechanics as for attributes.

[florianl]

I will set this change to draft for the moment, to wait for #42107 to land first, to reduce duplicate code, as well to address the mentioned feedback.

[github-actions]

This PR was marked stale due to lack of activity. It will be closed in 14 days.

[github-actions]

Closed as inactive. Feel free to reopen if this PR is still being worked on.

[github-actions]

This PR was marked stale due to lack of activity. It will be closed in 14 days.

[github-actions]

Closed as inactive. Feel free to reopen if this PR is still being worked on.