feat(spans): Equalize name and description for manual spans

CHANGELOG.md

@@ -6,6 +6,7 @@
 
 - Add metadata support for the `/upload` endpoint. ([#6028](https://github.com/getsentry/relay/pull/6028))
 - Infer user agents and client addresses in the V2 standalone span pipeline. ([#6047](https://github.com/getsentry/relay/pull/6047))
+- Equalize name and description for spans with "manual" origin. ([#6070](https://github.com/getsentry/relay/pull/6070))
 
 **Bug Fixes**:
 

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

@@ -11,7 +11,7 @@ use relay_conventions::attributes::*;
 use relay_conventions::{AttributeInfo, ReplacementName, WriteBehavior};
 use relay_event_schema::protocol::{Attribute, AttributeType, Attributes, BrowserContext, Geo};
 use relay_protocol::{Annotated, Error, ErrorKind, Meta, Remark, RemarkType, Value};
-use relay_spans::derive_op_for_v2_span;
+use relay_spans::{derive_description_for_v2_span, derive_op_for_v2_span};
 
 use crate::span::TABLE_NAME_REGEX;
 use crate::span::description::{scrub_db_query, scrub_http};
@@ -46,6 +46,33 @@ pub fn normalize_sentry_op(attributes: &mut Annotated<Attributes>) {
     attrs.insert_if_missing(SENTRY__OP, || inferred_op);
 }
 
+/// Normalizes a V2 span's [`SENTRY__DESCRIPTION`] attribute.
+///
+/// For now, this tries the following steps, in order:
+/// - backfill from the span's name if its [`SENTRY__ORIGIN`] attribute is `"manual"`
+/// - backfill from the span's [`DB__QUERY__TEXT`] attribute if it exists
+/// - backfill a combination of the span's [`HTTP__REQUEST__METHOD`] and
+///   [`URL__FULL`] attributes, if they both exists.
+///
+/// In the future, this logic will be partly moved to and extended in `sentry-conventions`.
+pub fn normalize_sentry_description(
+    attributes: &mut Annotated<Attributes>,
+    name: &Annotated<String>,
+) {
+    if attributes
+        .value()
+        .is_some_and(|attrs| attrs.contains_key(SENTRY__DESCRIPTION))
+    {
+        return;
+    }
+
+    if let Some(description) = derive_description_for_v2_span(attributes, name) {
+        attributes
+            .get_or_insert_with(Default::default)
+            .insert(SENTRY__DESCRIPTION, description);
+    }
+}
+
 /// Infers the sentry.category attribute and inserts it into `attributes` if not
 /// already set.  The category is derived from the span operation or other span
 /// attributes.
@@ -765,7 +792,6 @@ pub fn write_legacy_attributes(attributes: &mut Annotated<Attributes>) {
     )]
     let current_to_legacy_attributes = [
         // DB attributes
-        (DB__QUERY__TEXT, SENTRY__DESCRIPTION),
         (SENTRY__NORMALIZED_DB_QUERY, SENTRY__NORMALIZED_DESCRIPTION),
         (DB__OPERATION__NAME, SENTRY__ACTION),
         (DB__SYSTEM__NAME, DB__SYSTEM),
@@ -778,12 +804,15 @@ pub fn write_legacy_attributes(attributes: &mut Annotated<Attributes>) {
     ];
 
     for (current_attribute, legacy_attribute) in current_to_legacy_attributes {
-        if attributes.contains_key(current_attribute) {
-            let Some(attr) = attributes.get_attribute(current_attribute) else {
-                continue;
-            };
-            attributes.insert(legacy_attribute, attr.value.clone());
+        if attributes.contains_key(legacy_attribute) {
+            continue;
         }
+
+        let Some(attr) = attributes.get_attribute(current_attribute) else {
+            continue;
+        };
+
+        attributes.insert(legacy_attribute, attr.value.clone());
     }
 
     if !attributes.contains_key(SENTRY__DOMAIN)
@@ -803,12 +832,6 @@ pub fn write_legacy_attributes(attributes: &mut Annotated<Attributes>) {
             },
         );
     }
-
-    if let Some(&Value::String(method)) = attributes.get_value(HTTP__REQUEST__METHOD).as_ref()
-        && let Some(&Value::String(url)) = attributes.get_value(URL__FULL).as_ref()
-    {
-        attributes.insert(SENTRY__DESCRIPTION, format!("{method} {url}"))
-    }
 }
 
 #[cfg(test)]
@@ -2249,10 +2272,6 @@ mod tests {
             "type": "string",
             "value": "FIND"
           },
-          "sentry.description": {
-            "type": "string",
-            "value": "{\"find\": \"documents\", \"foo\": \"bar\"}"
-          },
           "sentry.domain": {
             "type": "string",
             "value": ",documents,"

relay-server/src/processing/spans/mod.rs

@@ -204,6 +204,7 @@ impl processing::Processor for SpansProcessor {
         };
 
         process::scrub(&mut spans, ctx);
+        process::backfill_description(&mut spans);
 
         match dynamic_sampling::try_split_indexed_and_total(spans, ctx) {
             Either::Left(spans) => Ok(Output::just(SpanOutput::TotalAndIndexed(spans))),

relay-server/src/processing/spans/process.rs

@@ -1,4 +1,5 @@
 use std::collections::BTreeMap;
+use std::convert::Infallible;
 use std::time::Duration;
 
 use relay_event_normalization::eap::ClientUserAgentInfo;
@@ -288,6 +289,21 @@ fn normalize_span(
     Ok(())
 }
 
+pub fn backfill_description(spans: &mut Managed<ExpandedSpans>) {
+    spans.retain_with_context(
+        |spans| (&mut spans.spans, &()),
+        |span, _, _| backfill_span_description(&mut span.span),
+    );
+}
+
+fn backfill_span_description(span: &mut Annotated<SpanV2>) -> Result<(), Infallible> {
+    let Some(span) = span.value_mut() else {
+        return Ok(());
+    };
+    eap::normalize_sentry_description(&mut span.attributes, &span.name);
+    Ok(())
+}
+
 /// Validates the start and end timestamps of a span.
 ///
 /// The start timestamp must not be after the end timestamp.
@@ -928,7 +944,6 @@ mod tests {
         assert_attributes_contains(
             &span,
             &[
-                (SENTRY__DESCRIPTION, "select * from users where id = 1"),
                 (
                     SENTRY__NORMALIZED_DESCRIPTION,
                     "SELECT * FROM users WHERE id = %s",
@@ -990,10 +1005,6 @@ mod tests {
             &[
                 (SENTRY__CATEGORY, "http"),
                 (SENTRY__OP, "http.client"),
-                (
-                    SENTRY__DESCRIPTION,
-                    "GET https://www.example.com/path?param=value",
-                ),
                 (SENTRY__ACTION, "GET"),
                 (SENTRY__DOMAIN, "*.example.com"),
             ],
@@ -1113,7 +1124,6 @@ mod tests {
             &span,
             &[
                 (SENTRY__OP, "db"),
-                (SENTRY__DESCRIPTION, "select * from users where id = 1"),
                 (
                     SENTRY__NORMALIZED_DESCRIPTION,
                     "SELECT * FROM users WHERE id = %s",

relay-server/src/processing/transactions/spans.rs

@@ -4,7 +4,7 @@ use crate::processing;
 use crate::processing::utils::event::event_type;
 use relay_base_schema::events::EventType;
 use relay_config::Config;
-use relay_event_schema::protocol::{Event, Measurement, Measurements, Span, SpanV2};
+use relay_event_schema::protocol::{Event, Measurement, Measurements, Span, SpanV2, TraceContext};
 use relay_metrics::MetricNamespace;
 use relay_metrics::{FractionUnit, MetricUnit};
 use relay_protocol::{Annotated, Empty};
@@ -41,6 +41,11 @@ pub fn extract_from_event(
 
     // Add child spans.
     if let Some(spans) = event.spans.value() {
+        let origin = event
+            .context::<TraceContext>()
+            .map(|trace| trace.origin.clone())
+            .unwrap_or_default();
+
         for span in spans {
             let Some(inner_span) = span.value() else {
                 continue;
@@ -54,6 +59,10 @@ pub fn extract_from_event(
             new_span.segment_id = transaction_span.segment_id.clone();
             new_span.platform = transaction_span.platform.clone();
 
+            if new_span.origin.value().is_none() {
+                new_span.origin = origin.clone();
+            }
+
             // If a profile is associated with the transaction, also associate it with its
             // child spans.
             new_span.profile_id = transaction_span.profile_id.clone();

relay-spans/src/description.rs

@@ -0,0 +1,40 @@
+use relay_conventions::attributes::*;
+use relay_event_schema::protocol::Attributes;
+use relay_protocol::{Annotated, Value};
+
+/// Derives a description for a V2 span, based on its name
+/// and attributes.
+///
+/// For now, this tries the following steps, in order:
+/// - returns the span's name if its [`SENTRY__ORIGIN`] attribute is `"manual"`
+/// - returns the span's [`DB__QUERY__TEXT`] attribute if it exists
+/// - returns a combination of the span's [`HTTP__REQUEST__METHOD`] and
+///   [`URL__FULL`] attributes, if they both exists.
+///
+/// In the future, this logic will be partly moved to and extended in `sentry-conventions`.
+pub fn derive_description_for_v2_span(
+    attributes: &Annotated<Attributes>,
+    name: &Annotated<String>,
+) -> Option<String> {
+    let attributes = attributes.value()?;
+
+    if attributes
+        .get_value(SENTRY__ORIGIN)
+        .and_then(|o| o.as_str())
+        == Some("manual")
+    {
+        return name.value().cloned();
+    }
+
+    if let Some(&Value::String(db_query)) = attributes.get_value(DB__QUERY__TEXT).as_ref() {
+        return Some(db_query.clone());
+    }
+
+    if let Some(&Value::String(method)) = attributes.get_value(HTTP__REQUEST__METHOD).as_ref()
+        && let Some(&Value::String(url)) = attributes.get_value(URL__FULL).as_ref()
+    {
+        return Some(format!("{method} {url}"));
+    }
+
+    None
+}

relay-spans/src/lib.rs

@@ -6,13 +6,15 @@
     html_favicon_url = "https://raw.githubusercontent.com/getsentry/relay/master/artwork/relay-icon.png"
 )]
 
+pub use crate::description::derive_description_for_v2_span;
 pub use crate::name::name_for_span;
 pub use crate::op::derive_op_for_v2_span;
 pub use crate::otel_to_sentry_v2::otel_to_sentry_span as otel_to_sentry_span_v2;
 pub use crate::v1_to_v2::span_v1_to_span_v2;
 
 pub use opentelemetry_proto::tonic::trace::v1 as otel_trace;
 
+mod description;
 mod name;
 mod op;
 mod otel_to_sentry_v2;

relay-spans/src/name.rs

@@ -1,10 +1,20 @@
-use relay_conventions::attributes::SENTRY__OP;
+use relay_conventions::attributes::{SENTRY__DESCRIPTION, SENTRY__OP, SENTRY__ORIGIN};
 use relay_conventions::name_for_op_and_attributes;
 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.
+/// Constructs a name attribute for a V1 span.
+///
+/// If the span's origin is `"manual"`, its description is used as the name.
+/// Otherwise, the name is constructed following the rules defined in sentry-conventions.
 pub fn name_for_span(span: &Span) -> Option<String> {
+    let origin = span.origin.value().map(|o| o.as_str());
+    let description = span.description.value().map(|d| d.as_str());
+
+    if let Some(name) = name_for_origin_and_description(origin, description) {
+        return Some(name);
+    }
+
     let op = span.op.value()?;
 
     let Some(data) = span.data.value() else {
@@ -20,12 +30,38 @@ pub fn name_for_span(span: &Span) -> Option<String> {
     ))
 }
 
-/// Constructs a name attribute for a span, following the rules defined in sentry-conventions.
+/// Constructs a name attribute for a V2 span, based on its attributes.
+///
+/// If the attributes contain [`SENTRY__ORIGIN`] with the value `"manual"`,
+/// the description (contained in [`SENTRY__DESCRIPTION`]) is used as the name.
+/// Otherwise, the name is constructed following the rules defined in sentry-conventions.
 pub fn name_for_attributes(attributes: &Attributes) -> Option<String> {
+    let origin = attributes
+        .get_value(SENTRY__ORIGIN)
+        .and_then(|o| o.as_str());
+    let description = attributes
+        .get_value(SENTRY__DESCRIPTION)
+        .and_then(|d| d.as_str());
+
+    if let Some(name) = name_for_origin_and_description(origin, description) {
+        return Some(name);
+    }
+
     let op = attributes.get_value(SENTRY__OP)?.as_str()?;
     Some(name_for_op_and_attributes(op, &AttributeGetter(attributes)))
 }
 
+fn name_for_origin_and_description(
+    origin: Option<&str>,
+    description: Option<&str>,
+) -> Option<String> {
+    if origin == Some("manual") {
+        description.map(String::from)
+    } else {
+        None
+    }
+}
+
 struct EmptyGetter {}
 
 impl Getter for EmptyGetter {
@@ -203,4 +239,67 @@ mod tests {
             Some("Database operation".to_owned())
         );
     }
+
+    #[test]
+    fn test_manual_spans_use_description_v1() {
+        let span = Span {
+            origin: Annotated::new("manual".to_owned()),
+            description: Annotated::new("Custom name".to_owned()),
+            op: Annotated::new("db".to_owned()),
+            data: Annotated::new(SpanData {
+                other: Object::from([
+                    (
+                        "db.query.summary".to_owned(),
+                        Value::String("SELECT users".to_owned()).into(),
+                    ),
+                    (
+                        "db.operation.name".to_owned(),
+                        Value::String("INSERT".to_owned()).into(),
+                    ),
+                    (
+                        "db.collection.name".to_owned(),
+                        Value::String("widgets".to_owned()).into(),
+                    ),
+                ]),
+                ..Default::default()
+            }),
+            ..Default::default()
+        };
+        assert_eq!(name_for_span(&span), Some("Custom name".to_owned()));
+    }
+
+    #[test]
+    fn test_manual_spans_use_description_v2() {
+        let attributes = Attributes::from([
+            (
+                "sentry.origin".to_owned(),
+                Annotated::new("manual".to_owned().into()),
+            ),
+            (
+                "sentry.description".to_owned(),
+                Annotated::new("Custom name".to_owned().into()),
+            ),
+            (
+                "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("Custom name".to_owned())
+        );
+    }
 }