fix(spansv2): Correctly map the span name from v1 to v2 (#5253)

Dav1dde · web-flow · commit f07959638f0c · 2025-10-16T06:07:51.000Z
The span name was only mapped but not inferred correctly (like we do for
standalone spans).

Also the span name was left in the attributes, when it should've been
removed, as it is redundant.

The segment processor in Sentry, then maps the top level `name` field
again correctly to a `sentry.name` attribute.
diff --git a/relay-event-schema/src/protocol/attributes.rs b/relay-event-schema/src/protocol/attributes.rs
@@ -30,6 +30,11 @@ impl Attribute {
             other: Object::new(),
         }
     }
+
+    /// Returns the string value of this attribute.
+    pub fn into_string(self) -> Option<String> {
+        self.value.value.into_value()?.into_string()
+    }
 }
 
 impl fmt::Debug for Attribute {
@@ -42,8 +47,12 @@ impl fmt::Debug for Attribute {
     }
 }
 
-impl From<AttributeValue> for Attribute {
-    fn from(value: AttributeValue) -> Self {
+impl<T> From<T> for Attribute
+where
+    AttributeValue: From<T>,
+{
+    fn from(value: T) -> Self {
+        let value = value.into();
         Self {
             value,
             other: Default::default(),
@@ -267,6 +276,15 @@ impl Attributes {
         self.0.contains_key(key)
     }
 
+    /// Removes an attribute from this collection.
+    pub fn remove<Q>(&mut self, key: &Q) -> Option<Annotated<Attribute>>
+    where
+        String: Borrow<Q>,
+        Q: Ord + ?Sized,
+    {
+        self.0.remove(key)
+    }
+
     /// Iterates over this collection's attribute keys and values.
     pub fn iter(&self) -> std::collections::btree_map::Iter<'_, String, Annotated<Attribute>> {
         self.0.iter()
@@ -296,6 +314,12 @@ impl FromIterator<(String, Annotated<Attribute>)> for Attributes {
     }
 }
 
+impl<const N: usize> From<[(String, Annotated<Attribute>); N]> for Attributes {
+    fn from(value: [(String, Annotated<Attribute>); N]) -> Self {
+        value.into_iter().collect()
+    }
+}
+
 impl ProcessValue for Attributes {
     #[inline]
     fn value_type(&self) -> EnumSet<ValueType> {
diff --git a/relay-protocol/src/annotated.rs b/relay-protocol/src/annotated.rs
@@ -238,6 +238,17 @@ impl<T> Annotated<T> {
     }
 }
 
+impl<T> Annotated<Option<T>> {
+    /// Transposes an `Annotated` of [`Option`] into a [`Option`] of `Annotated`.
+    pub fn transpose(self) -> Option<Annotated<T>> {
+        match self {
+            Annotated(Some(Some(value)), meta) => Some(Annotated(Some(value), meta)),
+            Annotated(Some(None), _) => None,
+            Annotated(None, meta) => Some(Annotated(None, meta)),
+        }
+    }
+}
+
 impl<T> Annotated<T>
 where
     T: AsRef<str>,
diff --git a/relay-protocol/src/value.rs b/relay-protocol/src/value.rs
@@ -70,6 +70,14 @@ impl Value {
         }
     }
 
+    /// Returns the string if this value is a string, otherwise `None`.
+    pub fn into_string(self) -> Option<String> {
+        match self {
+            Value::String(string) => Some(string),
+            _ => None,
+        }
+    }
+
     /// Returns a f64 if the value can be converted to it, otherwise `None`.
     pub fn as_f64(&self) -> Option<f64> {
         match self {
diff --git a/relay-spans/src/name.rs b/relay-spans/src/name.rs
@@ -1,5 +1,5 @@
 use relay_conventions::name_for_op_and_attributes;
-use relay_event_schema::protocol::Span;
+use relay_event_schema::protocol::{Attributes, Span};
 use relay_protocol::{Getter, GetterIter, Val};
 
 /// Constructs a name attribute for a span, following the rules defined in sentry-conventions.
@@ -19,6 +19,12 @@ pub fn name_for_span(span: &Span) -> Option<String> {
     ))
 }
 
+/// Constructs a name attribute for a span, following the rules defined in sentry-conventions.
+pub fn name_for_attributes(attributes: &Attributes) -> Option<String> {
+    let op = attributes.get_value("sentry.op")?.as_str()?;
+    Some(name_for_op_and_attributes(op, &AttributeGetter(attributes)))
+}
+
 struct EmptyGetter {}
 
 impl Getter for EmptyGetter {
@@ -39,6 +45,18 @@ impl<'a, T: Getter> Getter for EscapedGetter<'a, T> {
     }
 }
 
+/// A custom getter for [`Attributes`] which only resolves values based on the attribute name.
+///
+/// This [`Getter`] does not implement nested traversals, which is the behaviour required for
+/// [`name_for_op_and_attributes`].
+struct AttributeGetter<'a>(&'a Attributes);
+
+impl<'a> Getter for AttributeGetter<'a> {
+    fn get_value(&self, path: &str) -> Option<Val<'_>> {
+        self.0.get_value(path).map(|value| value.into())
+    }
+}
+
 #[cfg(test)]
 mod tests {
     use relay_event_schema::protocol::SpanData;
@@ -47,7 +65,7 @@ mod tests {
     use super::*;
 
     #[test]
-    fn falls_back_to_op_when_no_templates_defined() {
+    fn test_span_falls_back_to_op_when_no_templates_defined() {
         let span = Span {
             op: Annotated::new("foo".to_owned()),
             ..Default::default()
@@ -56,22 +74,32 @@ mod tests {
     }
 
     #[test]
-    fn uses_the_first_matching_template() {
+    fn test_attributes_falls_back_to_op_when_no_templates_defined() {
+        let attributes = Attributes::from([(
+            "sentry.op".to_owned(),
+            Annotated::new("foo".to_owned().into()),
+        )]);
+
+        assert_eq!(name_for_attributes(&attributes), Some("foo".to_owned()));
+    }
+
+    #[test]
+    fn test_span_uses_the_first_matching_template() {
         let span = Span {
             op: Annotated::new("db".to_owned()),
             data: Annotated::new(SpanData {
                 other: Object::from([
                     (
-                        "db.query.summary".into(),
-                        Value::String("SELECT users".into()).into(),
+                        "db.query.summary".to_owned(),
+                        Value::String("SELECT users".to_owned()).into(),
                     ),
                     (
-                        "db.operation.name".into(),
-                        Value::String("INSERT".into()).into(),
+                        "db.operation.name".to_owned(),
+                        Value::String("INSERT".to_owned()).into(),
                     ),
                     (
-                        "db.collection.name".into(),
-                        Value::String("widgets".into()).into(),
+                        "db.collection.name".to_owned(),
+                        Value::String("widgets".to_owned()).into(),
                     ),
                 ]),
                 ..Default::default()
@@ -82,18 +110,45 @@ mod tests {
     }
 
     #[test]
-    fn uses_fallback_templates_when_data_is_missing() {
+    fn test_attributes_uses_the_first_matching_template() {
+        let attributes = Attributes::from([
+            (
+                "sentry.op".to_owned(),
+                Annotated::new("db".to_owned().into()),
+            ),
+            (
+                "db.query.summary".to_owned(),
+                Annotated::new("SELECT users".to_owned().into()),
+            ),
+            (
+                "db.operation.name".to_owned(),
+                Annotated::new("INSERT".to_owned().into()),
+            ),
+            (
+                "db.collection.name".to_owned(),
+                Annotated::new("widgets".to_owned().into()),
+            ),
+        ]);
+
+        assert_eq!(
+            name_for_attributes(&attributes),
+            Some("SELECT users".to_owned())
+        );
+    }
+
+    #[test]
+    fn test_span_uses_fallback_templates_when_data_is_missing() {
         let span = Span {
             op: Annotated::new("db".to_owned()),
             data: Annotated::new(SpanData {
                 other: Object::from([
                     (
-                        "db.operation.name".into(),
-                        Value::String("INSERT".into()).into(),
+                        "db.operation.name".to_owned(),
+                        Value::String("INSERT".to_owned()).into(),
                     ),
                     (
-                        "db.collection.name".into(),
-                        Value::String("widgets".into()).into(),
+                        "db.collection.name".to_owned(),
+                        Value::String("widgets".to_owned()).into(),
                     ),
                 ]),
                 ..Default::default()
@@ -104,11 +159,47 @@ mod tests {
     }
 
     #[test]
-    fn falls_back_to_hardcoded_name_when_nothing_matches() {
+    fn test_attributes_uses_fallback_templates_when_data_is_missing() {
+        let attributes = Attributes::from([
+            (
+                "sentry.op".to_owned(),
+                Annotated::new("db".to_owned().into()),
+            ),
+            (
+                "db.operation.name".to_owned(),
+                Annotated::new("INSERT".to_owned().into()),
+            ),
+            (
+                "db.collection.name".to_owned(),
+                Annotated::new("widgets".to_owned().into()),
+            ),
+        ]);
+
+        assert_eq!(
+            name_for_attributes(&attributes),
+            Some("INSERT widgets".to_owned())
+        );
+    }
+
+    #[test]
+    fn test_span_falls_back_to_hardcoded_name_when_nothing_matches() {
         let span = Span {
             op: Annotated::new("db".to_owned()),
             ..Default::default()
         };
         assert_eq!(name_for_span(&span), Some("Database operation".to_owned()));
     }
+
+    #[test]
+    fn test_attributes_falls_back_to_hardcoded_name_when_nothing_matches() {
+        let attributes = Attributes::from([(
+            "sentry.op".to_owned(),
+            Annotated::new("db".to_owned().into()),
+        )]);
+
+        assert_eq!(
+            name_for_attributes(&attributes),
+            Some("Database operation".to_owned())
+        );
+    }
 }
diff --git a/relay-spans/src/v1_to_v2.rs b/relay-spans/src/v1_to_v2.rs
@@ -6,6 +6,8 @@ use relay_event_schema::protocol::{
 };
 use relay_protocol::{Annotated, Empty, Error, IntoValue, Meta, Value};
 
+use crate::name::name_for_attributes;
+
 /// Converts a legacy span to the new Span V2 schema.
 ///
 /// - `tags`, `sentry_tags`, `measurements` and `data` are transferred to `attributes`.
@@ -99,14 +101,16 @@ pub fn span_v1_to_span_v2(span_v1: SpanV1) -> SpanV2 {
         }
     }
 
+    let name = attributes
+        .remove("sentry.name")
+        .and_then(|name| name.map_value(|attr| attr.into_string()).transpose())
+        .unwrap_or_else(|| name_for_attributes(attributes).into());
+
     SpanV2 {
         trace_id,
         parent_span_id,
         span_id,
-        name: attributes
-            .get_value("sentry.name")
-            .and_then(|v| Some(v.as_str()?.to_owned()))
-            .into(),
+        name,
         status: Annotated::map_value(status, span_v1_status_to_span_v2_status)
             .or_else(|| SpanV2Status::Ok.into()),
         is_remote: is_remote.or_else(|| false.into()),
@@ -287,11 +291,12 @@ mod tests {
         let span_v2 = span_v1_to_span_v2(span_v1);
 
         let annotated_span_v2: Annotated<SpanV2> = Annotated::new(span_v2);
-        insta::assert_json_snapshot!(SerializableAnnotated(&annotated_span_v2), @r###"
+        insta::assert_json_snapshot!(SerializableAnnotated(&annotated_span_v2), @r#"
         {
           "trace_id": "4c79f60c11214eb38604f4ae0781bfb2",
           "parent_span_id": "fa90fdead5f74051",
           "span_id": "fa90fdead5f74052",
+          "name": "operation",
           "status": "ok",
           "is_remote": true,
           "kind": "server",
@@ -410,7 +415,7 @@ mod tests {
             }
           }
         }
-        "###);
+        "#);
     }
 
     #[test]
diff --git a/tests/integration/test_spans.py b/tests/integration/test_spans.py
@@ -146,7 +146,6 @@ def test_span_extraction(
         "attributes": {  # Backfilled from `sentry_tags`
             "sentry.category": {"type": "string", "value": "http"},
             "sentry.exclusive_time": {"type": "double", "value": 500.0},
-            "sentry.name": {"type": "string", "value": "http"},
             "sentry.normalized_description": {"type": "string", "value": "GET *"},
             "sentry.group": {"type": "string", "value": "37e3d9fab1ae9162"},
             "sentry.op": {"type": "string", "value": "http"},
@@ -223,7 +222,6 @@ def test_span_extraction(
             "sentry.description": {"type": "string", "value": "hi"},
             "sentry.exclusive_time": {"type": "double", "value": 1500.0},
             "sentry.is_segment": {"type": "boolean", "value": True},
-            "sentry.name": {"type": "string", "value": "hi"},
             "sentry.op": {"type": "string", "value": "hi"},
             "sentry.origin": {"type": "string", "value": "manual"},
             "sentry.platform": {"type": "string", "value": "other"},
@@ -682,7 +680,6 @@ def test_span_ingestion(
                 "sentry.file_extension": {"type": "string", "value": "js"},
                 "sentry.group": {"type": "string", "value": "8a97a9e43588e2bd"},
                 "sentry.is_segment": {"type": "boolean", "value": True},
-                "sentry.name": {"type": "string", "value": "resource.script"},
                 "sentry.normalized_description": {
                     "type": "string",
                     "value": "https://example.com/*/blah.js",
@@ -726,7 +723,6 @@ def test_span_ingestion(
                 },
                 "sentry.exclusive_time": {"type": "double", "value": 345.0},
                 "sentry.is_segment": {"type": "boolean", "value": False},
-                "sentry.name": {"type": "string", "value": "default"},
                 "sentry.op": {"type": "string", "value": "default"},
                 "sentry.segment.id": {"type": "string", "value": "968cff94913ebb07"},
                 "user_agent.original": {
@@ -755,7 +751,6 @@ def test_span_ingestion(
                 "sentry.description": {"type": "string", "value": "my 2nd OTel span"},
                 "sentry.exclusive_time": {"type": "double", "value": 500.0},
                 "sentry.is_segment": {"type": "boolean", "value": True},
-                "sentry.name": {"type": "string", "value": "my 2nd OTel span"},
                 "sentry.op": {"type": "string", "value": "default"},
                 "sentry.segment.id": {"type": "string", "value": "d342abb1214ca182"},
                 "sentry.status": {"type": "string", "value": "ok"},
@@ -793,7 +788,6 @@ def test_span_ingestion(
                 "sentry.browser.name": {"type": "string", "value": "Chrome"},
                 "sentry.exclusive_time": {"type": "double", "value": 345.0},
                 "sentry.is_segment": {"type": "boolean", "value": False},
-                "sentry.name": {"type": "string", "value": "default"},
                 "sentry.op": {"type": "string", "value": "default"},
                 "sentry.segment.id": {"type": "string", "value": "968cff94913ebb07"},
                 "user_agent.original": {
@@ -825,7 +819,6 @@ def test_span_ingestion(
                     "value": "my 3rd protobuf OTel span",
                 },
                 "sentry.exclusive_time": {"type": "double", "value": 500.0},
-                "sentry.name": {"type": "string", "value": "my 3rd protobuf OTel span"},
                 "sentry.op": {"type": "string", "value": "default"},
                 "sentry.status": {"type": "string", "value": "ok"},
                 "ui.component_name": {"type": "string", "value": "MyComponent"},

Original file line number	Diff line number	Diff line change
`@@ -70,6 +70,14 @@ impl Value {`
`70`	`70`	`}`
`71`	`71`	`}`
`72`	`72`
	`73`	+ /// Returns the string if this value is a string, otherwise `None`.
	`74`	`+ pub fn into_string(self) -> Option<String> {`
	`75`	`+ match self {`
	`76`	`+ Value::String(string) => Some(string),`
	`77`	`+ _ => None,`
	`78`	`+ }`
	`79`	`+ }`
	`80`	`+`
`73`	`81`	/// Returns a f64 if the value can be converted to it, otherwise `None`.
`74`	`82`	`pub fn as_f64(&self) -> Option<f64> {`
`75`	`83`	`match self {`