Skip to content

fix: match rate limits by data category not envelope item type#5412

Draft
jamescrosswell wants to merge 4 commits into
mainfrom
fix/rate-limit-data-categories
Draft

fix: match rate limits by data category not envelope item type#5412
jamescrosswell wants to merge 4 commits into
mainfrom
fix/rate-limit-data-categories

Conversation

@jamescrosswell

Copy link
Copy Markdown
Collaborator

Summary

Rate limits in the X-Sentry-Rate-Limits header are keyed by data category (error, monitor, log_item, metric_bucket, …), which is not always the same as the envelope item type (event, check_in, log, statsd).

RateLimitCategory.Matches compared the server's category name against the raw envelope item type, so category-specific limits whose name differs from the item type were silently ignored:

Item type Category Relay sends Old Matches result
event error ❌ never matched
check_in monitor ❌ never matched
log log_item ❌ never matched

The practical effect: when Sentry rate-limited errors, the SDK didn't recognise the limit, kept sending, got 429d downstream, and client reports mis-attributed the drops to default instead of the real category. This is the concern raised in #4535 — mishandling categories can cause the most important data (errors) to be dropped rather than cleanly backed off.

Changes

  • RateLimitCategory.Matches now compares the limit against the item's data category rather than the raw item type — so error / monitor / log_item limits are honoured. The special-case for statsdmetric_bucket is no longer needed.
  • EnvelopeItem.DataCategory maps every emitted item type to its correct category: check_inmonitor, loglog_item, trace_metrictrace_metric, statsdmetric_bucket. Client reports (client_report) now map to internal so they are never dropped by a default rate limit.
  • BatchProcessor records dropped logs/metrics under their real category (log_item / trace_metric) instead of default.
  • DataCategory gains the missing monitor / log_item / trace_metric / metric_bucket constants and the doc link is updated.
  • Tests updated to use the real category names (error, not event) and to guard against the regression (item-type strings must not match).

Verified: full Sentry.Tests suite passes on net9.0 (2465 passed, 4 environment-skips).

Closes #4535

Rate limits in the `X-Sentry-Rate-Limits` header are keyed by data
category (e.g. `error`, `monitor`, `log_item`, `metric_bucket`), which is
not always the same as the envelope item type (`event`, `check_in`, `log`,
`statsd`). `RateLimitCategory.Matches` compared the category name against
the raw item type, so limits for errors, check-ins and logs were silently
ignored — the SDK kept sending that data instead of backing off, and client
reports mis-attributed the resulting drops to `default`.

- Map every emitted item type to its correct data category, and add the
  missing `monitor`/`log_item`/`trace_metric`/`metric_bucket` constants
- Match rate limits against the item's data category
- Record dropped logs/metrics under their real category in client reports
- Never rate-limit client reports (they are the `internal` category)

Closes #4535

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Comment on lines +20 to +22
// Rate limits are keyed by data category (e.g. "error", "monitor", "metric_bucket"), which is not
// always the same as the envelope item type (e.g. "event", "check_in", "statsd"). Compare against the
// item's data category so limits for those categories are honoured rather than silently ignored.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Keep this concise...

Suggested change
// Rate limits are keyed by data category (e.g. "error", "monitor", "metric_bucket"), which is not
// always the same as the envelope item type (e.g. "event", "check_in", "statsd"). Compare against the
// item's data category so limits for those categories are honoured rather than silently ignored.
// Rate limits are keyed by data category (e.g. "error", "monitor", "metric_bucket")

@codecov

codecov Bot commented Jul 20, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 74.63%. Comparing base (4216c80) to head (4d97258).

Additional details and impacted files
@@                     Coverage Diff                     @@
##           fix/5406-pin-xcode-26.6    #5412      +/-   ##
===========================================================
+ Coverage                    74.60%   74.63%   +0.03%     
===========================================================
  Files                          512      512              
  Lines                        18666    18674       +8     
  Branches                      3659     3657       -2     
===========================================================
+ Hits                         13925    13937      +12     
+ Misses                        3867     3864       -3     
+ Partials                       874      873       -1     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

jamescrosswell and others added 2 commits July 21, 2026 10:41
The 10.0.302 workload ships the iOS SDK pack 26.5.10301, which requires
Xcode 26.6. CI was pinning Xcode 26.5, causing iOS/MacCatalyst builds and
device tests to fail. The macos-26 runner already used by these jobs has
Xcode 26.6 available, so switch the pin to it.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Dependabot bumped only codeql-action/init to 4.37.1, leaving analyze on
4.37.0. CodeQL requires all steps use the same version, so analyze failed
with "Loaded a configuration file for version '4.37.1', but running
version '4.37.0'". Pin analyze to the same v4.37.1 SHA.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

public RateLimitCategory(string name) => Name = name;

public bool Matches(EnvelopeItem item)

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This could be simplified to a lambda that simply returns IsMatchAll || string.Equals(...

@jamescrosswell
jamescrosswell changed the base branch from main to fix/5406-pin-xcode-26.6 July 21, 2026 01:06
Base automatically changed from fix/5406-pin-xcode-26.6 to main July 21, 2026 03:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Review and update Rate Limiting data categories

1 participant