Skip to content

feat: Add opportunistic Brotli compression #3612

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 12 commits into from
Oct 8, 2024
Merged

feat: Add opportunistic Brotli compression #3612

merged 12 commits into from
Oct 8, 2024

Conversation

BYK
Copy link
Member

@BYK BYK commented Oct 4, 2024

Brotli level 4 and 5 offer comparable or better compression to GZip
level 9 (which is our default) with better performance. This patch
adds opportunistic Brotli compression at level 4 (to be conservative)
when it detects the brotli module is available. It also provides some
escape hatches through transport_compression_level and
transport_compression_algo experiment configs to fine tune the behavior.

In the future, we may want to bump the default level from 4 to 5 for better
compression.

Brotli level 4 and 5 offer comparable or better compression to GZip
level 9 (which is our default) with better performance. This patch
adds opportunistic Brotli compression at level 4 (to be conservative)
when it detects the `brotli` module is available. It also provides some
escape hatches through `transport_compression_level` and
`transport_compression_algo` experiment configs to fine tune the behavior.

In the future, we may want to bump the default level from 4 to 5 for better
compression.
Copy link

codecov bot commented Oct 4, 2024

Codecov Report

Attention: Patch coverage is 88.00000% with 6 lines in your changes missing coverage. Please review.

Project coverage is 84.26%. Comparing base (4f79aec) to head (76d7bae).
Report is 1 commits behind head on master.

✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
sentry_sdk/transport.py 87.23% 5 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #3612      +/-   ##
==========================================
- Coverage   84.27%   84.26%   -0.01%     
==========================================
  Files         133      133              
  Lines       14031    14061      +30     
  Branches     2957     2964       +7     
==========================================
+ Hits        11825    11849      +24     
- Misses       1466     1471       +5     
- Partials      740      741       +1     
Files with missing lines Coverage Δ
sentry_sdk/consts.py 99.51% <100.00%> (+<0.01%) ⬆️
sentry_sdk/transport.py 84.67% <87.23%> (-0.53%) ⬇️

... and 2 files with indirect coverage changes

Copy link
Contributor

@sentrivana sentrivana left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

@sentrivana sentrivana added the Trigger: tests using secrets PR code is safe; run CI label Oct 7, 2024
@github-actions github-actions bot removed the Trigger: tests using secrets PR code is safe; run CI label Oct 7, 2024
@sentrivana sentrivana added the Trigger: tests using secrets PR code is safe; run CI label Oct 8, 2024
@sentrivana sentrivana merged commit d34c99a into getsentry:master Oct 8, 2024
137 of 139 checks passed
@BYK BYK deleted the byk/feat/opportunistic-brotli branch October 8, 2024 10:13
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
Trigger: tests using secrets PR code is safe; run CI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants