From cfe647fab0063cc3d2eb803cd5dfa16b608e66cc Mon Sep 17 00:00:00 2001 From: Daniel Szoke Date: Wed, 10 Jul 2024 14:15:23 +0200 Subject: [PATCH] ref(tracing): Remove `Hub` in `Transaction.finish` Rename `Transaction.finish` method's `hub` parameter to `scope` (in a backwards-compatible manner), and update the method so that it is using `Scope` API under the hood as much as possible. Prerequisite for #3265 --- sentry_sdk/tracing.py | 75 ++++++++++++++++++++++++++++---- tests/tracing/test_deprecated.py | 25 +++++++++++ 2 files changed, 92 insertions(+), 8 deletions(-) diff --git a/sentry_sdk/tracing.py b/sentry_sdk/tracing.py index 95a2d3469b..80a38b1e43 100644 --- a/sentry_sdk/tracing.py +++ b/sentry_sdk/tracing.py @@ -1,5 +1,6 @@ import uuid import random +import warnings from datetime import datetime, timedelta, timezone import sentry_sdk @@ -286,13 +287,23 @@ def __init__( self.op = op self.description = description self.status = status - self.hub = hub + self.hub = hub # backwards compatibility self.scope = scope self.origin = origin self._measurements = {} # type: Dict[str, MeasurementValue] self._tags = {} # type: MutableMapping[str, str] self._data = {} # type: Dict[str, Any] self._containing_transaction = containing_transaction + + 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 + if start_timestamp is None: start_timestamp = datetime.now(timezone.utc) elif isinstance(start_timestamp, float): @@ -823,15 +834,57 @@ def containing_transaction(self): # reference. return self - def finish(self, hub=None, end_timestamp=None): - # type: (Optional[Union[sentry_sdk.Hub, sentry_sdk.Scope]], Optional[Union[float, datetime]]) -> Optional[str] + 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] + end_timestamp=None, # type: Optional[Union[float, datetime]] + *, + hub=None, # type: Optional[sentry_sdk.Hub] + ): + # type: (...) -> Optional[str] """Finishes the transaction and sends it to Sentry. All finished spans in the transaction will also be sent to Sentry. - :param hub: The hub to use for this transaction. - If not provided, the current hub will be used. + :param scope: The Scope to use for this transaction. + If not provided, the current Scope will be used. :param end_timestamp: Optional timestamp that should be used as timestamp instead of the current time. + :param hub: The hub to use for this transaction. + This argument is DEPRECATED. Please use the `scope` + parameter, instead. :return: The event ID if the transaction was sent to Sentry, otherwise None. @@ -840,7 +893,13 @@ def finish(self, hub=None, end_timestamp=None): # This transaction is already finished, ignore. return None - hub = hub or self.hub or sentry_sdk.Hub.current + # 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.Scope.get_current_scope() client = sentry_sdk.Scope.get_client() if not client.is_active(): @@ -877,7 +936,7 @@ def finish(self, hub=None, end_timestamp=None): ) self.name = "" - super().finish(hub, end_timestamp) + super().finish(scope, end_timestamp) if not self.sampled: # At this point a `sampled = None` should have already been resolved @@ -930,7 +989,7 @@ def finish(self, hub=None, end_timestamp=None): if metrics_summary: event["_metrics_summary"] = metrics_summary - return hub.capture_event(event) + return scope.capture_event(event) def set_measurement(self, name, value, unit=""): # type: (str, float, MeasurementUnit) -> None diff --git a/tests/tracing/test_deprecated.py b/tests/tracing/test_deprecated.py index ba296350ec..8b7f34b6cb 100644 --- a/tests/tracing/test_deprecated.py +++ b/tests/tracing/test_deprecated.py @@ -1,4 +1,9 @@ +import warnings + import pytest + +import sentry_sdk +import sentry_sdk.tracing from sentry_sdk import start_span from sentry_sdk.tracing import Span @@ -20,3 +25,23 @@ def test_start_span_to_start_transaction(sentry_init, capture_events): assert len(events) == 2 assert events[0]["transaction"] == "/1/" assert events[1]["transaction"] == "/2/" + + +@pytest.mark.parametrize("parameter_value", (sentry_sdk.Hub(), sentry_sdk.Scope())) +def test_passing_hub_parameter_to_transaction_finish(parameter_value): + transaction = sentry_sdk.tracing.Transaction() + with pytest.warns(DeprecationWarning): + transaction.finish(hub=parameter_value) + + +def test_passing_hub_object_to_scope_transaction_finish(): + transaction = sentry_sdk.tracing.Transaction() + with pytest.warns(DeprecationWarning): + transaction.finish(sentry_sdk.Hub()) + + +def test_no_warnings_scope_to_transaction_finish(): + transaction = sentry_sdk.tracing.Transaction() + with warnings.catch_warnings(): + warnings.simplefilter("error") + transaction.finish(sentry_sdk.Scope())