Skip to content

fix(api-logs): preserve snuba policy info on throttles#116263

Merged
cvxluo merged 2 commits into
masterfrom
cvxluo/preserve-policy-metadata-when-429-came-from-a-thro
May 27, 2026
Merged

fix(api-logs): preserve snuba policy info on throttles#116263
cvxluo merged 2 commits into
masterfrom
cvxluo/preserve-policy-metadata-when-429-came-from-a-thro

Conversation

@cvxluo
Copy link
Copy Markdown
Contributor

@cvxluo cvxluo commented May 26, 2026

I'm noticing that several api access logs are missing snuba policy info, even though they show a rate limit category of 'snuba'. I believe this is because we (me 9 months ago) originally focused on only errors returned from snuba, writing this line rejection_threshold=policy_info["rejection_threshold"], assuming that rejection_threshold would be on every snuba error. However, throttling seems to be a big source of errors as well, and doesn't have that field, leading to us accidentally not attaching the policy info on throttled errors. Fix the error by using .get instead.

I chose this fix since I wanted to make the safest possible fix for now. We can look at putting 'throttled_by' in the rate limit error later.

@github-actions github-actions Bot added the Scope: Backend Automatically applied to PRs that change backend components label May 26, 2026
cvxluo and others added 2 commits May 26, 2026 16:10
…ot a rejection

When snuba returns 429 from a throttle path (e.g. TOO_MANY_BYTES from BytesScannedRejectingPolicy's limit_bytes_instead_of_rejecting), the response's quota_allowance.summary.throttled_by carries throttle_threshold rather than rejection_threshold. Subscripting policy_info["rejection_threshold"] raised KeyError, which was swallowed and fell through to a bare RateLimitExceeded with no policy/storage_key — wiping the SnubaRateLimitMeta downstream and leaving api.access logs with rate_limit_type=snuba but blank snuba_policy / snuba_storage_key.


Co-authored-by: Claude <noreply@anthropic.com>
@cvxluo cvxluo force-pushed the cvxluo/preserve-policy-metadata-when-429-came-from-a-thro branch from 44dcaf9 to 1bc1597 Compare May 26, 2026 23:10
@cvxluo cvxluo marked this pull request as ready for review May 26, 2026 23:22
@cvxluo cvxluo requested review from a team as code owners May 26, 2026 23:22
@cvxluo cvxluo requested a review from a team May 26, 2026 23:22
@cvxluo cvxluo merged commit 62f4dc8 into master May 27, 2026
94 of 118 checks passed
@cvxluo cvxluo deleted the cvxluo/preserve-policy-metadata-when-429-came-from-a-thro branch May 27, 2026 16:33
cvxluo added a commit that referenced this pull request May 27, 2026
…116338)

Follow up on #116263. Log the
throttled threshold when we get a 429 response due to throttling from
snuba.

Co-authored-by: Claude <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Scope: Backend Automatically applied to PRs that change backend components

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants