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

Profiles support in OTLP Exporter #11131

Closed
wants to merge 22 commits into from

Conversation

dmathieu
Copy link
Member

@dmathieu dmathieu commented Sep 11, 2024

Description

This adds profiles support to the OTLP exporters.
Note: This includes some commits from #11071 that were required in here too.

@dmathieu dmathieu force-pushed the otlpexporter branch 2 times, most recently from da2d91f to 001e190 Compare September 11, 2024 10:07
Copy link

codecov bot commented Sep 11, 2024

Codecov Report

Attention: Patch coverage is 91.75258% with 24 lines in your changes missing coverage. Please review.

Project coverage is 91.78%. Comparing base (1391fab) to head (713f25d).
Report is 138 commits behind head on main.

Files with missing lines Patch % Lines
.../exporterhelper/exporterhelperprofiles/profiles.go 91.80% 3 Missing and 2 partials ⚠️
exporter/exporterhelper/internal/obsexporter.go 0.00% 5 Missing ⚠️
exporter/otlphttpexporter/factory.go 76.47% 2 Missing and 2 partials ⚠️
exporter/otlphttpexporter/otlp.go 88.88% 3 Missing and 1 partial ⚠️
...terhelper/exporterhelperprofiles/profiles_batch.go 97.75% 1 Missing and 1 partial ⚠️
exporter/exporterhelper/internal/request.go 0.00% 2 Missing ⚠️
exporter/otlpexporter/otlp.go 84.61% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #11131      +/-   ##
==========================================
- Coverage   91.79%   91.78%   -0.02%     
==========================================
  Files         422      426       +4     
  Lines       20023    20285     +262     
==========================================
+ Hits        18380    18618     +238     
- Misses       1266     1283      +17     
- Partials      377      384       +7     

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

@dmathieu dmathieu marked this pull request as ready for review September 11, 2024 11:50
@dmathieu dmathieu requested review from a team and jpkrohling September 11, 2024 11:51
@mx-psi mx-psi added this to the Profiling support milestone Sep 12, 2024
exporter/otlpexporter/README.md Outdated Show resolved Hide resolved
exporter/exporterhelper/profiles.go Outdated Show resolved Hide resolved
@dmathieu dmathieu force-pushed the otlpexporter branch 4 times, most recently from 055a6bb to cb0a2a3 Compare September 12, 2024 13:26
mx-psi pushed a commit that referenced this pull request Sep 19, 2024
…#11215)

<!--Ex. Fixing a bug - Describe the bug and how this fixes the issue.
Ex. Adding a feature - Explain what this achieves.-->
#### Description

This is part of #11131, splitting the exporterhelper module with a new
`internal` submodule that holds everything shared by the signals.
This is so a new `exporterhelperprofiles` module can make use of the
shared structs.
jackgopack4 pushed a commit to jackgopack4/opentelemetry-collector that referenced this pull request Sep 19, 2024
…open-telemetry#11215)

<!--Ex. Fixing a bug - Describe the bug and how this fixes the issue.
Ex. Adding a feature - Explain what this achieves.-->
#### Description

This is part of open-telemetry#11131, splitting the exporterhelper module with a new
`internal` submodule that holds everything shared by the signals.
This is so a new `exporterhelperprofiles` module can make use of the
shared structs.
codeboten pushed a commit that referenced this pull request Sep 19, 2024
)

#### Description

Split off from #11131. Prepares OTLP HTTP exporter for supporting
profiles (which uses `/v1development`)

Co-authored-by: Damien Mathieu <42@dmathieu.com>
@dmathieu dmathieu requested a review from a team as a code owner September 20, 2024 07:22
@dmathieu dmathieu force-pushed the otlpexporter branch 3 times, most recently from bd6d2cd to d27e777 Compare September 20, 2024 07:45
dmathieu added a commit to dmathieu/opentelemetry-collector that referenced this pull request Sep 20, 2024
This was authored by @dmathieu on PR open-telemetry#11131

Co-authored-by: Damien Mathieu <42@dmathieu.com>
mx-psi pushed a commit that referenced this pull request Sep 20, 2024
<!--Ex. Fixing a bug - Describe the bug and how this fixes the issue.
Ex. Adding a feature - Explain what this achieves.-->
#### Description

This is an extraction from
#11131.

cc @mx-psi
dmathieu added a commit to dmathieu/opentelemetry-collector that referenced this pull request Sep 20, 2024
This was authored by @dmathieu on PR open-telemetry#11131

Co-authored-by: Damien Mathieu <42@dmathieu.com>
mx-psi pushed a commit that referenced this pull request Oct 4, 2024
<!--Ex. Fixing a bug - Describe the bug and how this fixes the issue.
Ex. Adding a feature - Explain what this achieves.-->
#### Description
This is the initial work towards #11155.
This PR adds profiles marshalers to `otlptext`, which will be used in
the `debugexporter` later on.

To continue the work on the `debugexporter` I'd wait on the PR #11131,
as it adds some shared components.

<!--Describe what testing was performed and which tests were added.-->
#### Testing
Added `TestProfilesText`, testing the following scenarios: 
- `empty_profiles`
- `two_profiles`
Copy link
Contributor

github-actions bot commented Oct 5, 2024

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

@github-actions github-actions bot added Stale and removed Stale labels Oct 5, 2024
jackgopack4 pushed a commit to jackgopack4/opentelemetry-collector that referenced this pull request Oct 8, 2024
<!--Ex. Fixing a bug - Describe the bug and how this fixes the issue.
Ex. Adding a feature - Explain what this achieves.-->
#### Description
This is the initial work towards open-telemetry#11155.
This PR adds profiles marshalers to `otlptext`, which will be used in
the `debugexporter` later on.

To continue the work on the `debugexporter` I'd wait on the PR open-telemetry#11131,
as it adds some shared components.

<!--Describe what testing was performed and which tests were added.-->
#### Testing
Added `TestProfilesText`, testing the following scenarios: 
- `empty_profiles`
- `two_profiles`
mx-psi added a commit that referenced this pull request Oct 9, 2024
<!--Ex. Fixing a bug - Describe the bug and how this fixes the issue.
Ex. Adding a feature - Explain what this achieves.-->
#### Description

Split off from #11131. Adds new module for consumer errors for profiles.

---------

Co-authored-by: Damien Mathieu <42@dmathieu.com>
@mx-psi
Copy link
Member

mx-psi commented Oct 9, 2024

@dmathieu Can you fix the merge conflicts?

@dmathieu
Copy link
Member Author

Closing this PR, as everything it contains has been split into multiple smaller ones.

@dmathieu dmathieu closed this Oct 15, 2024
@dmathieu dmathieu deleted the otlpexporter branch October 15, 2024 09:23
mx-psi added a commit that referenced this pull request Oct 16, 2024
<!--Ex. Fixing a bug - Describe the bug and how this fixes the issue.
Ex. Adding a feature - Explain what this achieves.-->
#### Description

This is extracted from #11131.
It adds profiles support for the epxorterhelper, so both the otlp and
otlphttp exporters can support that signal.

It requires #11207✅  and #11225✅  to be merged first.

---------

Co-authored-by: Pablo Baeyens <pbaeyens31+github@gmail.com>
Co-authored-by: Pablo Baeyens <pablo.baeyens@datadoghq.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants