Skip to content
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

fix: fix propagation of OTEL NonRecordingSpan #2187

Merged
merged 2 commits into from
Jun 22, 2023
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
14 changes: 8 additions & 6 deletions sentry_sdk/integrations/opentelemetry/propagator.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,9 @@
default_setter,
)
from opentelemetry.trace import ( # type: ignore
TraceFlags,
NonRecordingSpan,
SpanContext,
TraceFlags,
)
from sentry_sdk.integrations.opentelemetry.consts import (
SENTRY_BAGGAGE_KEY,
Expand Down Expand Up @@ -90,11 +90,12 @@ def inject(self, carrier, context=None, setter=default_setter):
context = get_current()

current_span = trace.get_current_span(context)
current_span_context = current_span.get_span_context()

if not current_span.context.is_valid:
if not current_span_context.is_valid:
return

span_id = trace.format_span_id(current_span.context.span_id)
span_id = trace.format_span_id(current_span_context.span_id)

span_map = SentrySpanProcessor().otel_span_map
sentry_span = span_map.get(span_id, None)
Expand All @@ -103,9 +104,10 @@ def inject(self, carrier, context=None, setter=default_setter):

setter.set(carrier, SENTRY_TRACE_HEADER_NAME, sentry_span.to_traceparent())

baggage = sentry_span.containing_transaction.get_baggage()
if baggage:
setter.set(carrier, BAGGAGE_HEADER_NAME, baggage.serialize())
if sentry_span.containing_transaction:
baggage = sentry_span.containing_transaction.get_baggage()
if baggage:
setter.set(carrier, BAGGAGE_HEADER_NAME, baggage.serialize())

@property
def fields(self):
Expand Down
23 changes: 12 additions & 11 deletions sentry_sdk/integrations/opentelemetry/span_processor.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,16 +29,15 @@
from urllib3.util import parse_url as urlparse

if TYPE_CHECKING:
from typing import Any
from typing import Dict
from typing import Union
from typing import Any, Dict, Optional, Union

from sentry_sdk._types import Event, Hint

OPEN_TELEMETRY_CONTEXT = "otel"


def link_trace_context_to_error_event(event, otel_span_map):
# type: (Event, Dict[str, Union[Transaction, OTelSpan]]) -> Event
# type: (Event, Dict[str, Union[Transaction, SentrySpan]]) -> Event
hub = Hub.current
if not hub:
return event
Expand Down Expand Up @@ -76,7 +75,7 @@ class SentrySpanProcessor(SpanProcessor): # type: ignore
"""

# The mapping from otel span ids to sentry spans
otel_span_map = {} # type: Dict[str, Union[Transaction, OTelSpan]]
otel_span_map = {} # type: Dict[str, Union[Transaction, SentrySpan]]

def __new__(cls):
# type: () -> SentrySpanProcessor
Expand All @@ -93,7 +92,7 @@ def global_event_processor(event, hint):
return link_trace_context_to_error_event(event, self.otel_span_map)

def on_start(self, otel_span, parent_context=None):
# type: (OTelSpan, SpanContext) -> None
# type: (OTelSpan, Optional[SpanContext]) -> None
hub = Hub.current
if not hub:
return
Expand All @@ -109,7 +108,7 @@ def on_start(self, otel_span, parent_context=None):
if hub.client and hub.client.options["instrumenter"] != INSTRUMENTER.OTEL:
return

if not otel_span.context.is_valid:
if not otel_span.get_span_context().is_valid:
return

if self._is_sentry_span(hub, otel_span):
Expand Down Expand Up @@ -152,10 +151,11 @@ def on_end(self, otel_span):
if hub.client and hub.client.options["instrumenter"] != INSTRUMENTER.OTEL:
return

if not otel_span.context.is_valid:
span_context = otel_span.get_span_context()
if not span_context.is_valid:
return

span_id = format_span_id(otel_span.context.span_id)
span_id = format_span_id(span_context.span_id)
sentry_span = self.otel_span_map.pop(span_id, None)
if not sentry_span:
return
Expand Down Expand Up @@ -211,11 +211,12 @@ def _get_trace_data(self, otel_span, parent_context):
Extracts tracing information from one OTel span and its parent OTel context.
"""
trace_data = {}
span_context = otel_span.get_span_context()

span_id = format_span_id(otel_span.context.span_id)
span_id = format_span_id(span_context.span_id)
trace_data["span_id"] = span_id

trace_id = format_trace_id(otel_span.context.trace_id)
trace_id = format_trace_id(span_context.trace_id)
trace_data["trace_id"] = trace_id

parent_span_id = (
Expand Down
6 changes: 3 additions & 3 deletions tests/integrations/opentelemetry/test_propagator.py
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ def test_inject_empty_otel_span_map():
is_remote=True,
)
span = MagicMock()
span.context = span_context
span.get_span_context.return_value = span_context

with mock.patch(
"sentry_sdk.integrations.opentelemetry.propagator.trace.get_current_span",
Expand Down Expand Up @@ -170,7 +170,7 @@ def test_inject_sentry_span_no_baggage():
is_remote=True,
)
span = MagicMock()
span.context = span_context
span.get_span_context.return_value = span_context

sentry_span = MagicMock()
sentry_span.to_traceparent = mock.Mock(
Expand Down Expand Up @@ -214,7 +214,7 @@ def test_inject_sentry_span_baggage():
is_remote=True,
)
span = MagicMock()
span.context = span_context
span.get_span_context.return_value = span_context

sentry_span = MagicMock()
sentry_span.to_traceparent = mock.Mock(
Expand Down
79 changes: 54 additions & 25 deletions tests/integrations/opentelemetry/test_span_processor.py
Original file line number Diff line number Diff line change
Expand Up @@ -62,9 +62,12 @@ def test_get_otel_context():

def test_get_trace_data_with_span_and_trace():
otel_span = MagicMock()
otel_span.context = MagicMock()
otel_span.context.trace_id = int("1234567890abcdef1234567890abcdef", 16)
otel_span.context.span_id = int("1234567890abcdef", 16)
span_context = SpanContext(
trace_id=int("1234567890abcdef1234567890abcdef", 16),
span_id=int("1234567890abcdef", 16),
is_remote=True,
)
otel_span.get_span_context.return_value = span_context
otel_span.parent = None

parent_context = {}
Expand All @@ -80,9 +83,12 @@ def test_get_trace_data_with_span_and_trace():

def test_get_trace_data_with_span_and_trace_and_parent():
otel_span = MagicMock()
otel_span.context = MagicMock()
otel_span.context.trace_id = int("1234567890abcdef1234567890abcdef", 16)
otel_span.context.span_id = int("1234567890abcdef", 16)
span_context = SpanContext(
trace_id=int("1234567890abcdef1234567890abcdef", 16),
span_id=int("1234567890abcdef", 16),
is_remote=True,
)
otel_span.get_span_context.return_value = span_context
otel_span.parent = MagicMock()
otel_span.parent.span_id = int("abcdef1234567890", 16)

Expand All @@ -99,9 +105,12 @@ def test_get_trace_data_with_span_and_trace_and_parent():

def test_get_trace_data_with_sentry_trace():
otel_span = MagicMock()
otel_span.context = MagicMock()
otel_span.context.trace_id = int("1234567890abcdef1234567890abcdef", 16)
otel_span.context.span_id = int("1234567890abcdef", 16)
span_context = SpanContext(
trace_id=int("1234567890abcdef1234567890abcdef", 16),
span_id=int("1234567890abcdef", 16),
is_remote=True,
)
otel_span.get_span_context.return_value = span_context
otel_span.parent = MagicMock()
otel_span.parent.span_id = int("abcdef1234567890", 16)

Expand Down Expand Up @@ -144,9 +153,12 @@ def test_get_trace_data_with_sentry_trace():

def test_get_trace_data_with_sentry_trace_and_baggage():
otel_span = MagicMock()
otel_span.context = MagicMock()
otel_span.context.trace_id = int("1234567890abcdef1234567890abcdef", 16)
otel_span.context.span_id = int("1234567890abcdef", 16)
span_context = SpanContext(
trace_id=int("1234567890abcdef1234567890abcdef", 16),
span_id=int("1234567890abcdef", 16),
is_remote=True,
)
otel_span.get_span_context.return_value = span_context
otel_span.parent = MagicMock()
otel_span.parent.span_id = int("abcdef1234567890", 16)

Expand Down Expand Up @@ -263,9 +275,12 @@ def test_on_start_transaction():
otel_span = MagicMock()
otel_span.name = "Sample OTel Span"
otel_span.start_time = time.time_ns()
otel_span.context = MagicMock()
otel_span.context.trace_id = int("1234567890abcdef1234567890abcdef", 16)
otel_span.context.span_id = int("1234567890abcdef", 16)
span_context = SpanContext(
trace_id=int("1234567890abcdef1234567890abcdef", 16),
span_id=int("1234567890abcdef", 16),
is_remote=True,
)
otel_span.get_span_context.return_value = span_context
otel_span.parent = MagicMock()
otel_span.parent.span_id = int("abcdef1234567890", 16)

Expand Down Expand Up @@ -305,9 +320,12 @@ def test_on_start_child():
otel_span = MagicMock()
otel_span.name = "Sample OTel Span"
otel_span.start_time = time.time_ns()
otel_span.context = MagicMock()
otel_span.context.trace_id = int("1234567890abcdef1234567890abcdef", 16)
otel_span.context.span_id = int("1234567890abcdef", 16)
span_context = SpanContext(
trace_id=int("1234567890abcdef1234567890abcdef", 16),
span_id=int("1234567890abcdef", 16),
is_remote=True,
)
otel_span.get_span_context.return_value = span_context
otel_span.parent = MagicMock()
otel_span.parent.span_id = int("abcdef1234567890", 16)

Expand Down Expand Up @@ -351,8 +369,12 @@ def test_on_end_no_sentry_span():
otel_span = MagicMock()
otel_span.name = "Sample OTel Span"
otel_span.end_time = time.time_ns()
otel_span.context = MagicMock()
otel_span.context.span_id = int("1234567890abcdef", 16)
span_context = SpanContext(
trace_id=int("1234567890abcdef1234567890abcdef", 16),
span_id=int("1234567890abcdef", 16),
is_remote=True,
)
otel_span.get_span_context.return_value = span_context

span_processor = SentrySpanProcessor()
span_processor.otel_span_map = {}
Expand All @@ -372,8 +394,12 @@ def test_on_end_sentry_transaction():
otel_span = MagicMock()
otel_span.name = "Sample OTel Span"
otel_span.end_time = time.time_ns()
otel_span.context = MagicMock()
otel_span.context.span_id = int("1234567890abcdef", 16)
span_context = SpanContext(
trace_id=int("1234567890abcdef1234567890abcdef", 16),
span_id=int("1234567890abcdef", 16),
is_remote=True,
)
otel_span.get_span_context.return_value = span_context

fake_sentry_span = MagicMock(spec=Transaction)
fake_sentry_span.set_context = MagicMock()
Expand All @@ -398,8 +424,12 @@ def test_on_end_sentry_span():
otel_span = MagicMock()
otel_span.name = "Sample OTel Span"
otel_span.end_time = time.time_ns()
otel_span.context = MagicMock()
otel_span.context.span_id = int("1234567890abcdef", 16)
span_context = SpanContext(
trace_id=int("1234567890abcdef1234567890abcdef", 16),
span_id=int("1234567890abcdef", 16),
is_remote=True,
)
otel_span.get_span_context.return_value = span_context

fake_sentry_span = MagicMock(spec=Span)
fake_sentry_span.set_context = MagicMock()
Expand All @@ -425,7 +455,6 @@ def test_link_trace_context_to_error_event():
"""
fake_client = MagicMock()
fake_client.options = {"instrumenter": "otel"}
fake_client

current_hub = MagicMock()
current_hub.client = fake_client
Expand Down