[receiver/pprof] attempt to convert from pprof to pdata and back by atoulme · Pull Request #40548 · open-telemetry/opentelemetry-collector-contrib

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

+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

+	// 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

Original payload refers to profiling data serialized under a different format (e.g. JFR or pprof) that was converted into the OTel profiling format. Note also how the documentation explicitly states that if the original data is in pprof format then it SHOULD not be included in this field (and thus one wouldn't set original_payload_format either), presumably because OTel profiling format is a superset of pprof and conversions can take place without loss of information.

I think you got confused because you probably read this bit (that is vague and doesn't specifically refer to profiling semantic conventions) and extrapolated that the semantic conventions it refers to (which AFAIK do not yet exist) are the profiling frame type conventions (which are completely unrelated). So to avoid this confusion in the future, we need to either reword this documentation comment about a semantic conventions reference that seemingly doesn't exist or introduce it.

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

+	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

+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

+	// 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

+	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.

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

+			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

+		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

+		// 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 · 2025-07-01T05:22:28Z

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

github-actions · 2025-07-15T05:22:58Z

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

github-actions · 2025-08-06T05:23:41Z

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

florianl · 2025-08-06T05:41:33Z

I'm interested in picking up this task, once the OTel Profiling signal settled more. Maybe with the next v1.8.0 release of OTel proto.

github-actions · 2025-08-22T05:21:56Z

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

felixge · 2025-09-04T18:07:41Z

I started a thread for this in slack, but I agree with @florianl that we need to wait for the collector to upgrade to v1.8.0 before proceeding here.

github-actions · 2025-09-20T05:20:44Z

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

Follow up to open-telemetry#40548 TODO: - [ ] verify result - [ ] more tests Signed-off-by: Florian Lehner <florian.lehner@elastic.co>

@atoulme

…les (#42843) Follow up to #40548 FYI: @atoulme @felixge @aalexand TODO: - [ ] verify result - [ ] more tests --------- Signed-off-by: Florian Lehner <florian.lehner@elastic.co> Co-authored-by: Roger Coll <roger.coll@elastic.co>

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>

github-actions Bot added the Stale label Jul 1, 2025

github-actions Bot closed this Jul 15, 2025

atoulme reopened this Jul 23, 2025

github-actions Bot removed the Stale label Jul 23, 2025

github-actions Bot added the Stale label Aug 6, 2025

github-actions Bot removed the Stale label Aug 7, 2025

github-actions Bot added the Stale label Aug 22, 2025

MovieStoreGuy removed the Stale label Aug 22, 2025

github-actions Bot added the Stale label Sep 20, 2025

florianl mentioned this pull request Sep 23, 2025

[receiver/pprof] initial commit to convert google/pprof to OTel profiles #42843

Merged

2 tasks

atoulme closed this Oct 4, 2025

atoulme mentioned this pull request Nov 5, 2025

New component: pprofreceiver #38260

Closed

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

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!

christos68k Sep 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

github-actions Bot commented Jul 1, 2025

Uh oh!

github-actions Bot commented Jul 15, 2025

Uh oh!

github-actions Bot commented Aug 6, 2025

Uh oh!

florianl commented Aug 6, 2025

Uh oh!

github-actions Bot commented Aug 22, 2025

Uh oh!

felixge commented Sep 4, 2025

Uh oh!

github-actions Bot commented Sep 20, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

christos68k Sep 4, 2025 •

edited

Loading