Skip to content

Commit

Permalink
fix(profiles): Rate limit profiles when transaction was sampled (#4195)
Browse files Browse the repository at this point in the history
fixes: #4280
  • Loading branch information
jjbayer authored Dec 6, 2024
1 parent 7ed88f2 commit f9a903a
Show file tree
Hide file tree
Showing 3 changed files with 101 additions and 0 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
**Bug Fixes**:

- Accept incoming requests even if there was an error fetching their project config. ([#4140](https://github.com/getsentry/relay/pull/4140))
- Rate limit profiles when transaction was sampled. ([#4195](https://github.com/getsentry/relay/pull/4195))

**Features**:

Expand Down
30 changes: 30 additions & 0 deletions relay-server/src/utils/rate_limits.rs
Original file line number Diff line number Diff line change
Expand Up @@ -677,6 +677,15 @@ where
scoping.item(DataCategory::Profile),
summary.profile_quantity,
)?;

// Profiles can persist in envelopes without transaction if the transaction item
// was dropped by dynamic sampling.
if profile_limits.is_empty() && summary.event_category.is_none() {
profile_limits = self
.check
.apply(scoping.item(DataCategory::Transaction), 0)?;
}

enforcement.profiles = CategoryLimit::new(
DataCategory::Profile,
summary.profile_quantity,
Expand Down Expand Up @@ -1427,6 +1436,27 @@ mod tests {
);
}

#[test]
fn test_enforce_transaction_standalone_profile_enforced() {
// When the transaction is sampled, the profile survives as standalone.
let mut envelope = envelope![Profile];

let mut mock = MockLimiter::default().deny(DataCategory::Transaction);
let (enforcement, _) = enforce_and_apply(&mut mock, &mut envelope, None);

assert!(enforcement.profiles.is_active());
mock.assert_call(DataCategory::Profile, 1);
mock.assert_call(DataCategory::Transaction, 0);

assert_eq!(
get_outcomes(enforcement),
vec![
(DataCategory::Profile, 1),
(DataCategory::ProfileIndexed, 1),
]
);
}

#[test]
fn test_enforce_transaction_attachment_enforced_indexing_quota() {
let mut envelope = envelope![Transaction, Attachment];
Expand Down
70 changes: 70 additions & 0 deletions tests/integration/test_outcome.py
Original file line number Diff line number Diff line change
Expand Up @@ -1666,6 +1666,76 @@ def test_profile_outcomes_rate_limited(
assert outcomes == expected_outcomes, outcomes


@pytest.mark.parametrize("quota_category", ["transaction", "transaction_indexed"])
def test_profile_outcomes_rate_limited_when_dynamic_sampling_drops(
mini_sentry,
relay,
quota_category,
):
project_id = 42
project_config = mini_sentry.add_full_project_config(project_id)["config"]

project_config.setdefault("features", []).append("organizations:profiling")
project_config["quotas"] = [
{
"id": f"test_rate_limiting_{uuid.uuid4().hex}",
"categories": [quota_category],
"limit": 0,
"reasonCode": "profiles_exceeded",
}
]

config = {
"outcomes": {
"emit_outcomes": True,
"batch_size": 1,
"batch_interval": 1,
"aggregator": {
"bucket_interval": 1,
"flush_interval": 0,
},
},
"aggregator": {
"bucket_interval": 1,
"initial_delay": 0,
},
}

relay = relay(mini_sentry, options=config)

with open(
RELAY_ROOT / "relay-profiling/tests/fixtures/sample/v1/valid.json",
"rb",
) as f:
profile = f.read()

# Create an envelope with an invalid profile:
envelope = Envelope()
profile_item = Item(payload=PayloadRef(bytes=profile), type="profile")
profile_item.headers["sampled"] = False
envelope.add_item(profile_item)
relay.send_envelope(project_id, envelope)

if quota_category == "transaction":
(outcome1,) = mini_sentry.captured_outcomes.get(timeout=10)["outcomes"]
(outcome2,) = mini_sentry.captured_outcomes.get(timeout=1)["outcomes"]
outcome1, outcome2 = sorted([outcome1, outcome2], key=lambda o: o["category"])
assert outcome1["outcome"] == 2 # rate limited
assert outcome1["category"] == DataCategory.PROFILE
assert outcome1["quantity"] == 1
assert outcome2["outcome"] == 2 # rate limited
assert outcome2["category"] == DataCategory.PROFILE_INDEXED
assert outcome2["quantity"] == 1

assert mini_sentry.captured_events.empty()
else:
# Do not rate limit if there is only a transaction_indexed quota.
envelope = mini_sentry.captured_events.get(timeout=10)
assert envelope.items[0].headers["type"] == "profile"

assert mini_sentry.captured_outcomes.empty()


def test_global_rate_limit(
mini_sentry, relay_with_processing, metrics_consumer, outcomes_consumer
):
Expand Down

0 comments on commit f9a903a

Please sign in to comment.