feat(normalization): Write default transaction source [INGEST-1511]

CHANGELOG.md

@@ -11,6 +11,8 @@
 - Support compressed project configs in redis cache. ([#1345](https://github.com/getsentry/relay/pull/1345))
 - Refactor profile processing into its own crate. ([#1340](https://github.com/getsentry/relay/pull/1340))
 - Treat "unknown" transaction source as low cardinality, except for JS. ([#1352](https://github.com/getsentry/relay/pull/1352))
+- Conditionally write a default transaction source to the transaction payload. ([#1354](https://github.com/getsentry/relay/pull/1354))
+
 ## 22.7.0
 
 **Features**:

relay-general/src/store/mod.rs

@@ -26,7 +26,9 @@ pub use normalize::breakdowns::{
     get_breakdown_measurements, BreakdownConfig, BreakdownsConfig, SpanOperationsConfig,
 };
 pub use normalize::{is_valid_platform, normalize_dist};
-pub use transactions::{get_measurement, get_transaction_op, validate_timestamps};
+pub use transactions::{
+    get_measurement, get_transaction_op, is_high_cardinality_sdk, validate_timestamps,
+};
 
 /// The config for store.
 #[derive(Serialize, Deserialize, Debug, Default)]

relay-general/src/store/transactions.rs

@@ -1,5 +1,7 @@
 use crate::processor::{ProcessValue, ProcessingState, Processor};
-use crate::protocol::{Context, ContextInner, Event, EventType, Span, Timestamp};
+use crate::protocol::{
+    ClientSdkInfo, Context, ContextInner, Event, EventType, Span, Timestamp, TransactionSource,
+};
 use crate::types::{Annotated, Meta, ProcessingAction, ProcessingResult};
 /// Rejects transactions based on required fields.
 pub struct TransactionsProcessor;
@@ -51,6 +53,49 @@ pub fn validate_timestamps(
     }
 }
 
+/// List of SDKs which we assume to produce high cardinality transaction names, such as
+/// "/user/123134/login".
+/// Newer SDK send the [`TransactionSource`] attribute, which we can rely on to determine cardinality,
+/// but for old SDKs, we fall back to this list.
+pub fn is_high_cardinality_sdk(client_sdk: &Annotated<ClientSdkInfo>) -> bool {
+    let sdk_name = client_sdk
+        .value()
+        .and_then(|sdk| sdk.name.value())
+        .map(|s| s.as_str())
+        .unwrap_or_default();
+
+    [
+        "sentry.javascript.angular",
+        "sentry.javascript.browser",
+        "sentry.javascript.ember",
+        "sentry.javascript.gatsby",
+        "sentry.javascript.react",
+        "sentry.javascript.remix",
+        "sentry.javascript.vue",
+    ]
+    .contains(&sdk_name)
+}
+
+/// Set a default transaction source if it is missing, but only if the transaction name was
+/// extracted as a metrics tag.
+/// This behavior makes it possible to identify transactions for which the transaction name was
+/// not extracted as a tag on the corresponding metrics, because
+///     source == null <=> transaction name == null
+/// See `relay_server::metrics_extraction::transactions::get_transaction_name`.
+fn set_default_transaction_source(event: &mut Event) {
+    let source = event
+        .transaction_info
+        .value()
+        .and_then(|info| info.source.value());
+
+    if source.is_none() && !is_high_cardinality_sdk(&event.client_sdk) {
+        let transaction_info = event.transaction_info.get_or_insert_with(Default::default);
+        transaction_info
+            .source
+            .set_value(Some(TransactionSource::Unknown));
+    }
+}
+
 impl Processor for TransactionsProcessor {
     fn process_event(
         &mut self,
@@ -122,6 +167,8 @@ impl Processor for TransactionsProcessor {
             }
         }
 
+        set_default_transaction_source(event);
+
         event.process_child_values(self, state)?;
 
         Ok(())
@@ -183,7 +230,7 @@ mod tests {
     use chrono::Utc;
 
     use crate::processor::process_value;
-    use crate::protocol::{Contexts, SpanId, TraceContext, TraceId};
+    use crate::protocol::{Contexts, SpanId, TraceContext, TraceId, TransactionSource};
     use crate::testutils::{assert_annotated_snapshot, assert_eq_dbg};
     use crate::types::Object;
 
@@ -833,6 +880,90 @@ mod tests {
         "###);
     }
 
+    #[test]
+    fn test_default_transaction_source_unknown() {
+        let mut event = Annotated::<Event>::from_json(
+            r###"
+        {
+          "type": "transaction",
+          "transaction": "/",
+          "timestamp": 946684810.0,
+          "start_timestamp": 946684800.0,
+          "contexts": {
+            "trace": {
+              "trace_id": "4c79f60c11214eb38604f4ae0781bfb2",
+              "span_id": "fa90fdead5f74053",
+              "op": "http.server",
+              "type": "trace"
+            }
+          },
+          "sdk": {
+            "name": "sentry.dart.flutter"
+          },
+          "spans": []
+        }
+        "###,
+        )
+        .unwrap();
+
+        process_value(
+            &mut event,
+            &mut TransactionsProcessor,
+            ProcessingState::root(),
+        )
+        .unwrap();
+
+        let source = event
+            .value()
+            .unwrap()
+            .transaction_info
+            .value()
+            .and_then(|info| info.source.value())
+            .unwrap();
+
+        assert_eq!(source, &TransactionSource::Unknown);
+    }
+
+    #[test]
+    fn test_default_transaction_source_none() {
+        let mut event = Annotated::<Event>::from_json(
+            r###"
+        {
+          "type": "transaction",
+          "transaction": "/",
+          "timestamp": 946684810.0,
+          "start_timestamp": 946684800.0,
+          "contexts": {
+            "trace": {
+              "trace_id": "4c79f60c11214eb38604f4ae0781bfb2",
+              "span_id": "fa90fdead5f74053",
+              "op": "http.server",
+              "type": "trace"
+            }
+          },
+          "sdk": {
+            "name": "sentry.javascript.browser"
+          },
+          "spans": []
+        }
+        "###,
+        )
+        .unwrap();
+
+        dbg!(event.value().map(|e| &e.client_sdk));
+
+        process_value(
+            &mut event,
+            &mut TransactionsProcessor,
+            ProcessingState::root(),
+        )
+        .unwrap();
+
+        let transaction_info = &event.value().unwrap().transaction_info;
+
+        assert!(transaction_info.value().is_none());
+    }
+
     #[test]
     fn test_allows_valid_transaction_event_with_spans() {
         let mut event = new_test_event();

relay-server/src/metrics_extraction/transactions.rs

@@ -215,26 +215,6 @@ fn is_low_cardinality(source: &TransactionSource, treat_unknown_as_low_cardinali
     }
 }
 
-fn is_browser_sdk(event: &Event) -> bool {
-    let sdk_name = event
-        .client_sdk
-        .value()
-        .and_then(|sdk| sdk.name.value())
-        .map(|s| s.as_str())
-        .unwrap_or_default();
-
-    [
-        "sentry.javascript.angular",
-        "sentry.javascript.browser",
-        "sentry.javascript.ember",
-        "sentry.javascript.gatsby",
-        "sentry.javascript.react",
-        "sentry.javascript.remix",
-        "sentry.javascript.vue",
-    ]
-    .contains(&sdk_name)
-}
-
 /// Decide whether we want to keep the transaction name.
 /// High-cardinality sources are excluded to protect our metrics infrastructure.
 /// Note that this will produce a discrepancy between metrics and raw transaction data.
@@ -254,7 +234,7 @@ fn get_transaction_name(
     let treat_unknown_as_low_cardinality = matches!(
         accept_transaction_names,
         AcceptTransactionNames::ClientBased
-    ) && !is_browser_sdk(event);
+    ) && !store::is_high_cardinality_sdk(&event.client_sdk);
 
     let source = event.get_transaction_source();
     let use_original_name = is_low_cardinality(source, treat_unknown_as_low_cardinality);

tests/integration/test_metrics.py

@@ -747,7 +747,7 @@ def test_graceful_shutdown(mini_sentry, relay):
     with pytest.raises(requests.ConnectionError):
         relay.send_metrics(project_id, metrics_payload, timestamp)
 
-    envelope = mini_sentry.captured_events.get(timeout=3)
+    envelope = mini_sentry.captured_events.get(timeout=5)
     assert len(envelope.items) == 1
     metrics_item = envelope.items[0]
     assert metrics_item.type == "metric_buckets"