[receiver/pprof] attempt to convert from pprof to pdata and back #40548

atoulme · 2025-06-09T07:06:45Z

This is not ready - opening for discussion and to help move the effort forward.

florianl

Just a first quick look

florianl · 2025-06-11T07:52:34Z

receiver/pprofreceiver/internal/pprof_to_pdata.go

+func pprofToPprofile(parsed *profile.Profile, originalPayload []byte) pprofile.Profiles {
+	result := pprofile.NewProfiles()
+	// create a new profile record
+	resultProfile := result.ResourceProfiles().AppendEmpty().ScopeProfiles().AppendEmpty().Profiles().AppendEmpty()


A single Resource Profile in OTel Profiling can have multiple Scope Profiles. And OTel eBPF Profiler is already using multiple Scope Profiles to differentiate between on CPU and off CPU profiling. Mixing these profiles will cause issues.

Is that defined somewhere I can reuse?

To add more complexity 🙈 open-telemetry/opentelemetry-ebpf-profiler#517 will implement the recent changes in the OTel Profiling signal to generate a single ResourceProfile per container and an additional ResourceProfile for all non-containerized processes. As, I think, google/pprof does not differentiate between containerized elements and others, I think it is hard to map this correctly. Google/pprof might be able to set attributes accordingly, but they might not be standartized.

But I think, there should be one OTel/ScopeProfile per Google/pprof Profile.sample_type. This will then also be according to open-telemetry/opentelemetry-proto#649.

OK, can this be clarified in a spec? This is a bit of a departure from other signals where scope typically is to identify the instrumentation used.

florianl · 2025-06-11T07:54:14Z

receiver/pprofreceiver/internal/pprof_to_pdata.go

+	// per https://github.com/open-telemetry/semantic-conventions/blob/main/model/profile/registry.yaml
+	resultProfile.SetOriginalPayloadFormat("go")


There is no direct correlation between the frame types that are reported with OTel Profiling (https://github.com/open-telemetry/semantic-conventions/blob/main/model/profile/registry.yaml) and the SetOriginalPayloadFormat. Regular OTel Profiles will also contain python and frames from other programing languges.

I didn't understand, sorry. Is this value invalid?

Sorry, I was confused about this element.

The best value, when converting Google/pprof to OTel Profiling should be pprof or something similar, as it should indicate the original resource (google/pprof in this case).

that's not what the spec says though - pointing back at the schema you linked, the exhaustive list of enums has go: https://github.com/open-telemetry/semantic-conventions/blob/main/model/profile/registry.yaml#L71

The spec you are referring to is about profile.frame.type and Describes the interpreter or compiler of a single frame.. The known list of frame types is in no relation to SetOriginalPayloadFormat.

The type of a single frame is different to the format of SetOriginalPayloadFormat. To my knowledge, there is no enum/list that describes known values for SetOriginalPayloadFormat.

The field original_payload_format is closely tight to the field original_payload. And in this context, JFR is named as a potential payload (and respective format value) in the proto file:

https://github.com/open-telemetry/opentelemetry-proto/blob/fe18c4b149ed308a7af4b47075c32742e30ebd59/opentelemetry/proto/profiles/v1development/profiles.proto#L264-L273

florianl · 2025-06-11T07:56:25Z

receiver/pprofreceiver/internal/pprof_to_pdata.go

+	hash = md5.Sum(originalPayload) //nolint:gosec
+
+	// set period
+	resultProfile.SetPeriod(parsed.Period)


Here is also a conflict with off CPU profiling as off CPU profiling does not have a regular period but is triggered by scheduler events. So its cadence can change depending on the workload and is not constant, like sampling based on CPU profiling.

should I put something else here? Or not set it in some situations?

It depends, I guess 😅

If the input originates from a sampling based approach, then this value should be set. Otherwise, it should be left out.

I cannot really know that, can I? Whatever pprof provides can be passed in and pprof can make that assertion?

florianl · 2025-06-14T10:46:17Z

receiver/pprofreceiver/internal/pprof_to_pdata.go

+func pprofToPprofile(parsed *profile.Profile, originalPayload []byte) pprofile.Profiles {
+	result := pprofile.NewProfiles()
+	// create a new profile record
+	resultProfile := result.ResourceProfiles().AppendEmpty().ScopeProfiles().AppendEmpty().Profiles().AppendEmpty()


To add more complexity 🙈 open-telemetry/opentelemetry-ebpf-profiler#517 will implement the recent changes in the OTel Profiling signal to generate a single ResourceProfile per container and an additional ResourceProfile for all non-containerized processes. As, I think, google/pprof does not differentiate between containerized elements and others, I think it is hard to map this correctly. Google/pprof might be able to set attributes accordingly, but they might not be standartized.

But I think, there should be one OTel/ScopeProfile per Google/pprof Profile.sample_type. This will then also be according to open-telemetry/opentelemetry-proto#649.

florianl · 2025-06-14T10:48:45Z

receiver/pprofreceiver/internal/pprof_to_pdata.go

+	// per https://github.com/open-telemetry/semantic-conventions/blob/main/model/profile/registry.yaml
+	resultProfile.SetOriginalPayloadFormat("go")


Sorry, I was confused about this element.

The best value, when converting Google/pprof to OTel Profiling should be pprof or something similar, as it should indicate the original resource (google/pprof in this case).

florianl · 2025-06-14T10:59:46Z

receiver/pprofreceiver/internal/pprof_to_pdata.go

+	hash = md5.Sum(originalPayload) //nolint:gosec
+
+	// set period
+	resultProfile.SetPeriod(parsed.Period)


It depends, I guess 😅

If the input originates from a sampling based approach, then this value should be set. Otherwise, it should be left out.

receiver/pprofreceiver/internal/pprof_to_pdata.go

florianl · 2025-06-14T11:08:01Z

receiver/pprofreceiver/internal/pprof_to_pdata.go

+			resultL := result.ProfilesDictionary().LocationTable().AppendEmpty()
+			resultL.SetAddress(l.Address)
+			resultL.SetIsFolded(l.IsFolded)
+			for i, m := range result.ProfilesDictionary().MappingTable().All() {


This might not scale well, if OTels result.ProfilesDictionary().MappingTable() is prepopulated and for each Location all populated elements are iterated. Maybe just add the mapping from google/pprof into ProfilesDictionary().MappingTable at this point and set the OTel Location.mapping_index accordingly.

we are not trying to scale, just make a roundtrip pprof -> otlp -> pprof. After that works we can fine tune.

florianl · 2025-06-14T11:11:57Z

receiver/pprofreceiver/internal/pprof_to_pdata.go

+		resultS.SetLocationsLength(int32(len(parsed.Location)))
+	}
+
+	for _, f := range parsed.Function {


Similar to mappings, the resulting connection between OTel Sample.Location.Line.Function is missing. So I would also suggest to add the google/pprof Function to the OTel Function Dictionary while encountering it while iterating over google Samples.

florianl · 2025-06-14T11:14:48Z

receiver/pprofreceiver/internal/pprof_to_pdata.go

+		// TODO set aggregation temporality
+	}
+
+	commentsIndices := make([]int32, len(parsed.Comments))


As google/pprof mixes sample types and profiles in general, it might be hard to assign the google/Profile.comment to the corresponding OTel Profile.comment_strindices, as there could be multiple OTel Profiles per google/Profile.

Co-authored-by: Florian Lehner <florianl@users.noreply.github.com>

github-actions bot added the receiver/pprof label Jun 9, 2025

github-actions bot requested a review from MovieStoreGuy June 9, 2025 07:07

atoulme force-pushed the pprofreceiver_impl branch 3 times, most recently from aa49ba8 to 856302b Compare June 9, 2025 23:21

[receiver/pprof] attempt to convert from pprof to pdata and back

9ed9b02

atoulme force-pushed the pprofreceiver_impl branch from 856302b to 9ed9b02 Compare June 9, 2025 23:29

florianl reviewed Jun 11, 2025

View reviewed changes

florianl reviewed Jun 14, 2025

View reviewed changes

Update receiver/pprofreceiver/internal/pprof_to_pdata.go

2bef57a

Co-authored-by: Florian Lehner <florianl@users.noreply.github.com>

		// per https://github.com/open-telemetry/semantic-conventions/blob/main/model/profile/registry.yaml
		resultProfile.SetOriginalPayloadFormat("go")

[receiver/pprof] attempt to convert from pprof to pdata and back #40548

Are you sure you want to change the base?

[receiver/pprof] attempt to convert from pprof to pdata and back #40548

Uh oh!

Conversation

atoulme commented Jun 9, 2025

Uh oh!

florianl left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!