Skip to content

ref(tracing): Remove Hub in Transaction.finish #3267

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

Merged
merged 2 commits into from
Jul 10, 2024
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
88 changes: 78 additions & 10 deletions sentry_sdk/tracing.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import uuid
import random
import warnings
from datetime import datetime, timedelta, timezone

import sentry_sdk
Expand Down Expand Up @@ -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):
Expand Down Expand Up @@ -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]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The signature of NoOpSpan.finish() probably also needs an update.

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.
Expand All @@ -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():
Expand Down Expand Up @@ -877,7 +936,7 @@ def finish(self, hub=None, end_timestamp=None):
)
self.name = "<unlabeled transaction>"

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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -1146,8 +1205,17 @@ def get_profile_context(self):
# type: () -> Any
return {}

def finish(self, hub=None, end_timestamp=None):
# type: (Optional[Union[sentry_sdk.Hub, sentry_sdk.Scope]], Optional[Union[float, datetime]]) -> Optional[str]
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]
"""
The `hub` parameter is deprecated. Please use the `scope` parameter, instead.
"""
pass

def set_measurement(self, name, value, unit=""):
Expand Down
25 changes: 25 additions & 0 deletions tests/tracing/test_deprecated.py
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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())
Loading