fix(spans): Remove mobile outliers

CHANGELOG.md

@@ -6,14 +6,19 @@
 
 - Disable resource link span ingestion. ([#2647](https://github.com/getsentry/relay/pull/2647))
 
+**Features**:
+
+- Filter outliers (>180s) for mobile measurements. ([#2649](https://github.com/getsentry/relay/pull/2649))
+- Allow access to more context fields in dynamic sampling and metric extraction. ([#2607](https://github.com/getsentry/relay/pull/2607), [#2640](https://github.com/getsentry/relay/pull/2640))
+
 ## 23.10.1
 
 **Features**:
 
 - Update Docker Debian image from 10 to 12. ([#2622](https://github.com/getsentry/relay/pull/2622))
 - Remove event spans starting or ending before January 1, 1970 UTC. ([#2627](https://github.com/getsentry/relay/pull/2627))
 - Remove event breadcrumbs dating before January 1, 1970 UTC. ([#2635](https://github.com/getsentry/relay/pull/2635))
-- Allow access to more context fields in dynamic sampling and metric extraction. ([#2607](https://github.com/getsentry/relay/pull/2607), [#2640](https://github.com/getsentry/relay/pull/2640))
+
 
 **Internal**:
 

relay-dynamic-config/src/defaults.rs

@@ -1,6 +1,8 @@
 use relay_base_schema::data_category::DataCategory;
 use relay_common::glob2::LazyGlob;
+use relay_event_normalization::utils::MAX_DURATION_MOBILE_MS;
 use relay_protocol::RuleCondition;
+use serde_json::Number;
 
 use crate::feature::Feature;
 use crate::metrics::{MetricExtractionConfig, MetricSpec, TagMapping, TagSpec};
@@ -64,12 +66,19 @@ pub fn add_span_metrics(project_config: &mut ProjectConfig) {
         conditions
     };
 
+    // For mobile spans, only extract duration metrics when they are below a threshold.
+    let duration_condition = RuleCondition::negate(RuleCondition::glob("span.op", MOBILE_OPS))
+        | RuleCondition::lte(
+            "span.exclusive_time",
+            Number::from_f64(MAX_DURATION_MOBILE_MS).unwrap_or(0.into()),
+        );
+
     config.metrics.extend([
         MetricSpec {
             category: DataCategory::Span,
             mri: "d:spans/exclusive_time@millisecond".into(),
             field: Some("span.exclusive_time".into()),
-            condition: Some(span_op_conditions.clone()),
+            condition: Some(span_op_conditions.clone() & duration_condition.clone()),
             tags: vec![TagSpec {
                 key: "transaction".into(),
                 field: Some("span.sentry_tags.transaction".into()),
@@ -81,7 +90,7 @@ pub fn add_span_metrics(project_config: &mut ProjectConfig) {
             category: DataCategory::Span,
             mri: "d:spans/exclusive_time_light@millisecond".into(),
             field: Some("span.exclusive_time".into()),
-            condition: Some(span_op_conditions.clone()),
+            condition: Some(span_op_conditions.clone() & duration_condition.clone()),
             tags: Default::default(),
         },
         MetricSpec {
@@ -154,6 +163,7 @@ pub fn add_span_metrics(project_config: &mut ProjectConfig) {
             }))
             .collect(),
         },
+        // Mobile-specific tags:
         TagMapping {
             metrics: vec![LazyGlob::new("d:spans/exclusive_time*@millisecond".into())],
             tags: ["release", "device.class"] // TODO: sentry PR for static strings

relay-event-normalization/src/normalize/mod.rs

@@ -28,6 +28,7 @@ use smallvec::SmallVec;
 
 use crate::span::tag_extraction::{self, extract_span_tags};
 use crate::timestamp::TimestampProcessor;
+use crate::utils::MAX_DURATION_MOBILE_MS;
 use crate::{
     schema, transactions, trimming, BreakdownsConfig, ClockDriftProcessor, GeoIpLookup,
     RawUserAgentInfo, SpanDescriptionRule, StoreConfig, TransactionNameConfig,
@@ -439,6 +440,29 @@ fn normalize_app_start_measurements(measurements: &mut Measurements) {
     }
 }
 
+/// New SDKs do not send measurements when they exceed 180 seconds.
+///
+/// Drop those outlier measurements for older SDKs.
+fn filter_mobile_outliers(measurements: &mut Measurements) {
+    for key in [
+        "app_start_cold",
+        "app_start_warm",
+        "time_to_initial_display",
+        "time_to_full_display",
+    ] {
+        if let Some(value) = measurements.get_value(key) {
+            if value > MAX_DURATION_MOBILE_MS {
+                measurements.remove(key);
+            }
+        }
+    }
+}
+
+fn normalize_mobile_measurements(measurements: &mut Measurements) {
+    normalize_app_start_measurements(measurements);
+    filter_mobile_outliers(measurements);
+}
+
 fn normalize_units(measurements: &mut Measurements) {
     for (name, measurement) in measurements.iter_mut() {
         let measurement = match measurement.value_mut() {
@@ -464,7 +488,7 @@ fn normalize_measurements(
         // Only transaction events may have a measurements interface
         event.measurements = Annotated::empty();
     } else if let Annotated(Some(ref mut measurements), ref mut meta) = event.measurements {
-        normalize_app_start_measurements(measurements);
+        normalize_mobile_measurements(measurements);
         normalize_units(measurements);
         if let Some(measurements_config) = measurements_config {
             remove_invalid_measurements(measurements, meta, measurements_config, max_mri_len);
@@ -3710,6 +3734,18 @@ mod tests {
         "###);
     }
 
+    #[test]
+    fn test_filter_mobile_outliers() {
+        let mut measurements =
+            Annotated::<Measurements>::from_json(r#"{"app_start_warm": {"value": 180001}}"#)
+                .unwrap()
+                .into_value()
+                .unwrap();
+        assert_eq!(measurements.len(), 1);
+        filter_mobile_outliers(&mut measurements);
+        assert_eq!(measurements.len(), 0);
+    }
+
     #[test]
     fn test_reject_stale_transaction() {
         let json = r#"{

relay-event-normalization/src/normalize/utils.rs

@@ -13,6 +13,12 @@ pub const MOBILE_SDKS: [&str; 4] = [
     "sentry.javascript.react-native",
 ];
 
+/// Maximum length of a mobile span or measurement in milliseconds.
+///
+/// Spans like `ui.load` with an `exclusive_time` that exceeds this number will be removed,
+/// as well as mobile measurements (on transactions) such as `app.start.cold`, etc.
+pub const MAX_DURATION_MOBILE_MS: f64 = 180_000.0;
+
 /// Extract the HTTP status code from the span data.
 pub fn http_status_code_from_span(span: &Span) -> Option<String> {
     // For SDKs which put the HTTP status code into the span data.

relay-server/src/metrics_extraction/event.rs

@@ -61,8 +61,10 @@ pub fn extract_metrics(event: &Event, config: &MetricExtractionConfig) -> Vec<Bu
 
 #[cfg(test)]
 mod tests {
+    use chrono::{DateTime, Utc};
     use relay_dynamic_config::{Feature, FeatureSet, ProjectConfig};
     use relay_event_normalization::LightNormalizationConfig;
+    use relay_event_schema::protocol::Timestamp;
     use relay_protocol::Annotated;
     use std::collections::BTreeSet;
 
@@ -1082,4 +1084,72 @@ mod tests {
         let metrics = extract_metrics(event.value().unwrap(), &config);
         insta::assert_debug_snapshot!((&event.value().unwrap().spans, metrics));
     }
+
+    /// Helper function for span metric extraction tests.
+    fn extract_span_metrics(span_op: &str, duration_millis: f64) -> Vec<Bucket> {
+        let mut span = Span::default();
+        span.timestamp
+            .set_value(Some(Timestamp::from(DateTime::<Utc>::MAX_UTC))); // whatever
+        span.op.set_value(Some(span_op.into()));
+        span.exclusive_time.set_value(Some(duration_millis));
+
+        let mut config = ProjectConfig::default();
+        config.features.0.insert(Feature::SpanMetricsExtraction);
+        config.sanitize(); // apply defaults for span extraction
+
+        let extraction_config = config.metric_extraction.ok().unwrap();
+        generic::extract_metrics(&span, &extraction_config)
+    }
+
+    #[test]
+    fn test_app_start_cold_inlier() {
+        assert_eq!(2, extract_span_metrics("app.start.cold", 180000.0).len());
+    }
+
+    #[test]
+    fn test_app_start_cold_outlier() {
+        assert_eq!(0, extract_span_metrics("app.start.cold", 181000.0).len());
+    }
+
+    #[test]
+    fn test_app_start_warm_inlier() {
+        assert_eq!(2, extract_span_metrics("app.start.warm", 180000.0).len());
+    }
+
+    #[test]
+    fn test_app_start_warm_outlier() {
+        assert_eq!(0, extract_span_metrics("app.start.warm", 181000.0).len());
+    }
+
+    #[test]
+    fn test_ui_load_initial_display_inlier() {
+        assert_eq!(
+            2,
+            extract_span_metrics("ui.load.initial_display", 180000.0).len()
+        );
+    }
+
+    #[test]
+    fn test_ui_load_initial_display_outlier() {
+        assert_eq!(
+            0,
+            extract_span_metrics("ui.load.initial_display", 181000.0).len()
+        );
+    }
+
+    #[test]
+    fn test_ui_load_full_display_inlier() {
+        assert_eq!(
+            2,
+            extract_span_metrics("ui.load.full_display", 180000.0).len()
+        );
+    }
+
+    #[test]
+    fn test_ui_load_full_display_outlier() {
+        assert_eq!(
+            0,
+            extract_span_metrics("ui.load.full_display", 181000.0).len()
+        );
+    }
 }