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

CHANGELOG.md

@@ -4,6 +4,10 @@
 
 **Bug Fixes**:
 
+- Fix Android trace and ANR profile parsing. Serialize Android trace chunks with `version: "2.android-trace"`. Custom
+  Android trace `profile_chunk` producers should send `version: "2.android-trace"`; legacy `version: "2"` is accepted
+  only for Android `sampled_profile` payloads. `version: "1"` and versionless Android trace chunks are rejected.
+  ([#6183](https://github.com/getsentry/relay/pull/6183))
 - Defer dynamic sampling until metrics config is valid. ([#6246](https://github.com/getsentry/relay/pull/6246))
 
 ## 26.7.1

relay-profiling/src/android/chunk.rs

@@ -17,6 +17,7 @@
 
 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 @@
     platform: String,
     release: String,
 
+    #[serde(default)]
+    version: Version,
+
     #[serde(skip_serializing_if = "Option::is_none")]
     debug_meta: Option<DebugMeta>,
 
@@ -114,6 +118,11 @@
         // 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 legacy Android trace version ("2") to the corrected version
+        // ("2.android-trace"). We do so during parsing rather than
+        // serialization 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() {
@@ -179,6 +188,21 @@
         assert!(Chunk::parse(&(data.unwrap())[..]).is_ok());
     }
 
+    #[test]
+    fn test_parse_corrects_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]
     fn test_remove_invalid_events() {
         let payload =

relay-profiling/src/lib.rs

@@ -33,7 +33,7 @@
 //! Each item type expects a different format.
 //!
 //! For `Profile` item type, we expect the Sample format v1 or Android format.
-//! For `ProfileChunk` item type, we expect the Sample format v2.
+//! For `ProfileChunk` item type, we expect the Sample format v2 or Android trace chunk format.
 //!
 //! # Ingestion
 //!

relay-profiling/src/profile_chunk.rs

@@ -1,7 +1,8 @@
 use serde::Deserialize;
 
 use crate::{
-    AndroidProfileChunk, PerfettoProfileChunk, ProfileError, ProfileType, V2ProfileChunk, sample,
+    AndroidProfileChunk, PerfettoProfileChunk, ProfileError, ProfileType, V2ProfileChunk,
+    sample::Version,
 };
 
 /// Minimum interface all profile chunk types must implement.
@@ -94,6 +95,7 @@ impl relay_filter::Filterable for AnyProfileChunk {
 }
 
 /// Either an [`AndroidProfileChunk`] or a [`V2ProfileChunk`].
+#[derive(Debug)]
 pub enum AndroidOrV2ProfileChunk {
     Android(Box<AndroidProfileChunk>),
     V2(Box<V2ProfileChunk>),
@@ -122,24 +124,152 @@ impl AndroidOrV2ProfileChunk {
         struct MinimalProfile {
             platform: String,
             #[serde(default)]
-            version: sample::Version,
+            version: 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)
+        // 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.
+
+        // Android trace profiles:
+        // ---------------
+        // Version: 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
+
+        // Android ANR profiles:
+        // ---------------
+        // Version: 2
+        // Platform: java (incorrect), android (corrected)
+        // Content field: profile (i.e., standardized stacks/frames/samples format)
+        // Destination type: V2ProfileChunk
+
+        // We also need to handle non-Android profile chunks.
+
+        // Non-Android profiles:
+        // ---------------
+        // Version: 2
+        // Platform: cocoa, javascript, etc.
+        // Content field: profile (i.e., standardized stacks/frames/samples format)
+        // Destination type: V2ProfileChunk
+
+        let is_android_trace_profile =
+            minimal.platform == "android" && minimal.sampled_profile.is_some();
+
+        match minimal.version {
+            Version::V2AndroidTrace => AndroidProfileChunk::parse(data)
+                .map(Box::new)
+                .map(Self::Android),
+            // Account for legacy submissions that don't use the 2.android-trace version.
+            Version::V2 if is_android_trace_profile => 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),
+            Version::V2 => V2ProfileChunk::parse(data).map(Box::new).map(Self::V2),
+            Version::V1 | Version::Unknown => Err(ProfileError::PlatformNotSupported),
+        }
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use std::assert_matches;
+
+    use serde_json::{Value, json};
+
+    use super::*;
+
+    #[test]
+    fn test_parse_correctly_versioned_android_trace_profile_into_android_profile_chunk() {
+        let mut payload: Value =
+            serde_json::from_slice(include_bytes!("../tests/fixtures/android/chunk/valid.json"))
+                .unwrap();
+        payload["version"] = json!("2.android-trace");
+        let data = serde_json::to_vec(&payload).unwrap();
+
+        // 1. Fresh SDK-shaped payload: `sampled_profile` populated, `profile` absent.
+        let sdk_chunk = AndroidOrV2ProfileChunk::parse(&data).unwrap();
+        assert_matches!(sdk_chunk, AndroidOrV2ProfileChunk::Android(_));
+
+        // 2. Relay's own re-serialized shape: `sampled_profile` absent, `profile` populated.
+        let AndroidOrV2ProfileChunk::Android(android_chunk) = sdk_chunk else {
+            unreachable!()
+        };
+        let reserialized = serde_json::to_vec(&android_chunk).unwrap();
+        let value: Value = serde_json::from_slice(&reserialized).unwrap();
+        assert!(value.get("sampled_profile").is_none());
+        assert!(value.get("profile").is_some());
+
+        let round_tripped = AndroidOrV2ProfileChunk::parse(&reserialized).unwrap();
+        assert_matches!(round_tripped, AndroidOrV2ProfileChunk::Android(_));
+    }
+
+    #[test]
+    fn test_parse_legacy_versioned_android_trace_profile_into_android_profile_chunk() {
+        let mut payload: Value =
+            serde_json::from_slice(include_bytes!("../tests/fixtures/android/chunk/valid.json"))
+                .unwrap();
+        payload["version"] = json!("2");
+        let data = serde_json::to_vec(&payload).unwrap();
+
+        let chunk = AndroidOrV2ProfileChunk::parse(&data).unwrap();
+        assert_matches!(chunk, AndroidOrV2ProfileChunk::Android(_));
+    }
+
+    #[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", "javascript", "python"] {
+            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(_));
+        }
+    }
+
+    #[test]
+    fn test_return_error_for_version_1_profile() {
+        for payload in [
+            &include_bytes!("../tests/fixtures/sample/v2/valid.json")[..],
+            &include_bytes!("../tests/fixtures/android/chunk/valid.json")[..],
+            &include_bytes!("../tests/fixtures/android/chunk/valid-rn.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);
+        }
+    }
+
+    #[test]
+    fn test_return_error_for_unknown_version_profile() {
+        for payload in [
+            &include_bytes!("../tests/fixtures/sample/v2/valid.json")[..],
+            &include_bytes!("../tests/fixtures/android/chunk/valid.json")[..],
+            &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);
         }
     }
 }

relay-profiling/src/sample/mod.rs

@@ -6,7 +6,7 @@ use relay_event_schema::protocol::Addr;
 pub mod v1;
 pub mod v2;
 
-/// Possible values for the version field of the Sample Format.
+/// Possible values for profile payload versions.
 #[derive(Debug, Serialize, Deserialize, Copy, Clone, Default, PartialEq, Eq)]
 pub enum Version {
     #[default]
@@ -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",
     [