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): Add optional js profile to Android profiles events #2706

Merged
merged 18 commits into from
Nov 29, 2023
Merged
Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
6d87ca4
feat(profiling): Add optional js profile to Android profiles events
krystofwoldrich Nov 9, 2023
ff675a2
Add validation comment
krystofwoldrich Nov 9, 2023
6348709
Add js debug meta to android profile
krystofwoldrich Nov 21, 2023
e054389
Merge remote-tracking branch 'origin/master' into kw-android-with-js-…
krystofwoldrich Nov 21, 2023
2e3c12b
Move validations and processing to profile.profile
krystofwoldrich Nov 22, 2023
04227c9
Add js profile validation to android
krystofwoldrich Nov 22, 2023
457187b
Merge remote-tracking branch 'origin/master' into kw-android-with-js-…
krystofwoldrich Nov 22, 2023
cc926af
add changelog
krystofwoldrich Nov 22, 2023
c42a945
Merge remote-tracking branch 'origin/master' into kw-android-with-js-…
krystofwoldrich Nov 22, 2023
748d510
fix changelog
krystofwoldrich Nov 22, 2023
a0e342d
Add docs to sample profile normalize fun
krystofwoldrich Nov 22, 2023
c89d7e5
Refactor normalization utils funs
krystofwoldrich Nov 22, 2023
479bfca
Merge branch 'master' into kw-android-with-js-profile
krystofwoldrich Nov 23, 2023
8edde5c
Fix review notes
krystofwoldrich Nov 23, 2023
0e561f8
Merge remote-tracking branch 'origin/kw-android-with-js-profile' into…
krystofwoldrich Nov 23, 2023
3dbaf1e
Merge remote-tracking branch 'origin/master' into kw-android-with-js-…
krystofwoldrich Nov 23, 2023
067a491
Merge remote-tracking branch 'origin/master' into kw-android-with-js-…
krystofwoldrich Nov 29, 2023
f64844f
Merge branch 'master' into kw-android-with-js-profile
krystofwoldrich Nov 29, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

- `normalize_performance_score` stores 0 to 1 cdf score instead of weighted score for each performance score component. ([#2734](https://github.com/getsentry/relay/pull/2734))
- Add Bytespider (Bytedance) to web crawler filter. ([#2747](https://github.com/getsentry/relay/pull/2747))
- Add Mixed JS/Android Profiles events processing. ([#2706](https://github.com/getsentry/relay/pull/2706))
krystofwoldrich marked this conversation as resolved.
Show resolved Hide resolved

**Bug Fixes**:

Expand Down
28 changes: 28 additions & 0 deletions relay-profiling/src/android.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ use relay_event_schema::protocol::EventId;
use serde::{Deserialize, Serialize};

use crate::measurements::Measurement;
use crate::native_debug_image::NativeDebugImage;
use crate::sample::Profile as SampleProfile;
krystofwoldrich marked this conversation as resolved.
Show resolved Hide resolved
use crate::transaction_metadata::TransactionMetadata;
use crate::utils::{deserialize_number_from_string, is_zero};
use crate::{ProfileError, MAX_PROFILE_DURATION};
Expand Down Expand Up @@ -78,6 +80,9 @@ pub struct ProfileMetadata {

#[serde(default, skip_serializing_if = "BTreeMap::is_empty")]
transaction_tags: BTreeMap<String, String>,

#[serde(skip_serializing_if = "Option::is_none")]
debug_meta: Option<DebugMeta>,
}

#[derive(Debug, Serialize, Deserialize, Clone)]
Expand All @@ -88,10 +93,18 @@ struct AndroidProfile {
#[serde(default, skip_serializing)]
sampled_profile: String,

#[serde(default, skip_serializing_if = "Option::is_none")]
js_profile: Option<SampleProfile>,

#[serde(default = "AndroidProfile::default")]
profile: AndroidTraceLog,
}

#[derive(Default, Debug, Serialize, Deserialize, Clone)]
struct DebugMeta {
images: Vec<NativeDebugImage>,
}

impl AndroidProfile {
fn default() -> AndroidTraceLog {
AndroidTraceLog {
Expand Down Expand Up @@ -158,6 +171,10 @@ fn parse_profile(payload: &[u8]) -> Result<AndroidProfile, ProfileError> {
return Err(ProfileError::NoTransactionAssociated);
}

if let Some(ref mut js_profile) = profile.js_profile {
js_profile.parse(profile.metadata.platform.as_str(), "")?;
}

if !profile.sampled_profile.is_empty() {
profile.parse()?;
}
Expand Down Expand Up @@ -221,6 +238,17 @@ mod tests {
);
}

#[test]
fn test_roundtrip_react_native() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test validates the parsing doesn't fail, but we don't know if the contents are what we expect. What do you think about validating that too?

Copy link
Member Author

@krystofwoldrich krystofwoldrich Nov 23, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tests ensuring values are in current places are in sample.rs.

let payload = include_bytes!("../tests/fixtures/profiles/android/roundtrip.rn.json");
let profile = parse_profile(payload);
assert!(profile.is_ok());
let data = serde_json::to_vec(&profile.unwrap());
assert!(
parse_android_profile(&(data.unwrap())[..], BTreeMap::new(), BTreeMap::new()).is_ok()
);
}

#[test]
fn test_no_transaction() {
let payload = include_bytes!("../tests/fixtures/profiles/android/no_transaction.json");
Expand Down
Loading
Loading