fix(limits): Enforce size limits consistently for all enevelope items#6250
fix(limits): Enforce size limits consistently for all enevelope items#6250Dav1dde wants to merge 4 commits into
Conversation
a0056d0 to
1401bbc
Compare
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
1401bbc to
046d868
Compare
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
046d868 to
0513a8e
Compare
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
| item_size: config.max_profile_size(), | ||
| total_count: 1, | ||
| total_size: config.max_profile_size(), | ||
| }; |
| None => { | ||
| // Shouldn't be possible, if it still happens due to a bug, be defensive. | ||
| debug_assert!(false); | ||
| &mut empty |
There was a problem hiding this comment.
Would it make sense to create a sentry error here?
There was a problem hiding this comment.
I considered it but it's a lot of noise for something that is already enforced at compile time through the match. I think I'd rather look into getting rid of the None case so we can fully rely on the compiler.
| "reason": "profiling_too_many_profiles", | ||
| "timestamp": time_within_delta(), | ||
| "quantity": 2, | ||
| "reason": "too_large:profile", |
There was a problem hiding this comment.
Something to consider: add a separate too_many reason to differentiate between count vs size exceeds.
There was a problem hiding this comment.
Agreed, I also ideally want to improve the error message returned to the SDK. Will keep that as a follow-up.
3 participants
Closes: INGEST-1057
Enforces size, total size and count limits for all envelope items, instead of only selectively. By adding these limits we effectively make accepted envelopes much stricter, it also adds a new metric to verify the limits are appropriate.
Also adds a few more limits that were missing. All limits configured to
1are forced by the protocol to be max1. Items which share the same limit of1are mutually exclusive in an envelope.There is a future improvement we can do in #6249 to improve how the limits are configured.