ref(profiling): Log better errors (#2644)

relay-profiling/Cargo.toml

@@ -14,9 +14,11 @@ android_trace_log = { version = "0.3.0", features = ["serde"] }
 chrono = { workspace = true }
 data-encoding = "2.3.3"
 relay-event-schema = { path = "../relay-event-schema" }
+relay-log = { path = "../relay-log" }
 relay-protocol = { path = "../relay-protocol" }
 serde = { workspace = true }
 serde_json = { workspace = true }
+serde_path_to_error = "0.1.14"
 thiserror = { workspace = true }
 
 [dev-dependencies]

relay-profiling/src/android.rs

@@ -128,8 +128,9 @@ impl AndroidProfile {
 }
 
 fn parse_profile(payload: &[u8]) -> Result<AndroidProfile, ProfileError> {
+    let d = &mut serde_json::Deserializer::from_slice(payload);
     let mut profile: AndroidProfile =
-        serde_json::from_slice(payload).map_err(ProfileError::InvalidJson)?;
+        serde_path_to_error::deserialize(d).map_err(ProfileError::InvalidJson)?;
 
     let transaction_opt = profile.metadata.transactions.drain(..).next();
     if let Some(transaction) = transaction_opt {
@@ -271,7 +272,9 @@ mod tests {
         let profile_json = parse_android_profile(payload, transaction_metadata, BTreeMap::new());
         assert!(profile_json.is_ok());
 
-        let output: AndroidProfile = serde_json::from_slice(&profile_json.unwrap()[..])
+        let payload = profile_json.unwrap();
+        let d = &mut serde_json::Deserializer::from_slice(&payload[..]);
+        let output: AndroidProfile = serde_path_to_error::deserialize(d)
             .map_err(ProfileError::InvalidJson)
             .unwrap();
         assert_eq!(output.metadata.release, "some-random-release".to_string());

relay-profiling/src/error.rs

@@ -3,7 +3,7 @@ use thiserror::Error;
 #[derive(Debug, Error)]
 pub enum ProfileError {
     #[error(transparent)]
-    InvalidJson(#[from] serde_json::Error),
+    InvalidJson(#[from] serde_path_to_error::Error<serde_json::Error>),
     #[error("invalid base64 value")]
     InvalidBase64Value,
     #[error("invalid sampled profile")]
@@ -31,3 +31,12 @@ pub enum ProfileError {
     #[error("duration is too long")]
     DurationIsTooLong,
 }
+
+impl ProfileError {
+    pub fn path(self) -> String {
+        match self {
+            Self::InvalidJson(err) => err.path().to_string(),
+            _ => "".into(),
+        }
+    }
+}

relay-profiling/src/lib.rs

@@ -93,10 +93,19 @@
 //! }
 //! ```
 
-use serde::Deserialize;
 use std::collections::BTreeMap;
+use std::error::Error;
 use std::time::Duration;
 
+use relay_event_schema::protocol::{Event, EventId};
+use serde::Deserialize;
+use serde_json::Deserializer;
+
+use crate::extract_from_transaction::{extract_transaction_metadata, extract_transaction_tags};
+
+pub use crate::error::ProfileError;
+pub use crate::outcomes::discard_reason;
+
 mod android;
 mod error;
 mod extract_from_transaction;
@@ -107,13 +116,6 @@ mod sample;
 mod transaction_metadata;
 mod utils;
 
-use relay_event_schema::protocol::{Event, EventId};
-
-use crate::extract_from_transaction::{extract_transaction_metadata, extract_transaction_tags};
-
-pub use crate::error::ProfileError;
-pub use crate::outcomes::discard_reason;
-
 const MAX_PROFILE_DURATION: Duration = Duration::from_secs(30);
 
 #[derive(Debug, Deserialize)]
@@ -126,26 +128,46 @@ struct MinimalProfile {
 }
 
 fn minimal_profile_from_json(payload: &[u8]) -> Result<MinimalProfile, ProfileError> {
-    serde_json::from_slice(payload).map_err(ProfileError::InvalidJson)
+    let d = &mut Deserializer::from_slice(payload);
+    serde_path_to_error::deserialize(d).map_err(ProfileError::InvalidJson)
 }
 
 pub fn parse_metadata(payload: &[u8]) -> Result<(), ProfileError> {
     let profile = match minimal_profile_from_json(payload) {
         Ok(profile) => profile,
-        Err(err) => return Err(err),
+        Err(err) => {
+            relay_log::warn!(error = &err as &dyn Error, "invalid profile (minimal)");
+            return Err(err);
+        }
     };
     match profile.version {
         sample::Version::V1 => {
-            let _: sample::ProfileMetadata = match serde_json::from_slice(payload) {
+            let d = &mut Deserializer::from_slice(payload);
+            let _: sample::ProfileMetadata = match serde_path_to_error::deserialize(d) {
                 Ok(profile) => profile,
-                Err(err) => return Err(ProfileError::InvalidJson(err)),
+                Err(err) => {
+                    relay_log::warn!(
+                        error = &err as &dyn Error,
+                        "invalid profile (platform: {}, version: {:?})",
+                        profile.platform,
+                        profile.version,
+                    );
+                    return Err(ProfileError::InvalidJson(err));
+                }
             };
         }
         _ => match profile.platform.as_str() {
             "android" => {
-                let _: android::ProfileMetadata = match serde_json::from_slice(payload) {
+                let d = &mut Deserializer::from_slice(payload);
+                let _: android::ProfileMetadata = match serde_path_to_error::deserialize(d) {
                     Ok(profile) => profile,
-                    Err(err) => return Err(ProfileError::InvalidJson(err)),
+                    Err(err) => {
+                        relay_log::warn!(
+                            error = &err as &dyn Error,
+                            "invalid profile (platform: android)",
+                        );
+                        return Err(ProfileError::InvalidJson(err));
+                    }
                 };
             }
             _ => return Err(ProfileError::PlatformNotSupported),
@@ -180,7 +202,23 @@ pub fn expand_profile(
             _ => return Err(ProfileError::PlatformNotSupported),
         },
     };
-    processed_payload.map(|payload| (profile.event_id, payload))
+    match processed_payload {
+        Ok(payload) => Ok((profile.event_id, payload)),
+        Err(err) => match err {
+            ProfileError::InvalidJson(err) => {
+                relay_log::warn!(
+                    error = &err as &dyn Error,
+                    "invalid profile (platform: {})",
+                    profile.platform,
+                );
+                Err(ProfileError::InvalidJson(err))
+            }
+            _ => {
+                relay_log::debug!(error = &err as &dyn Error, "invalid profile");
+                Err(err)
+            }
+        },
+    }
 }
 
 #[cfg(test)]

relay-profiling/src/sample.rs

@@ -311,8 +311,9 @@ impl SampleProfile {
 }
 
 fn parse_profile(payload: &[u8]) -> Result<SampleProfile, ProfileError> {
+    let d = &mut serde_json::Deserializer::from_slice(payload);
     let mut profile: SampleProfile =
-        serde_json::from_slice(payload).map_err(ProfileError::InvalidJson)?;
+        serde_path_to_error::deserialize(d).map_err(ProfileError::InvalidJson)?;
 
     if !profile.valid() {
         return Err(ProfileError::MissingProfileMetadata);
@@ -912,7 +913,9 @@ mod tests {
         let profile_json = parse_sample_profile(payload, transaction_metadata, BTreeMap::new());
         assert!(profile_json.is_ok());
 
-        let output: SampleProfile = serde_json::from_slice(&profile_json.unwrap()[..])
+        let payload = profile_json.unwrap();
+        let d = &mut serde_json::Deserializer::from_slice(&payload[..]);
+        let output: SampleProfile = serde_path_to_error::deserialize(d)
             .map_err(ProfileError::InvalidJson)
             .unwrap();
         assert_eq!(output.metadata.release, "some-random-release".to_string());

relay-server/src/actors/processor.rs

@@ -1166,17 +1166,9 @@ impl EnvelopeProcessorService {
                             )))
                         }
                     }
-                    Err(err) => {
-                        match err {
-                            relay_profiling::ProfileError::InvalidJson(_) => {
-                                relay_log::warn!(error = &err as &dyn Error, "invalid profile");
-                            }
-                            _ => relay_log::debug!(error = &err as &dyn Error, "invalid profile"),
-                        };
-                        ItemAction::Drop(Outcome::Invalid(DiscardReason::Profiling(
-                            relay_profiling::discard_reason(err),
-                        )))
-                    }
+                    Err(err) => ItemAction::Drop(Outcome::Invalid(DiscardReason::Profiling(
+                        relay_profiling::discard_reason(err),
+                    ))),
                 }
             }
             _ => ItemAction::Keep,