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(profiling): Reject profiles longer than 30s #2168

Merged
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

- Add support for X-Vercel-Forwarded-For header. ([#2124](https://github.com/getsentry/relay/pull/2124))
- Add `lock` attribute to the frame protocol. ([#2171](https://github.com/getsentry/relay/pull/2171))
- Reject profiles longer than 30s. ([#2168](https://github.com/getsentry/relay/pull/2168))

## 23.5.2

Expand Down
6 changes: 5 additions & 1 deletion relay-profiling/src/android.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ use serde::{Deserialize, Serialize};
use crate::measurements::Measurement;
use crate::transaction_metadata::TransactionMetadata;
use crate::utils::{deserialize_number_from_string, is_zero};
use crate::ProfileError;
use crate::{ProfileError, MAX_PROFILE_DURATION};

#[derive(Debug, Serialize, Deserialize, Clone)]
pub struct ProfileMetadata {
Expand Down Expand Up @@ -183,6 +183,10 @@ fn parse_profile(payload: &[u8]) -> Result<AndroidProfile, ProfileError> {
return Err(ProfileError::NotEnoughSamples);
}

if profile.profile.elapsed_time > MAX_PROFILE_DURATION {
return Err(ProfileError::DurationIsTooLong);
}

Ok(profile)
}

Expand Down
2 changes: 2 additions & 0 deletions relay-profiling/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,4 +26,6 @@ pub enum ProfileError {
MalformedSamples,
#[error("exceed size limit")]
ExceedSizeLimit,
#[error("duration is too long")]
DurationIsTooLong,
}
3 changes: 3 additions & 0 deletions relay-profiling/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,7 @@

use serde::Deserialize;
use std::collections::BTreeMap;
use std::time::Duration;

mod android;
mod cocoa;
Expand All @@ -114,6 +115,8 @@ use crate::extract_from_transaction::{extract_transaction_metadata, extract_tran
pub use crate::error::ProfileError;
pub use crate::outcomes::discard_reason;

const MAX_PROFILE_DURATION: Duration = Duration::from_secs(30);

#[derive(Debug, Deserialize)]
struct MinimalProfile {
#[serde(alias = "profile_id")]
Expand Down
1 change: 1 addition & 0 deletions relay-profiling/src/outcomes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,5 +14,6 @@ pub fn discard_reason(err: ProfileError) -> &'static str {
ProfileError::NoTransactionAssociated => "profiling_no_transaction_associated",
ProfileError::NotEnoughSamples => "profiling_not_enough_samples",
ProfileError::PlatformNotSupported => "profiling_platform_not_supported",
ProfileError::DurationIsTooLong => "profiling_duration_is_too_long",
}
}
59 changes: 59 additions & 0 deletions relay-profiling/src/sample.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,9 @@ use crate::measurements::Measurement;
use crate::native_debug_image::NativeDebugImage;
use crate::transaction_metadata::TransactionMetadata;
use crate::utils::deserialize_number_from_string;
use crate::MAX_PROFILE_DURATION;

const MAX_PROFILE_DURATION_NS: u64 = MAX_PROFILE_DURATION.as_nanos() as u64;

#[derive(Debug, Serialize, Deserialize, Clone)]
struct Frame {
Expand Down Expand Up @@ -213,6 +216,13 @@ impl SampleProfile {
true
}

fn is_above_max_duration(&self) -> bool {
if let Some(sample) = &self.profile.samples.last() {
return sample.elapsed_since_start_ns > MAX_PROFILE_DURATION_NS;
}
false
}

/// Removes a sample when it's the only sample on its thread
fn remove_single_samples_per_thread(&mut self) {
let mut sample_count_by_thread_id: HashMap<u64, u32> = HashMap::new();
Expand Down Expand Up @@ -327,6 +337,10 @@ fn parse_profile(payload: &[u8]) -> Result<SampleProfile, ProfileError> {
return Err(ProfileError::MalformedStacks);
}

if profile.is_above_max_duration() {
return Err(ProfileError::DurationIsTooLong);
}

profile.strip_pointer_authentication_code();
profile.cleanup_thread_metadata();
profile.cleanup_queue_metadata();
Expand Down Expand Up @@ -368,6 +382,7 @@ pub fn parse_sample_profile(
#[cfg(test)]
mod tests {
use super::*;
use std::time::Duration;

#[test]
fn test_roundtrip() {
Expand Down Expand Up @@ -909,4 +924,48 @@ mod tests {
"some-random-transaction".to_string()
);
}

#[test]
fn test_keep_profile_under_max_duration() {
let mut profile = generate_profile();
profile.profile.samples.extend(vec![
Sample {
stack_id: 0,
queue_address: Some("0xdeadbeef".to_string()),
elapsed_since_start_ns: 10,
thread_id: 1,
},
Sample {
stack_id: 0,
queue_address: Some("0xdeadbeef".to_string()),
elapsed_since_start_ns: (MAX_PROFILE_DURATION - Duration::from_secs(1)).as_nanos()
as u64,
thread_id: 2,
},
]);

assert!(!profile.is_above_max_duration());
}

#[test]
fn test_reject_profile_over_max_duration() {
let mut profile = generate_profile();
profile.profile.samples.extend(vec![
Sample {
stack_id: 0,
queue_address: Some("0xdeadbeef".to_string()),
elapsed_since_start_ns: 10,
thread_id: 1,
},
Sample {
stack_id: 0,
queue_address: Some("0xdeadbeef".to_string()),
elapsed_since_start_ns: (MAX_PROFILE_DURATION + Duration::from_secs(1)).as_nanos()
as u64,
thread_id: 2,
},
]);

assert!(profile.is_above_max_duration());
}
}