ref(tracing): rework tracing to Sentry span name/op conversion

CHANGELOG.md

@@ -4,6 +4,28 @@
 
 ### Breaking changes
 
+- ref(tracing): rework tracing to Sentry span name/op conversion ([#887](https://github.com/getsentry/sentry-rust/pull/887)) by @lcian
+  - The `tracing` integration now uses the tracing span name as the Sentry span name by default.
+  - Before this change, the span name would be set based on the `tracing` span target (<module>::<function> when using the `tracing::instrument` macro).
+  - The `tracing` integration now uses `default` as the default Sentry span op.
+  - Before this change, the span op would be set based on the `tracing` span name.
+  - When upgrading, please ensure to adapt any queries, metrics or dashboards to use the new span names/ops.
+  - Additional special fields have been added that allow overriding certain data on the Sentry span:
+    - `sentry.op`: override the Sentry span op.
+    - `sentry.name`: override the Sentry span name.
+    - `sentry.trace`: given a string matching a valid `sentry-trace` header (sent automatically by client SDKs), continues the distributed trace instead of starting a new one. If the value is not a valid `sentry-trace` header or a trace is already started, this value is ignored.
+  - `sentry.op` and `sentry.name` can also be applied retroactively by declaring fields with value `tracing::field::Empty` and then recorded using `tracing::Span::record`.
+  - Example usage:
+    ```rust
+    #[tracing::instrument(skip_all, fields(
+        sentry.op = "http.server",
+        sentry.name = "GET /payments",
+        sentry.trace = headers.get("sentry-trace").unwrap_or(&"".to_owned()),
+    ))]
+    async fn handle_request(headers: std::collections::HashMap<String, String>) {
+        // ...
+    }
+    ```
 - fix(actix): capture only server errors ([#877](https://github.com/getsentry/sentry-rust/pull/877))
   - The Actix integration now properly honors the `capture_server_errors` option (enabled by default), capturing errors returned by middleware only if they are server errors (HTTP status code 5xx).
   - Previously, if a middleware were to process the request after the Sentry middleware and return an error, our middleware would always capture it and send it to Sentry, regardless if it was a client, server or some other kind of error.

sentry-core/src/performance.rs

@@ -465,6 +465,22 @@ impl TransactionOrSpan {
         }
     }
 
+    /// Set the operation for this Transaction/Span.
+    pub fn set_op(&self, op: &str) {
+        match self {
+            TransactionOrSpan::Transaction(transaction) => transaction.set_op(op),
+            TransactionOrSpan::Span(span) => span.set_op(op),
+        }
+    }
+
+    /// Set the name (description) for this Transaction/Span.
+    pub fn set_name(&self, name: &str) {
+        match self {
+            TransactionOrSpan::Transaction(transaction) => transaction.set_name(name),
+            TransactionOrSpan::Span(span) => span.set_name(name),
+        }
+    }
+
     /// Set the HTTP request information for this Transaction/Span.
     pub fn set_request(&self, request: protocol::Request) {
         match self {
@@ -781,6 +797,20 @@ impl Transaction {
         inner.context.status = Some(status);
     }
 
+    /// Set the operation of the Transaction.
+    pub fn set_op(&self, op: &str) {
+        let mut inner = self.inner.lock().unwrap();
+        inner.context.op = Some(op.to_string());
+    }
+
+    /// Set the name of the Transaction.
+    pub fn set_name(&self, name: &str) {
+        let mut inner = self.inner.lock().unwrap();
+        if let Some(transaction) = inner.transaction.as_mut() {
+            transaction.name = Some(name.to_string());
+        }
+    }
+
     /// Set the HTTP request information for this Transaction.
     pub fn set_request(&self, request: protocol::Request) {
         let mut inner = self.inner.lock().unwrap();
@@ -1018,6 +1048,18 @@ impl Span {
         span.status = Some(status);
     }
 
+    /// Set the operation of the Span.
+    pub fn set_op(&self, op: &str) {
+        let mut span = self.span.lock().unwrap();
+        span.op = Some(op.to_string());
+    }
+
+    /// Set the name (description) of the Span.
+    pub fn set_name(&self, name: &str) {
+        let mut span = self.span.lock().unwrap();
+        span.description = Some(name.to_string());
+    }
+
     /// Set the HTTP request information for this Span.
     pub fn set_request(&self, request: protocol::Request) {
         let mut span = self.span.lock().unwrap();

sentry-tracing/src/layer.rs

@@ -12,6 +12,9 @@ use tracing_subscriber::layer::{Context, Layer};
 use tracing_subscriber::registry::LookupSpan;
 
 use crate::converters::*;
+use crate::SENTRY_NAME_FIELD;
+use crate::SENTRY_OP_FIELD;
+use crate::SENTRY_TRACE_FIELD;
 use crate::TAGS_PREFIX;
 
 bitflags! {
@@ -292,27 +295,26 @@ where
             return;
         }
 
-        let (description, data) = extract_span_data(attrs);
-        let op = span.name();
-
-        // Spans don't always have a description, this ensures our data is not empty,
-        // therefore the Sentry UI will be a lot more valuable for navigating spans.
-        let description = description.unwrap_or_else(|| {
-            let target = span.metadata().target();
-            if target.is_empty() {
-                op.to_string()
-            } else {
-                format!("{target}::{op}")
-            }
-        });
+        let (data, sentry_name, sentry_op, sentry_trace) = extract_span_data(attrs);
+        let sentry_name = sentry_name.as_deref().unwrap_or_else(|| span.name());
+        let sentry_op = sentry_op.as_deref().unwrap_or("default");
 
         let hub = sentry_core::Hub::current();
         let parent_sentry_span = hub.configure_scope(|scope| scope.get_span());
 
         let sentry_span: sentry_core::TransactionOrSpan = match &parent_sentry_span {
-            Some(parent) => parent.start_child(op, &description).into(),
+            Some(parent) => parent.start_child(sentry_op, sentry_name).into(),
             None => {
-                let ctx = sentry_core::TransactionContext::new(&description, op);
+                let ctx = if let Some(trace_header) = sentry_trace {
+                    sentry_core::TransactionContext::continue_from_headers(
+                        sentry_name,
+                        sentry_op,
+                        [("sentry-trace", trace_header.as_str())],
+                    )
+                } else {
+                    sentry_core::TransactionContext::new(sentry_name, sentry_op)
+                };
+
                 let tx = sentry_core::start_transaction(ctx);
                 tx.set_data("origin", "auto.tracing".into());
                 tx.into()
@@ -397,6 +399,32 @@ where
         let mut data = FieldVisitor::default();
         values.record(&mut data);
 
+        let sentry_name = data
+            .json_values
+            .remove(SENTRY_NAME_FIELD)
+            .and_then(|v| match v {
+                Value::String(s) => Some(s),
+                _ => None,
+            });
+
+        let sentry_op = data
+            .json_values
+            .remove(SENTRY_OP_FIELD)
+            .and_then(|v| match v {
+                Value::String(s) => Some(s),
+                _ => None,
+            });
+
+        // `sentry.trace` cannot be applied retroactively
+        data.json_values.remove(SENTRY_TRACE_FIELD);
+
+        if let Some(name) = sentry_name {
+            span.set_name(&name);
+        }
+        if let Some(op) = sentry_op {
+            span.set_op(&op);
+        }
+
         record_fields(span, data.json_values);
     }
 }
@@ -410,7 +438,14 @@ where
 }
 
 /// Extracts the message and attributes from a span
-fn extract_span_data(attrs: &span::Attributes) -> (Option<String>, BTreeMap<&'static str, Value>) {
+fn extract_span_data(
+    attrs: &span::Attributes,
+) -> (
+    BTreeMap<&'static str, Value>,
+    Option<String>,
+    Option<String>,
+    Option<String>,
+) {
     let mut json_values = VISITOR_BUFFER.with_borrow_mut(|debug_buffer| {
         let mut visitor = SpanFieldVisitor {
             debug_buffer,
@@ -420,13 +455,24 @@ fn extract_span_data(attrs: &span::Attributes) -> (Option<String>, BTreeMap<&'st
         visitor.json_values
     });
 
-    // Find message of the span, if any
-    let message = json_values.remove("message").and_then(|v| match v {
+    let name = json_values.remove(SENTRY_NAME_FIELD).and_then(|v| match v {
+        Value::String(s) => Some(s),
+        _ => None,
+    });
+
+    let op = json_values.remove(SENTRY_OP_FIELD).and_then(|v| match v {
         Value::String(s) => Some(s),
         _ => None,
     });
 
-    (message, json_values)
+    let sentry_trace = json_values
+        .remove(SENTRY_TRACE_FIELD)
+        .and_then(|v| match v {
+            Value::String(s) => Some(s),
+            _ => None,
+        });
+
+    (json_values, name, op, sentry_trace)
 }
 
 thread_local! {

sentry-tracing/src/lib.rs

@@ -169,7 +169,7 @@
 //! # Tracing Spans
 //!
 //! The integration automatically tracks `tracing` spans as spans in Sentry. A convenient way to do
-//! this is with the `#[instrument]` attribute macro, which creates a transaction for the function
+//! this is with the `#[instrument]` attribute macro, which creates a span/transaction for the function
 //! in Sentry.
 //!
 //! Function arguments are added as context fields automatically, which can be configured through
@@ -180,8 +180,8 @@
 //!
 //! use tracing_subscriber::prelude::*;
 //!
-//! // Functions instrumented by tracing automatically report
-//! // their span as transactions.
+//! // Functions instrumented by tracing automatically
+//! // create spans/transactions around their execution.
 //! #[tracing::instrument]
 //! async fn outer() {
 //!     for i in 0..10 {
@@ -198,6 +198,42 @@
 //!     tokio::time::sleep(Duration::from_millis(100)).await;
 //! }
 //! ```
+//!
+//! By default, the name of the span sent to Sentry matches the name of the `tracing` span, which
+//! is the name of the function when using `tracing::instrument`, or the name passed to the
+//! `tracing::<level>_span` macros.
+//!
+//! By default, the `op` of the span sent to Sentry is `default`.
+//!
+//! ## Special Span Fields
+//!
+//! Some fields on spans are treated specially by the Sentry tracing integration:
+//! - `sentry.name`: overrides the span name sent to Sentry.
+//!   This is useful to customize the span name when using `#[tracing::instrument]`, or to update
+//!   it retroactively (using `span.record`) after the span has been created, as `tracing` doesn't allow doing it.
+//! - `sentry.op`: overrides the span `op` sent to Sentry.
+//! - `sentry.trace`: in Sentry, the `sentry-trace` header is sent with HTTP requests to achieve distributed tracing.
+//!   If the value of this field is set to the value of a valid `sentry-trace` header, which
+//!   frontend SDKs send automatically with outgoing requests, then the SDK will continue the trace using the given distributed tracing information.
+//!   This is useful to achieve distributed tracing at service boundaries by using only the
+//!   `tracing` API.
+//!   Note that this will only be effective on span creation (cannot be applied retroactively) and
+//!   requires the span it's applied to to be a root span, i.e. no span should active upon its
+//!   creation.
+//!
+//!
+//! Example:
+//!
+//! ```
+//! #[tracing::instrument(skip_all, fields(
+//!     sentry.name = "GET /payments",
+//!     sentry.op = "http.server",
+//!     sentry.trace = headers.get("sentry-trace").unwrap_or(&"".to_owned()),
+//! ))]
+//! async fn handle_request(headers: std::collections::HashMap<String, String>) {
+//!     // ...
+//! }
+//! ```
 
 #![doc(html_favicon_url = "https://sentry-brand.storage.googleapis.com/favicon.ico")]
 #![doc(html_logo_url = "https://sentry-brand.storage.googleapis.com/sentry-glyph-black.png")]
@@ -210,3 +246,6 @@ pub use converters::*;
 pub use layer::*;
 
 const TAGS_PREFIX: &str = "tags.";
+const SENTRY_OP_FIELD: &str = "sentry.op";
+const SENTRY_NAME_FIELD: &str = "sentry.name";
+const SENTRY_TRACE_FIELD: &str = "sentry.trace";

sentry-tracing/tests/name_op_updates.rs

@@ -0,0 +1,48 @@
+mod shared;
+
+#[tracing::instrument(fields(
+    some = "value",
+    sentry.name = tracing::field::Empty,
+    sentry.op = tracing::field::Empty,
+))]
+fn test_fun_record_on_creation() {
+    tracing::Span::current().record("sentry.name", "updated name");
+    tracing::Span::current().record("sentry.op", "updated op");
+}
+
+#[tracing::instrument(fields(
+    some = "value",
+    sentry.name = tracing::field::Empty,
+    sentry.op = tracing::field::Empty,
+))]
+fn test_fun_record_later() {
+    tracing::Span::current().record("sentry.name", "updated name");
+    tracing::Span::current().record("sentry.op", "updated op");
+}
+
+#[test]
+fn should_update_sentry_op_and_name_based_on_fields() {
+    let transport = shared::init_sentry(1.0);
+
+    for f in [test_fun_record_on_creation, test_fun_record_later] {
+        f();
+
+        let data = transport.fetch_and_clear_envelopes();
+        assert_eq!(data.len(), 1);
+        // Confirm transaction has updated values
+        let transaction = data.first().expect("should have 1 transaction");
+        let transaction = match transaction.items().next().unwrap() {
+            sentry::protocol::EnvelopeItem::Transaction(transaction) => transaction,
+            unexpected => panic!("Expected transaction, but got {unexpected:#?}"),
+        };
+
+        assert_eq!(transaction.name.as_deref().unwrap(), "updated name");
+        let ctx = transaction.contexts.get("trace");
+        match ctx {
+            Some(sentry::protocol::Context::Trace(trace_ctx)) => {
+                assert_eq!(trace_ctx.op, Some("updated op".to_owned()))
+            }
+            _ => panic!("expected trace context"),
+        }
+    }
+}

sentry-tracing/tests/smoke.rs

@@ -24,7 +24,7 @@ fn should_instrument_function_with_event() {
         sentry::protocol::Context::Trace(trace) => trace,
         unexpected => panic!("Expected trace context but got {unexpected:?}"),
     };
-    assert_eq!(trace.op.as_deref().unwrap(), "function_with_tags");
+    assert_eq!(trace.op.as_deref().unwrap(), "default");
 
     //Confirm transaction values
     let transaction = data.get(1).expect("should have 1 transaction");