Repository: getsentry/sentry-rust
Pull request #687 (2024-09-26 00:05:12)

feat(sentry-core): add support for w3c traceparent
This change does three things:
1. Add a function to allow creating a `TransactionContext` with a custom Trace ID without a parent span ID.
  My company is testing traces on Sentry in parallel to our current trace solution, for ease of use, we'd like to keep using the same trace ID on both platforms for now. Currently, my approach is to create a `sentry-trace` header with a zeroed span ID, but the UI has this bright red alert telling me that the parent span ID is invalid, and this is bugging me. I doubt I'm the only one who has faced this issue before.
2. Implements support for the `traceparent` as stated in getsentry/team-sdks#41. `sentry-trace` still takes precedence. It does **not** emit the header, though. Should it? It only allows for parsing for now.
3. Creates a builder allowing to customize the creation of a transaction.

saiintbrisson: I need to figure some stuff out in #686 before doing this one.
codecov[bot]: ## [Codecov](https://app.codecov.io/gh/getsentry/sentry-rust/pull/687?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=getsentry) Report
Attention: Patch coverage is `91.30435%` with `4 lines` in your changes missing coverage. Please review.
> Project coverage is 73.26%. Comparing base [(`e3b0fba`)](https://app.codecov.io/gh/getsentry/sentry-rust/commit/e3b0fbae44e2fa90a519a93ceb5585a1b2ddc25b?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=getsentry) to head [(`4ca67ff`)](https://app.codecov.io/gh/getsentry/sentry-rust/commit/4ca67ff9887913c40de3981a0cadc63ec27993d9?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=getsentry).
> Report is 7 commits behind head on master.

<details><summary>Additional details and impacted files</summary>


```diff
@@            Coverage Diff             @@
##           master     #687      +/-   ##
==========================================
+ Coverage   73.25%   73.26%   +0.01%     
==========================================
  Files          66       66              
  Lines        7754     7781      +27     
==========================================
+ Hits         5680     5701      +21     
- Misses       2074     2080       +6     
```

</details>
saiintbrisson: @Swatinem this one is also ready to be reviewed.

Patch:
diff --git a/sentry-core/src/hub_impl.rs b/sentry-core/src/hub_impl.rs
index 1a2de2c..59c91bf 100644
--- a/sentry-core/src/hub_impl.rs
+++ b/sentry-core/src/hub_impl.rs
@@ -21,12 +21,17 @@ thread_local! {
     );
 }
 
-pub(crate) struct SwitchGuard {
+/// A Hub switch guard used to temporarily swap
+/// active hub in thread local storage.
+pub struct SwitchGuard {
     inner: Option<(Arc<Hub>, bool)>,
 }
 
 impl SwitchGuard {
-    pub(crate) fn new(mut hub: Arc<Hub>) -> Self {
+    /// Swaps the current thread's Hub by the one provided
+    /// and returns a guard that, when dropped, replaces it
+    /// to the previous one.
+    pub fn new(mut hub: Arc<Hub>) -> Self {
         let inner = THREAD_HUB.with(|(thread_hub, is_process_hub)| {
             // SAFETY: `thread_hub` will always be a valid thread local hub,
             // by definition not shared between threads.
diff --git a/sentry-core/src/lib.rs b/sentry-core/src/lib.rs
index 1f7d8c3..5cc39cd 100644
--- a/sentry-core/src/lib.rs
+++ b/sentry-core/src/lib.rs
@@ -149,8 +149,9 @@ pub mod metrics;
 mod session;
 #[cfg(all(feature = "client", feature = "metrics"))]
 mod units;
+
 #[cfg(feature = "client")]
-pub use crate::client::Client;
+pub use crate::{client::Client, hub_impl::SwitchGuard as HubSwitchGuard};
 
 // test utilities
 #[cfg(feature = "test")]
diff --git a/sentry-core/src/performance.rs b/sentry-core/src/performance.rs
index 59cca2a..3255f5d 100644
--- a/sentry-core/src/performance.rs
+++ b/sentry-core/src/performance.rs
@@ -69,12 +69,15 @@ pub struct TransactionContext {
     op: String,
     trace_id: protocol::TraceId,
     parent_span_id: Option<protocol::SpanId>,
+    span_id: protocol::SpanId,
     sampled: Option<bool>,
     custom: Option<CustomTransactionContext>,
 }
 
 impl TransactionContext {
-    /// Creates a new Transaction Context with the given `name` and `op`.
+    /// Creates a new Transaction Context with the given `name` and `op`. A random
+    /// `trace_id` is assigned. Use [`TransactionContext::new_with_trace_id`] to
+    /// specify a custom trace ID.
     ///
     /// See <https://docs.sentry.io/platforms/native/enriching-events/transaction-name/>
     /// for an explanation of a Transaction's `name`, and
@@ -85,13 +88,33 @@ impl TransactionContext {
     /// can be used for distributed tracing.
     #[must_use = "this must be used with `start_transaction`"]
     pub fn new(name: &str, op: &str) -> Self {
-        Self::continue_from_headers(name, op, std::iter::empty())
+        Self::new_with_trace_id(name, op, protocol::TraceId::default())
+    }
+
+    /// Creates a new Transaction Context with the given `name`, `op`, and `trace_id`.
+    ///
+    /// See <https://docs.sentry.io/platforms/native/enriching-events/transaction-name/>
+    /// for an explanation of a Transaction's `name`, and
+    /// <https://develop.sentry.dev/sdk/performance/span-operations/> for conventions
+    /// around an `operation`'s value.
+    #[must_use = "this must be used with `start_transaction`"]
+    pub fn new_with_trace_id(name: &str, op: &str, trace_id: protocol::TraceId) -> Self {
+        Self {
+            name: name.into(),
+            op: op.into(),
+            trace_id,
+            parent_span_id: None,
+            span_id: Default::default(),
+            sampled: None,
+            custom: None,
+        }
     }
 
     /// Creates a new Transaction Context based on the distributed tracing `headers`.
     ///
-    /// The `headers` in particular need to include the `sentry-trace` header,
-    /// which is used to associate the transaction with a distributed trace.
+    /// The `headers` in particular need to include either the `sentry-trace` or W3C
+    /// `traceparent` header, which is used to associate the transaction with a distributed
+    /// trace. If both are present, `sentry-trace` takes precedence.
     #[must_use = "this must be used with `start_transaction`"]
     pub fn continue_from_headers<'a, I: IntoIterator<Item = (&'a str, &'a str)>>(
         name: &str,
@@ -102,6 +125,11 @@ impl TransactionContext {
         for (k, v) in headers.into_iter() {
             if k.eq_ignore_ascii_case("sentry-trace") {
                 trace = parse_sentry_trace(v);
+                break;
+            }
+
+            if k.eq_ignore_ascii_case("traceparent") {
+                trace = parse_w3c_traceparent(v);
             }
         }
 
@@ -115,6 +143,7 @@ impl TransactionContext {
             op: op.into(),
             trace_id,
             parent_span_id,
+            span_id: Default::default(),
             sampled,
             custom: None,
         }
@@ -152,6 +181,7 @@ impl TransactionContext {
             op: op.into(),
             trace_id,
             parent_span_id: Some(parent_span_id),
+            span_id: protocol::SpanId::default(),
             sampled,
             custom: None,
         }
@@ -185,6 +215,11 @@ impl TransactionContext {
         self.trace_id
     }
 
+    /// Get the Span ID of this Transaction.
+    pub fn span_id(&self) -> protocol::SpanId {
+        self.span_id
+    }
+
     /// Get the custom context of this Transaction.
     pub fn custom(&self) -> Option<&CustomTransactionContext> {
         self.custom.as_ref()
@@ -220,6 +255,80 @@ impl TransactionContext {
         std::mem::swap(&mut self.custom, &mut Some(custom));
         existing_value
     }
+
+    /// Creates a transaction context builder initialized with the given `name` and `op`.
+    ///
+    /// See <https://docs.sentry.io/platforms/native/enriching-events/transaction-name/>
+    /// for an explanation of a Transaction's `name`, and
+    /// <https://develop.sentry.dev/sdk/performance/span-operations/> for conventions
+    /// around an `operation`'s value.
+    #[must_use]
+    pub fn builder(name: &str, op: &str) -> TransactionContextBuilder {
+        TransactionContextBuilder {
+            ctx: TransactionContext::new(name, op),
+        }
+    }
+}
+
+/// A transaction context builder created by [`TransactionContext::builder`].
+pub struct TransactionContextBuilder {
+    ctx: TransactionContext,
+}
+
+impl TransactionContextBuilder {
+    /// Defines the name of the transaction.
+    #[must_use]
+    pub fn with_name(mut self, name: String) -> Self {
+        self.ctx.name = name;
+        self
+    }
+
+    /// Defines the operation of the transaction.
+    #[must_use]
+    pub fn with_op(mut self, op: String) -> Self {
+        self.ctx.op = op;
+        self
+    }
+
+    /// Defines the trace ID.
+    #[must_use]
+    pub fn with_trace_id(mut self, trace_id: protocol::TraceId) -> Self {
+        self.ctx.trace_id = trace_id;
+        self
+    }
+
+    /// Defines a parent span ID for the created transaction.
+    #[must_use]
+    pub fn with_parent_span_id(mut self, parent_span_id: Option<protocol::SpanId>) -> Self {
+        self.ctx.parent_span_id = parent_span_id;
+        self
+    }
+
+    /// Defines the span ID to be used when creating the transaction.
+    #[must_use]
+    pub fn with_span_id(mut self, span_id: protocol::SpanId) -> Self {
+        self.ctx.span_id = span_id;
+        self
+    }
+
+    /// Defines whether the transaction will be sampled.
+    #[must_use]
+    pub fn with_sampled(mut self, sampled: Option<bool>) -> Self {
+        self.ctx.sampled = sampled;
+        self
+    }
+
+    /// Adds a custom key and value to the transaction context.
+    #[must_use]
+    pub fn with_custom(mut self, key: String, value: serde_json::Value) -> Self {
+        self.ctx.custom_insert(key, value);
+        self
+    }
+
+    /// Finishes building a transaction.
+    pub fn finish(self) -> TransactionContext {
+        self.ctx
+    }
 }
 
 /// A function to be run for each new transaction, to determine the rate at which
@@ -467,6 +576,7 @@ impl Transaction {
         let context = protocol::TraceContext {
             trace_id: ctx.trace_id,
             parent_span_id: ctx.parent_span_id,
+            span_id: ctx.span_id,
             op: Some(ctx.op),
             ..Default::default()
         };
@@ -717,6 +827,12 @@ impl Span {
         transaction.context.clone()
     }
 
+    /// Get the current span ID.
+    pub fn get_span_id(&self) -> protocol::SpanId {
+        let span = self.span.lock().unwrap();
+        span.span_id
+    }
+
     /// Get the status of the Span.
     pub fn get_status(&self) -> Option<protocol::SpanStatus> {
         let span = self.span.lock().unwrap();
@@ -858,6 +974,23 @@ fn parse_sentry_trace(header: &str) -> Option<SentryTrace> {
     Some(SentryTrace(trace_id, parent_span_id, parent_sampled))
 }
 
+/// Parses a W3C traceparent header.
+/// Reference: <https://w3c.github.io/trace-context/#traceparent-header-field-values>
+fn parse_w3c_traceparent(header: &str) -> Option<SentryTrace> {
+    let header = header.trim();
+    let mut parts = header.splitn(4, '-');
+
+    let _version = parts.next()?;
+    let trace_id = parts.next()?.parse().ok()?;
+    let parent_span_id = parts.next()?.parse().ok()?;
+    let parent_sampled = parts
+        .next()
+        .and_then(|sampled| u8::from_str_radix(sampled, 16).ok())
+        .map(|flag| flag & 1 != 0);
+
+    Some(SentryTrace(trace_id, parent_span_id, parent_sampled))
+}
+
 impl std::fmt::Display for SentryTrace {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
         write!(f, "{}-{}", self.0, self.1)?;
@@ -890,6 +1023,26 @@ mod tests {
         assert_eq!(parsed, Some(trace));
     }
 
+    #[test]
+    fn parses_traceparent() {
+        let trace_id = protocol::TraceId::from_str("4bf92f3577b34da6a3ce929d0e0e4736").unwrap();
+        let parent_trace_id = protocol::SpanId::from_str("00f067aa0ba902b7").unwrap();
+
+        let trace =
+            parse_w3c_traceparent("00-4bf92f3577b34da6a3ce929d0e0e4736-00f067aa0ba902b7-01");
+        assert_eq!(
+            trace,
+            Some(SentryTrace(trace_id, parent_trace_id, Some(true)))
+        );
+
+        let trace =
+            parse_w3c_traceparent("00-4bf92f3577b34da6a3ce929d0e0e4736-00f067aa0ba902b7-00");
+        assert_eq!(
+            trace,
+            Some(SentryTrace(trace_id, parent_trace_id, Some(false)))
+        );
+    }
+
     #[test]
     fn disabled_forwards_trace_id() {
         let headers = [(
diff --git a/sentry-tracing/src/layer.rs b/sentry-tracing/src/layer.rs
index 01930f8..0540fe7 100644
--- a/sentry-tracing/src/layer.rs
+++ b/sentry-tracing/src/layer.rs
@@ -1,6 +1,7 @@
 use std::borrow::Cow;
 use std::cell::RefCell;
 use std::collections::BTreeMap;
+use std::sync::Arc;
 
 use sentry_core::protocol::Value;
 use sentry_core::{Breadcrumb, TransactionOrSpan};
@@ -197,6 +198,8 @@ fn record_fields<'a, K: AsRef<str> + Into<Cow<'a, str>>>(
 pub(super) struct SentrySpanData {
     pub(super) sentry_span: TransactionOrSpan,
     parent_sentry_span: Option<TransactionOrSpan>,
+    hub: Arc<sentry_core::Hub>,
+    hub_switch_guard: Option<sentry_core::HubSwitchGuard>,
 }
 
 impl<S> Layer<S> for SentryLayer<S>
@@ -256,7 +259,9 @@ where
             }
         });
 
-        let parent_sentry_span = sentry_core::configure_scope(|s| s.get_span());
+        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(),
             None => {
@@ -268,15 +273,48 @@ where
         // This comes from typically the `fields` in `tracing::instrument`.
         record_fields(&sentry_span, data);
 
-        sentry_core::configure_scope(|scope| scope.set_span(Some(sentry_span.clone())));
-
         let mut extensions = span.extensions_mut();
         extensions.insert(SentrySpanData {
             sentry_span,
             parent_sentry_span,
+            hub,
+            hub_switch_guard: None,
         });
     }
 
+    /// Sets entered span as *current* sentry span. A tracing span can be
+    /// entered and existed multiple times, for example, when using a `tracing::Instrumented` future.
+    fn on_enter(&self, id: &span::Id, ctx: Context<'_, S>) {
+        let span = match ctx.span(id) {
+            Some(span) => span,
+            None => return,
+        };
+
+        let mut extensions = span.extensions_mut();
+        if let Some(data) = extensions.get_mut::<SentrySpanData>() {
+            data.hub_switch_guard = Some(sentry_core::HubSwitchGuard::new(data.hub.clone()));
+            data.hub.configure_scope(|scope| {
+                scope.set_span(Some(data.sentry_span.clone()));
+            })
+        }
+    }
+
+    /// Set exited span's parent as *current* sentry span.
+    fn on_exit(&self, id: &span::Id, ctx: Context<'_, S>) {
+        let span = match ctx.span(id) {
+            Some(span) => span,
+            None => return,
+        };
+
+        let mut extensions = span.extensions_mut();
+        if let Some(data) = extensions.get_mut::<SentrySpanData>() {
+            data.hub.configure_scope(|scope| {
+                scope.set_span(data.parent_sentry_span.clone());
+            });
+            data.hub_switch_guard.take();
+        }
+    }
+
     /// When a span gets closed, finish the underlying sentry span, and set back
     /// its parent as the *current* sentry span.
     fn on_close(&self, id: span::Id, ctx: Context<'_, S>) {
@@ -286,16 +324,12 @@ where
         };
 
         let mut extensions = span.extensions_mut();
-        let SentrySpanData {
-            sentry_span,
-            parent_sentry_span,
-        } = match extensions.remove::<SentrySpanData>() {
+        let SentrySpanData { sentry_span, .. } = match extensions.remove::<SentrySpanData>() {
             Some(data) => data,
             None => return,
         };
 
         sentry_span.finish();
-        sentry_core::configure_scope(|scope| scope.set_span(parent_sentry_span));
     }
 
     /// Implement the writing of extra data to span


Test patch:
diff --git a/sentry-tracing/tests/tracing.rs b/sentry-tracing/tests/tracing.rs
new file mode 100644
index 0000000..c77183c
--- /dev/null
+++ b/sentry-tracing/tests/tracing.rs
@@ -0,0 +1,76 @@
+use sentry::{ClientOptions, Hub};
+use sentry_core::test::TestTransport;
+
+use std::sync::Arc;
+
+fn init_sentry() -> Arc<TestTransport> {
+    use tracing_subscriber::prelude::*;
+
+    let transport = TestTransport::new();
+    let options = ClientOptions {
+        dsn: Some("https://test@sentry-tracing.com/test".parse().unwrap()),
+        transport: Some(Arc::new(transport.clone())),
+        sample_rate: 1.0,
+        traces_sample_rate: 1.0,
+        ..ClientOptions::default()
+    };
+    Hub::current().bind_client(Some(Arc::new(options.into())));
+
+    let _ = tracing_subscriber::registry()
+        .with(sentry_tracing::layer().enable_span_attributes())
+        .try_init();
+
+    transport
+}
+
+#[tracing::instrument(fields(tags.tag = "key", not_tag = "value"))]
+fn function_with_tags(value: i32) {
+    tracing::error!(value, "event");
+}
+
+#[test]
+fn should_instrument_function_with_event() {
+    let transport = init_sentry();
+
+    function_with_tags(1);
+
+    let data = transport.fetch_and_clear_envelopes();
+    assert_eq!(data.len(), 2);
+    let event = data.first().expect("should have 1 event");
+    let event = match event.items().next().unwrap() {
+        sentry::protocol::EnvelopeItem::Event(event) => event,
+        unexpected => panic!("Expected event, but got {:#?}", unexpected),
+    };
+
+    //Validate transaction is created
+    let trace = match event.contexts.get("trace").expect("to get 'trace' context") {
+        sentry::protocol::Context::Trace(trace) => trace,
+        unexpected => panic!("Expected trace context but got {:?}", unexpected),
+    };
+    assert_eq!(trace.op.as_deref().unwrap(), "function_with_tags");
+
+    //Confirm transaction values
+    let transaction = data.get(1).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.tags.len(), 1);
+    assert_eq!(transaction.extra.len(), 2);
+
+    let tag = transaction
+        .tags
+        .get("tag")
+        .expect("to have tag with name 'tag'");
+    assert_eq!(tag, "key");
+    let not_tag = transaction
+        .extra
+        .get("not_tag")
+        .expect("to have extra with name 'not_tag'");
+    assert_eq!(not_tag, "value");
+    let value = transaction
+        .extra
+        .get("value")
+        .expect("to have extra with name 'value'");
+    assert_eq!(value, 1);
+}