Sentry: Per-project rate limits not working correctly

[MeredithAnya]

As reported from a customer, per-project rate limiting is not respecting the limit they have set. They have limit set to 1000 events in 12 hrs but are seeing upwards of 5K errors in one day.

@getsentry/platform

[dcramer]

@mattrobenolt @tkaemming could one of you take a look at this today? its a high priority concern

[tkaemming]

The frontend is sending this data in minute units, while the quota backend expects them to be seconds. So, what the UI is representing as "12 hour" window (720 minutes) is actually being treated as a "12 minute" (720 second) window by the backend.

Backend Implementation

The key quota query object is constructed as part of RedisQuota.get_quotas:

https://github.com/getsentry/sentry/blob/39353021d957bb47be27b8643e8a20154abfbe1e/src/sentry/quotas/redis.py#L81-L90

The parameters used to construct the quota query object are derived from BaseQuota.get_key_quota, which returns the value directly from the ProjectKey.rate_limit property:

https://github.com/getsentry/sentry/blob/39353021d957bb47be27b8643e8a20154abfbe1e/src/sentry/quotas/base.py#L69-L74

https://github.com/getsentry/sentry/blob/39353021d957bb47be27b8643e8a20154abfbe1e/src/sentry/models/projectkey.py#L123-L127

So, we have the ProjectKey.rate_limit_window value directly propagating to the BasicRedisQuota quota query object without any transformation and being set as BasicRedisQuota.window, which is defined in second units:

https://github.com/getsentry/sentry/blob/39353021d957bb47be27b8643e8a20154abfbe1e/src/sentry/quotas/redis.py#L29-L30

Management UI

The PUT method on the ProjectKeyDetailsEndpoint saves the rate limit details directly from the RateLimitSerializer, which also does no transformation of the input data:

https://github.com/getsentry/sentry/blob/39353021d957bb47be27b8643e8a20154abfbe1e/src/sentry/api/endpoints/project_key_details.py#L99-L105

https://github.com/getsentry/sentry/blob/39353021d957bb47be27b8643e8a20154abfbe1e/src/sentry/api/endpoints/project_key_details.py#L36-L38

On the front end, the rate limit windows are defined here with minute units:

https://github.com/getsentry/sentry/blob/39353021d957bb47be27b8643e8a20154abfbe1e/src/sentry/static/sentry/app/views/projectKeyDetails.jsx#L234-L247

https://github.com/getsentry/sentry/blob/39353021d957bb47be27b8643e8a20154abfbe1e/src/sentry/static/sentry/app/views/projectKeyDetails.jsx#L343-L356

[EvanPurkhiser]

Great investigation overview @tkaemming 🥇

[tkaemming]

I also checked on the customer account that reported this — the UI shows them with a "1000 event(s) in 12 hours" rate limit (as noted in the original issue comment) that was being stored as 'rate_limit_count': 1000, 'rate_limit_window': 720 on the ProjectKey instance to verify.

[dcramer]

@tkaemming would you mind putting up a quick fix for it? we could add a data migration as well, but there's only a handful of accounts using it, so fixing it in prod would also be fine.

[tkaemming]

Yup, was planning on doing that today. I was thinking about fixing it at the model level to avoid the data migration, but I can do the data migration instead if that's your preference.

[dcramer]

no strong opinion -- it'll never be less than 1 minute