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.

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(replays): lower max replay recording segment size to 10 MiB #2031

Merged
merged 6 commits into from
Apr 19, 2023
Merged
Show file tree
Hide file tree
Changes from 2 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
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,10 @@ Metrics:
- The `route` tag of request metrics uses the route pattern instead of schematic names. There is an exact replacement for every previous route. For example, `"store-default"` is now tagged as `"/api/:project_id/store/"`.
- Statsd metrics `event.size_bytes.raw` and `event.size_bytes.uncompressed` have been removed.

Replays:

- Lower max compressed replay recording segment size to 10 MiB. ([#2031](https://github.com/getsentry/relay/pull/2031))
JoshFerge marked this conversation as resolved.
Show resolved Hide resolved

JoshFerge marked this conversation as resolved.
Show resolved Hide resolved
**Features**:

- Allow monitor checkins to paass `monitor_config` for monitor upserts. ([#1962](https://github.com/getsentry/relay/pull/1962))
Expand Down
20 changes: 14 additions & 6 deletions relay-config/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -542,8 +542,10 @@ struct Limits {
max_api_chunk_upload_size: ByteSize,
/// The maximum payload size for a profile
max_profile_size: ByteSize,
/// The maximum payload size for a replay.
max_replay_size: ByteSize,
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we keep old config and use max_replay_size as incoming check, the one is currently called max_replay_compressed_size, which clearly signals what the check is for, and if this is really necessary also use serde alias to rename it to the new name?

What do you think?

Copy link
Member

Choose a reason for hiding this comment

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

The new naming looks OK to me, but I would add the old name as an alias as @olksdr suggests, such that this is not a breaking change.

/// The maximum payload size for a compressed replay.
max_replay_compressed_size: ByteSize,
/// The maximum payload size for an uncompressed replay.
max_replay_uncompressed_size: ByteSize,
/// The maximum number of threads to spawn for CPU and web work, each.
///
/// The total number of threads spawned will roughly be `2 * max_thread_count + 1`. Defaults to
Expand Down Expand Up @@ -577,7 +579,8 @@ impl Default for Limits {
max_api_file_upload_size: ByteSize::mebibytes(40),
max_api_chunk_upload_size: ByteSize::mebibytes(100),
max_profile_size: ByteSize::mebibytes(50),
max_replay_size: ByteSize::mebibytes(100),
max_replay_compressed_size: ByteSize::mebibytes(10),
max_replay_uncompressed_size: ByteSize::mebibytes(100),
max_thread_count: num_cpus::get(),
query_timeout: 30,
shutdown_timeout: 10,
Expand Down Expand Up @@ -1822,9 +1825,14 @@ impl Config {
self.values.limits.max_profile_size.as_bytes()
}

/// Returns the maximum payload size for a replay.
pub fn max_replay_size(&self) -> usize {
self.values.limits.max_replay_size.as_bytes()
/// Returns the maximum payload size for an compressed replay.
JoshFerge marked this conversation as resolved.
Show resolved Hide resolved
pub fn max_replay_compressed_size(&self) -> usize {
self.values.limits.max_replay_compressed_size.as_bytes()
}

/// Returns the maximum payload size for an uncompressed replay.
pub fn max_replay_uncompressed_size(&self) -> usize {
self.values.limits.max_replay_uncompressed_size.as_bytes()
}

/// Returns the maximum number of active requests
Expand Down
2 changes: 1 addition & 1 deletion relay-server/src/actors/processor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1134,7 +1134,7 @@ impl EnvelopeProcessorService {
let client_addr = meta.client_addr();
let event_id = state.envelope().event_id();

let limit = self.config.max_replay_size();
let limit = self.config.max_replay_uncompressed_size();
let config = project_state.config();
let datascrubbing_config = config
.datascrubbing_settings
Expand Down
7 changes: 6 additions & 1 deletion relay-server/src/utils/sizes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,13 +32,18 @@ pub fn check_envelope_size_limits(config: &Config, envelope: &Envelope) -> bool
| ItemType::FormData => {
event_size += item.len();
}
ItemType::Attachment | ItemType::UnrealReport | ItemType::ReplayRecording => {
ItemType::Attachment | ItemType::UnrealReport => {
if item.len() > config.max_attachment_size() {
return false;
}

attachments_size += item.len()
}
ItemType::ReplayRecording => {
if item.len() > config.max_replay_compressed_size() {
return false;
}
}
ItemType::Session | ItemType::Sessions => {
session_count += 1;
}
Expand Down