Support for gzip compression in OTLP exporter#6494
Support for gzip compression in OTLP exporter#6494hannahhaering wants to merge 26 commits intoopen-telemetry:mainfrom
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #6494 +/- ##
==========================================
+ Coverage 86.63% 86.83% +0.20%
==========================================
Files 258 258
Lines 11878 11907 +29
==========================================
+ Hits 10290 10340 +50
+ Misses 1588 1567 -21
Flags with carried forward coverage won't be shown. Click here to find out more.
|
|
@hannahhaering The OTLP Exporter currently lacks integration tests. With your proposed changes, did you test them with telemetry systems, and what were the results? Please update the PR description to include more details about how you tested with the collector or other services. It would be beneficial to verify with multiple sources. |
…ahhaering/opentelemetry-dotnet into add-compression-in-exporter
…pcMessageHandler.cs Co-authored-by: Martin Costello <martin@martincostello.com>
…Options.cs Co-authored-by: Martin Costello <martin@martincostello.com>
…pression.cs Co-authored-by: Martin Costello <martin@martincostello.com>
…ahhaering/opentelemetry-dotnet into add-compression-in-exporter
|
This PR was marked stale due to lack of activity and will be closed in 7 days. Commenting or pushing will instruct the bot to automatically remove the label. This bot runs once per day. |
|
This PR was marked stale due to lack of activity and will be closed in 7 days. Commenting or pushing will instruct the bot to automatically remove the label. This bot runs once per day. |
|
This PR was marked stale due to lack of activity and will be closed in 7 days. Commenting or pushing will instruct the bot to automatically remove the label. This bot runs once per day. |
|
This PR was marked stale due to lack of activity and will be closed in 7 days. Commenting or pushing will instruct the bot to automatically remove the label. This bot runs once per day. |
|
This PR was marked stale due to lack of activity and will be closed in 7 days. Commenting or pushing will instruct the bot to automatically remove the label. This bot runs once per day. |
| } | ||
|
|
||
| // TODO: Support compression. | ||
| Stream data = new MemoryStream(buffer, 0, contentLength); |
There was a problem hiding this comment.
The change from ByteArrayContent to MemoryStream + StreamContent introduces a performance regression for the non-compressed path.
When compression is not enabled (which is the default), we're adding an extra allocation per export (2 objects vs. 1 previously). This is a hot path executed on every export batch.
Users who haven't opted into compression should not pay this cost.
rajkumar-rangaraj
left a comment
There was a problem hiding this comment.
We should also explore if compression can be achieved without the additional allocation overhead (e.g., compress directly to network stream, pooled buffers).
|
This PR was marked stale due to lack of activity and will be closed in 7 days. Commenting or pushing will instruct the bot to automatically remove the label. This bot runs once per day. |
|
This PR was marked stale due to lack of activity and will be closed in 7 days. Commenting or pushing will instruct the bot to automatically remove the label. This bot runs once per day. |
|
Closed as inactive. Feel free to reopen if this PR is still being worked on. |
Add support for GZip compression to the OTLP exporter. Picks up from open-telemetry#6494. Resolves open-telemetry#3961. Co-Authored-By: Hannah Haering <157852144+hannahhaering@users.noreply.github.com>
Add support for GZip compression to the OTLP exporter. Picks up from open-telemetry#6494. Resolves open-telemetry#3961. Co-Authored-By: Hannah Haering <157852144+hannahhaering@users.noreply.github.com>
Implements #3961
Changes
Validation/Testing
Merge requirement checklist
CHANGELOG.mdfiles updated for non-trivial changes