Skip to content

Commit

Permalink
Remove Hub from our test suite (#3197)
Browse files Browse the repository at this point in the history
Remove Hub usage from our test suite. We keep the tests that test the hubs/scopes-refactoring until we actually remove the Hub from the public API. Also removing Hub usage from some of our integrations.
  • Loading branch information
antonpirker authored Jun 25, 2024
1 parent 6c7374e commit ac5c8e8
Show file tree
Hide file tree
Showing 18 changed files with 151 additions and 178 deletions.
4 changes: 2 additions & 2 deletions sentry_sdk/integrations/_asgi_common.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import urllib

from sentry_sdk.hub import _should_send_default_pii
from sentry_sdk.scope import should_send_default_pii
from sentry_sdk.integrations._wsgi_common import _filter_headers
from sentry_sdk._types import TYPE_CHECKING

Expand Down Expand Up @@ -101,7 +101,7 @@ def _get_request_data(asgi_scope):
)

client = asgi_scope.get("client")
if client and _should_send_default_pii():
if client and should_send_default_pii():
request_data["env"] = {"REMOTE_ADDR": _get_ip(asgi_scope)}

return request_data
4 changes: 2 additions & 2 deletions sentry_sdk/integrations/gnu_backtrace.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import re

from sentry_sdk.hub import Hub
import sentry_sdk
from sentry_sdk.integrations import Integration
from sentry_sdk.scope import add_global_event_processor
from sentry_sdk.utils import capture_internal_exceptions
Expand Down Expand Up @@ -49,7 +49,7 @@ def process_gnu_backtrace(event, hint):

def _process_gnu_backtrace(event, hint):
# type: (Event, dict[str, Any]) -> Event
if Hub.current.get_integration(GnuBacktraceIntegration) is None:
if sentry_sdk.get_client().get_integration(GnuBacktraceIntegration) is None:
return event

exc_info = hint.get("exc_info", None)
Expand Down
6 changes: 3 additions & 3 deletions sentry_sdk/integrations/wsgi.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
from sentry_sdk._werkzeug import get_host, _get_headers
from sentry_sdk.api import continue_trace
from sentry_sdk.consts import OP
from sentry_sdk.hub import _should_send_default_pii
from sentry_sdk.scope import should_send_default_pii
from sentry_sdk.integrations._wsgi_common import _filter_headers
from sentry_sdk.sessions import (
auto_session_tracking_scope as auto_session_tracking,
Expand Down Expand Up @@ -143,7 +143,7 @@ def _get_environ(environ):
capture (server name, port and remote addr if pii is enabled).
"""
keys = ["SERVER_NAME", "SERVER_PORT"]
if _should_send_default_pii():
if should_send_default_pii():
# make debugging of proxy setup easier. Proxy headers are
# in headers.
keys += ["REMOTE_ADDR"]
Expand Down Expand Up @@ -266,7 +266,7 @@ def event_processor(event, hint):
# if the code below fails halfway through we at least have some data
request_info = event.setdefault("request", {})

if _should_send_default_pii():
if should_send_default_pii():
user_info = event.setdefault("user", {})
if client_ip:
user_info.setdefault("ip_address", client_ip)
Expand Down
10 changes: 4 additions & 6 deletions sentry_sdk/metrics.py
Original file line number Diff line number Diff line change
Expand Up @@ -720,20 +720,18 @@ def _tags_to_dict(tags):

def _get_aggregator():
# type: () -> Optional[MetricsAggregator]
hub = sentry_sdk.Hub.current
client = hub.client
client = sentry_sdk.get_client()
return (
client.metrics_aggregator
if client is not None and client.metrics_aggregator is not None
if client.is_active() and client.metrics_aggregator is not None
else None
)


def _get_aggregator_and_update_tags(key, value, unit, tags):
# type: (str, Optional[MetricValue], MeasurementUnit, Optional[MetricTags]) -> Tuple[Optional[MetricsAggregator], Optional[LocalAggregator], Optional[MetricTags]]
hub = sentry_sdk.Hub.current
client = hub.client
if client is None or client.metrics_aggregator is None:
client = sentry_sdk.get_client()
if not client.is_active() or client.metrics_aggregator is None:
return None, None, tags

updated_tags = dict(tags or ()) # type: Dict[str, MetricTagValue]
Expand Down
44 changes: 1 addition & 43 deletions tests/integrations/celery/test_celery.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
from celery import Celery, VERSION
from celery.bin import worker

from sentry_sdk import Hub, configure_scope, start_transaction, get_current_span
from sentry_sdk import configure_scope, start_transaction, get_current_span
from sentry_sdk.integrations.celery import (
CeleryIntegration,
_wrap_apply_async,
Expand Down Expand Up @@ -60,9 +60,6 @@ def inner(
celery.conf.result_backend = "redis://127.0.0.1:6379"
celery.conf.task_always_eager = False

Hub.main.bind_client(Hub.current.client)
request.addfinalizer(lambda: Hub.main.bind_client(None))

# Once we drop celery 3 we can use the celery_worker fixture
if VERSION < (5,):
worker_fn = worker.worker(app=celery).run
Expand Down Expand Up @@ -302,45 +299,6 @@ def dummy_task(x, y):
assert not events


@pytest.mark.skip(
reason="This tests for a broken rerun in Celery 3. We don't support Celery 3 anymore."
)
def test_broken_prerun(init_celery, connect_signal):
from celery.signals import task_prerun

stack_lengths = []

def crash(*args, **kwargs):
# scope should exist in prerun
stack_lengths.append(len(Hub.current._stack))
1 / 0

# Order here is important to reproduce the bug: In Celery 3, a crashing
# prerun would prevent other preruns from running.

connect_signal(task_prerun, crash)
celery = init_celery()

assert len(Hub.current._stack) == 1

@celery.task(name="dummy_task")
def dummy_task(x, y):
stack_lengths.append(len(Hub.current._stack))
return x / y

if VERSION >= (4,):
dummy_task.delay(2, 2)
else:
with pytest.raises(ZeroDivisionError):
dummy_task.delay(2, 2)

assert len(Hub.current._stack) == 1
if VERSION < (4,):
assert stack_lengths == [2]
else:
assert stack_lengths == [2, 2]


@pytest.mark.xfail(
(4, 2, 0) <= VERSION < (4, 4, 3),
strict=True,
Expand Down
3 changes: 3 additions & 0 deletions tests/integrations/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,9 @@ def inner():
old_capture_event_scope = sentry_sdk.Scope.capture_event

def capture_event_hub(self, event, hint=None, scope=None):
"""
Can be removed when we remove push_scope and the Hub from the SDK.
"""
if hint:
if "exc_info" in hint:
error = hint["exc_info"][1]
Expand Down
22 changes: 19 additions & 3 deletions tests/test_basics.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@

from tests.conftest import patch_start_tracing_child

import sentry_sdk
from sentry_sdk import (
push_scope,
configure_scope,
Expand Down Expand Up @@ -220,7 +221,7 @@ def before_breadcrumb(crumb, hint):
events = capture_events()

monkeypatch.setattr(
Hub.current.client.transport, "record_lost_event", record_lost_event
sentry_sdk.get_client().transport, "record_lost_event", record_lost_event
)

def do_this():
Expand Down Expand Up @@ -269,7 +270,7 @@ def test_option_enable_tracing(
updated_traces_sample_rate,
):
sentry_init(enable_tracing=enable_tracing, traces_sample_rate=traces_sample_rate)
options = Hub.current.client.options
options = sentry_sdk.get_client().options
assert has_tracing_enabled(options) is tracing_enabled
assert options["traces_sample_rate"] == updated_traces_sample_rate

Expand Down Expand Up @@ -311,6 +312,9 @@ def test_push_scope(sentry_init, capture_events):


def test_push_scope_null_client(sentry_init, capture_events):
"""
This test can be removed when we remove push_scope and the Hub from the SDK.
"""
sentry_init()
events = capture_events()

Expand All @@ -331,6 +335,9 @@ def test_push_scope_null_client(sentry_init, capture_events):
)
@pytest.mark.parametrize("null_client", (True, False))
def test_push_scope_callback(sentry_init, null_client, capture_events):
"""
This test can be removed when we remove push_scope and the Hub from the SDK.
"""
sentry_init()

if null_client:
Expand Down Expand Up @@ -439,6 +446,9 @@ def test_integration_scoping(sentry_init, capture_events):
reason="This test is not valid anymore, because with the new Scopes calling bind_client on the Hub sets the client on the global scope. This test should be removed once the Hub is removed"
)
def test_client_initialized_within_scope(sentry_init, caplog):
"""
This test can be removed when we remove push_scope and the Hub from the SDK.
"""
caplog.set_level(logging.WARNING)

sentry_init()
Expand All @@ -455,6 +465,9 @@ def test_client_initialized_within_scope(sentry_init, caplog):
reason="This test is not valid anymore, because with the new Scopes the push_scope just returns the isolation scope. This test should be removed once the Hub is removed"
)
def test_scope_leaks_cleaned_up(sentry_init, caplog):
"""
This test can be removed when we remove push_scope and the Hub from the SDK.
"""
caplog.set_level(logging.WARNING)

sentry_init()
Expand All @@ -475,6 +488,9 @@ def test_scope_leaks_cleaned_up(sentry_init, caplog):
reason="This test is not valid anymore, because with the new Scopes there is not pushing and popping of scopes. This test should be removed once the Hub is removed"
)
def test_scope_popped_too_soon(sentry_init, caplog):
"""
This test can be removed when we remove push_scope and the Hub from the SDK.
"""
caplog.set_level(logging.ERROR)

sentry_init()
Expand Down Expand Up @@ -719,7 +735,7 @@ def test_functions_to_trace_with_class(sentry_init, capture_events):
def test_redis_disabled_when_not_installed(sentry_init):
sentry_init()

assert Hub.current.get_integration(RedisIntegration) is None
assert sentry_sdk.get_client().get_integration(RedisIntegration) is None


def test_multiple_setup_integrations_calls():
Expand Down
19 changes: 14 additions & 5 deletions tests/test_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@

import pytest

import sentry_sdk
from sentry_sdk import (
Hub,
Client,
Expand Down Expand Up @@ -563,7 +564,11 @@ def capture_envelope(self, envelope):


def test_configure_scope_available(sentry_init, request, monkeypatch):
# Test that scope is configured if client is configured
"""
Test that scope is configured if client is configured
This test can be removed once configure_scope and the Hub are removed.
"""
sentry_init()

with configure_scope() as scope:
Expand All @@ -585,7 +590,9 @@ def callback(scope):
def test_client_debug_option_enabled(sentry_init, caplog):
sentry_init(debug=True)

Hub.current._capture_internal_exception((ValueError, ValueError("OK"), None))
sentry_sdk.Scope.get_isolation_scope()._capture_internal_exception(
(ValueError, ValueError("OK"), None)
)
assert "OK" in caplog.text


Expand All @@ -595,7 +602,9 @@ def test_client_debug_option_disabled(with_client, sentry_init, caplog):
if with_client:
sentry_init()

Hub.current._capture_internal_exception((ValueError, ValueError("OK"), None))
sentry_sdk.Scope.get_isolation_scope()._capture_internal_exception(
(ValueError, ValueError("OK"), None)
)
assert "OK" not in caplog.text


Expand Down Expand Up @@ -949,7 +958,7 @@ def test_init_string_types(dsn, sentry_init):
# extra code
sentry_init(dsn)
assert (
Hub.current.client.dsn
sentry_sdk.get_client().dsn
== "http://894b7d594095440f8dfea9b300e6f572@localhost:8000/2"
)

Expand Down Expand Up @@ -1047,7 +1056,7 @@ def test_debug_option(
else:
sentry_init(debug=client_option)

Hub.current._capture_internal_exception(
sentry_sdk.Scope.get_isolation_scope()._capture_internal_exception(
(ValueError, ValueError("something is wrong"), None)
)
if debug_output_expected:
Expand Down
55 changes: 24 additions & 31 deletions tests/test_crons.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
import pytest

import sentry_sdk
from sentry_sdk import Hub, configure_scope, set_level

from sentry_sdk.crons import capture_checkin


Expand Down Expand Up @@ -322,6 +322,8 @@ def test_scope_data_in_checkin(sentry_init, capture_envelopes):
# Optional event keys
"release",
"environment",
"server_name",
"sdk",
# Mandatory check-in specific keys
"check_in_id",
"monitor_slug",
Expand All @@ -330,42 +332,33 @@ def test_scope_data_in_checkin(sentry_init, capture_envelopes):
"duration",
"monitor_config",
"contexts", # an event processor adds this
# TODO: These fields need to be checked if valid for checkin:
"_meta",
"tags",
"extra", # an event processor adds this
"modules",
"server_name",
"sdk",
]

hub = Hub.current
with configure_scope() as scope:
# Add some data to the scope
set_level("warning")
hub.add_breadcrumb(message="test breadcrumb")
scope.set_tag("test_tag", "test_value")
scope.set_extra("test_extra", "test_value")
scope.set_context("test_context", {"test_key": "test_value"})
# Add some data to the scope
sentry_sdk.add_breadcrumb(message="test breadcrumb")
sentry_sdk.set_context("test_context", {"test_key": "test_value"})
sentry_sdk.set_extra("test_extra", "test_value")
sentry_sdk.set_level("warning")
sentry_sdk.set_tag("test_tag", "test_value")

capture_checkin(
monitor_slug="abc123",
check_in_id="112233",
status="ok",
duration=123,
)
capture_checkin(
monitor_slug="abc123",
check_in_id="112233",
status="ok",
duration=123,
)

(envelope,) = envelopes
check_in_event = envelope.items[0].payload.json
(envelope,) = envelopes
check_in_event = envelope.items[0].payload.json

invalid_keys = []
for key in check_in_event.keys():
if key not in valid_keys:
invalid_keys.append(key)
invalid_keys = []
for key in check_in_event.keys():
if key not in valid_keys:
invalid_keys.append(key)

assert len(invalid_keys) == 0, "Unexpected keys found in checkin: {}".format(
invalid_keys
)
assert len(invalid_keys) == 0, "Unexpected keys found in checkin: {}".format(
invalid_keys
)


@pytest.mark.asyncio
Expand Down
Loading

0 comments on commit ac5c8e8

Please sign in to comment.