From 646d23a658d89579829ae40946230bd226ed8a4a Mon Sep 17 00:00:00 2001 From: Daniel Szoke <7881302+szokeasaurusrex@users.noreply.github.com> Date: Mon, 22 Jul 2024 15:09:12 +0200 Subject: [PATCH] feat(hub): Emit deprecation warnings from `Hub` API (#3280) `sentry_sdk.Hub` has been deprecated since Sentry SDK version 2.0.0 per our docs; however, we waited with adding deprecation warnings because the SDK itself was still using `Hub` APIs until recently. Since we no longer use `Hub` APIs in the SDK (except in `Hub` APIs which are themselves deprecated), we can now start emitting deprecation warnings. Closes #3265 --- sentry_sdk/hub.py | 39 +++++++++++++++++++-- tests/conftest.py | 12 +++++++ tests/new_scopes_compat/conftest.py | 2 +- tests/profiler/test_transaction_profiler.py | 2 +- tests/test_basics.py | 18 ++++++++++ tests/tracing/test_deprecated.py | 20 ++++++++--- 6 files changed, 85 insertions(+), 8 deletions(-) diff --git a/sentry_sdk/hub.py b/sentry_sdk/hub.py index 47975eee80..d514c168fa 100644 --- a/sentry_sdk/hub.py +++ b/sentry_sdk/hub.py @@ -1,3 +1,4 @@ +import warnings from contextlib import contextmanager from sentry_sdk._compat import with_metaclass @@ -55,6 +56,32 @@ def overload(x): return x +class SentryHubDeprecationWarning(DeprecationWarning): + """ + A custom deprecation warning to inform users that the Hub is deprecated. + """ + + _MESSAGE = ( + "`sentry_sdk.Hub` is deprecated and will be removed in a future major release. " + "Please consult our 1.x to 2.x migration guide for details on how to migrate " + "`Hub` usage to the new API: " + "https://docs.sentry.io/platforms/python/migration/1.x-to-2.x" + ) + + def __init__(self, *_): + # type: (*object) -> None + super().__init__(self._MESSAGE) + + +@contextmanager +def _suppress_hub_deprecation_warning(): + # type: () -> Generator[None, None, None] + """Utility function to suppress deprecation warnings for the Hub.""" + with warnings.catch_warnings(): + warnings.filterwarnings("ignore", category=SentryHubDeprecationWarning) + yield + + _local = ContextVar("sentry_current_hub") @@ -63,9 +90,12 @@ class HubMeta(type): def current(cls): # type: () -> Hub """Returns the current instance of the hub.""" + warnings.warn(SentryHubDeprecationWarning(), stacklevel=2) rv = _local.get(None) if rv is None: - rv = Hub(GLOBAL_HUB) + with _suppress_hub_deprecation_warning(): + # This will raise a deprecation warning; supress it since we already warned above. + rv = Hub(GLOBAL_HUB) _local.set(rv) return rv @@ -73,6 +103,7 @@ def current(cls): def main(cls): # type: () -> Hub """Returns the main instance of the hub.""" + warnings.warn(SentryHubDeprecationWarning(), stacklevel=2) return GLOBAL_HUB @@ -103,6 +134,7 @@ def __init__( scope=None, # type: Optional[Any] ): # type: (...) -> None + warnings.warn(SentryHubDeprecationWarning(), stacklevel=2) current_scope = None @@ -689,7 +721,10 @@ def trace_propagation_meta(self, span=None): ) -GLOBAL_HUB = Hub() +with _suppress_hub_deprecation_warning(): + # Suppress deprecation warning for the Hub here, since we still always + # import this module. + GLOBAL_HUB = Hub() _local.set(GLOBAL_HUB) diff --git a/tests/conftest.py b/tests/conftest.py index 048f8bc140..52e0c75c5c 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -1,6 +1,7 @@ import json import os import socket +import warnings from threading import Thread from contextlib import contextmanager from http.server import BaseHTTPRequestHandler, HTTPServer @@ -561,6 +562,17 @@ def teardown_profiling(): teardown_continuous_profiler() +@pytest.fixture() +def suppress_deprecation_warnings(): + """ + Use this fixture to suppress deprecation warnings in a test. + Useful for testing deprecated SDK features. + """ + with warnings.catch_warnings(): + warnings.simplefilter("ignore", DeprecationWarning) + yield + + class MockServerRequestHandler(BaseHTTPRequestHandler): def do_GET(self): # noqa: N802 # Process an HTTP GET request and return a response with an HTTP 200 status. diff --git a/tests/new_scopes_compat/conftest.py b/tests/new_scopes_compat/conftest.py index 3afcf91704..9f16898dea 100644 --- a/tests/new_scopes_compat/conftest.py +++ b/tests/new_scopes_compat/conftest.py @@ -3,6 +3,6 @@ @pytest.fixture(autouse=True) -def isolate_hub(): +def isolate_hub(suppress_deprecation_warnings): with sentry_sdk.Hub(None): yield diff --git a/tests/profiler/test_transaction_profiler.py b/tests/profiler/test_transaction_profiler.py index d657bec506..142fd7d78c 100644 --- a/tests/profiler/test_transaction_profiler.py +++ b/tests/profiler/test_transaction_profiler.py @@ -817,7 +817,7 @@ def test_profile_processing( assert processed["samples"] == expected["samples"] -def test_hub_backwards_compatibility(): +def test_hub_backwards_compatibility(suppress_deprecation_warnings): hub = sentry_sdk.Hub() with pytest.warns(DeprecationWarning): diff --git a/tests/test_basics.py b/tests/test_basics.py index 52eb5045d8..2c31cfa3ae 100644 --- a/tests/test_basics.py +++ b/tests/test_basics.py @@ -871,3 +871,21 @@ def test_last_event_id_scope(sentry_init): # Should not crash with isolation_scope() as scope: assert scope.last_event_id() is None + + +def test_hub_constructor_deprecation_warning(): + with pytest.warns(sentry_sdk.hub.SentryHubDeprecationWarning): + Hub() + + +def test_hub_current_deprecation_warning(): + with pytest.warns(sentry_sdk.hub.SentryHubDeprecationWarning) as warning_records: + Hub.current + + # Make sure we only issue one deprecation warning + assert len(warning_records) == 1 + + +def test_hub_main_deprecation_warnings(): + with pytest.warns(sentry_sdk.hub.SentryHubDeprecationWarning): + Hub.main diff --git a/tests/tracing/test_deprecated.py b/tests/tracing/test_deprecated.py index 8b7f34b6cb..fb58e43ebf 100644 --- a/tests/tracing/test_deprecated.py +++ b/tests/tracing/test_deprecated.py @@ -27,17 +27,29 @@ def test_start_span_to_start_transaction(sentry_init, capture_events): 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): +@pytest.mark.parametrize( + "parameter_value_getter", + # Use lambda to avoid Hub deprecation warning here (will suppress it in the test) + (lambda: sentry_sdk.Hub(), lambda: sentry_sdk.Scope()), +) +def test_passing_hub_parameter_to_transaction_finish( + suppress_deprecation_warnings, parameter_value_getter +): + parameter_value = parameter_value_getter() transaction = sentry_sdk.tracing.Transaction() with pytest.warns(DeprecationWarning): transaction.finish(hub=parameter_value) -def test_passing_hub_object_to_scope_transaction_finish(): +def test_passing_hub_object_to_scope_transaction_finish(suppress_deprecation_warnings): transaction = sentry_sdk.tracing.Transaction() + + # Do not move the following line under the `with` statement. Otherwise, the Hub.__init__ deprecation + # warning will be confused with the transaction.finish deprecation warning that we are testing. + hub = sentry_sdk.Hub() + with pytest.warns(DeprecationWarning): - transaction.finish(sentry_sdk.Hub()) + transaction.finish(hub) def test_no_warnings_scope_to_transaction_finish():