Skip to content

Commit e5bd64e

Browse files
authored
fix(access-logs): rate limit type not correctly set to snuba (#97306)
Follow up to #97199. We're now seeing logs with snuba info (see [here](https://console.cloud.google.com/logs/query;query=resource.type%3D%22k8s_container%22%0Alabels.name%3D%22sentry.access.api%22%0A-jsonPayload.snuba_policy%3D%22None%22%0A--%20jsonPayload.response%3D%22429%22;summaryFields=jsonPayload%252Fsnuba_policy:true:32:beginning;cursorTimestamp=2025-08-06T15:30:30.107622095Z;duration=PT6H?project=internal-sentry&inv=1&invt=Ab4xUQ&rapt=AEjHL4NivrJYEIhr36atfUalUZIcwCYZjvYww0eGEMey3GYzVzSRQyr1DFLozgYmc-quqALJKFUH-jXq5G72I_WT6iWlAjpbgdwqwr9qw6IsGJn43ToXwto&pli=1)), but those logs have a `rate_limit_type` of `RateLimitType.NOT_LIMITED` rather than `RateLimitType.SNUBA`. This is because rate limit metadata is returned on all endpoints implementing rate limiting, even when the request is not limited, when access logs assumed previously that it would not be returned at all.
1 parent 7fde502 commit e5bd64e

File tree

2 files changed

+24
-8
lines changed

2 files changed

+24
-8
lines changed

src/sentry/middleware/access_log.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ def _get_rate_limit_stats_dict(request: Request) -> dict[str, str]:
5757
rate_limit_type = "DNE"
5858
if rate_limit_metadata:
5959
rate_limit_type = str(getattr(rate_limit_metadata, "rate_limit_type", "DNE"))
60-
elif snuba_rate_limit_metadata:
60+
if snuba_rate_limit_metadata:
6161
rate_limit_type = "RateLimitType.SNUBA"
6262

6363
rate_limit_stats = {

tests/sentry/middleware/test_access_log_middleware.py

Lines changed: 23 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@
1616
from sentry.silo.base import SiloMode
1717
from sentry.testutils.cases import APITestCase
1818
from sentry.testutils.silo import all_silo_test, assume_test_silo_mode, control_silo_test
19-
from sentry.types.ratelimit import RateLimit, RateLimitCategory
19+
from sentry.types.ratelimit import RateLimit, RateLimitCategory, RateLimitMeta, RateLimitType
2020
from sentry.utils.snuba import RateLimitExceeded
2121

2222

@@ -38,6 +38,22 @@ class SnubaRateLimitedEndpoint(Endpoint):
3838
permission_classes = (AllowAny,)
3939

4040
def get(self, request):
41+
42+
# Rate limit middleware will set metadata to indicate the request is not limited by the endpoint itself
43+
request._request.rate_limit_metadata = RateLimitMeta(
44+
rate_limit_type=RateLimitType.NOT_LIMITED,
45+
concurrent_limit=123,
46+
concurrent_requests=1,
47+
reset_time=123,
48+
group="test_group",
49+
limit=123,
50+
window=123,
51+
current=1,
52+
remaining=122,
53+
)
54+
55+
# However, snuba's 429 will be caught by the custom handler and raise an exception
56+
# with the snuba metadata
4157
raise RateLimitExceeded(
4258
"Query on could not be run due to allocation policies, ... 'rejection_threshold': 40, 'quota_used': 41, ...",
4359
policy="ConcurrentRateLimitAllocationPolicy",
@@ -190,12 +206,12 @@ def test_access_log_snuba_rate_limited(self) -> None:
190206
assert self.captured_logs[0].rate_limit_type == "RateLimitType.SNUBA"
191207
assert self.captured_logs[0].rate_limited == "True"
192208

193-
# All the types from the standard rate limit metadata should not be set
194-
assert self.captured_logs[0].remaining == "None"
195-
assert self.captured_logs[0].concurrent_limit == "None"
196-
assert self.captured_logs[0].concurrent_requests == "None"
197-
assert self.captured_logs[0].limit == "None"
198-
assert self.captured_logs[0].reset_time == "None"
209+
# All the types from the standard rate limit metadata should be set
210+
assert self.captured_logs[0].remaining == "122"
211+
assert self.captured_logs[0].concurrent_limit == "123"
212+
assert self.captured_logs[0].concurrent_requests == "1"
213+
assert self.captured_logs[0].limit == "123"
214+
assert self.captured_logs[0].reset_time == "123"
199215

200216
# Snuba rate limit specific fields should be set
201217
assert self.captured_logs[0].snuba_policy == "ConcurrentRateLimitAllocationPolicy"

0 commit comments

Comments
 (0)