Feat: Sentry rate limiting - #2366
Conversation
Signed-off-by: Nicolas Höning <nicolas@seita.nl>
… warn Signed-off-by: Nicolas Höning <nicolas@seita.nl>
Documentation build overview
8 files changed ·
|
Signed-off-by: Felix Claessen <30658763+Flix6x@users.noreply.github.com>
Signed-off-by: Mohamed Belhsan Hmida <mohamedbelhsanhmida@gmail.com>
BelhsanHmida
left a comment
BelhsanHmida
left a comment
There was a problem hiding this comment.
Looks good to me. I left a couple of comments on some things i wanted to point out before approving.
| ) | ||
| else: | ||
| app.logger.error( | ||
| app.logger.warning( |
There was a problem hiding this comment.
just confirming the intention here: since production logging defaults to ERROR, changing this to WARNING means the schema mismatch won't appear in production logs or Sentry.
| hour=0, minute=0, second=0, microsecond=0 | ||
| ) + timedelta(days=1) | ||
| pipeline = redis_connection.pipeline() | ||
| pipeline.incr(counter_key) |
There was a problem hiding this comment.
small edge case: if a host configures error sampling, sampled-out events will still use this allowance because sampling happens after before_send.
There was a problem hiding this comment.
Does "error sampling" refer to a Sentry setting, where only randomized events are sent (I see the traces_sample_rate param).
That might not be a small edge case - those two approaches might not work well, at least we will actually see much less events actually in Sentry if both are active.
Full disclosure: I am not educated in Sentry error sampling right now.
| pipeline = redis_connection.pipeline() | ||
| pipeline.incr(counter_key) | ||
| pipeline.expireat(counter_key, int(expires_at.timestamp())) | ||
| event_count, _ = pipeline.execute() |
There was a problem hiding this comment.
this runs inside Sentry's before_send and the Redis client doesn't have a timeout. If Redis accepts the connection but stops responding, the request/job could hang here instead of failing open. Should we use a separate Redis client with a short timeout for this?
3 participants
Description
On Sentry, we might be on a certain plan, e.g. 5k errors per month.
This PR adds a config setting to rate limit Sentry errors we send to the platform, capped at some amount each day. That way, hosts can spread out errors over the month, instead of spending it all early, then seeing no activity for the remainder.
That setting FLEXMEASURES_SENTRY_DAILY_RATE_LIMIT defaults to None, which leads to no rate limit, or should be a positive integer otherwise.
documentation/changelog.rst