feat(profiling): Log SDK name and version in errors (#2673)

Follow-up to my previous PR to make logs even more useful by adding the
previous error containing the value with the issue and the SDK name and
version.

#skip-changelog

---------

Co-authored-by: Iker Barriocanal <32816711+iker-barriocanal@users.noreply.github.com>

relay-profiling/Cargo.toml

@@ -13,6 +13,7 @@ publish = false
 android_trace_log = { version = "0.3.0", features = ["serde"] }
 chrono = { workspace = true }
 data-encoding = "2.3.3"
+relay-base-schema = { path = "../relay-base-schema" }
 relay-event-schema = { path = "../relay-event-schema" }
 relay-log = { path = "../relay-log" }
 relay-protocol = { path = "../relay-protocol" }

relay-profiling/src/lib.rs

@@ -93,10 +93,10 @@
 //! }
 //! ```
 
-use std::collections::BTreeMap;
 use std::error::Error;
 use std::time::Duration;
 
+use relay_base_schema::project::ProjectId;
 use relay_event_schema::protocol::{Event, EventId};
 use serde::Deserialize;
 use serde_json::Deserializer;
@@ -127,17 +127,23 @@ struct MinimalProfile {
  version: sample::Version,
 }
 
-fn minimal_profile_from_json(payload: &[u8]) -> Result<MinimalProfile, ProfileError> {
+fn minimal_profile_from_json(
+ payload: &[u8],
+) -> Result<MinimalProfile, serde_path_to_error::Error<serde_json::Error>> {
  let d = &mut Deserializer::from_slice(payload);
- serde_path_to_error::deserialize(d).map_err(ProfileError::InvalidJson)
+ serde_path_to_error::deserialize(d)
 }
 
-pub fn parse_metadata(payload: &[u8]) -> Result<(), ProfileError> {
+pub fn parse_metadata(payload: &[u8], project_id: ProjectId) -> Result<(), ProfileError> {
  let profile = match minimal_profile_from_json(payload) {
  Ok(profile) => profile,
  Err(err) => {
- relay_log::warn!(error = &err as &dyn Error, "invalid profile (minimal)");
- return Err(err);
+ relay_log::warn!(
+ error = &err as &dyn Error,
+ from = "minimal",
+ project_id = project_id.value(),
+ );
+ return Err(ProfileError::InvalidJson(err));
  }
  };
  match profile.version {
@@ -148,9 +154,10 @@ pub fn parse_metadata(payload: &[u8]) -> Result<(), ProfileError> {
  Err(err) => {
  relay_log::warn!(
  error = &err as &dyn Error,
- "invalid profile (platform: {}, version: {:?})",
- profile.platform,
- profile.version,
+ from = "metadata",
+ platform = profile.platform,
+ project_id = project_id.value(),
+ "invalid profile",
  );
  return Err(ProfileError::InvalidJson(err));
  }
@@ -164,7 +171,10 @@ pub fn parse_metadata(payload: &[u8]) -> Result<(), ProfileError> {
  Err(err) => {
  relay_log::warn!(
  error = &err as &dyn Error,
- "invalid profile (platform: android)",
+ from = "metadata",
+ platform = "android",
+ project_id = project_id.value(),
+ "invalid profile",
  );
  return Err(ProfileError::InvalidJson(err));
  }
@@ -176,21 +186,24 @@ pub fn parse_metadata(payload: &[u8]) -> Result<(), ProfileError> {
  Ok(())
 }
 
-pub fn expand_profile(
- payload: &[u8],
- event: Option<&Event>,
-) -> Result<(EventId, Vec<u8>), ProfileError> {
+pub fn expand_profile(payload: &[u8], event: &Event) -> Result<(EventId, Vec<u8>), ProfileError> {
  let profile = match minimal_profile_from_json(payload) {
  Ok(profile) => profile,
- Err(err) => return Err(err),
- };
- let (transaction_metadata, transaction_tags) = match event {
- Some(event) => (
- extract_transaction_metadata(event),
- extract_transaction_tags(event),
- ),
- _ => (BTreeMap::new(), BTreeMap::new()),
+ Err(err) => {
+ relay_log::warn!(
+ error = &err as &dyn Error,
+ from = "minimal",
+ platform = event.platform.as_str(),
+ project_id = event.project.value().unwrap_or(&0),
+ sdk_name = event.sdk_name(),
+ sdk_version = event.sdk_version(),
+ "invalid profile",
+ );
+ return Err(ProfileError::InvalidJson(err));
+ }
  };
+ let transaction_metadata = extract_transaction_metadata(event);
+ let transaction_tags = extract_transaction_tags(event);
  let processed_payload = match profile.version {
  sample::Version::V1 => {
  sample::parse_sample_profile(payload, transaction_metadata, transaction_tags)
@@ -208,13 +221,25 @@ pub fn expand_profile(
  ProfileError::InvalidJson(err) => {
  relay_log::warn!(
  error = &err as &dyn Error,
- "invalid profile (platform: {})",
- profile.platform,
+ from = "parsing",
+ platform = profile.platform,
+ project_id = event.project.value().unwrap_or(&0),
+ sdk_name = event.sdk_name(),
+ sdk_version = event.sdk_version(),
+ "invalid profile",
  );
  Err(ProfileError::InvalidJson(err))
  }
  _ => {
- relay_log::debug!(error = &err as &dyn Error, "invalid profile");
+ relay_log::warn!(
+ error = &err as &dyn Error,
+ from = "parsing",
+ platform = profile.platform,
+ project_id = event.project.value().unwrap_or(&0),
+ sdk_name = event.sdk_name(),
+ sdk_version = event.sdk_version(),
+ "invalid profile",
+ );
  Err(err)
  }
  },
@@ -244,12 +269,12 @@ mod tests {
  #[test]
  fn test_expand_profile_with_version() {
  let payload = include_bytes!("../tests/fixtures/profiles/sample/roundtrip.json");
- assert!(expand_profile(payload, Some(&Event::default())).is_ok());
+ assert!(expand_profile(payload, &Event::default()).is_ok());
  }
 
  #[test]
  fn test_expand_profile_without_version() {
  let payload = include_bytes!("../tests/fixtures/profiles/android/roundtrip.json");
- assert!(expand_profile(payload, Some(&Event::default())).is_ok());
+ assert!(expand_profile(payload, &Event::default()).is_ok());
  }
 }

relay-profiling/src/measurements.rs

@@ -39,12 +39,17 @@ mod tests {
  #[test]
  fn test_value_as_float() {
  let measurement_json = r#"{"elapsed_since_start_ns":1234567890,"value":1234.56789}"#;
- assert!(serde_json::from_str::<MeasurementValue>(measurement_json).is_ok());
+ let measurement = serde_json::from_str::<MeasurementValue>(measurement_json);
+ assert!(measurement.is_ok());
+ assert_eq!(measurement.unwrap().value, 1234.56789);
  }
 
  #[test]
  fn test_value_as_string() {
  let measurement_json = r#"{"elapsed_since_start_ns":1234567890,"value":"1234.56789"}"#;
  assert!(serde_json::from_str::<MeasurementValue>(measurement_json).is_ok());
+ let measurement = serde_json::from_str::<MeasurementValue>(measurement_json);
+ assert!(measurement.is_ok());
+ assert_eq!(measurement.unwrap().value, 1234.56789);
  }
 }

relay-profiling/src/utils.rs

@@ -2,6 +2,7 @@ use std::fmt::Display;
 use std::str::FromStr;
 
 use serde::{de, Deserialize};
+use serde_json::{Map, Value};
 
 pub fn is_zero(n: &u64) -> bool {
  *n == 0
@@ -15,13 +16,28 @@ where
 {
  #[derive(Deserialize)]
  #[serde(untagged)]
- enum StringOrInt<T> {
+ enum StringOrNumber<T> {
  String(String),
  Number(T),
+ Bool(bool),
+ Array(Vec<Value>),
+ Object(Map<String, Value>),
  }
 
- match StringOrInt::<T>::deserialize(deserializer)? {
- StringOrInt::String(s) => s.parse::<T>().map_err(serde::de::Error::custom),
- StringOrInt::Number(i) => Ok(i),
+ match StringOrNumber::<T>::deserialize(deserializer)? {
+ StringOrNumber::String(s) => s.parse::<T>().map_err(serde::de::Error::custom),
+ StringOrNumber::Number(n) => Ok(n),
+ StringOrNumber::Bool(v) => Err(serde::de::Error::custom(format!(
+ "unsupported value: {:?}",
+ v
+ ))),
+ StringOrNumber::Array(v) => Err(serde::de::Error::custom(format!(
+ "unsupported value: {:?}",
+ v
+ ))),
+ StringOrNumber::Object(v) => Err(serde::de::Error::custom(format!(
+ "unsupported value: {:?}",
+ v
+ ))),
  }
 }

relay-server/src/actors/processor.rs

@@ -1097,7 +1097,7 @@ impl EnvelopeProcessorService {
  ItemType::Profile if transaction_count == 0 => ItemAction::DropSilently,
  // First profile found in the envelope, we'll keep it if metadata are valid.
  ItemType::Profile if !found_profile => {
- match relay_profiling::parse_metadata(&item.payload()) {
+ match relay_profiling::parse_metadata(&item.payload(), state.project_id) {
  Ok(_) => {
  found_profile = true;
  ItemAction::Keep
@@ -1152,7 +1152,11 @@ impl EnvelopeProcessorService {
  if !profiling_enabled {
  return ItemAction::DropSilently;
  }
- match relay_profiling::expand_profile(&item.payload(), state.event.value()) {
+ // If we don't have an event at this stage, we need to drop the profile.
+ let Some(event) = state.event.value() else {
+ return ItemAction::DropSilently;
+ };
+ match relay_profiling::expand_profile(&item.payload(), event) {
  Ok((profile_id, payload)) => {
  if payload.len() <= self.inner.config.max_profile_size() {
  found_profile_id = Some(profile_id);