fix(profiling): Fix accidentally correct Android profile chunk parsing

CHANGELOG.md

@@ -23,11 +23,13 @@
 - Unset segment info for web vital spans. ([#6042](https://github.com/getsentry/relay/pull/6042))
 - Set sentry.trace.status on segment spans. ([#6140](https://github.com/getsentry/relay/pull/6140))
 - Don't modify segment information for V2 web vital spans. ([#6160](https://github.com/getsentry/relay/pull/6160))
-
 - Support compressed minidumps when the `relay-minidump-uploads` feature is enabled. ([#6151](https://github.com/getsentry/relay/pull/6151))
 - Make `--log-level` and `--log-format` take effect again and accept them on all subcommands. ([#6198](https://github.com/getsentry/relay/pull/6198))
 - Parse two-component versions in iOS and iPadOS `raw_description` into `version` instead of `kernel_version`. ([#6197](https://github.com/getsentry/relay/pull/6197))
 - Normalize segment names for standalone spans in the experimental pipeline. ([#6163](https://github.com/getsentry/relay/pull/6163))
+- Fix Android trace and ANR profile parsing. Serialize Android trace chunks with `version: "2.android-trace"`. Custom
+  Android trace `profile_chunk` producers must send `version: "2"` or `version: "2.android-trace"`; versionless Android
+  trace chunks are rejected. ([#6183](https://github.com/getsentry/relay/pull/6183))
 
 **Internal**:
 

relay-profiling/src/android/chunk.rs

@@ -17,6 +17,7 @@ use serde::{Deserialize, Serialize};
 
 use crate::debug_image::get_proguard_image;
 use crate::measurements::ChunkMeasurement;
+use crate::sample::Version;
 use crate::sample::v2::ProfileData;
 use crate::types::{ClientSdk, DebugMeta};
 use crate::{MAX_PROFILE_CHUNK_DURATION, ProfileError};
@@ -35,6 +36,9 @@ pub struct Metadata {
     platform: String,
     release: String,
 
+    #[serde(default)]
+    version: Version,
+
     #[serde(skip_serializing_if = "Option::is_none")]
     debug_meta: Option<DebugMeta>,
 
@@ -114,6 +118,11 @@ impl Chunk {
         // Use duration given by the profiler and not reported by the SDK.
         profile.metadata.duration_ns = profile.profile.elapsed_time.as_nanos() as u64;
 
+        // Convert the accepted legacy Android trace version ("2") to the new wire version
+        // ("2.android-trace"). We do so during parsing rather than when serializing because raw
+        // serde doesn't validate the trace payload.
+        profile.metadata.version = Version::V2AndroidTrace;
+
         // If build_id is not empty but we don't have any DebugImage set,
         // we create the proper Proguard image and set the uuid.
         if !profile.metadata.build_id.is_empty() && profile.metadata.debug_meta.is_none() {
@@ -175,8 +184,26 @@ mod tests {
     fn test_roundtrip_react_native() {
         let payload = include_bytes!("../../tests/fixtures/android/chunk/valid-rn.json");
         let profile = Chunk::parse(payload).unwrap();
-        let data = serde_json::to_vec(&profile);
-        assert!(Chunk::parse(&(data.unwrap())[..]).is_ok());
+        let data = serde_json::to_vec(&profile).unwrap();
+        let output: serde_json::Value = serde_json::from_slice(&data).unwrap();
+
+        assert_eq!(output["version"], "2.android-trace");
+        assert!(Chunk::parse(&data).is_ok());
+    }
+
+    #[test]
+    fn test_parse_canonicalizes_android_trace_profile_version() {
+        let payload = include_bytes!("../../tests/fixtures/android/chunk/valid.json");
+        let input: serde_json::Value = serde_json::from_slice(payload).unwrap();
+        assert_eq!(input["version"], "2");
+
+        let profile = Chunk::parse(payload).unwrap();
+        assert_eq!(profile.metadata.version, Version::V2AndroidTrace);
+
+        let output = serde_json::to_value(&profile).unwrap();
+
+        assert_eq!(output["version"], "2.android-trace");
+        assert!(output.get("sampled_profile").is_none());
     }
 
     #[test]

relay-profiling/src/profile_chunk.rs

@@ -94,6 +94,7 @@ impl relay_filter::Filterable for AnyProfileChunk {
 }
 
 /// Either an [`AndroidProfileChunk`] or a [`V2ProfileChunk`].
+#[derive(Debug)]
 pub enum AndroidOrV2ProfileChunk {
     Android(Box<AndroidProfileChunk>),
     V2(Box<V2ProfileChunk>),
@@ -123,23 +124,188 @@ impl AndroidOrV2ProfileChunk {
             platform: String,
             #[serde(default)]
             version: sample::Version,
+            #[serde(default)]
+            sampled_profile: Option<serde::de::IgnoredAny>,
         }
 
         let minimal: MinimalProfile = {
             let d = &mut serde_json::Deserializer::from_slice(data);
             serde_path_to_error::deserialize(d)
         }?;
 
-        match (minimal.platform.as_str(), minimal.version) {
-            // This has always been parsed with higher priority than `v2`, so this was kept as-is
-            // when refactoring, but from the looks of it, this may cause issues with v2 profiles
-            // which happen to be sent from android.
-            ("android", _) => AndroidProfileChunk::parse(data)
+        let is_android_trace_profile = minimal.platform == "android"
+            && matches!(
+                minimal.version,
+                sample::Version::V2 | sample::Version::V2AndroidTrace
+            )
+            && minimal.sampled_profile.is_some();
+
+        match (minimal.version, is_android_trace_profile) {
+            // Android SDKs produce two profile_chunk types that pass through this method: trace
+            // profiles and Application-Not-Responding (ANR) profiles. They come in multiple
+            // varieties, each of which needs to be accounted for.
+
+            // Trace profiles:
+            // ---------------
+            // Versions: 2 (incorrect), 2.android-trace (corrected)
+            // Platform: android
+            // Content field: sampled_profile (i.e., Android Runtime's event-based format, aka
+            //   "traces")
+            // Destination type: AndroidProfileChunk
+
+            // ANR profiles:
+            // ---------------
+            // Version: 2
+            // Platform: java (incorrect), android (corrected)
+            // Content field: profile (i.e., standardized stacks/frames/samples format)
+            // Destination type: V2ProfileChunk
+
+            // First handle Android trace profiles...
+            (_, true) => AndroidProfileChunk::parse(data)
                 .map(Box::new)
                 .map(Self::Android),
-            (_, sample::Version::V2) => V2ProfileChunk::parse(data).map(Box::new).map(Self::V2),
-            (_, sample::Version::V1) => Err(ProfileError::PlatformNotSupported),
-            (_, sample::Version::Unknown) => Err(ProfileError::PlatformNotSupported),
+
+            // ...then handle everything else (Android ANRs, Cocoa profiles, etc.).
+            (sample::Version::V2, _) => V2ProfileChunk::parse(data).map(Box::new).map(Self::V2),
+            (
+                sample::Version::V2AndroidTrace | sample::Version::V1 | sample::Version::Unknown,
+                _,
+            ) => Err(ProfileError::PlatformNotSupported),
+        }
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use serde_json::{Value, json};
+
+    use super::*;
+
+    #[test]
+    fn test_parse_android_trace_profile_into_android_profile_chunk() {
+        let base_payload: Value =
+            serde_json::from_slice(include_bytes!("../tests/fixtures/android/chunk/valid.json"))
+                .unwrap();
+
+        for version in ["2", "2.android-trace"] {
+            let mut payload = base_payload.clone();
+            payload["version"] = json!(version);
+            let data = serde_json::to_vec(&payload).unwrap();
+
+            let chunk = AndroidOrV2ProfileChunk::parse(&data).unwrap();
+
+            assert!(
+                matches!(chunk, AndroidOrV2ProfileChunk::Android(_)),
+                "expected Android profile chunk for version {version:?}"
+            );
+        }
+    }
+
+    #[test]
+    fn test_return_error_for_android_trace_profile_without_version() {
+        for (fixture, payload) in [
+            (
+                "android trace format",
+                &include_bytes!("../tests/fixtures/android/chunk/valid.json")[..],
+            ),
+            (
+                "react native android trace format",
+                &include_bytes!("../tests/fixtures/android/chunk/valid-rn.json")[..],
+            ),
+        ] {
+            let mut payload: Value = serde_json::from_slice(payload).unwrap();
+            payload.as_object_mut().unwrap().remove("version");
+            let data = serde_json::to_vec(&payload).unwrap();
+
+            let err = AndroidOrV2ProfileChunk::parse(&data).unwrap_err();
+            assert!(
+                matches!(err, ProfileError::PlatformNotSupported),
+                "expected unsupported platform error for {fixture}"
+            );
+        }
+    }
+
+    #[test]
+    fn test_parse_sample_v2_profile_into_v2_profile_chunk() {
+        let base_payload: Value =
+            serde_json::from_slice(include_bytes!("../tests/fixtures/sample/v2/valid.json"))
+                .unwrap();
+
+        for platform in ["android", "cocoa"] {
+            let mut payload = base_payload.clone();
+            payload["platform"] = json!(platform);
+            let data = serde_json::to_vec(&payload).unwrap();
+
+            let chunk = AndroidOrV2ProfileChunk::parse(&data).unwrap();
+
+            assert!(
+                matches!(chunk, AndroidOrV2ProfileChunk::V2(_)),
+                "expected v2 profile chunk for platform {platform:?}"
+            );
+        }
+    }
+
+    #[test]
+    fn test_return_error_for_android_trace_version_without_android_trace_payload() {
+        let base_payload: Value =
+            serde_json::from_slice(include_bytes!("../tests/fixtures/sample/v2/valid.json"))
+                .unwrap();
+
+        for platform in ["android", "cocoa"] {
+            let mut payload = base_payload.clone();
+            payload["platform"] = json!(platform);
+            payload["version"] = json!("2.android-trace");
+            let data = serde_json::to_vec(&payload).unwrap();
+
+            let err = AndroidOrV2ProfileChunk::parse(&data).unwrap_err();
+            assert!(
+                matches!(err, ProfileError::PlatformNotSupported),
+                "expected unsupported platform error for platform {platform:?}"
+            );
+        }
+    }
+
+    #[test]
+    fn test_return_error_for_version_1_profile() {
+        for (fixture, payload) in [
+            (
+                "sample v2 format",
+                &include_bytes!("../tests/fixtures/sample/v2/valid.json")[..],
+            ),
+            (
+                "android trace format",
+                &include_bytes!("../tests/fixtures/android/chunk/valid.json")[..],
+            ),
+        ] {
+            let mut payload: Value = serde_json::from_slice(payload).unwrap();
+            payload["version"] = json!("1");
+            let data = serde_json::to_vec(&payload).unwrap();
+
+            let err = AndroidOrV2ProfileChunk::parse(&data).unwrap_err();
+            assert!(
+                matches!(err, ProfileError::PlatformNotSupported),
+                "expected unsupported platform error for {fixture}"
+            );
+        }
+    }
+
+    #[test]
+    fn test_return_error_for_unknown_version_profile() {
+        let base_payload: Value =
+            serde_json::from_slice(include_bytes!("../tests/fixtures/sample/v2/valid.json"))
+                .unwrap();
+
+        for platform in ["android", "cocoa"] {
+            let mut payload = base_payload.clone();
+            payload["platform"] = json!(platform);
+            payload.as_object_mut().unwrap().remove("version");
+            let data = serde_json::to_vec(&payload).unwrap();
+
+            let err = AndroidOrV2ProfileChunk::parse(&data).unwrap_err();
+            assert!(
+                matches!(err, ProfileError::PlatformNotSupported),
+                "expected unsupported platform error for platform {platform:?}"
+            );
         }
     }
 }

relay-profiling/src/sample/mod.rs

@@ -15,6 +15,9 @@ pub enum Version {
     V1,
     #[serde(rename = "2")]
     V2,
+    /// Special-cased chunk format for Android trace profiles, distinct from sample-format v2.
+    #[serde(rename = "2.android-trace")]
+    V2AndroidTrace,
 }
 
 /// Holds information about a single stacktrace frame.

tests/integration/test_profile_chunks.py

@@ -1,3 +1,4 @@
+import json
 import uuid
 from copy import deepcopy
 from pathlib import Path
@@ -321,6 +322,41 @@ def test_profile_chunk_outcomes_rate_limited_fast(
         assert mini_sentry.captured_envelopes.empty()
 
 
+@pytest.mark.parametrize(
+    ["envelope_factory", "expected_version"],
+    [
+        pytest.param(sample_profile_v2_envelope, "2", id="profile v2"),
+        pytest.param(
+            android_profile_chunk_envelope,
+            "2.android-trace",
+            id="android chunk",
+        ),
+    ],
+)
+def test_profile_chunk_version_is_forwarded(
+    mini_sentry,
+    relay_with_processing,
+    profiles_consumer,
+    envelope_factory,
+    expected_version,
+):
+    profiles_consumer = profiles_consumer()
+
+    project_id = 42
+    project_config = mini_sentry.add_full_project_config(project_id)["config"]
+
+    project_config.setdefault("features", []).append(
+        "organizations:continuous-profiling"
+    )
+
+    upstream = relay_with_processing(TEST_CONFIG)
+    upstream.send_envelope(project_id, envelope_factory())
+
+    profile, headers = profiles_consumer.get_profile()
+    assert headers == [("project_id", b"42")]
+    assert json.loads(profile["payload"])["version"] == expected_version
+
+
 @pytest.mark.parametrize(
     "platform, category, filter_context",
     [