Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

**Internal**:

- Enforce span limits for transactions and vice versa. ([#4963](https://github.com/getsentry/relay/pull/4963))
- Emit outcomes for skipped large attachments on playstation crashes. ([#4862](https://github.com/getsentry/relay/pull/4862))
- Disable span metrics. ([#4931](https://github.com/getsentry/relay/pull/4931),[#4955](https://github.com/getsentry/relay/pull/4955))

Expand Down
45 changes: 43 additions & 2 deletions relay-quotas/src/rate_limit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ use std::str::FromStr;
use std::sync::{Arc, Mutex, PoisonError};
use std::time::{Duration, Instant};

use relay_base_schema::data_category::DataCategory;
use relay_base_schema::metrics::MetricNamespace;
use relay_base_schema::organization::OrganizationId;
use relay_base_schema::project::{ProjectId, ProjectKey};
Expand Down Expand Up @@ -482,8 +483,31 @@ impl CachedRateLimits {

let mut inner = self.0.lock().unwrap_or_else(PoisonError::into_inner);
let current = Arc::make_mut(&mut inner);
for limit in limits {
current.add(limit)
for mut limit in limits {
// To spice it up, we do have some special casing here for 'inherited categories',
// e.g. spans and transactions.
//
// The tldr; is, as transactions are just containers for spans,
// we can enforce span limits on transactions but also vice versa.
//
// So this is largely an enforcement problem, but since Relay propagates
// rate limits to clients, we clone the limits with the inherited category.
// This ensures old SDKs rate limit correctly, but also it simplifies client
// implementations. Only Relay needs to make this decision.
for i in 0..limit.categories.len() {
let Some(category) = limit.categories.get(i) else {
Comment on lines +497 to +498
Copy link
Contributor

Choose a reason for hiding this comment

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

This is written with a loop over the index because of iterator invalidation?

Copy link
Member Author

Choose a reason for hiding this comment

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

Correct, I cannot iterate and extend at the same time.

debug_assert!(false, "logical error");
break;
};

for inherited in inherited_categories(category) {
if !limit.categories.contains(inherited) {
limit.categories.push(*inherited);
}
}
}

current.add(limit);
}
}

Expand All @@ -499,6 +523,23 @@ impl CachedRateLimits {
}
}

/// Returns inherited rate limit categories for the passed category.
///
/// When a rate limit for a category can also be enforced in a different category,
/// then it's an inherited category.
///
/// For example, a transaction rate limit can also be applied to spans and vice versa.
///
/// For a detailed explanation on span/transaction enforcement see:
/// <https://develop.sentry.dev/ingestion/relay/transaction-span-ratelimits/>.
fn inherited_categories(category: &DataCategory) -> &'static [DataCategory] {
match category {
DataCategory::Transaction => &[DataCategory::Span],
DataCategory::Span => &[DataCategory::Transaction],
_ => &[],
}
}

#[cfg(test)]
mod tests {
use smallvec::smallvec;
Expand Down
6 changes: 5 additions & 1 deletion tests/integration/test_store.py
Original file line number Diff line number Diff line change
Expand Up @@ -468,7 +468,11 @@ def transform(e):
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:get_lost" % category
if event_type == "transaction":
# Transaction limits also apply to spans.
assert rest == "transaction;span:key:get_lost"
else:
assert rest == "%s:key:get_lost" % category
if outcomes_consumer is not None:
outcomes_consumer.assert_rate_limited(
"get_lost", key_id=key_id, categories=[category]
Expand Down
Loading