-
Notifications
You must be signed in to change notification settings - Fork 221
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
Enable batching multiple events into a single POST request over buffer size if maxPostBytes setting is followed #1356
Conversation
…r size if maxPostBytes setting is followed
fb581b2
to
7a9c12a
Compare
BundleMonFiles added (6)
Total files change +104.95KB 0% Final result: ✅ View report in BundleMon website ➡️ |
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.
Nice fix! Batching works as expected again with this change, I can trigger 100 events in quick succession and it's split into 3 batches, rather than 100 requests without this change.
The Content-Length
is slightly larger than maxPostBytes
though, should we take this opportunity to account for payload_data
overhead or something? Even just a static >= (maxPostBytes - 200)
or something would probably be adequate. (We've suggested this setting to some people who have a WAF in front of the collector that blocks requests over a certain size; ideally we'd treat it a bit more strictly, but doesn't have to be perfect; we can always just advice under-specifying it.)
My comment above should help address #1355, let me know if you want me to do a dedicated PR or you can just include it here.
…r size if maxPostBytes setting is followed (#1356)
…r size if maxPostBytes setting is followed (#1356)
…r size if maxPostBytes setting is followed (#1356)
…r size if maxPostBytes setting is followed (#1356)
…r size if maxPostBytes setting is followed (#1356)
…r size if maxPostBytes setting is followed (#1356)
This PR enables POST requests to contain more events than configured through
bufferSize
as long as they don't surpass themaxPostBytes
.The reason for this is to send events queued up in the event store in a single POST request rather than individually. This may happen for instance if some events didn't manage to be saved on the previous page visit (or the collector was not reachable). With this feature, they should be sent in one batch request on the next visit.
It may also happen in events are tracked right after each other – if they manage to be saved to the event store before the flush function is called, they will be batched together.
The change might be useful also with issue #1355 where part of the problem could be too many requests done shortly after each other.