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(profile): Introduce a common sample format #1462

Merged
merged 23 commits into from Sep 27, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
f2bb0bc
feat(profile): Introduce a common sample format
phacops Sep 12, 2022
81561f1
Remove unnecessary code
phacops Sep 12, 2022
edd820d
Support frame indexing
phacops Sep 12, 2022
07923cc
Support frames without addresses
phacops Sep 13, 2022
e1221c3
Add a changelog entry
phacops Sep 13, 2022
b49c885
Merge branch 'master' into pierre/profiling-sample-format
phacops Sep 13, 2022
c5b8146
Use more efficient syntax where suggested
phacops Sep 13, 2022
08e1210
Add a thread_id field to the transaction metadata to collect the thre…
phacops Sep 14, 2022
ab0f69c
Merge branch 'master' into pierre/profiling-sample-format
phacops Sep 14, 2022
d6048d5
Generate a new event_id when duplicating the profile
phacops Sep 14, 2022
20c33ba
Avoid potential panic if end < start
phacops Sep 14, 2022
b3fc60e
Match on specific version instead of just testing for existence
phacops Sep 14, 2022
ce2f060
Do not filter frames or stacks since they are indexed
phacops Sep 14, 2022
af1de3b
Explain why we're duplicating profiles
phacops Sep 14, 2022
fdae8a7
Adjust some types
phacops Sep 15, 2022
9adabfe
Do not serialize zero values
phacops Sep 15, 2022
9661180
Rename field to clarify its use
phacops Sep 15, 2022
cfb397f
Merge branch 'master' into pierre/profiling-sample-format
phacops Sep 15, 2022
760a62a
Merge branch 'master' into pierre/profiling-sample-format
phacops Sep 19, 2022
895c195
Rename a field for clarity
phacops Sep 26, 2022
77e5b01
Rename key in test data as well
phacops Sep 26, 2022
b4af3bc
Merge branch 'master' into pierre/profiling-sample-format
phacops Sep 27, 2022
0601e39
Remove check for thread_id since nodejs thread IDs can be equal to 0
phacops Sep 27, 2022
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Expand Up @@ -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))
Expand Down
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 2 additions & 1 deletion relay-profiling/Cargo.toml
Expand Up @@ -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"
Expand Down
2 changes: 1 addition & 1 deletion relay-profiling/src/android.rs
Expand Up @@ -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 {
Expand Down
57 changes: 5 additions & 52 deletions relay-profiling/src/cocoa.rs
Expand Up @@ -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
Expand Down Expand Up @@ -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>,
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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");
Expand Down
2 changes: 2 additions & 0 deletions relay-profiling/src/error.rs
Expand Up @@ -18,4 +18,6 @@ pub enum ProfileError {
NoTransactionAssociated,
#[fail(display = "invalid transaction metadata")]
InvalidTransactionMetadata,
#[fail(display = "missing profile metadata")]
MissingProfileMetadata,
}
86 changes: 68 additions & 18 deletions relay-profiling/src/lib.rs
Expand Up @@ -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;
Expand All @@ -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());
}
}
58 changes: 58 additions & 0 deletions 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);
}
}
2 changes: 1 addition & 1 deletion relay-profiling/src/rust.rs
Expand Up @@ -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)]
Expand Down