Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Enforce limit:0 quotas on profiles #2207

Closed
jjbayer opened this issue Jun 13, 2023 · 0 comments · Fixed by #2212
Closed

Enforce limit:0 quotas on profiles #2207

jjbayer opened this issue Jun 13, 2023 · 0 comments · Fixed by #2212
Assignees
@jjbayer

Comments

@jjbayer
Copy link
Member

jjbayer commented Jun 13, 2023

In #2163 we decided not to enforce rate limits on processed profiles with limit > 0, to prevent complexity (see PR description).

There is at least one case where a quota with limit:0 is also ignored though: When the metric gets extracted by an external relay (which does not receive quota config).

Investigate whether this can also happen for PoPs.

Possible solution: Reopen #2178
@jjbayer jjbayer self-assigned this Jun 13, 2023
@jjbayer jjbayer mentioned this issue Jun 13, 2023
Merged
jjbayer added a commit that referenced this issue Jun 13, 2023
@jjbayer
fix(profiles): Enforce profile quotas on metrics buckets (#2212)
94c2a3a 
In #2163 we decided not to
enforce rate limits on processed profiles with `limit > 0`, to prevent
complexity (see PR description).

I wrongly assumed that quotas (i.e. rate limits with `limit:0`) would be
correctly enforced by dropping the profile envelope item before metrics
are extracted. I overlooked the fact that PoP Relays (where metrics are
extracted) do not _receive_ quota configuration, because they do not
request it:


https://github.com/getsentry/relay/blob/0505c88f932d89bd42ee75114440f7f08fe7d938/relay-server/src/actors/project_upstream.rs#L273

Note that this only affects rate limits that are issued _only_ for
profiles. Rate limits issued for _transactions_ correctly drop the
processed profiles.

This PR fixes the issue by stripping the `has_profile` tag off metrics
buckets if a `Profile` rate limit is active.

Fixes #2207
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@jjbayer jjbayer

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix(profiles): Enforce profile quotas on metrics buckets getsentry/relay
1 participant
@jjbayer