feat(protocol): Track transaction name changes (#1466)

Introduces `propagations` and `changes` that provides more insight into how a
transaction name was changed during the lifetime of a transaction span. Since
the transaction name is propagated to downstream SDKs as part of the Dynamic
Sampling Context (DSC), this is a relevant quality metric for sampling traces by
transaction name.

All traces where propagations occur before the final transaction name is set
may not be sampled consistently if there are rules matching on the transaction
name.

Author: jan-auer

CHANGELOG.md

@@ -26,6 +26,7 @@
 - Add InvalidReplayEvent outcome. ([#1455](https://github.com/getsentry/relay/pull/1455))
 - Add replay and replay-recording rate limiter. ([#1456](https://github.com/getsentry/relay/pull/1456))
 - Support profiles tagged for many transactions. ([#1444](https://github.com/getsentry/relay/pull/1444)), ([#1463](https://github.com/getsentry/relay/pull/1463)), ([#1464](https://github.com/getsentry/relay/pull/1464))
+- Track metrics for changes to the transaction name and DSC propagations. ([#1466](https://github.com/getsentry/relay/pull/1466))
 
 **Features**:
 

relay-general/src/protocol/event.rs

@@ -560,6 +560,26 @@ impl Event {
             .map(|m| m.contains_key(module_name))
             .unwrap_or(false)
     }
+
+    pub fn sdk_name(&self) -> &str {
+        if let Some(client_sdk) = self.client_sdk.value() {
+            if let Some(name) = client_sdk.name.as_str() {
+                return name;
+            }
+        }
+
+        "unknown"
+    }
+
+    pub fn sdk_version(&self) -> &str {
+        if let Some(client_sdk) = self.client_sdk.value() {
+            if let Some(version) = client_sdk.version.as_str() {
+                return version;
+            }
+        }
+
+        "unknown"
+    }
 }
 
 #[cfg(test)]

relay-general/src/protocol/transaction.rs

@@ -2,6 +2,7 @@ use std::fmt;
 use std::str::FromStr;
 
 use crate::processor::ProcessValue;
+use crate::protocol::Timestamp;
 use crate::types::{Annotated, Empty, ErrorKind, FromValue, IntoValue, SkipSerialization, Value};
 
 /// Describes how the name of the transaction was determined.
@@ -28,6 +29,21 @@ pub enum TransactionSource {
     Other(String),
 }
 
+impl TransactionSource {
+    pub fn as_str(&self) -> &str {
+        match self {
+            Self::Custom => "custom",
+            Self::Url => "url",
+            Self::Route => "route",
+            Self::View => "view",
+            Self::Component => "component",
+            Self::Task => "task",
+            Self::Unknown => "unknown",
+            Self::Other(ref s) => s,
+        }
+    }
+}
+
 impl FromStr for TransactionSource {
     type Err = std::convert::Infallible;
 
@@ -47,16 +63,7 @@ impl FromStr for TransactionSource {
 
 impl fmt::Display for TransactionSource {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        match self {
-            Self::Custom => write!(f, "custom"),
-            Self::Url => write!(f, "url"),
-            Self::Route => write!(f, "route"),
-            Self::View => write!(f, "view"),
-            Self::Component => write!(f, "component"),
-            Self::Task => write!(f, "task"),
-            Self::Unknown => write!(f, "unknown"),
-            Self::Other(s) => write!(f, "{}", s),
-        }
+        write!(f, "{}", self.as_str())
     }
 }
 
@@ -108,6 +115,21 @@ impl IntoValue for TransactionSource {
 
 impl ProcessValue for TransactionSource {}
 
+#[derive(Clone, Debug, PartialEq, Empty, FromValue, IntoValue, ProcessValue)]
+#[cfg_attr(feature = "jsonschema", derive(JsonSchema))]
+pub struct TransactionNameChange {
+    /// Describes how the previous transaction name was determined.
+    pub source: Annotated<TransactionSource>,
+
+    /// The number of propagations from the start of the transaction to this change.
+    pub propagations: Annotated<u64>,
+
+    /// Timestamp when the transaction name was changed.
+    ///
+    /// This adheres to the event timestamp specification.
+    pub timestamp: Annotated<Timestamp>,
+}
+
 /// Additional information about the name of the transaction.
 #[derive(Clone, Debug, Default, PartialEq, Empty, FromValue, IntoValue, ProcessValue)]
 #[cfg_attr(feature = "jsonschema", derive(JsonSchema))]
@@ -123,10 +145,20 @@ pub struct TransactionInfo {
     /// This value will only be set if the transaction name was modified during event processing.
     #[metastructure(max_chars = "culprit", trim_whitespace = "true")]
     pub original: Annotated<String>,
+
+    /// A list of changes prior to the final transaction name.
+    ///
+    /// This list must be empty if the transaction name is set at the beginning of the transaction
+    /// and never changed. There is no placeholder entry for the initial transaction name.
+    pub changes: Annotated<Vec<Annotated<TransactionNameChange>>>,
+
+    /// The total number of propagations during the transaction.
+    pub propagations: Annotated<u64>,
 }
 
 #[cfg(test)]
 mod tests {
+    use chrono::{TimeZone, Utc};
     use similar_asserts::assert_eq;
 
     use super::*;
@@ -143,13 +175,27 @@ mod tests {
     #[test]
     fn test_transaction_info_roundtrip() {
         let json = r#"{
-  "source": "url",
-  "original": "/auth/login/john123/"
+  "source": "route",
+  "original": "/auth/login/john123/",
+  "changes": [
+    {
+      "source": "url",
+      "propagations": 1,
+      "timestamp": 946684800.0
+    }
+  ],
+  "propagations": 2
 }"#;
 
         let info = Annotated::new(TransactionInfo {
-            source: Annotated::new(TransactionSource::Url),
+            source: Annotated::new(TransactionSource::Route),
             original: Annotated::new("/auth/login/john123/".to_owned()),
+            changes: Annotated::new(vec![Annotated::new(TransactionNameChange {
+                source: Annotated::new(TransactionSource::Url),
+                propagations: Annotated::new(1),
+                timestamp: Annotated::new(Utc.ymd(2000, 1, 1).and_hms(0, 0, 0).into()),
+            })]),
+            propagations: Annotated::new(2),
         });
 
         assert_eq!(info, Annotated::from_json(json).unwrap());

relay-general/tests/snapshots/test_fixtures__event_schema.snap

@@ -1,7 +1,6 @@
 ---
 source: relay-general/tests/test_fixtures.rs
-assertion_line: 110
-expression: event_json_schema()
+expression: "relay_general::protocol::event_json_schema()"
 ---
 {
   "$schema": "http://json-schema.org/draft-07/schema#",
@@ -2983,6 +2982,24 @@ expression: event_json_schema()
         {
           "type": "object",
           "properties": {
+            "changes": {
+              "description": " A list of changes prior to the final transaction name.\n\n This list must be empty if the transaction name is set at the beginning of the transaction\n and never changed. There is no placeholder entry for the initial transaction name.",
+              "default": null,
+              "type": [
+                "array",
+                "null"
+              ],
+              "items": {
+                "anyOf": [
+                  {
+                    "$ref": "#/definitions/TransactionNameChange"
+                  },
+                  {
+                    "type": "null"
+                  }
+                ]
+              }
+            },
             "original": {
               "description": " The unmodified transaction name as obtained by the source.\n\n This value will only be set if the transaction name was modified during event processing.",
               "default": null,
@@ -2991,6 +3008,16 @@ expression: event_json_schema()
                 "null"
               ]
             },
+            "propagations": {
+              "description": " The total number of propagations during the transaction.",
+              "default": null,
+              "type": [
+                "integer",
+                "null"
+              ],
+              "format": "uint64",
+              "minimum": 0.0
+            },
             "source": {
               "description": " Describes how the name of the transaction was determined.\n\n This will be used by the server to decide whether or not to scrub identifiers from the\n transaction name, or replace the entire name with a placeholder.",
               "default": null,
@@ -3008,6 +3035,50 @@ expression: event_json_schema()
         }
       ]
     },
+    "TransactionNameChange": {
+      "anyOf": [
+        {
+          "type": "object",
+          "properties": {
+            "propagations": {
+              "description": " The number of propagations from the start of the transaction to this change.",
+              "default": null,
+              "type": [
+                "integer",
+                "null"
+              ],
+              "format": "uint64",
+              "minimum": 0.0
+            },
+            "source": {
+              "description": " Describes how the previous transaction name was determined.",
+              "default": null,
+              "anyOf": [
+                {
+                  "$ref": "#/definitions/TransactionSource"
+                },
+                {
+                  "type": "null"
+                }
+              ]
+            },
+            "timestamp": {
+              "description": " Timestamp when the transaction name was changed.\n\n This adheres to the event timestamp specification.",
+              "default": null,
+              "anyOf": [
+                {
+                  "$ref": "#/definitions/Timestamp"
+                },
+                {
+                  "type": "null"
+                }
+              ]
+            }
+          },
+          "additionalProperties": false
+        }
+      ]
+    },
     "TransactionSource": {
       "description": "Describes how the name of the transaction was determined.",
       "type": "string",

relay-server/src/actors/processor.rs

@@ -32,7 +32,7 @@ use relay_log::LogError;
 use relay_metrics::{Bucket, Metric};
 use relay_quotas::{DataCategory, RateLimits, ReasonCode};
 use relay_redis::RedisPool;
-use relay_sampling::RuleId;
+use relay_sampling::{DynamicSamplingContext, RuleId};
 use relay_statsd::metric;
 
 use crate::actors::envelopes::{EnvelopeManager, SendEnvelope, SendEnvelopeError, SubmitEnvelope};
@@ -45,7 +45,7 @@ use crate::envelope::{AttachmentType, ContentType, Envelope, Item, ItemType};
 use crate::metrics_extraction::sessions::{extract_session_metrics, SessionMetricsConfig};
 use crate::metrics_extraction::transactions::extract_transaction_metrics;
 use crate::service::{ServerError, REGISTRY};
-use crate::statsd::{RelayCounters, RelayTimers};
+use crate::statsd::{RelayCounters, RelayHistograms, RelayTimers};
 use crate::utils::{
     self, ChunkedFormDataAggregator, EnvelopeContext, ErrorBoundary, FormDataIter, SamplingResult,
 };
@@ -344,6 +344,94 @@ fn outcome_from_profile_error(err: relay_profiling::ProfileError) -> Outcome {
     Outcome::Invalid(discard_reason)
 }
 
+fn track_sampling_metrics(
+    project_state: &ProjectState,
+    context: &DynamicSamplingContext,
+    event: &Event,
+) {
+    // We only collect this metric for the root transaction event, so ignore secondary projects.
+    if !project_state.is_matching_key(context.public_key) {
+        return;
+    }
+
+    let transaction_info = match event.transaction_info.value() {
+        Some(info) => info,
+        None => return,
+    };
+
+    let changes = match transaction_info.changes.value() {
+        Some(value) => value.as_slice(),
+        None => return,
+    };
+
+    let last_change = changes.last().and_then(|a| a.value());
+    let source = event.get_transaction_source().as_str();
+    let platform = event.platform.as_str().unwrap_or("other");
+    let sdk_name = event.sdk_name();
+    let sdk_version = event.sdk_version();
+
+    metric!(
+        histogram(RelayHistograms::DynamicSamplingChanges) = changes.len() as u64,
+        source = source,
+        platform = platform,
+        sdk_name = sdk_name,
+        sdk_version = sdk_version,
+    );
+
+    if let Some(&total) = transaction_info.propagations.value() {
+        // If there was no change, there were no propagations that happened with a wrong name.
+        let change = last_change
+            .and_then(|c| c.propagations.value())
+            .map_or(0, |v| *v);
+
+        metric!(
+            histogram(RelayHistograms::DynamicSamplingPropagationCount) = change,
+            source = source,
+            platform = platform,
+            sdk_name = sdk_name,
+            sdk_version = sdk_version,
+        );
+
+        let percentage = ((change as f64) / (total as f64)).min(1.0);
+        metric!(
+            histogram(RelayHistograms::DynamicSamplingPropagationPercentage) = percentage,
+            source = source,
+            platform = platform,
+            sdk_name = sdk_name,
+            sdk_version = sdk_version,
+        );
+    }
+
+    if let (Some(&start), Some(&change), Some(&end)) = (
+        event.start_timestamp.value(),
+        last_change.and_then(|c| c.timestamp.value()),
+        event.timestamp.value(),
+    ) {
+        let delay_ms = (change - start).num_milliseconds();
+        if delay_ms >= 0 {
+            metric!(
+                histogram(RelayHistograms::DynamicSamplingChangeDuration) = delay_ms as u64,
+                source = source,
+                platform = platform,
+                sdk_name = sdk_name,
+                sdk_version = sdk_version,
+            );
+        }
+
+        let duration_ms = (end - start).num_milliseconds() as f64;
+        if delay_ms >= 0 && duration_ms >= 0.0 {
+            let percentage = ((delay_ms as f64) / duration_ms).min(1.0);
+            metric!(
+                histogram(RelayHistograms::DynamicSamplingChangePercentage) = percentage,
+                source = source,
+                platform = platform,
+                sdk_name = sdk_name,
+                sdk_version = sdk_version,
+            );
+        }
+    }
+}
+
 /// Synchronous service for processing envelopes.
 pub struct EnvelopeProcessor {
     config: Arc<Config>,
@@ -1625,6 +1713,10 @@ impl EnvelopeProcessor {
                 }
             );
             state.transaction_metrics_extracted = true;
+
+            if let Some(context) = state.envelope.sampling_context() {
+                track_sampling_metrics(&state.project_state, context, event);
+            }
         }
         Ok(())
     }