diff --git a/CHANGELOG.md b/CHANGELOG.md index 2127cd47..c89740cf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,7 +4,13 @@ ### Breaking changes -- fix(actix): capture only server errors ([#877](https://github.com/getsentry/sentry-rust/pull/877)) +- 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 (:: 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. +- fix(actix): capture only server errors ([#877](https://github.com/getsentry/sentry-rust/pull/877)) by @lcian - 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. - With this change, we capture errors returned by middleware only if those errors can be classified as server errors. @@ -17,6 +23,27 @@ ### Features +- ref(tracing): rework tracing to Sentry span name/op conversion ([#887](https://github.com/getsentry/sentry-rust/pull/887)) by @lcian + - 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) { + // ... + } + ``` + - Additional attributes are sent along with each span by default: + - `sentry.tracing.target`: corresponds to the `tracing` span's `metadata.target()` + - `code.module.name`, `code.file.path`, `code.line.number` + - feat(core): add Response context ([#874](https://github.com/getsentry/sentry-rust/pull/874)) by @lcian - The `Response` context can now be attached to events, to include information about HTTP responses such as headers, cookies and status code. - Example: diff --git a/sentry-core/src/performance.rs b/sentry-core/src/performance.rs index 4e7a3ac3..7e6b723b 100644 --- a/sentry-core/src/performance.rs +++ b/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(); diff --git a/sentry-tracing/src/layer.rs b/sentry-tracing/src/layer.rs index 5d3c59ec..202f7cf3 100644 --- a/sentry-tracing/src/layer.rs +++ b/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! { @@ -298,27 +301,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(), + let mut sentry_span: sentry_core::TransactionOrSpan = match &parent_sentry_span { + 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() @@ -328,6 +330,8 @@ where // This comes from typically the `fields` in `tracing::instrument`. record_fields(&sentry_span, data); + set_default_attributes(&mut sentry_span, span.metadata()); + let mut extensions = span.extensions_mut(); extensions.insert(SentrySpanData { sentry_span, @@ -403,10 +407,52 @@ 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); } } +fn set_default_attributes(span: &mut TransactionOrSpan, metadata: &Metadata<'_>) { + span.set_data("sentry.tracing.target", metadata.target().into()); + + if let Some(module) = metadata.module_path() { + span.set_data("code.module.name", module.into()); + } + + if let Some(file) = metadata.file() { + span.set_data("code.file.path", file.into()); + } + + if let Some(line) = metadata.line() { + span.set_data("code.line.number", line.into()); + } +} + /// Creates a default Sentry layer pub fn layer() -> SentryLayer where @@ -415,8 +461,16 @@ where Default::default() } -/// Extracts the message and attributes from a span -fn extract_span_data(attrs: &span::Attributes) -> (Option, BTreeMap<&'static str, Value>) { +/// Extracts the attributes from a span, +/// returning the values of SENTRY_NAME_FIELD, SENTRY_OP_FIELD, SENTRY_TRACE_FIELD separately +fn extract_span_data( + attrs: &span::Attributes, +) -> ( + BTreeMap<&'static str, Value>, + Option, + Option, + Option, +) { let mut json_values = VISITOR_BUFFER.with_borrow_mut(|debug_buffer| { let mut visitor = SpanFieldVisitor { debug_buffer, @@ -426,13 +480,24 @@ fn extract_span_data(attrs: &span::Attributes) -> (Option, 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, }); - (message, json_values) + let op = json_values.remove(SENTRY_OP_FIELD).and_then(|v| match v { + Value::String(s) => Some(s), + _ => None, + }); + + 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! { diff --git a/sentry-tracing/src/lib.rs b/sentry-tracing/src/lib.rs index df688718..100b8efd 100644 --- a/sentry-tracing/src/lib.rs +++ b/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::_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. +//! - `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 +//! other Sentry 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 `sentry.trace` will only be effective on span creation (it 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) { +//! // ... +//! } +//! ``` #![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"; diff --git a/sentry-tracing/tests/name_op_updates.rs b/sentry-tracing/tests/name_op_updates.rs new file mode 100644 index 00000000..a6f3d15a --- /dev/null +++ b/sentry-tracing/tests/name_op_updates.rs @@ -0,0 +1,45 @@ +mod shared; + +#[tracing::instrument(fields( + some = "value", + sentry.name = "updated name", + sentry.op = "updated op", +))] +fn test_fun_record_on_creation() {} + +#[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); + + 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"), + } + } +} diff --git a/sentry-tracing/tests/smoke.rs b/sentry-tracing/tests/smoke.rs index cb4f04e3..cca7b1f4 100644 --- a/sentry-tracing/tests/smoke.rs +++ b/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"); @@ -32,8 +32,9 @@ fn should_instrument_function_with_event() { sentry::protocol::EnvelopeItem::Transaction(transaction) => transaction, unexpected => panic!("Expected transaction, but got {unexpected:#?}"), }; + assert_eq!(transaction.name, Some("function_with_tags".into())); assert_eq!(transaction.tags.len(), 1); - assert_eq!(trace.data.len(), 3); + assert_eq!(trace.data.len(), 7); let tag = transaction .tags @@ -50,4 +51,15 @@ fn should_instrument_function_with_event() { .get("value") .expect("to have data attribute with name 'value'"); assert_eq!(value, 1); + + assert_eq!( + trace.data.get("sentry.tracing.target"), + Some("smoke".into()).as_ref() + ); + assert_eq!( + trace.data.get("code.module.name"), + Some("smoke".into()).as_ref() + ); + assert!(trace.data.contains_key("code.file.path")); + assert!(trace.data.contains_key("code.line.number")); }