Send client report when flushing queue#1757
Merged
mattjohnsonpint merged 5 commits intomainfrom Jun 28, 2022
Merged
Conversation
Also cleaned up FlushAsync implementation
mattjohnsonpint
requested review from
SimonCropp and
bruno-garcia
as code owners
bruno-garcia
approved these changes
Jun 28, 2022
3 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
During shutdown (and when manually requested), we flush the worker queue of all events. However, if there are no events to send, we'll potentially lose accumulated client report information.
This updates the
BackgroundWorker.FlushAsyncmethod to send a final client report after flushing, if there is one to send. That also ensures that if events are rate-limited as they are flushed, that the count of discarded events from rate limiting will be sent.Note that we still respect the shutdown timeout, so if the queue is very backed up there's a chance the client report won't get sent. If instead we wanted to ensure that it always get sent, then we would need to be ok with going past the timeout period.
Additionally, this PR includes some general implementation improvements with
FlushAsync, and an unrelated typo fixed.