[DO NOT MERGE] Showcase PR: Add profiling pdata #10109

dmathieu · 2024-05-07T12:03:18Z

Description

This adds the profiling proto schema to pdata, so the receiver/consumer interfaces can then start accepting that data.

Proto files:
https://github.com/open-telemetry/opentelemetry-proto/tree/main/opentelemetry/proto/profiles/v1experimental

Testing

This is being tested by auto-generated tests.

cc @open-telemetry/profiling-approvers @open-telemetry/profiling-maintainers @open-telemetry/profiling-triagers

bogdandrutu

Please do not add an experimental API into the stable pdata module. Split the pprofile part into separate module, and when no longer experimental maybe we can include back into the main pdata.

dmathieu · 2024-05-07T20:02:12Z

Sure.
Should I put it into experimental/pdata/pprofile (in which case pdatagen can't be used, has to be refactored or something else, as it only handles putting generated code into the pdata package), or pdata/pprofileexperimental?

bogdandrutu · 2024-05-08T03:48:53Z

Sure.
Should I put it into experimental/pdata/pprofile (in which case pdatagen can't be used, has to be refactored or something else, as it only handles putting generated code into the pdata package), or pdata/pprofileexperimental?

Just have a different module for pdata/pprofile

dmathieu · 2024-05-08T07:42:04Z

I have moved it into pdata/experimental.
I couldn't move it to experimental/pdata, as we need access to the pdata/internal package.

mx-psi

This doesn't need to be in an experimental folder, we just need it to be in a different Go module versioned 0.x. That way when we go from 0.x to 1.x, we won't need to change the imports for this package

dmathieu · 2024-05-08T17:58:12Z

This is very unclear.
To try to clarify, you want it in pdata/pprofile, but with its own go.mod?

mx-psi · 2024-05-09T10:15:18Z

This is very unclear. To try to clarify, you want it in pdata/pprofile, but with its own go.mod?

Sorry, yes, that's the preferred way of going about this. There is some prior discussion on #6508 and #4705, in particular see the summary on #6508 (comment)

The idea is to minimize changes in import paths while honoring semver (since profiles are still experimental, we need its related packages to be 0.x)

dmathieu · 2024-05-11T09:38:47Z

Thank you very much for your input 😃
I've moved it back into pdata/pprofile, with its own go.mod.

codecov · 2024-05-11T09:54:59Z

Codecov Report

Attention: Patch coverage is 98.12030% with 25 lines in your changes are missing coverage. Please review.

Project coverage is 92.09%. Comparing base (1d52fb9) to head (661d072).

Files	Patch %	Lines
pdata/internal/generated_wrapper_byteslice.go	0.00%	5 Missing ⚠️
pdata/internal/generated_wrapper_float64slice.go	0.00%	5 Missing ⚠️
pdata/internal/generated_wrapper_int64slice.go	54.54%	5 Missing ⚠️
pdata/internal/generated_wrapper_stringslice.go	54.54%	5 Missing ⚠️
pdata/internal/generated_wrapper_uint64slice.go	0.00%	5 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #10109      +/-   ##
==========================================
+ Coverage   91.63%   92.09%   +0.46%     
==========================================
  Files         356      386      +30     
  Lines       16849    18179    +1330     
==========================================
+ Hits        15439    16742    +1303     
- Misses       1068     1094      +26     
- Partials      342      343       +1

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

mx-psi

How are the pcommon slice changes related to the profile? Could we get those in first in a separate PR?

dmathieu · 2024-05-13T18:22:36Z

It's required because the primitive slices struct type only allowed numbers, not strings.
Sure, I'm happy to split this into a set of smaller PRs.

mx-psi · 2024-05-13T19:42:16Z

Sure, I'm happy to split this into a set of smaller PRs.

👌 Thanks, since pcommon is part of a 1.0 module I would like us to consider this changes separately and carefully

@mx-psi

#### Description This adds an `originFieldName` option to `sliceField`, `messageValueField` and `primitiveField`. This option already exists for `primitiveTypedField`, `oneOfField`, `oneOfPrimitiveValue` and `oneOfMessageValue`. #### Link to tracking issue This is needed for #10109.  #### Testing This is unit tested. cc @mx-psi

mx-psi

Note: we need to update the README per

opentelemetry-collector/CONTRIBUTING.md

Line 690 in 27f59cb

8. Update the supported OTLP version in [README.md](./README.md).

on this PR

@mx-psi

#### Description This upgrades the proto to their latest version, and generates the profiles.  #### Link to tracking issue Part of #10109 cc @mx-psi

mx-psi · 2024-05-15T18:58:52Z

#10155 caused a conflict

dmathieu · 2024-05-16T07:24:20Z

Yes, but the commit history isn't the same between my extracted PRs and this one. So I will be opening a new one with the actual pdata changes.

#### Description We upgraded the proto version, so we need to upgrade it in the readme as well.  #### Link to tracking issue * #10109 (review) * #10155

@mx-psi

…umber slices (#10148)  #### Description This adds a string slice to pcommon, that will be used by the profiles pdata. #### Link to tracking issue Part of #10109. #### Testing This is unit-tested. cc @mx-psi --------- Co-authored-by: Pablo Baeyens <pablo.baeyens@datadoghq.com>

dmathieu · 2024-05-21T16:05:00Z

Closing this in favor of the last step PR, #10195.

@mx-psi

#### Description This generates the pdata for profiles.  #### Link to tracking issue Last step of #10109.  #### Testing The generated code is unit-tested by generated tests. cc @mx-psi --------- Co-authored-by: Pablo Baeyens <pbaeyens31+github@gmail.com> Co-authored-by: Pablo Baeyens <pablo.baeyens@datadoghq.com>

@mx-psi

#### Description This adds an `originFieldName` option to `sliceField`, `messageValueField` and `primitiveField`. This option already exists for `primitiveTypedField`, `oneOfField`, `oneOfPrimitiveValue` and `oneOfMessageValue`. #### Link to tracking issue This is needed for open-telemetry#10109.  #### Testing This is unit tested. cc @mx-psi

@mx-psi

#### Description This upgrades the proto to their latest version, and generates the profiles.  #### Link to tracking issue Part of open-telemetry#10109 cc @mx-psi

#### Description We upgraded the proto version, so we need to upgrade it in the readme as well.  #### Link to tracking issue * open-telemetry#10109 (review) * open-telemetry#10155

@mx-psi

…umber slices (open-telemetry#10148)  #### Description This adds a string slice to pcommon, that will be used by the profiles pdata. #### Link to tracking issue Part of open-telemetry#10109. #### Testing This is unit-tested. cc @mx-psi --------- Co-authored-by: Pablo Baeyens <pablo.baeyens@datadoghq.com>

@mx-psi

#### Description This generates the pdata for profiles.  #### Link to tracking issue Last step of open-telemetry#10109.  #### Testing The generated code is unit-tested by generated tests. cc @mx-psi --------- Co-authored-by: Pablo Baeyens <pbaeyens31+github@gmail.com> Co-authored-by: Pablo Baeyens <pablo.baeyens@datadoghq.com>

dmathieu marked this pull request as ready for review May 7, 2024 13:27

dmathieu requested a review from a team as a code owner May 7, 2024 13:27

dmathieu requested a review from songy23 May 7, 2024 13:27

bogdandrutu requested changes May 7, 2024

View reviewed changes

dmathieu force-pushed the profiles branch from 0e69d56 to 8720733 Compare May 8, 2024 07:41

dmathieu requested a review from bogdandrutu May 8, 2024 08:16

mx-psi requested changes May 8, 2024

View reviewed changes

dmathieu added 11 commits May 11, 2024 11:38

generate profiles in pdata

2080995

start generating pprof

9480b87

fully generate the sample

bb2f720

add mapping

afa2b5a

generate location and function

3887436

add attributes

0853d58

generate the remaining fields

f52272e

add changelog entry

c5f86ca

make string slice a primitive type

74b2d8a

move profiling pdata into experimental

a6960f3

move pprofile back into pdata/pprofile, as its own module

f14d107

dmathieu force-pushed the profiles branch from bc6e2c7 to f14d107 Compare May 11, 2024 09:38

add pprofile makefile

0c89c37

dmathieu force-pushed the profiles branch from 5c87b3a to 0c89c37 Compare May 11, 2024 09:45

add pprofile to versions.yaml

b5ebbb4

dmathieu requested a review from mx-psi May 11, 2024 15:16

generate string primitive slice struct

661d072

mx-psi reviewed May 13, 2024

View reviewed changes

This was referenced May 14, 2024

Add stringSlice and int64Slice to pcommon, and let tests handle non-number slices #10148

Merged

Add origin field name option to base fields #10149

Merged

petethepig mentioned this pull request May 15, 2024

Adds Profiles Signal #10154

Draft

dmathieu mentioned this pull request May 15, 2024

Upgrade proto and generate profiles #10155

Merged

petethepig approved these changes May 15, 2024

View reviewed changes

mx-psi reviewed May 15, 2024

View reviewed changes

dmathieu changed the title ~~Add profiling pdata~~ [DO NOT MERGE] Showcase PR: Add profiling pdata May 16, 2024

dmathieu mentioned this pull request May 16, 2024

Upgrade the used otlp version in readme #10167

Merged

dmathieu mentioned this pull request May 21, 2024

Generate profiling pdata #10195

Merged

dmathieu closed this May 21, 2024

dmathieu deleted the profiles branch May 21, 2024 16:05

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[DO NOT MERGE] Showcase PR: Add profiling pdata #10109

[DO NOT MERGE] Showcase PR: Add profiling pdata #10109

dmathieu commented May 7, 2024

bogdandrutu left a comment

dmathieu commented May 7, 2024

bogdandrutu commented May 8, 2024

dmathieu commented May 8, 2024

mx-psi left a comment

dmathieu commented May 8, 2024

mx-psi commented May 9, 2024 •

edited

dmathieu commented May 11, 2024

codecov bot commented May 11, 2024 •

edited

mx-psi left a comment

dmathieu commented May 13, 2024

mx-psi commented May 13, 2024

mx-psi left a comment

mx-psi commented May 15, 2024

dmathieu commented May 16, 2024

dmathieu commented May 21, 2024

[DO NOT MERGE] Showcase PR: Add profiling pdata #10109

[DO NOT MERGE] Showcase PR: Add profiling pdata #10109

Conversation

dmathieu commented May 7, 2024

Description

Testing

bogdandrutu left a comment

Choose a reason for hiding this comment

dmathieu commented May 7, 2024

bogdandrutu commented May 8, 2024

dmathieu commented May 8, 2024

mx-psi left a comment

Choose a reason for hiding this comment

dmathieu commented May 8, 2024

mx-psi commented May 9, 2024 • edited

dmathieu commented May 11, 2024

codecov bot commented May 11, 2024 • edited

Codecov Report

mx-psi left a comment

Choose a reason for hiding this comment

dmathieu commented May 13, 2024

mx-psi commented May 13, 2024

mx-psi left a comment

Choose a reason for hiding this comment

mx-psi commented May 15, 2024

dmathieu commented May 16, 2024

dmathieu commented May 21, 2024

mx-psi commented May 9, 2024 •

edited

codecov bot commented May 11, 2024 •

edited