-
Notifications
You must be signed in to change notification settings - Fork 94
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.
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
feat(quota): Rate limit attachments by item count #4377
Merged
Merged
Changes from 13 commits
Commits
Show all changes
15 commits
Select commit
Hold shift + click to select a range
03ad403
quantity
jjbayer 66ada11
Merge remote-tracking branch 'origin/master' into feat/rate-limit-att…
jjbayer b60f8eb
add some failing tests
jjbayer 6a91076
apply rate limits
jjbayer 54ff151
test
jjbayer b5e322c
keep sessions
jjbayer b76dbda
lint
jjbayer bde2f7e
sessions: valid unit
jjbayer ea38468
test
jjbayer 5fddf83
fix: store limit
jjbayer 83e06a7
wip
jjbayer 41e05d4
fix
jjbayer 68e3506
fix
jjbayer 4665bbf
Merge remote-tracking branch 'origin/master' into feat/rate-limit-att…
jjbayer b816e68
changelog
jjbayer File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -49,7 +49,7 @@ use relay_quotas::DataCategory; | |
use relay_sampling::DynamicSamplingContext; | ||
use serde::de::DeserializeOwned; | ||
use serde::{Deserialize, Serialize}; | ||
use smallvec::SmallVec; | ||
use smallvec::{smallvec, SmallVec}; | ||
|
||
use crate::constants::DEFAULT_EVENT_RETENTION; | ||
use crate::extractors::{PartialMeta, RequestMeta}; | ||
|
@@ -632,6 +632,14 @@ pub struct Item { | |
payload: Bytes, | ||
} | ||
|
||
/// Expresses the purpose of counting quantities. | ||
/// | ||
/// Sessions are counted for rate limiting enforcement but not for outcome reporting. | ||
pub enum CountFor { | ||
RateLimits, | ||
Outcomes, | ||
} | ||
|
||
impl Item { | ||
/// Creates a new item with the given type. | ||
pub fn new(ty: ItemType) -> Self { | ||
|
@@ -670,13 +678,38 @@ impl Item { | |
/// Returns the number used for counting towards rate limits and producing outcomes. | ||
/// | ||
/// For attachments, we count the number of bytes. Other items are counted as 1. | ||
pub fn quantity(&self) -> usize { | ||
pub fn quantities(&self, purpose: CountFor) -> SmallVec<[(DataCategory, usize); 1]> { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This function now unites the old We might be able to absorb the |
||
match self.ty() { | ||
ItemType::Attachment => self.len().max(1), | ||
// NOTE: This is semantically wrong. An otel trace contains may contain many spans, | ||
// but we cannot easily count these before converting the trace into a series of spans. | ||
ItemType::OtelTracesData => 1, | ||
_ => 1, | ||
ItemType::Event => smallvec![(DataCategory::Error, 1)], | ||
ItemType::Transaction => smallvec![(DataCategory::Transaction, 1)], | ||
ItemType::Security | ItemType::RawSecurity => { | ||
smallvec![(DataCategory::Security, 1)] | ||
} | ||
ItemType::Nel => smallvec![], | ||
ItemType::UnrealReport => smallvec![(DataCategory::Error, 1)], | ||
ItemType::Attachment => smallvec![ | ||
(DataCategory::Attachment, self.len().max(1)), | ||
(DataCategory::AttachmentItem, 1) | ||
], | ||
ItemType::Session | ItemType::Sessions => match purpose { | ||
CountFor::RateLimits => smallvec![(DataCategory::Session, 1)], | ||
CountFor::Outcomes => smallvec![], | ||
}, | ||
ItemType::Statsd | ItemType::MetricBuckets => smallvec![], | ||
ItemType::FormData => smallvec![], | ||
ItemType::UserReport => smallvec![], | ||
ItemType::UserReportV2 => smallvec![(DataCategory::UserReportV2, 1)], | ||
ItemType::Profile => smallvec![(DataCategory::Profile, 1)], | ||
ItemType::ReplayEvent | ItemType::ReplayRecording | ItemType::ReplayVideo => { | ||
smallvec![(DataCategory::Replay, 1)] | ||
} | ||
ItemType::ClientReport => smallvec![], | ||
ItemType::CheckIn => smallvec![(DataCategory::Monitor, 1)], | ||
ItemType::Span | ItemType::OtelSpan => smallvec![(DataCategory::Span, 1)], | ||
// NOTE: semantically wrong, but too expensive to parse. | ||
ItemType::OtelTracesData => smallvec![(DataCategory::Span, 1)], | ||
ItemType::ProfileChunk => smallvec![(DataCategory::ProfileChunk, 1)], // TODO: should be seconds? | ||
ItemType::Unknown(_) => smallvec![], | ||
} | ||
} | ||
|
||
|
@@ -688,35 +721,6 @@ impl Item { | |
) | ||
} | ||
|
||
/// Returns the data category used for generating outcomes. | ||
/// | ||
/// Returns `None` if outcomes are not generated for this type (e.g. sessions). | ||
pub fn outcome_category(&self) -> Option<DataCategory> { | ||
match self.ty() { | ||
ItemType::Event => Some(DataCategory::Error), | ||
ItemType::Transaction => Some(DataCategory::Transaction), | ||
ItemType::Security | ItemType::RawSecurity => Some(DataCategory::Security), | ||
ItemType::Nel => None, | ||
ItemType::UnrealReport => Some(DataCategory::Error), | ||
ItemType::Attachment => Some(DataCategory::Attachment), | ||
ItemType::Session | ItemType::Sessions => None, | ||
ItemType::Statsd | ItemType::MetricBuckets => None, | ||
ItemType::FormData => None, | ||
ItemType::UserReport => None, | ||
ItemType::UserReportV2 => Some(DataCategory::UserReportV2), | ||
ItemType::Profile => Some(DataCategory::Profile), | ||
ItemType::ReplayEvent | ItemType::ReplayRecording | ItemType::ReplayVideo => { | ||
Some(DataCategory::Replay) | ||
} | ||
ItemType::ClientReport => None, | ||
ItemType::CheckIn => Some(DataCategory::Monitor), | ||
ItemType::Span | ItemType::OtelSpan => Some(DataCategory::Span), | ||
ItemType::OtelTracesData => None, | ||
ItemType::ProfileChunk => Some(DataCategory::ProfileChunk), | ||
ItemType::Unknown(_) => None, | ||
} | ||
} | ||
|
||
/// Returns `true` if this item's payload is empty. | ||
pub fn is_empty(&self) -> bool { | ||
self.payload.is_empty() | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
An aggregate session item does not support accurate counts, but this does not matter since sessions don't produce outcomes and are only rate possibly limited by setting quota to zero.