From d4cbe804492743d9c44f762153013db9b935e7da Mon Sep 17 00:00:00 2001 From: Neel Shah Date: Tue, 3 Sep 2024 13:33:30 +0200 Subject: [PATCH 1/5] Fix some methods in POTelSpan * Use `ReadableSpan` checks for attribute and parent access (handles the `NonRecordingSpan` case as a result) * deleted methods `continue_from_environ`, `continue_from_headers` and `from_traceparent` from `POTelSpan`, the new interface will not support them, but we might shim them later to not break code from outside the otel stuff * replaced `continue_from_headers` with `continue_trace` in grpc --- sentry_sdk/integrations/grpc/aio/server.py | 40 +++++----- sentry_sdk/integrations/grpc/server.py | 26 +++--- sentry_sdk/tracing.py | 92 +++++++--------------- 3 files changed, 61 insertions(+), 97 deletions(-) diff --git a/sentry_sdk/integrations/grpc/aio/server.py b/sentry_sdk/integrations/grpc/aio/server.py index addc6bee36..6d38e91363 100644 --- a/sentry_sdk/integrations/grpc/aio/server.py +++ b/sentry_sdk/integrations/grpc/aio/server.py @@ -2,7 +2,7 @@ from sentry_sdk.consts import OP from sentry_sdk.integrations import DidNotEnable from sentry_sdk.integrations.grpc.consts import SPAN_ORIGIN -from sentry_sdk.tracing import Transaction, TRANSACTION_SOURCE_CUSTOM +from sentry_sdk.tracing import TRANSACTION_SOURCE_CUSTOM from sentry_sdk.utils import event_from_exception from typing import TYPE_CHECKING @@ -44,26 +44,24 @@ async def wrapped(request, context): return await handler(request, context) # What if the headers are empty? - transaction = Transaction.continue_from_headers( - dict(context.invocation_metadata()), - op=OP.GRPC_SERVER, - name=name, - source=TRANSACTION_SOURCE_CUSTOM, - origin=SPAN_ORIGIN, - ) - - with sentry_sdk.start_transaction(transaction=transaction): - try: - return await handler.unary_unary(request, context) - except AbortError: - raise - except Exception as exc: - event, hint = event_from_exception( - exc, - mechanism={"type": "grpc", "handled": False}, - ) - sentry_sdk.capture_event(event, hint=hint) - raise + with sentry_sdk.continue_trace(dict(context.invocation_metadata())): + with sentry_sdk.start_transaction( + op=OP.GRPC_SERVER, + name=name, + source=TRANSACTION_SOURCE_CUSTOM, + origin=SPAN_ORIGIN, + ): + try: + return await handler.unary_unary(request, context) + except AbortError: + raise + except Exception as exc: + event, hint = event_from_exception( + exc, + mechanism={"type": "grpc", "handled": False}, + ) + sentry_sdk.capture_event(event, hint=hint) + raise elif not handler.request_streaming and handler.response_streaming: handler_factory = grpc.unary_stream_rpc_method_handler diff --git a/sentry_sdk/integrations/grpc/server.py b/sentry_sdk/integrations/grpc/server.py index a640df5e11..fb123c5ca4 100644 --- a/sentry_sdk/integrations/grpc/server.py +++ b/sentry_sdk/integrations/grpc/server.py @@ -2,7 +2,7 @@ from sentry_sdk.consts import OP from sentry_sdk.integrations import DidNotEnable from sentry_sdk.integrations.grpc.consts import SPAN_ORIGIN -from sentry_sdk.tracing import Transaction, TRANSACTION_SOURCE_CUSTOM +from sentry_sdk.tracing import TRANSACTION_SOURCE_CUSTOM from typing import TYPE_CHECKING @@ -38,19 +38,17 @@ def behavior(request, context): if name: metadata = dict(context.invocation_metadata()) - transaction = Transaction.continue_from_headers( - metadata, - op=OP.GRPC_SERVER, - name=name, - source=TRANSACTION_SOURCE_CUSTOM, - origin=SPAN_ORIGIN, - ) - - with sentry_sdk.start_transaction(transaction=transaction): - try: - return handler.unary_unary(request, context) - except BaseException as e: - raise e + with sentry_sdk.continue_trace(metadata): + with sentry_sdk.start_transaction( + op=OP.GRPC_SERVER, + name=name, + source=TRANSACTION_SOURCE_CUSTOM, + origin=SPAN_ORIGIN, + ): + try: + return handler.unary_unary(request, context) + except BaseException as e: + raise e else: return handler.unary_unary(request, context) diff --git a/sentry_sdk/tracing.py b/sentry_sdk/tracing.py index 66a3a7b7c7..932efc61db 100644 --- a/sentry_sdk/tracing.py +++ b/sentry_sdk/tracing.py @@ -5,6 +5,7 @@ from opentelemetry import trace as otel_trace, context from opentelemetry.trace import format_trace_id, format_span_id from opentelemetry.trace.status import StatusCode +from opentelemetry.sdk.trace import ReadableSpan import sentry_sdk from sentry_sdk.consts import SPANSTATUS, SPANDATA @@ -37,7 +38,6 @@ R = TypeVar("R") import sentry_sdk.profiler - from sentry_sdk.scope import Scope from sentry_sdk._types import ( Event, MeasurementUnit, @@ -1198,7 +1198,6 @@ def __init__( op=None, # type: Optional[str] description=None, # type: Optional[str] status=None, # type: Optional[str] - scope=None, # type: Optional[Scope] start_timestamp=None, # type: Optional[Union[datetime, float]] origin=None, # type: Optional[str] name=None, # type: Optional[str] @@ -1218,10 +1217,9 @@ def __init__( # OTel timestamps have nanosecond precision start_timestamp = convert_to_otel_timestamp(start_timestamp) - # XXX deal with _otel_span being a NonRecordingSpan self._otel_span = tracer.start_span( description or op or "", start_time=start_timestamp - ) # XXX + ) self.origin = origin or DEFAULT_SPAN_ORIGIN self.op = op @@ -1267,12 +1265,18 @@ def __exit__(self, ty, value, tb): # XXX set status to error if unset and an exception occurred? context.detach(self._ctx_token) + def _get_attribute(self, name): + # type: (str) -> Optional[Any] + if not isinstance(self._otel_span, ReadableSpan): + return None + self._otel_span.attributes.get(name) + @property def description(self): # type: () -> Optional[str] from sentry_sdk.integrations.opentelemetry.consts import SentrySpanAttribute - return self._otel_span.attributes.get(SentrySpanAttribute.DESCRIPTION) + return self._get_attribute(SentrySpanAttribute.DESCRIPTION) @description.setter def description(self, value): @@ -1287,7 +1291,7 @@ def origin(self): # type: () -> Optional[str] from sentry_sdk.integrations.opentelemetry.consts import SentrySpanAttribute - return self._otel_span.attributes.get(SentrySpanAttribute.ORIGIN) + return self._get_attribute(SentrySpanAttribute.ORIGIN) @origin.setter def origin(self, value): @@ -1299,7 +1303,7 @@ def origin(self, value): @property def containing_transaction(self): - # type: () -> Optional[Transaction] + # type: () -> Optional[POTelSpan] """ Get the transaction this span is a child of. @@ -1311,16 +1315,6 @@ def containing_transaction(self): ) return self.root_span - @containing_transaction.setter - def containing_transaction(self, value): - # type: (Span) -> None - """ - Set this span's transaction. - .. deprecated:: 3.0.0 - Use :func:`root_span` instead. - """ - pass - @property def root_span(self): # type: () -> Optional[POTelSpan] @@ -1329,21 +1323,22 @@ def root_span(self): # not sure if there's a way to retrieve the parent with pure otel. return None - @root_span.setter - def root_span(self, value): - pass - @property def is_root_span(self): - if isinstance(self._otel_span, otel_trace.NonRecordingSpan): - return False - - return self._otel_span.parent is None + # type: () -> bool + return ( + isinstance(self._otel_span, ReadableSpan) and self._otel_span.parent is None + ) @property def parent_span_id(self): # type: () -> Optional[str] - return self._otel_span.parent if hasattr(self._otel_span, "parent") else None + if ( + not isinstance(self._otel_span, ReadableSpan) + or self._otel_span.parent is None + ): + return None + return format_span_id(self._otel_span.parent.span_id) @property def trace_id(self): @@ -1370,7 +1365,7 @@ def op(self): # type: () -> Optional[str] from sentry_sdk.integrations.opentelemetry.consts import SentrySpanAttribute - return self._otel_span.attributes.get(SentrySpanAttribute.OP) + self._get_attribute(SentrySpanAttribute.OP) @op.setter def op(self, value): @@ -1385,7 +1380,7 @@ def name(self): # type: () -> Optional[str] from sentry_sdk.integrations.opentelemetry.consts import SentrySpanAttribute - return self._otel_span.attributes.get(SentrySpanAttribute.NAME) + self._get_attribute(SentrySpanAttribute.NAME) @name.setter def name(self, value): @@ -1408,6 +1403,9 @@ def source(self, value): @property def start_timestamp(self): # type: () -> Optional[datetime] + if not isinstance(self._otel_span, ReadableSpan): + return None + start_time = self._otel_span.start_time if start_time is None: return None @@ -1421,6 +1419,9 @@ def start_timestamp(self): @property def timestamp(self): # type: () -> Optional[datetime] + if not isinstance(self._otel_span, ReadableSpan): + return None + end_time = self._otel_span.end_time if end_time is None: return None @@ -1432,49 +1433,16 @@ def timestamp(self): return convert_from_otel_timestamp(end_time) def start_child(self, **kwargs): - # type: (str, **Any) -> POTelSpan + # type: (**Any) -> POTelSpan kwargs.setdefault("sampled", self.sampled) span = POTelSpan(**kwargs) return span - @classmethod - def continue_from_environ( - cls, - environ, # type: Mapping[str, str] - **kwargs, # type: Any - ): - # type: (...) -> POTelSpan - # XXX actually propagate - span = POTelSpan(**kwargs) - return span - - @classmethod - def continue_from_headers( - cls, - headers, # type: Mapping[str, str] - **kwargs, # type: Any - ): - # type: (...) -> POTelSpan - # XXX actually propagate - span = POTelSpan(**kwargs) - return span - def iter_headers(self): # type: () -> Iterator[Tuple[str, str]] pass - @classmethod - def from_traceparent( - cls, - traceparent, # type: Optional[str] - **kwargs, # type: Any - ): - # type: (...) -> Optional[Transaction] - # XXX actually propagate - span = POTelSpan(**kwargs) - return span - def to_traceparent(self): # type: () -> str if self.sampled is True: From 75928343845f6e1fdf548271185cbc79ca33b858 Mon Sep 17 00:00:00 2001 From: Neel Shah Date: Tue, 3 Sep 2024 16:53:41 +0200 Subject: [PATCH 2/5] Update sentry_sdk/tracing.py Co-authored-by: Ivana Kellyer --- sentry_sdk/tracing.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sentry_sdk/tracing.py b/sentry_sdk/tracing.py index 932efc61db..a3bf48a4b3 100644 --- a/sentry_sdk/tracing.py +++ b/sentry_sdk/tracing.py @@ -1269,7 +1269,7 @@ def _get_attribute(self, name): # type: (str) -> Optional[Any] if not isinstance(self._otel_span, ReadableSpan): return None - self._otel_span.attributes.get(name) + return self._otel_span.attributes.get(name) @property def description(self): From ce88720b822f23c11e16f7076d3ce0c4490767c0 Mon Sep 17 00:00:00 2001 From: Neel Shah Date: Tue, 3 Sep 2024 16:54:07 +0200 Subject: [PATCH 3/5] Update sentry_sdk/tracing.py Co-authored-by: Ivana Kellyer --- sentry_sdk/tracing.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sentry_sdk/tracing.py b/sentry_sdk/tracing.py index a3bf48a4b3..9b425c75f7 100644 --- a/sentry_sdk/tracing.py +++ b/sentry_sdk/tracing.py @@ -1365,7 +1365,7 @@ def op(self): # type: () -> Optional[str] from sentry_sdk.integrations.opentelemetry.consts import SentrySpanAttribute - self._get_attribute(SentrySpanAttribute.OP) + return self._get_attribute(SentrySpanAttribute.OP) @op.setter def op(self, value): From 545f3a124fe4bc6cadd835f3b11f92c74fd9b028 Mon Sep 17 00:00:00 2001 From: Neel Shah Date: Tue, 3 Sep 2024 16:54:13 +0200 Subject: [PATCH 4/5] Update sentry_sdk/tracing.py Co-authored-by: Ivana Kellyer --- sentry_sdk/tracing.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sentry_sdk/tracing.py b/sentry_sdk/tracing.py index 9b425c75f7..136b4c0c18 100644 --- a/sentry_sdk/tracing.py +++ b/sentry_sdk/tracing.py @@ -1380,7 +1380,7 @@ def name(self): # type: () -> Optional[str] from sentry_sdk.integrations.opentelemetry.consts import SentrySpanAttribute - self._get_attribute(SentrySpanAttribute.NAME) + return self._get_attribute(SentrySpanAttribute.NAME) @name.setter def name(self, value): From 292467e3c940ec65429b61da6e408ba40b6f168a Mon Sep 17 00:00:00 2001 From: Neel Shah Date: Fri, 6 Sep 2024 15:20:15 +0200 Subject: [PATCH 5/5] Add to migration guide --- MIGRATION_GUIDE.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/MIGRATION_GUIDE.md b/MIGRATION_GUIDE.md index a587a6b827..f360820dc3 100644 --- a/MIGRATION_GUIDE.md +++ b/MIGRATION_GUIDE.md @@ -29,6 +29,8 @@ Looking to upgrade from Sentry SDK 2.x to 3.x? Here's a comprehensive list of wh - Utility function `is_auto_session_tracking_enabled()` has been removed. There is no public replacement. There is a private `_is_auto_session_tracking_enabled()` (if you absolutely need this function) It accepts a `scope` parameter instead of the previously used `hub` parameter. - Utility function `is_auto_session_tracking_enabled_scope()` has been removed. There is no public replacement. There is a private `_is_auto_session_tracking_enabled()` (if you absolutely need this function) - Setting `scope.level` has been removed. Use `scope.set_level` instead. +- `span.containing_transaction` has been removed. Use `span.root_span` instead. +- `continue_from_headers`, `continue_from_environ` and `from_traceparent` have been removed, please use top-level API `sentry_sdk.continue_trace` instead.` ### Deprecated