diff --git a/sentry_sdk/api.py b/sentry_sdk/api.py index e56109cbd0..18a2da37f6 100644 --- a/sentry_sdk/api.py +++ b/sentry_sdk/api.py @@ -354,6 +354,13 @@ def start_span( **kwargs, # type: Any ): # type: (...) -> Span + if kwargs.pop("scope", None) is not None: + warnings.warn( + "The `scope` parameter is deprecated, and its value is ignored.", + DeprecationWarning, + stacklevel=2, + ) + return get_current_scope().start_span(**kwargs) diff --git a/sentry_sdk/integrations/langchain.py b/sentry_sdk/integrations/langchain.py index 431fc46bec..81e8a07263 100644 --- a/sentry_sdk/integrations/langchain.py +++ b/sentry_sdk/integrations/langchain.py @@ -123,7 +123,7 @@ def _handle_error(self, run_id, error): span_data = self.span_map[run_id] if not span_data: return - sentry_sdk.capture_exception(error, span_data.span.scope) + sentry_sdk.capture_exception(error) span_data.span.__exit__(None, None, None) del self.span_map[run_id] diff --git a/sentry_sdk/scope.py b/sentry_sdk/scope.py index f346569255..5f1a201980 100644 --- a/sentry_sdk/scope.py +++ b/sentry_sdk/scope.py @@ -1122,32 +1122,36 @@ def start_span(self, instrumenter=INSTRUMENTER.SENTRY, **kwargs): stacklevel=2, ) - with new_scope(): - kwargs.setdefault("scope", self) + if kwargs.pop("scope", None) is not None: + warnings.warn( + "The `scope` parameter is deprecated, and its value is ignored.", + DeprecationWarning, + stacklevel=2, + ) - client = self.get_client() + client = self.get_client() - configuration_instrumenter = client.options["instrumenter"] + configuration_instrumenter = client.options["instrumenter"] - if instrumenter != configuration_instrumenter: - return NoOpSpan() + if instrumenter != configuration_instrumenter: + return NoOpSpan() - # get current span or transaction - span = self.span or self.get_isolation_scope().span + # get current span or transaction + span = self.span or self.get_isolation_scope().span - if span is None: - # New spans get the `trace_id` from the scope - if "trace_id" not in kwargs: - propagation_context = self.get_active_propagation_context() - if propagation_context is not None: - kwargs["trace_id"] = propagation_context.trace_id + if span is None: + # New spans get the `trace_id` from the scope + if "trace_id" not in kwargs: + propagation_context = self.get_active_propagation_context() + if propagation_context is not None: + kwargs["trace_id"] = propagation_context.trace_id - span = Span(**kwargs) - else: - # Children take `trace_id`` from the parent span. - span = span.start_child(**kwargs) + span = Span(**kwargs) + else: + # Children take `trace_id`` from the parent span. + span = span.start_child(**kwargs) - return span + return span def continue_trace( self, environ_or_headers, op=None, name=None, source=None, origin="manual" diff --git a/sentry_sdk/tracing.py b/sentry_sdk/tracing.py index fc40221b9f..95958c41b9 100644 --- a/sentry_sdk/tracing.py +++ b/sentry_sdk/tracing.py @@ -281,6 +281,7 @@ class Span: "name", "_flags", "_flags_capacity", + "scope_manager", ) def __init__( @@ -301,6 +302,24 @@ def __init__( name=None, # type: Optional[str] ): # type: (...) -> None + if hub is not None: + warnings.warn( + "The `hub` parameter is deprecated, and its value is ignored.", + DeprecationWarning, + stacklevel=2, + ) + + del hub + + if scope is not None: + warnings.warn( + "The `scope` parameter is deprecated, and its value is ignored.", + DeprecationWarning, + stacklevel=2, + ) + + del scope + self.trace_id = trace_id or uuid.uuid4().hex self.span_id = span_id or uuid.uuid4().hex[16:] self.parent_span_id = parent_span_id @@ -309,8 +328,6 @@ def __init__( self.op = op self.description = name or description self.status = status - self.hub = hub # backwards compatibility - self.scope = scope self.origin = origin self._measurements = {} # type: Dict[str, MeasurementValue] self._tags = {} # type: MutableMapping[str, str] @@ -319,14 +336,9 @@ def __init__( self._flags = {} # type: Dict[str, bool] self._flags_capacity = 10 - if hub is not None: - warnings.warn( - "The `hub` parameter is deprecated. Please use `scope` instead.", - DeprecationWarning, - stacklevel=2, - ) - - self.scope = self.scope or hub.scope + # Backwards compatibility: allow these values to be get and set (but we don't use them) + self.hub = None # type: Optional[sentry_sdk.Hub] + self.scope = None # type: Optional[sentry_sdk.Scope] if start_timestamp is None: start_timestamp = datetime.now(timezone.utc) @@ -381,21 +393,19 @@ def __repr__(self): def __enter__(self): # type: () -> Span - scope = self.scope or sentry_sdk.get_current_scope() - old_span = scope.span + self.scope_manager = sentry_sdk.new_scope() + scope = self.scope_manager.__enter__() scope.span = self - self._context_manager_state = (scope, old_span) return self def __exit__(self, ty, value, tb): # type: (Optional[Any], Optional[Any], Optional[Any]) -> None - if value is not None and should_be_treated_as_error(ty, value): - self.set_status(SPANSTATUS.INTERNAL_ERROR) - - scope, old_span = self._context_manager_state - del self._context_manager_state - self.finish(scope) - scope.span = old_span + try: + if value is not None and should_be_treated_as_error(ty, value): + self.set_status(SPANSTATUS.INTERNAL_ERROR) + self.finish() + finally: + self.scope_manager.__exit__(ty, value, tb) @property def containing_transaction(self): @@ -429,6 +439,13 @@ def start_child(self, instrumenter=INSTRUMENTER.SENTRY, **kwargs): stacklevel=2, ) + if kwargs.pop("scope", None) is not None: + warnings.warn( + "The `scope` parameter is deprecated, and its value is ignored.", + DeprecationWarning, + stacklevel=2, + ) + configuration_instrumenter = sentry_sdk.get_client().options["instrumenter"] if instrumenter != configuration_instrumenter: @@ -684,8 +701,7 @@ def finish(self, scope=None, end_timestamp=None): except AttributeError: self.timestamp = datetime.now(timezone.utc) - scope = scope or sentry_sdk.get_current_scope() - maybe_create_breadcrumbs_from_span(scope, self) + maybe_create_breadcrumbs_from_span(self) return None @@ -819,6 +835,13 @@ def __init__( # type: ignore[misc] ): # type: (...) -> None + if kwargs.pop("scope", None) is not None: + warnings.warn( + "The `scope` parameter is deprecated, and its value is ignored.", + DeprecationWarning, + stacklevel=2, + ) + super().__init__(**kwargs) self.name = name @@ -906,39 +929,6 @@ def containing_transaction(self): # reference. return self - def _get_scope_from_finish_args( - self, - scope_arg, # type: Optional[Union[sentry_sdk.Scope, sentry_sdk.Hub]] - hub_arg, # type: Optional[Union[sentry_sdk.Scope, sentry_sdk.Hub]] - ): - # type: (...) -> Optional[sentry_sdk.Scope] - """ - Logic to get the scope from the arguments passed to finish. This - function exists for backwards compatibility with the old finish. - - TODO: Remove this function in the next major version. - """ - scope_or_hub = scope_arg - if hub_arg is not None: - warnings.warn( - "The `hub` parameter is deprecated. Please use the `scope` parameter, instead.", - DeprecationWarning, - stacklevel=3, - ) - - scope_or_hub = hub_arg - - if isinstance(scope_or_hub, sentry_sdk.Hub): - warnings.warn( - "Passing a Hub to finish is deprecated. Please pass a Scope, instead.", - DeprecationWarning, - stacklevel=3, - ) - - return scope_or_hub.scope - - return scope_or_hub - def finish( self, scope=None, # type: Optional[sentry_sdk.Scope] @@ -961,17 +951,28 @@ def finish( :return: The event ID if the transaction was sent to Sentry, otherwise None. """ + if hub is not None: + warnings.warn( + "The `hub` parameter is deprecated, and its value is ignored.", + DeprecationWarning, + stacklevel=2, + ) + + del hub + + if scope is not None: + warnings.warn( + "The `scope` parameter is deprecated, and its value is ignored.", + DeprecationWarning, + stacklevel=2, + ) + + del scope + if self.timestamp is not None: # This transaction is already finished, ignore. return None - # For backwards compatibility, we must handle the case where `scope` - # or `hub` could both either be a `Scope` or a `Hub`. - scope = self._get_scope_from_finish_args( - scope, hub - ) # type: Optional[sentry_sdk.Scope] - - scope = scope or self.scope or sentry_sdk.get_current_scope() client = sentry_sdk.get_client() if not client.is_active(): @@ -1008,7 +1009,7 @@ def finish( ) self.name = "" - super().finish(scope, end_timestamp) + super().finish(end_timestamp=end_timestamp) if not self.sampled: # At this point a `sampled = None` should have already been resolved @@ -1067,7 +1068,7 @@ def finish( if metrics_summary: event["_metrics_summary"] = metrics_summary - return scope.capture_event(event) + return sentry_sdk.capture_event(event) def set_measurement(self, name, value, unit=""): # type: (str, float, MeasurementUnit) -> None diff --git a/sentry_sdk/tracing_utils.py b/sentry_sdk/tracing_utils.py index 552f4fd59a..912cdd4bf1 100644 --- a/sentry_sdk/tracing_utils.py +++ b/sentry_sdk/tracing_utils.py @@ -158,10 +158,10 @@ def record_sql_queries( yield span -def maybe_create_breadcrumbs_from_span(scope, span): - # type: (sentry_sdk.Scope, sentry_sdk.tracing.Span) -> None +def maybe_create_breadcrumbs_from_span(span): + # type: (sentry_sdk.tracing.Span) -> None if span.op == OP.DB_REDIS: - scope.add_breadcrumb( + sentry_sdk.add_breadcrumb( message=span.description, type="redis", category="redis", data=span._tags ) @@ -175,14 +175,14 @@ def maybe_create_breadcrumbs_from_span(scope, span): level = "warning" if level: - scope.add_breadcrumb( + sentry_sdk.add_breadcrumb( type="http", category="httplib", data=span._data, level=level ) else: - scope.add_breadcrumb(type="http", category="httplib", data=span._data) + sentry_sdk.add_breadcrumb(type="http", category="httplib", data=span._data) elif span.op == "subprocess": - scope.add_breadcrumb( + sentry_sdk.add_breadcrumb( type="subprocess", category="subprocess", message=span.description, diff --git a/tests/integrations/httpx/test_httpx.py b/tests/integrations/httpx/test_httpx.py index 5a35b68076..5f43da5eb8 100644 --- a/tests/integrations/httpx/test_httpx.py +++ b/tests/integrations/httpx/test_httpx.py @@ -30,14 +30,20 @@ def before_breadcrumb(crumb, hint): events = capture_events() if asyncio.iscoroutinefunction(httpx_client.get): + + async def get_response_and_capture_message(): + response = await httpx_client.get(url) + capture_message("Testing!") + return response + response = asyncio.get_event_loop().run_until_complete( - httpx_client.get(url) + get_response_and_capture_message() ) else: response = httpx_client.get(url) + capture_message("Testing!") assert response.status_code == 200 - capture_message("Testing!") (event,) = events @@ -84,14 +90,20 @@ def test_crumb_capture_client_error( events = capture_events() if asyncio.iscoroutinefunction(httpx_client.get): + + async def get_response_and_capture_message(): + response = await httpx_client.get(url) + capture_message("Testing!") + return response + response = asyncio.get_event_loop().run_until_complete( - httpx_client.get(url) + get_response_and_capture_message() ) else: response = httpx_client.get(url) + capture_message("Testing!") assert response.status_code == status_code - capture_message("Testing!") (event,) = events diff --git a/tests/tracing/test_async_context.py b/tests/tracing/test_async_context.py new file mode 100644 index 0000000000..4f24b8ee30 --- /dev/null +++ b/tests/tracing/test_async_context.py @@ -0,0 +1,48 @@ +import asyncio +import sys + + +import pytest +import sentry_sdk + + +async def concurrent_task(task_id): + with sentry_sdk.start_span(op="task", name=f"concurrent_task_{task_id}") as span: + await asyncio.sleep(0.01) + return span + + +async def parent_task(): + with sentry_sdk.start_span(op="task", name="parent_task") as parent_span: + futures = [concurrent_task(i) for i in range(3)] + spans = await asyncio.gather(*futures) + return parent_span, spans + + +@pytest.mark.skipif( + sys.version_info < (3, 7), + reason="python 3.6 lacks proper contextvar support. Contextvars are needed to prevent scope bleed in async context", +) +@pytest.mark.asyncio +async def test_concurrent_spans_are_children(): + sentry_sdk.init( + debug=True, + traces_sample_rate=1.0, + ) + + with sentry_sdk.start_transaction( + op="test", name="test_transaction" + ) as transaction: + parent_span, child_spans = await parent_task() + + # Verify parent span is a child of the transaction + assert parent_span.parent_span_id == transaction.span_id + # Verify all child spans are children of the parent span + for span in child_spans: + print(span.description) + assert span.parent_span_id == parent_span.span_id + assert span.op == "task" + assert span.description.startswith("concurrent_task_") + + # Verify we have the expected number of child spans + assert len(child_spans) == 3 diff --git a/tests/tracing/test_deprecated.py b/tests/tracing/test_deprecated.py index fb58e43ebf..1ea89a2fe5 100644 --- a/tests/tracing/test_deprecated.py +++ b/tests/tracing/test_deprecated.py @@ -1,5 +1,3 @@ -import warnings - import pytest import sentry_sdk @@ -52,8 +50,7 @@ def test_passing_hub_object_to_scope_transaction_finish(suppress_deprecation_war transaction.finish(hub) -def test_no_warnings_scope_to_transaction_finish(): +def test_warning_scope_to_transaction_finish(): transaction = sentry_sdk.tracing.Transaction() - with warnings.catch_warnings(): - warnings.simplefilter("error") + with pytest.warns(DeprecationWarning): transaction.finish(sentry_sdk.Scope()) diff --git a/tests/tracing/test_misc.py b/tests/tracing/test_misc.py index b954d36e1a..b16fde127c 100644 --- a/tests/tracing/test_misc.py +++ b/tests/tracing/test_misc.py @@ -449,9 +449,8 @@ def test_should_propagate_trace_to_sentry( def test_start_transaction_updates_scope_name_source(sentry_init): sentry_init(traces_sample_rate=1.0) - scope = sentry_sdk.get_current_scope() - with start_transaction(name="foobar", source="route"): + scope = sentry_sdk.get_current_scope() assert scope._transaction == "foobar" assert scope._transaction_info == {"source": "route"}