diff --git a/Cargo.lock b/Cargo.lock index e80ed925e8..93e4427ede 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3785,6 +3785,7 @@ dependencies = [ "chrono", "data-encoding", "insta", + "relay-base-schema", "relay-event-schema", "relay-log", "relay-protocol", diff --git a/relay-profiling/Cargo.toml b/relay-profiling/Cargo.toml index 96d9d5d1a2..16a240d452 100644 --- a/relay-profiling/Cargo.toml +++ b/relay-profiling/Cargo.toml @@ -13,6 +13,7 @@ publish = false android_trace_log = { version = "0.3.0", features = ["serde"] } chrono = { workspace = true } data-encoding = "2.3.3" +relay-base-schema = { path = "../relay-base-schema" } relay-event-schema = { path = "../relay-event-schema" } relay-log = { path = "../relay-log" } relay-protocol = { path = "../relay-protocol" } diff --git a/relay-profiling/src/lib.rs b/relay-profiling/src/lib.rs index 9683f39af2..8eca3e2ad2 100644 --- a/relay-profiling/src/lib.rs +++ b/relay-profiling/src/lib.rs @@ -93,10 +93,10 @@ //! } //! ``` -use std::collections::BTreeMap; use std::error::Error; use std::time::Duration; +use relay_base_schema::project::ProjectId; use relay_event_schema::protocol::{Event, EventId}; use serde::Deserialize; use serde_json::Deserializer; @@ -127,17 +127,23 @@ struct MinimalProfile { version: sample::Version, } -fn minimal_profile_from_json(payload: &[u8]) -> Result { +fn minimal_profile_from_json( + payload: &[u8], +) -> Result> { let d = &mut Deserializer::from_slice(payload); - serde_path_to_error::deserialize(d).map_err(ProfileError::InvalidJson) + serde_path_to_error::deserialize(d) } -pub fn parse_metadata(payload: &[u8]) -> Result<(), ProfileError> { +pub fn parse_metadata(payload: &[u8], project_id: ProjectId) -> Result<(), ProfileError> { let profile = match minimal_profile_from_json(payload) { Ok(profile) => profile, Err(err) => { - relay_log::warn!(error = &err as &dyn Error, "invalid profile (minimal)"); - return Err(err); + relay_log::warn!( + error = &err as &dyn Error, + from = "minimal", + project_id = project_id.value(), + ); + return Err(ProfileError::InvalidJson(err)); } }; match profile.version { @@ -148,9 +154,10 @@ pub fn parse_metadata(payload: &[u8]) -> Result<(), ProfileError> { Err(err) => { relay_log::warn!( error = &err as &dyn Error, - "invalid profile (platform: {}, version: {:?})", - profile.platform, - profile.version, + from = "metadata", + platform = profile.platform, + project_id = project_id.value(), + "invalid profile", ); return Err(ProfileError::InvalidJson(err)); } @@ -164,7 +171,10 @@ pub fn parse_metadata(payload: &[u8]) -> Result<(), ProfileError> { Err(err) => { relay_log::warn!( error = &err as &dyn Error, - "invalid profile (platform: android)", + from = "metadata", + platform = "android", + project_id = project_id.value(), + "invalid profile", ); return Err(ProfileError::InvalidJson(err)); } @@ -176,21 +186,24 @@ pub fn parse_metadata(payload: &[u8]) -> Result<(), ProfileError> { Ok(()) } -pub fn expand_profile( - payload: &[u8], - event: Option<&Event>, -) -> Result<(EventId, Vec), ProfileError> { +pub fn expand_profile(payload: &[u8], event: &Event) -> Result<(EventId, Vec), ProfileError> { let profile = match minimal_profile_from_json(payload) { Ok(profile) => profile, - Err(err) => return Err(err), - }; - let (transaction_metadata, transaction_tags) = match event { - Some(event) => ( - extract_transaction_metadata(event), - extract_transaction_tags(event), - ), - _ => (BTreeMap::new(), BTreeMap::new()), + Err(err) => { + relay_log::warn!( + error = &err as &dyn Error, + from = "minimal", + platform = event.platform.as_str(), + project_id = event.project.value().unwrap_or(&0), + sdk_name = event.sdk_name(), + sdk_version = event.sdk_version(), + "invalid profile", + ); + return Err(ProfileError::InvalidJson(err)); + } }; + let transaction_metadata = extract_transaction_metadata(event); + let transaction_tags = extract_transaction_tags(event); let processed_payload = match profile.version { sample::Version::V1 => { sample::parse_sample_profile(payload, transaction_metadata, transaction_tags) @@ -208,13 +221,25 @@ pub fn expand_profile( ProfileError::InvalidJson(err) => { relay_log::warn!( error = &err as &dyn Error, - "invalid profile (platform: {})", - profile.platform, + from = "parsing", + platform = profile.platform, + project_id = event.project.value().unwrap_or(&0), + sdk_name = event.sdk_name(), + sdk_version = event.sdk_version(), + "invalid profile", ); Err(ProfileError::InvalidJson(err)) } _ => { - relay_log::debug!(error = &err as &dyn Error, "invalid profile"); + relay_log::warn!( + error = &err as &dyn Error, + from = "parsing", + platform = profile.platform, + project_id = event.project.value().unwrap_or(&0), + sdk_name = event.sdk_name(), + sdk_version = event.sdk_version(), + "invalid profile", + ); Err(err) } }, @@ -244,12 +269,12 @@ mod tests { #[test] fn test_expand_profile_with_version() { let payload = include_bytes!("../tests/fixtures/profiles/sample/roundtrip.json"); - assert!(expand_profile(payload, Some(&Event::default())).is_ok()); + assert!(expand_profile(payload, &Event::default()).is_ok()); } #[test] fn test_expand_profile_without_version() { let payload = include_bytes!("../tests/fixtures/profiles/android/roundtrip.json"); - assert!(expand_profile(payload, Some(&Event::default())).is_ok()); + assert!(expand_profile(payload, &Event::default()).is_ok()); } } diff --git a/relay-profiling/src/measurements.rs b/relay-profiling/src/measurements.rs index 9d1a9750fb..8f29afed0b 100644 --- a/relay-profiling/src/measurements.rs +++ b/relay-profiling/src/measurements.rs @@ -39,12 +39,17 @@ mod tests { #[test] fn test_value_as_float() { let measurement_json = r#"{"elapsed_since_start_ns":1234567890,"value":1234.56789}"#; - assert!(serde_json::from_str::(measurement_json).is_ok()); + let measurement = serde_json::from_str::(measurement_json); + assert!(measurement.is_ok()); + assert_eq!(measurement.unwrap().value, 1234.56789); } #[test] fn test_value_as_string() { let measurement_json = r#"{"elapsed_since_start_ns":1234567890,"value":"1234.56789"}"#; assert!(serde_json::from_str::(measurement_json).is_ok()); + let measurement = serde_json::from_str::(measurement_json); + assert!(measurement.is_ok()); + assert_eq!(measurement.unwrap().value, 1234.56789); } } diff --git a/relay-profiling/src/utils.rs b/relay-profiling/src/utils.rs index 39cc2ffb98..2705f8230b 100644 --- a/relay-profiling/src/utils.rs +++ b/relay-profiling/src/utils.rs @@ -2,6 +2,7 @@ use std::fmt::Display; use std::str::FromStr; use serde::{de, Deserialize}; +use serde_json::{Map, Value}; pub fn is_zero(n: &u64) -> bool { *n == 0 @@ -15,13 +16,28 @@ where { #[derive(Deserialize)] #[serde(untagged)] - enum StringOrInt { + enum StringOrNumber { String(String), Number(T), + Bool(bool), + Array(Vec), + Object(Map), } - match StringOrInt::::deserialize(deserializer)? { - StringOrInt::String(s) => s.parse::().map_err(serde::de::Error::custom), - StringOrInt::Number(i) => Ok(i), + match StringOrNumber::::deserialize(deserializer)? { + StringOrNumber::String(s) => s.parse::().map_err(serde::de::Error::custom), + StringOrNumber::Number(n) => Ok(n), + StringOrNumber::Bool(v) => Err(serde::de::Error::custom(format!( + "unsupported value: {:?}", + v + ))), + StringOrNumber::Array(v) => Err(serde::de::Error::custom(format!( + "unsupported value: {:?}", + v + ))), + StringOrNumber::Object(v) => Err(serde::de::Error::custom(format!( + "unsupported value: {:?}", + v + ))), } } diff --git a/relay-server/src/actors/processor.rs b/relay-server/src/actors/processor.rs index 61c072de6b..81736c331e 100644 --- a/relay-server/src/actors/processor.rs +++ b/relay-server/src/actors/processor.rs @@ -1097,7 +1097,7 @@ impl EnvelopeProcessorService { ItemType::Profile if transaction_count == 0 => ItemAction::DropSilently, // First profile found in the envelope, we'll keep it if metadata are valid. ItemType::Profile if !found_profile => { - match relay_profiling::parse_metadata(&item.payload()) { + match relay_profiling::parse_metadata(&item.payload(), state.project_id) { Ok(_) => { found_profile = true; ItemAction::Keep @@ -1152,7 +1152,11 @@ impl EnvelopeProcessorService { if !profiling_enabled { return ItemAction::DropSilently; } - match relay_profiling::expand_profile(&item.payload(), state.event.value()) { + // If we don't have an event at this stage, we need to drop the profile. + let Some(event) = state.event.value() else { + return ItemAction::DropSilently; + }; + match relay_profiling::expand_profile(&item.payload(), event) { Ok((profile_id, payload)) => { if payload.len() <= self.inner.config.max_profile_size() { found_profile_id = Some(profile_id);