Skip to content

fix(otel): fix span and trace ids for distributed tracing #801

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
May 9, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ An OpenTelemetry integration has been released. Please refer to the changelog en
- build: add `sentry-opentelemetry` to workspace (#789) by @lcian
- docs: update docs including OTEL and other integrations (#790) by @lcian
- fix(otel): fix doctests (#794) by @lcian
- fix(otel): fix span and trace ids for distributed tracing (#801) by @lcian

## 0.37.0

Expand Down
14 changes: 10 additions & 4 deletions sentry-core/src/performance.rs
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,7 @@ impl TransactionContext {
headers: I,
) -> Self {
parse_headers(headers)
.map(|sentry_trace| Self::continue_from_sentry_trace(name, op, &sentry_trace))
.map(|sentry_trace| Self::continue_from_sentry_trace(name, op, &sentry_trace, None))
.unwrap_or_else(|| Self {
name: name.into(),
op: op.into(),
Expand All @@ -192,17 +192,23 @@ impl TransactionContext {
})
}

/// Creates a new Transaction Context based on the provided distributed tracing data.
pub fn continue_from_sentry_trace(name: &str, op: &str, sentry_trace: &SentryTrace) -> Self {
/// Creates a new Transaction Context based on the provided distributed tracing data,
/// optionally creating the `TransactionContext` with the provided `span_id`.
pub fn continue_from_sentry_trace(
name: &str,
op: &str,
sentry_trace: &SentryTrace,
span_id: Option<SpanId>,
) -> Self {
let (trace_id, parent_span_id, sampled) =
(sentry_trace.0, Some(sentry_trace.1), sentry_trace.2);
Self {
name: name.into(),
op: op.into(),
trace_id,
parent_span_id,
span_id: Default::default(),
sampled,
span_id: span_id.unwrap_or_default(),
custom: None,
}
}
Expand Down
38 changes: 25 additions & 13 deletions sentry-opentelemetry/src/processor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
//! setting up both is provided in the [crate-level documentation](../).

use std::collections::HashMap;
use std::sync::{Arc, Mutex};
use std::sync::{Arc, LazyLock, Mutex};
use std::time::SystemTime;

use opentelemetry::global::ObjectSafeSpan;
Expand All @@ -31,12 +31,12 @@ use crate::converters::{
/// to track OTEL spans across start/end calls.
type SpanMap = Arc<Mutex<HashMap<sentry_core::protocol::SpanId, TransactionOrSpan>>>;

static SPAN_MAP: LazyLock<SpanMap> = LazyLock::new(|| Arc::new(Mutex::new(HashMap::new())));

/// An OpenTelemetry SpanProcessor that converts OTEL spans to Sentry spans/transactions and sends
/// them to Sentry.
#[derive(Debug, Clone)]
pub struct SentrySpanProcessor {
span_map: SpanMap,
}
pub struct SentrySpanProcessor {}

impl SentrySpanProcessor {
/// Creates a new `SentrySpanProcessor`.
Expand All @@ -46,10 +46,23 @@ impl SentrySpanProcessor {
// This works as long as all Sentry spans/transactions are managed exclusively through OTEL APIs.
scope.add_event_processor(|mut event| {
get_active_span(|otel_span| {
let (span_id, trace_id) = (
convert_span_id(&otel_span.span_context().span_id()),
convert_trace_id(&otel_span.span_context().trace_id()),
);
let span_map = SPAN_MAP.lock().unwrap();

let Some(sentry_span) =
span_map.get(&convert_span_id(&otel_span.span_context().span_id()))
else {
return;
};

let (span_id, trace_id) = match sentry_span {
TransactionOrSpan::Transaction(transaction) => (
transaction.get_trace_context().span_id,
transaction.get_trace_context().trace_id,
),
TransactionOrSpan::Span(span) => {
(span.get_span_id(), span.get_trace_context().trace_id)
}
};

if let Some(sentry_core::protocol::Context::Trace(trace_context)) =
event.contexts.get_mut("trace")
Expand All @@ -71,9 +84,7 @@ impl SentrySpanProcessor {
Some(event)
});
});
Self {
span_map: Default::default(),
}
Self {}
}
}

Expand All @@ -89,7 +100,7 @@ impl SpanProcessor for SentrySpanProcessor {
let span_id = span.span_context().span_id();
let trace_id = span.span_context().trace_id();

let mut span_map = self.span_map.lock().unwrap();
let mut span_map = SPAN_MAP.lock().unwrap();

let mut span_description = String::new();
let mut span_op = String::new();
Expand Down Expand Up @@ -123,6 +134,7 @@ impl SpanProcessor for SentrySpanProcessor {
span_description,
span_op,
sentry_trace,
Some(convert_span_id(&span_id)),
)
} else {
// start a new trace
Expand All @@ -146,7 +158,7 @@ impl SpanProcessor for SentrySpanProcessor {
fn on_end(&self, data: SpanData) {
let span_id = data.span_context.span_id();

let mut span_map = self.span_map.lock().unwrap();
let mut span_map = SPAN_MAP.lock().unwrap();

let Some(sentry_span) = span_map.remove(&convert_span_id(&span_id)) else {
return;
Expand Down
7 changes: 6 additions & 1 deletion sentry-opentelemetry/tests/creates_distributed_trace.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,13 @@ fn test_creates_distributed_trace() {
});

// Now simulate the second service receiving the headers and continuing the trace
let tracer_provider = SdkTracerProvider::builder()
.with_span_processor(SentrySpanProcessor::new())
.build();
let tracer = tracer_provider.tracer("test_2".to_string());
let propagator = SentryPropagator::new();
let second_service_ctx =
propagator.extract_with_context(&Context::current(), &TestExtractor(&headers));
propagator.extract_with_context(&Context::new(), &TestExtractor(&headers));

// Create a second service span that continues the trace
// We need to use start_with_context here to connect with the previous context
Expand Down