-
-
Notifications
You must be signed in to change notification settings - Fork 56
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
feat: Compress uploads #286
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you, @timfish! LGTM.
As a suggestion, you could only trigger compression if the body exceeds a certain size, since gzip also comes with a CPU penalty.
What would be a good threshold? |
I did some quick research, and without explicit benchmarking the threshold is hard to determine. Most resources I found talk about a limit lower than 1k, which would be almost every outgoing request. There is also https://medium.com/@joehonton/the-gzip-penalty-d31bd697f1a2, which seems to tip over at 32k. This gets me thinking: We probably don't need compression for regular event submission, and events are usually under 32k, so let's go with that? |
So |
49ddb85
to
65d3241
Compare
Now with streams and gzip threshold! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great, thanks for going the extra mile.
Any particular reason why this one is not merged yet? |
Fixes #285