From 4e8d50fa752d228354f076ce18d03a59af17fb02 Mon Sep 17 00:00:00 2001 From: Christian Hartung Date: Wed, 21 Jun 2023 19:13:57 -0300 Subject: [PATCH] fix: fix propagation of OTEL NonRecordingSpan --- .../integrations/opentelemetry/propagator.py | 7 +- .../opentelemetry/span_processor.py | 23 +++--- .../opentelemetry/test_propagator.py | 6 +- .../opentelemetry/test_span_processor.py | 79 +++++++++++++------ 4 files changed, 73 insertions(+), 42 deletions(-) diff --git a/sentry_sdk/integrations/opentelemetry/propagator.py b/sentry_sdk/integrations/opentelemetry/propagator.py index 3e1f696939..51d613878a 100644 --- a/sentry_sdk/integrations/opentelemetry/propagator.py +++ b/sentry_sdk/integrations/opentelemetry/propagator.py @@ -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, @@ -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) diff --git a/sentry_sdk/integrations/opentelemetry/span_processor.py b/sentry_sdk/integrations/opentelemetry/span_processor.py index 9b74d993dc..afcb4dbbb7 100644 --- a/sentry_sdk/integrations/opentelemetry/span_processor.py +++ b/sentry_sdk/integrations/opentelemetry/span_processor.py @@ -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 @@ -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 @@ -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 @@ -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): @@ -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 @@ -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 = ( diff --git a/tests/integrations/opentelemetry/test_propagator.py b/tests/integrations/opentelemetry/test_propagator.py index d3e29707e5..510118f67f 100644 --- a/tests/integrations/opentelemetry/test_propagator.py +++ b/tests/integrations/opentelemetry/test_propagator.py @@ -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", @@ -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( @@ -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( diff --git a/tests/integrations/opentelemetry/test_span_processor.py b/tests/integrations/opentelemetry/test_span_processor.py index 0db2a942a5..6ecd3dddb7 100644 --- a/tests/integrations/opentelemetry/test_span_processor.py +++ b/tests/integrations/opentelemetry/test_span_processor.py @@ -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 = {} @@ -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) @@ -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) @@ -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) @@ -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) @@ -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) @@ -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 = {} @@ -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() @@ -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() @@ -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