feat(profile): Introduce a common sample format (#1462)

getsentry · Sep 27, 2022 · 4894c1d · 4894c1d
1 parent af8c6bb
commit 4894c1d
Show file tree

Hide file tree

Showing 16 changed files with 879 additions and 87 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -9,6 +9,7 @@
 
 **Internal**:
 
+- Introduce a new profile format called `sample`. ([#1462](https://github.com/getsentry/relay/pull/1462))
 - Generate a new profile ID when splitting a profile for multiple transactions. ([#1473](https://github.com/getsentry/relay/pull/1473))
 - Pin Rust version to 1.63.0 in Dockerfile. ([#1482](https://github.com/getsentry/relay/pull/1482))
 - Normalize measurement units in event payload. ([#1488](https://github.com/getsentry/relay/pull/1488))

diff --git a/Cargo.lock b/Cargo.lock
diff --git a/relay-profiling/Cargo.toml b/relay-profiling/Cargo.toml
@@ -10,10 +10,11 @@ license-file = "../LICENSE"
 publish = false
 
 [dependencies]
-failure = "0.1.8"
 android_trace_log = { version = "0.2.0", features = ["serde"] }
 base64 = "0.10.1"
 bytes = { version = "0.4.12", features = ["serde"] }
+chrono = { version = "0.4", features = ["serde"] }
+failure = "0.1.8"
 relay-general = { path = "../relay-general" }
 serde = { version = "1.0.114", features = ["derive"] }
 serde_json = "1.0.55"

diff --git a/relay-profiling/src/android.rs b/relay-profiling/src/android.rs
@@ -94,7 +94,7 @@ impl AndroidProfile {
         self.transaction_name = transaction.name.clone();
         self.transaction_id = transaction.id;
         self.trace_id = transaction.trace_id;
-        self.duration_ns = transaction.relative_end_ns - transaction.relative_start_ns;
+        self.duration_ns = transaction.duration_ns();
     }
 
     fn has_transaction_metadata(&self) -> bool {

diff --git a/relay-profiling/src/cocoa.rs b/relay-profiling/src/cocoa.rs
@@ -2,11 +2,12 @@ use std::collections::HashMap;
 
 use serde::{de, Deserialize, Serialize};
 
-use relay_general::protocol::{Addr, DebugId, EventId, NativeImagePath};
+use relay_general::protocol::{Addr, EventId};
 
+use crate::error::ProfileError;
+use crate::native_debug_image::NativeDebugImage;
 use crate::transaction_metadata::TransactionMetadata;
 use crate::utils::{deserialize_number_from_string, is_zero};
-use crate::ProfileError;
 
 fn strip_pointer_authentication_code<'de, D>(deserializer: D) -> Result<Addr, D::Error>
 where
@@ -65,31 +66,6 @@ struct SampledProfile {
     queue_metadata: HashMap<String, QueueMetadata>,
 }
 
-#[derive(Debug, Serialize, Deserialize, PartialEq, Clone)]
-#[serde(rename_all = "lowercase")]
-enum ImageType {
-    MachO,
-    Symbolic,
-}
-
-#[derive(Debug, Serialize, Deserialize, PartialEq, Clone)]
-pub struct NativeDebugImage {
-    #[serde(alias = "name")]
-    code_file: NativeImagePath,
-    #[serde(alias = "id")]
-    debug_id: DebugId,
-    image_addr: Addr,
-
-    #[serde(default)]
-    image_vmaddr: Addr,
-
-    #[serde(deserialize_with = "deserialize_number_from_string")]
-    image_size: u64,
-
-    #[serde(rename = "type")]
-    image_type: ImageType,
-}
-
 #[derive(Debug, Serialize, Deserialize, Clone)]
 struct DebugMeta {
     images: Vec<NativeDebugImage>,
@@ -137,7 +113,7 @@ impl CocoaProfile {
         self.transaction_name = transaction.name.clone();
         self.transaction_id = transaction.id;
         self.trace_id = transaction.trace_id;
-        self.duration_ns = transaction.relative_end_ns - transaction.relative_start_ns;
+        self.duration_ns = transaction.duration_ns();
     }
 
     fn has_transaction_metadata(&self) -> bool {
@@ -234,38 +210,15 @@ fn parse_cocoa_profile(payload: &[u8]) -> Result<CocoaProfile, ProfileError> {
 mod tests {
     use super::*;
 
-    use relay_general::protocol::{DebugImage, NativeDebugImage as RelayNativeDebugImage};
-    use relay_general::types::{Annotated, Map};
-
     #[test]
     fn test_roundtrip_cocoa() {
-        let payload = include_bytes!("../tests/fixtures/profiles/cocoa/valid.json");
+        let payload = include_bytes!("../tests/fixtures/profiles/cocoa/roundtrip.json");
         let profile = parse_cocoa_profile(payload);
         assert!(profile.is_ok());
         let data = serde_json::to_vec(&profile.unwrap());
         assert!(parse_cocoa_profile(&data.unwrap()[..]).is_ok());
     }
 
-    #[test]
-    fn test_ios_debug_image_compatibility() {
-        let image_json = r#"{"debug_id":"32420279-25E2-34E6-8BC7-8A006A8F2425","image_addr":"0x000000010258c000","code_file":"/private/var/containers/Bundle/Application/C3511752-DD67-4FE8-9DA2-ACE18ADFAA61/TrendingMovies.app/TrendingMovies","type":"macho","image_size":1720320,"image_vmaddr":"0x0000000100000000"}"#;
-        let image: NativeDebugImage = serde_json::from_str(image_json).unwrap();
-        let json = serde_json::to_string(&image).unwrap();
-        let annotated = Annotated::from_json(&json[..]).unwrap();
-        assert_eq!(
-            Annotated::new(DebugImage::MachO(Box::new(RelayNativeDebugImage {
-                arch: Annotated::empty(),
-                code_file: Annotated::new("/private/var/containers/Bundle/Application/C3511752-DD67-4FE8-9DA2-ACE18ADFAA61/TrendingMovies.app/TrendingMovies".into()),
-                code_id: Annotated::empty(),
-                debug_file: Annotated::empty(),
-                debug_id: Annotated::new("32420279-25E2-34E6-8BC7-8A006A8F2425".parse().unwrap()),
-                image_addr: Annotated::new(Addr(4334338048)),
-                image_size: Annotated::new(1720320),
-                image_vmaddr: Annotated::new(Addr(4294967296)),
-                other: Map::new(),
-            }))), annotated);
-    }
-
     #[test]
     fn test_parse_multiple_transactions() {
         let payload = include_bytes!("../tests/fixtures/profiles/cocoa/multiple_transactions.json");

diff --git a/relay-profiling/src/error.rs b/relay-profiling/src/error.rs
@@ -18,4 +18,6 @@ pub enum ProfileError {
     NoTransactionAssociated,
     #[fail(display = "invalid transaction metadata")]
     InvalidTransactionMetadata,
+    #[fail(display = "missing profile metadata")]
+    MissingProfileMetadata,
 }
diff --git a/relay-profiling/src/lib.rs b/relay-profiling/src/lib.rs
@@ -93,13 +93,15 @@
 //! }
 //! ```
 
-use serde::Deserialize;
+use serde::{Deserialize, Serialize};
 
 mod android;
 mod cocoa;
 mod error;
+mod native_debug_image;
 mod python;
 mod rust;
+mod sample;
 mod transaction_metadata;
 mod typescript;
 mod utils;
@@ -108,36 +110,84 @@ use crate::android::expand_android_profile;
 use crate::cocoa::expand_cocoa_profile;
 use crate::python::parse_python_profile;
 use crate::rust::parse_rust_profile;
+use crate::sample::{expand_sample_profile, Version};
 use crate::typescript::parse_typescript_profile;
 
 pub use crate::error::ProfileError;
 
+#[derive(Debug, Serialize, Deserialize, Clone)]
+#[serde(rename_all = "lowercase")]
+enum Platform {
+    Android,
+    Cocoa,
+    Node,
+    Python,
+    Rust,
+    Typescript,
+}
+
 #[derive(Debug, Deserialize)]
 struct MinimalProfile {
-    platform: String,
+    platform: Platform,
+    #[serde(default)]
+    version: Version,
 }
 
 fn minimal_profile_from_json(data: &[u8]) -> Result<MinimalProfile, ProfileError> {
     serde_json::from_slice(data).map_err(ProfileError::InvalidJson)
 }
 
 pub fn expand_profile(payload: &[u8]) -> Result<Vec<Vec<u8>>, ProfileError> {
-    let minimal_profile: MinimalProfile = minimal_profile_from_json(payload)?;
-    let platform = minimal_profile.platform.as_str();
-    match platform {
-        "android" => expand_android_profile(payload),
-        "cocoa" => expand_cocoa_profile(payload),
-        _ => {
-            let payload = match platform {
-                "python" => parse_python_profile(payload),
-                "rust" => parse_rust_profile(payload),
-                "typescript" => parse_typescript_profile(payload),
-                _ => Err(ProfileError::PlatformNotSupported),
-            };
-            match payload {
-                Ok(payload) => Ok(vec![payload]),
-                Err(payload) => Err(payload),
+    let profile: MinimalProfile = minimal_profile_from_json(payload)?;
+    match profile.version {
+        Version::V1 => expand_sample_profile(payload),
+        Version::Unknown => match profile.platform {
+            Platform::Android => expand_android_profile(payload),
+            Platform::Cocoa => expand_cocoa_profile(payload),
+            _ => {
+                let payload = match profile.platform {
+                    Platform::Python => parse_python_profile(payload),
+                    Platform::Rust => parse_rust_profile(payload),
+                    Platform::Typescript => parse_typescript_profile(payload),
+                    _ => Err(ProfileError::PlatformNotSupported),
+                };
+                Ok(vec![payload?])
             }
-        }
+        },
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn test_minimal_profile_with_version() {
+        let data = r#"{"version": "1", "platform": "cocoa"}"#;
+        let profile = minimal_profile_from_json(data.as_bytes());
+        assert!(profile.is_ok());
+        assert_eq!(profile.unwrap().version, Version::V1);
+    }
+
+    #[test]
+    fn test_minimal_profile_without_version() {
+        let data = r#"{"platform": "cocoa"}"#;
+        let profile = minimal_profile_from_json(data.as_bytes());
+        assert!(profile.is_ok());
+        assert_eq!(profile.unwrap().version, Version::Unknown);
+    }
+
+    #[test]
+    fn test_expand_profile_with_version() {
+        let payload = include_bytes!("../tests/fixtures/profiles/sample/roundtrip.json");
+        let profile = expand_profile(payload);
+        assert!(profile.is_ok());
+    }
+
+    #[test]
+    fn test_expand_profile_without_version() {
+        let payload = include_bytes!("../tests/fixtures/profiles/cocoa/roundtrip.json");
+        let profile = expand_profile(payload);
+        assert!(profile.is_ok());
     }
 }
diff --git a/relay-profiling/src/native_debug_image.rs b/relay-profiling/src/native_debug_image.rs
@@ -0,0 +1,58 @@
+use serde::{Deserialize, Serialize};
+
+use relay_general::protocol::{Addr, DebugId, NativeImagePath};
+
+use crate::utils::deserialize_number_from_string;
+
+#[derive(Debug, Serialize, Deserialize, PartialEq, Clone)]
+#[serde(rename_all = "lowercase")]
+enum ImageType {
+    MachO,
+    Symbolic,
+}
+
+#[derive(Debug, Serialize, Deserialize, PartialEq, Clone)]
+pub struct NativeDebugImage {
+    #[serde(alias = "name")]
+    code_file: NativeImagePath,
+    #[serde(alias = "id")]
+    debug_id: DebugId,
+    image_addr: Addr,
+
+    #[serde(default)]
+    image_vmaddr: Addr,
+
+    #[serde(deserialize_with = "deserialize_number_from_string")]
+    image_size: u64,
+
+    #[serde(rename = "type")]
+    image_type: ImageType,
+}
+
+#[cfg(test)]
+mod tests {
+    use super::NativeDebugImage;
+
+    use relay_general::protocol::{Addr, DebugImage, NativeDebugImage as RelayNativeDebugImage};
+    use relay_general::types::{Annotated, Map};
+
+    #[test]
+    fn test_native_debug_image_compatibility() {
+        let image_json = r#"{"debug_id":"32420279-25E2-34E6-8BC7-8A006A8F2425","image_addr":"0x000000010258c000","code_file":"/private/var/containers/Bundle/Application/C3511752-DD67-4FE8-9DA2-ACE18ADFAA61/TrendingMovies.app/TrendingMovies","type":"macho","image_size":1720320,"image_vmaddr":"0x0000000100000000"}"#;
+        let image: NativeDebugImage = serde_json::from_str(image_json).unwrap();
+        let json = serde_json::to_string(&image).unwrap();
+        let annotated = Annotated::from_json(&json[..]).unwrap();
+        assert_eq!(
+            Annotated::new(DebugImage::MachO(Box::new(RelayNativeDebugImage {
+                arch: Annotated::empty(),
+                code_file: Annotated::new("/private/var/containers/Bundle/Application/C3511752-DD67-4FE8-9DA2-ACE18ADFAA61/TrendingMovies.app/TrendingMovies".into()),
+                code_id: Annotated::empty(),
+                debug_file: Annotated::empty(),
+                debug_id: Annotated::new("32420279-25E2-34E6-8BC7-8A006A8F2425".parse().unwrap()),
+                image_addr: Annotated::new(Addr(4334338048)),
+                image_size: Annotated::new(1720320),
+                image_vmaddr: Annotated::new(Addr(4294967296)),
+                other: Map::new(),
+            }))), annotated);
+    }
+}
diff --git a/relay-profiling/src/rust.rs b/relay-profiling/src/rust.rs
@@ -2,7 +2,7 @@ use serde::{Deserialize, Serialize};
 
 use relay_general::protocol::EventId;
 
-use crate::cocoa::NativeDebugImage;
+use crate::native_debug_image::NativeDebugImage;
 use crate::ProfileError;
 
 #[derive(Debug, Deserialize, Serialize)]