From 80c1d878a8579d67101cfac2000b36baebf1e715 Mon Sep 17 00:00:00 2001 From: Markus Unterwaditzer <markus-honeypot@unterwaditzer.net> Date: Mon, 2 Nov 2020 16:19:03 +0100 Subject: [PATCH 1/3] fix(quotas): Apply grace only to redis ttl, not the actual retry-after --- relay-quotas/src/redis.rs | 4 ++-- tests/integration/test_store.py | 16 +++++++++++----- 2 files changed, 13 insertions(+), 7 deletions(-) diff --git a/relay-quotas/src/redis.rs b/relay-quotas/src/redis.rs index 60a6a5bceb1..736ea38406e 100644 --- a/relay-quotas/src/redis.rs +++ b/relay-quotas/src/redis.rs @@ -93,7 +93,7 @@ impl<'a> RedisQuota<'a> { fn expiry(&self) -> UnixTimestamp { let next_slot = self.slot() + 1; let next_start = next_slot * self.window + self.shift(); - UnixTimestamp::from_secs(next_start + GRACE) + UnixTimestamp::from_secs(next_start) } fn key(&self) -> String { @@ -197,7 +197,7 @@ impl RedisRateLimiter { invocation.key(refund_key); invocation.arg(quota.limit()); - invocation.arg(quota.expiry().as_secs()); + invocation.arg(quota.expiry().as_secs() + GRACE); invocation.arg(quantity); tracked_quotas.push(quota); diff --git a/tests/integration/test_store.py b/tests/integration/test_store.py index 4bcc283812b..083d71edc1d 100644 --- a/tests/integration/test_store.py +++ b/tests/integration/test_store.py @@ -8,6 +8,7 @@ import socket import threading import pytest +import time from requests.exceptions import HTTPError from flask import abort, Response @@ -462,6 +463,10 @@ def test_processing( assert event.get("version") is not None +@pytest.mark.parametrize("window,max_rate_limit", [ + (100, 300), + (300, 100), +]) @pytest.mark.parametrize("event_type", ["default", "error", "transaction"]) def test_processing_quotas( mini_sentry, @@ -470,10 +475,12 @@ def test_processing_quotas( events_consumer, transactions_consumer, event_type, + window, + max_rate_limit ): from time import sleep - relay = relay_with_processing({"processing": {"max_rate_limit": 120}}) + relay = relay_with_processing({"processing": {"max_rate_limit": max_rate_limit}}) mini_sentry.project_configs[42] = projectconfig = mini_sentry.full_project_config() public_keys = projectconfig["publicKeys"] @@ -489,7 +496,7 @@ def test_processing_quotas( "scopeId": six.text_type(key_id), "categories": [category], "limit": 5, - "window": 3600, + "window": window, "reasonCode": "get_lost", } ] @@ -543,10 +550,9 @@ def test_processing_quotas( relay.send_event(42, transform({"message": "rate_limited"})) headers = excinfo.value.response.headers - # The rate limit is actually for 1 hour, but we cap at 120s with the - # max_rate_limit parameter retry_after = headers["retry-after"] - assert int(retry_after) <= 120 + assert int(retry_after) <= window + assert int(retry_after) <= max_rate_limit retry_after2, rest = headers["x-sentry-rate-limits"].split(":", 1) assert int(retry_after2) == int(retry_after) assert rest == "%s:key" % category From 4a6aa1b3f5ba6a4b2c98ef295216057f3868fa36 Mon Sep 17 00:00:00 2001 From: Markus Unterwaditzer <markus-honeypot@unterwaditzer.net> Date: Mon, 2 Nov 2020 16:30:17 +0100 Subject: [PATCH 2/3] add changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 91e4a8250e3..d67ac299c63 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ **Bug Fixes**: - Accept sessions with IP address set to `{{auto}}`. This was previously rejected and silently dropped. ([#827](https://github.com/getsentry/relay/pull/827)) +- Fix an issue where every retry-after response would be too large by one minute. ([#829](https://github.com/getsentry/relay/pull/829)) **Internal**: From 5d362e22d8a909dbf318763862938ecf89c0b4d8 Mon Sep 17 00:00:00 2001 From: Markus Unterwaditzer <markus-honeypot@unterwaditzer.net> Date: Mon, 2 Nov 2020 17:17:31 +0100 Subject: [PATCH 3/3] fix formatting --- tests/integration/test_store.py | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/tests/integration/test_store.py b/tests/integration/test_store.py index 083d71edc1d..444f05874f8 100644 --- a/tests/integration/test_store.py +++ b/tests/integration/test_store.py @@ -463,10 +463,7 @@ def test_processing( assert event.get("version") is not None -@pytest.mark.parametrize("window,max_rate_limit", [ - (100, 300), - (300, 100), -]) +@pytest.mark.parametrize("window,max_rate_limit", [(100, 300), (300, 100),]) @pytest.mark.parametrize("event_type", ["default", "error", "transaction"]) def test_processing_quotas( mini_sentry, @@ -476,7 +473,7 @@ def test_processing_quotas( transactions_consumer, event_type, window, - max_rate_limit + max_rate_limit, ): from time import sleep