Add ability to discard events under load #849
Conversation
ananthakumaran
force-pushed
the
discard-threshold
branch
from
0984473 to
f604b83
Compare
| @@ -319,6 +329,10 @@ defmodule Sentry.LoggerHandler do | |||
| config.rate_limiting && RateLimiter.increment(handler_id) == :rate_limited -> | |||
| :ok | |||
|
|
|||
| config.discard_threshold > 0 && | |||
There was a problem hiding this comment.
Nit, these are both strictly booleans so you should do
| config.discard_threshold > 0 && | |
| config.discard_threshold > 0 and |
| *since v10.8.2* - The number of queued events after which this handler will start | ||
| to discard events. If you don't want to discard, set this option to `0`. This option |
There was a problem hiding this comment.
I think it would be worth documenting how this interacts with :sync_threshold?
There was a problem hiding this comment.
discard_threshold takes precedence over sync_threshold:
- If
discard_thresholdis set lower thansync_threshold, events will be dropped, and the logger handler will not switch to sync mode. - If
discard_thresholdis set higher thansync_threshold, the logger handler will switch to sync mode, but once thediscard_thresholdis reached, events will be discarded instead of being queued.
Would this work?
There was a problem hiding this comment.
I’m unclear on the second point—if discard_threshold is higher, we will stop queuing events when we reach sync_threshold. Those events will actually block? I am not sure these options are compatible with each other basically.
There was a problem hiding this comment.
My bad, I was assuming queue counter was always increased, but that's not the case. So the counter will never go over the sync_threshold.
discard_threshold and sync_threshold operate independently, with the lower threshold determining the behavior:
- If
discard_thresholdis lower thansync_threshold, events will be discarded once thediscard_thresholdis reached, and the logger handler will not switch to sync mode. - If
sync_thresholdis lower thandiscard_threshold, the logger handler will switch to sync mode once thesync_thresholdis reached, and events will no longer be queued, meaning thediscard_thresholdwill not be reached.
would this work? it seems a bit wordy.
There was a problem hiding this comment.
Follow up question, should I set the default to 0 then? 500 doesn't make sense considering the sync_threshold is 100
Closes #848.