From f44dec539501a0c556b7768da8c6650d7c56a6cd Mon Sep 17 00:00:00 2001 From: Tony Xiao Date: Mon, 18 Mar 2024 17:08:59 -0400 Subject: [PATCH 01/20] feat(profiling): Add thread data to spans As per getsentry/rfc#75, this adds the thread data to the spans. This will be needed for the continuous profiling mode in #2830. --- sentry_sdk/consts.py | 12 +++++++ sentry_sdk/profiler.py | 70 +++------------------------------------- sentry_sdk/tracing.py | 19 ++++++++++- sentry_sdk/utils.py | 55 +++++++++++++++++++++++++++++++ tests/test_profiler.py | 23 ------------- tests/test_utils.py | 73 ++++++++++++++++++++++++++++++++++++++++++ 6 files changed, 163 insertions(+), 89 deletions(-) diff --git a/sentry_sdk/consts.py b/sentry_sdk/consts.py index 83076c762f..4b23caf465 100644 --- a/sentry_sdk/consts.py +++ b/sentry_sdk/consts.py @@ -191,6 +191,18 @@ class SPANDATA: Example: "http.handler" """ + THREAD_ID = "thread.id" + """ + The thread id within which the span was started. This should be a string. + Example: "7972576320" + """ + + THREAD_NAME = "thread.name" + """ + The thread name within which the span was started. This should be a string. + Example: "MainThread" + """ + class OP: CACHE_GET_ITEM = "cache.get_item" diff --git a/sentry_sdk/profiler.py b/sentry_sdk/profiler.py index ef4868f745..4fa3e481ae 100644 --- a/sentry_sdk/profiler.py +++ b/sentry_sdk/profiler.py @@ -42,6 +42,8 @@ from sentry_sdk.utils import ( capture_internal_exception, filename_for_module, + get_current_thread_meta, + is_gevent, is_valid_sample_rate, logger, nanosecond_time, @@ -126,32 +128,16 @@ try: - from gevent import get_hub as get_gevent_hub # type: ignore - from gevent.monkey import get_original, is_module_patched # type: ignore + from gevent.monkey import get_original # type: ignore from gevent.threadpool import ThreadPool # type: ignore thread_sleep = get_original("time", "sleep") except ImportError: - - def get_gevent_hub(): - # type: () -> Any - return None - thread_sleep = time.sleep - def is_module_patched(*args, **kwargs): - # type: (*Any, **Any) -> bool - # unable to import from gevent means no modules have been patched - return False - ThreadPool = None -def is_gevent(): - # type: () -> bool - return is_module_patched("threading") or is_module_patched("_thread") - - _scheduler = None # type: Optional[Scheduler] # The default sampling frequency to use. This is set at 101 in order to @@ -389,52 +375,6 @@ def get_frame_name(frame): MAX_PROFILE_DURATION_NS = int(3e10) # 30 seconds -def get_current_thread_id(thread=None): - # type: (Optional[threading.Thread]) -> Optional[int] - """ - Try to get the id of the current thread, with various fall backs. - """ - - # if a thread is specified, that takes priority - if thread is not None: - try: - thread_id = thread.ident - if thread_id is not None: - return thread_id - except AttributeError: - pass - - # if the app is using gevent, we should look at the gevent hub first - # as the id there differs from what the threading module reports - if is_gevent(): - gevent_hub = get_gevent_hub() - if gevent_hub is not None: - try: - # this is undocumented, so wrap it in try except to be safe - return gevent_hub.thread_ident - except AttributeError: - pass - - # use the current thread's id if possible - try: - current_thread_id = threading.current_thread().ident - if current_thread_id is not None: - return current_thread_id - except AttributeError: - pass - - # if we can't get the current thread id, fall back to the main thread id - try: - main_thread_id = threading.main_thread().ident - if main_thread_id is not None: - return main_thread_id - except AttributeError: - pass - - # we've tried everything, time to give up - return None - - class Profile(object): def __init__( self, @@ -456,7 +396,7 @@ def __init__( # Various framework integrations are capable of overwriting the active thread id. # If it is set to `None` at the end of the profile, we fall back to the default. - self._default_active_thread_id = get_current_thread_id() or 0 # type: int + self._default_active_thread_id = get_current_thread_meta()[0] or 0 # type: int self.active_thread_id = None # type: Optional[int] try: @@ -479,7 +419,7 @@ def __init__( def update_active_thread_id(self): # type: () -> None - self.active_thread_id = get_current_thread_id() + self.active_thread_id = get_current_thread_meta()[0] logger.debug( "[Profiling] updating active thread id to {tid}".format( tid=self.active_thread_id diff --git a/sentry_sdk/tracing.py b/sentry_sdk/tracing.py index bac1ceaa60..4383e61df8 100644 --- a/sentry_sdk/tracing.py +++ b/sentry_sdk/tracing.py @@ -5,7 +5,12 @@ import sentry_sdk from sentry_sdk.consts import INSTRUMENTER -from sentry_sdk.utils import is_valid_sample_rate, logger, nanosecond_time +from sentry_sdk.utils import ( + get_current_thread_meta, + is_valid_sample_rate, + logger, + nanosecond_time, +) from sentry_sdk._compat import datetime_utcnow, utc_from_timestamp, PY2 from sentry_sdk.consts import SPANDATA from sentry_sdk._types import TYPE_CHECKING @@ -172,6 +177,9 @@ def __init__( self._span_recorder = None # type: Optional[_SpanRecorder] self._local_aggregator = None # type: Optional[LocalAggregator] + thread_id, thread_name = get_current_thread_meta() + self.set_thread(thread_id, thread_name) + # TODO this should really live on the Transaction class rather than the Span # class def init_span_recorder(self, maxlen): @@ -418,6 +426,15 @@ def set_status(self, value): # type: (str) -> None self.status = value + def set_thread(self, thread_id, thread_name): + # type: (Optional[int], Optional[str]) -> None + + if thread_id is not None: + self.set_data(SPANDATA.THREAD_ID, thread_id) + + if thread_name is not None: + self.set_data(SPANDATA.THREAD_NAME, thread_name) + def set_http_status(self, http_status): # type: (int) -> None self.set_tag( diff --git a/sentry_sdk/utils.py b/sentry_sdk/utils.py index 150130a057..b6e3d8e477 100644 --- a/sentry_sdk/utils.py +++ b/sentry_sdk/utils.py @@ -1746,8 +1746,12 @@ def now(): try: + from gevent import get_hub as get_gevent_hub from gevent.monkey import is_module_patched except ImportError: + def get_gevent_hub(): + # type: () -> Any + return None def is_module_patched(*args, **kwargs): # type: (*Any, **Any) -> bool @@ -1758,3 +1762,54 @@ def is_module_patched(*args, **kwargs): def is_gevent(): # type: () -> bool return is_module_patched("threading") or is_module_patched("_thread") + + +def get_current_thread_meta(thread=None): + # type: (Optional[threading.Thread]) -> Tuple[Optional[int], Optional[str]] + """ + Try to get the id of the current thread, with various fall backs. + """ + + # if a thread is specified, that takes priority + if thread is not None: + try: + thread_id = thread.ident + thread_name = thread.name + if thread_id is not None: + return thread_id, thread_name + except AttributeError: + pass + + # if the app is using gevent, we should look at the gevent hub first + # as the id there differs from what the threading module reports + if is_gevent(): + gevent_hub = get_gevent_hub() + if gevent_hub is not None: + try: + # this is undocumented, so wrap it in try except to be safe + return gevent_hub.thread_ident, gevent_hub.name + except AttributeError: + pass + + # use the current thread's id if possible + try: + thread = threading.current_thread() + thread_id = thread.ident + thread_name = thread.name + if thread_id is not None: + return thread_id, thread_name + except AttributeError: + pass + + # if we can't get the current thread id, fall back to the main thread id + try: + thread = threading.main_thread() + thread_id = thread.ident + thread_name = thread.name + if thread_id is not None: + return thread_id, thread_name + except AttributeError: + pass + + # we've tried everything, time to give up + return None, None diff --git a/tests/test_profiler.py b/tests/test_profiler.py index 94659ff02f..f6212f8a57 100644 --- a/tests/test_profiler.py +++ b/tests/test_profiler.py @@ -16,7 +16,6 @@ extract_frame, extract_stack, frame_id, - get_current_thread_id, get_frame_name, setup_profiler, ) @@ -556,28 +555,6 @@ def test_extract_stack_with_cache(frame, depth): assert frame1 is frame2, i -@requires_python_version(3, 3) -def test_get_current_thread_id_explicit_thread(): - results = Queue(maxsize=1) - - def target1(): - pass - - def target2(): - results.put(get_current_thread_id(thread1)) - - thread1 = threading.Thread(target=target1) - thread1.start() - - thread2 = threading.Thread(target=target2) - thread2.start() - - thread2.join() - thread1.join() - - assert thread1.ident == results.get(timeout=1) - - @requires_python_version(3, 3) @requires_gevent def test_get_current_thread_id_gevent_in_thread(): diff --git a/tests/test_utils.py b/tests/test_utils.py index 147064b541..788652807a 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -1,12 +1,15 @@ import pytest import re import sys +import threading from datetime import timedelta from sentry_sdk._compat import duration_in_milliseconds +from sentry_sdk._queue import Queue from sentry_sdk.utils import ( Components, Dsn, + get_current_thread_meta, get_default_release, get_error_message, get_git_revision, @@ -29,6 +32,11 @@ except ImportError: import mock # python < 3.3 +try: + import gevent +except ImportError: + gevent = None + try: # Python 3 FileNotFoundError @@ -607,3 +615,68 @@ def test_default_release_empty_string(): ) def test_duration_in_milliseconds(timedelta, expected_milliseconds): assert duration_in_milliseconds(timedelta) == expected_milliseconds + + +def test_get_current_thread_id_explicit_thread(): + results = Queue(maxsize=1) + + def target1(): + pass + + def target2(): + results.put(get_current_thread_meta(thread1)) + + thread1 = threading.Thread(target=target1) + thread1.start() + + thread2 = threading.Thread(target=target2) + thread2.start() + + thread2.join() + thread1.join() + + assert (thread1.ident, thread1.name) == results.get(timeout=1) + + +@pytest.mark.skipif(gevent is None, reason="gevent not enabled") +def test_get_current_thread_id_gevent_in_thread(): + results = Queue(maxsize=1) + + def target(): + job = gevent.spawn(get_current_thread_meta) + job.join() + results.put(job.value) + + thread = threading.Thread(target=target) + thread.start() + thread.join() + assert (thread.ident, thread.name) == results.get(timeout=1) + + +def test_get_current_thread_id_running_thread(): + results = Queue(maxsize=1) + + def target(): + results.put(get_current_thread_meta()) + + thread = threading.Thread(target=target) + thread.start() + thread.join() + assert (thread.ident, thread.name) == results.get(timeout=1) + + +@pytest.mark.skipif(sys.version_info < (3, 4), reason="threading.main_thread() Not available") +def test_get_current_thread_id_main_thread(): + results = Queue(maxsize=1) + + def target(): + # mock that somehow the current thread doesn't exist + with mock.patch("threading.current_thread", side_effect=[None]): + results.put(get_current_thread_meta()) + + main_thread = threading.main_thread() + + thread = threading.Thread(target=target) + thread.start() + thread.join() + assert (main_thread.ident, main_thread.name) == results.get(timeout=1) From 95331f53e00fb3b91426fe00e096425350e23fac Mon Sep 17 00:00:00 2001 From: Tony Xiao Date: Mon, 18 Mar 2024 17:41:08 -0400 Subject: [PATCH 02/20] fix some tests --- tests/conftest.py | 12 +++++ tests/integrations/boto3/test_s3.py | 13 ++++-- tests/integrations/stdlib/test_subprocess.py | 3 +- tests/test_profiler.py | 46 -------------------- tests/test_utils.py | 8 ++-- 5 files changed, 27 insertions(+), 55 deletions(-) diff --git a/tests/conftest.py b/tests/conftest.py index 85c65462cb..c87111cbf7 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -652,3 +652,15 @@ def patch_start_tracing_child(fake_transaction_is_none=False): return_value=fake_transaction, ): yield fake_start_child + + +class ApproxDict(dict): + def __eq__(self, other): + # For an ApproxDict to equal another dict, the other dict just needs to contain + # all the keys from the ApproxDict with the same values. + # + # The other dict may contain additional keys with any value. + return all(key in other and other[key] == value for key, value in self.items()) + + def __ne__(self, other): + return not self.__eq__(other) diff --git a/tests/integrations/boto3/test_s3.py b/tests/integrations/boto3/test_s3.py index 5812c2c1bb..c2b17797e8 100644 --- a/tests/integrations/boto3/test_s3.py +++ b/tests/integrations/boto3/test_s3.py @@ -4,6 +4,7 @@ from sentry_sdk import Hub from sentry_sdk.integrations.boto3 import Boto3Integration +from tests.conftest import ApproxDict from tests.integrations.boto3.aws_mock import MockResponse from tests.integrations.boto3 import read_fixture @@ -65,12 +66,12 @@ def test_streaming(sentry_init, capture_events): span1 = event["spans"][0] assert span1["op"] == "http.client" assert span1["description"] == "aws.s3.GetObject" - assert span1["data"] == { + assert span1["data"] == ApproxDict({ "http.method": "GET", "aws.request.url": "https://bucket.s3.amazonaws.com/foo.pdf", "http.fragment": "", "http.query": "", - } + }) span2 = event["spans"][1] assert span2["op"] == "http.client.stream" @@ -123,7 +124,11 @@ def test_omit_url_data_if_parsing_fails(sentry_init, capture_events): transaction.finish() (event,) = events - assert event["spans"][0]["data"] == { + assert event["spans"][0]["data"] == ApproxDict({ "http.method": "GET", # no url data - } + }) + + assert "aws.request.url" not in event["spans"][0]["data"] + assert "http.fragment" not in event["spans"][0]["data"] + assert "http.query" not in event["spans"][0]["data"] diff --git a/tests/integrations/stdlib/test_subprocess.py b/tests/integrations/stdlib/test_subprocess.py index 31da043ac3..d61be35fd2 100644 --- a/tests/integrations/stdlib/test_subprocess.py +++ b/tests/integrations/stdlib/test_subprocess.py @@ -8,6 +8,7 @@ from sentry_sdk import capture_message, start_transaction from sentry_sdk._compat import PY2 from sentry_sdk.integrations.stdlib import StdlibIntegration +from tests.conftest import ApproxDict if PY2: @@ -125,7 +126,7 @@ def test_subprocess_basic( assert message_event["message"] == "hi" - data = {"subprocess.cwd": os.getcwd()} if with_cwd else {} + data = ApproxDict({"subprocess.cwd": os.getcwd()} if with_cwd else {}) (crumb,) = message_event["breadcrumbs"]["values"] assert crumb == { diff --git a/tests/test_profiler.py b/tests/test_profiler.py index f6212f8a57..707b9979b7 100644 --- a/tests/test_profiler.py +++ b/tests/test_profiler.py @@ -555,52 +555,6 @@ def test_extract_stack_with_cache(frame, depth): assert frame1 is frame2, i -@requires_python_version(3, 3) -@requires_gevent -def test_get_current_thread_id_gevent_in_thread(): - results = Queue(maxsize=1) - - def target(): - job = gevent.spawn(get_current_thread_id) - job.join() - results.put(job.value) - - thread = threading.Thread(target=target) - thread.start() - thread.join() - assert thread.ident == results.get(timeout=1) - - -@requires_python_version(3, 3) -def test_get_current_thread_id_running_thread(): - results = Queue(maxsize=1) - - def target(): - results.put(get_current_thread_id()) - - thread = threading.Thread(target=target) - thread.start() - thread.join() - assert thread.ident == results.get(timeout=1) - - -@requires_python_version(3, 3) -def test_get_current_thread_id_main_thread(): - results = Queue(maxsize=1) - - def target(): - # mock that somehow the current thread doesn't exist - with mock.patch("threading.current_thread", side_effect=[None]): - results.put(get_current_thread_id()) - - thread_id = threading.main_thread().ident if sys.version_info >= (3, 4) else None - - thread = threading.Thread(target=target) - thread.start() - thread.join() - assert thread_id == results.get(timeout=1) - - def get_scheduler_threads(scheduler): return [thread for thread in threading.enumerate() if thread.name == scheduler.name] diff --git a/tests/test_utils.py b/tests/test_utils.py index 788652807a..5ccafa4d23 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -617,7 +617,7 @@ def test_duration_in_milliseconds(timedelta, expected_milliseconds): assert duration_in_milliseconds(timedelta) == expected_milliseconds -def test_get_current_thread_id_explicit_thread(): +def test_get_current_thread_meta_explicit_thread(): results = Queue(maxsize=1) def target1(): @@ -639,7 +639,7 @@ def target2(): @pytest.mark.skipif(gevent is None, reason="gevent not enabled") -def test_get_current_thread_id_gevent_in_thread(): +def test_get_current_thread_meta_gevent_in_thread(): results = Queue(maxsize=1) def target(): @@ -653,7 +653,7 @@ def target(): assert (thread.ident, thread.name) == results.get(timeout=1) -def test_get_current_thread_id_running_thread(): +def test_get_current_thread_meta_running_thread(): results = Queue(maxsize=1) def target(): @@ -666,7 +666,7 @@ def target(): @pytest.mark.skipif(sys.version_info < (3, 4), reason="threading.main_thread() Not available") -def test_get_current_thread_id_main_thread(): +def test_get_current_thread_meta_main_thread(): results = Queue(maxsize=1) def target(): From 0bcd02f391249faa4f81d62dda0f11eb601d3c2c Mon Sep 17 00:00:00 2001 From: Tony Xiao Date: Mon, 18 Mar 2024 17:58:22 -0400 Subject: [PATCH 03/20] fix more tests --- tests/integrations/celery/test_celery.py | 2 ++ tests/integrations/socket/test_socket.py | 5 +++-- tests/integrations/stdlib/test_httplib.py | 14 +++++++------- 3 files changed, 12 insertions(+), 9 deletions(-) diff --git a/tests/integrations/celery/test_celery.py b/tests/integrations/celery/test_celery.py index 0d44ee992e..c6eb55536c 100644 --- a/tests/integrations/celery/test_celery.py +++ b/tests/integrations/celery/test_celery.py @@ -10,6 +10,7 @@ ) from sentry_sdk._compat import text_type +from tests.conftest import ApproxDict from celery import Celery, VERSION from celery.bin import worker @@ -218,6 +219,7 @@ def dummy_task(x, y): assert execution_event["spans"] == [] assert submission_event["spans"] == [ { + "data": ApproxDict(), "description": "dummy_task", "op": "queue.submit.celery", "parent_span_id": submission_event["contexts"]["trace"]["span_id"], diff --git a/tests/integrations/socket/test_socket.py b/tests/integrations/socket/test_socket.py index 914ba0bf84..e5f0a6e9c9 100644 --- a/tests/integrations/socket/test_socket.py +++ b/tests/integrations/socket/test_socket.py @@ -2,6 +2,7 @@ from sentry_sdk import start_transaction from sentry_sdk.integrations.socket import SocketIntegration +from tests.conftest import ApproxDict def test_getaddrinfo_trace(sentry_init, capture_events): @@ -37,11 +38,11 @@ def test_create_connection_trace(sentry_init, capture_events): assert connect_span["op"] == "socket.connection" assert connect_span["description"] == "example.com:443" - assert connect_span["data"] == { + assert connect_span["data"] == ApproxDict({ "address": ["example.com", 443], "timeout": timeout, "source_address": None, - } + }) assert dns_span["op"] == "socket.dns" assert dns_span["description"] == "example.com:443" diff --git a/tests/integrations/stdlib/test_httplib.py b/tests/integrations/stdlib/test_httplib.py index d50bf42e21..3b5b2736d8 100644 --- a/tests/integrations/stdlib/test_httplib.py +++ b/tests/integrations/stdlib/test_httplib.py @@ -27,7 +27,7 @@ from sentry_sdk.tracing import Transaction from sentry_sdk.integrations.stdlib import StdlibIntegration -from tests.conftest import create_mock_http_server +from tests.conftest import ApproxDict, create_mock_http_server PORT = create_mock_http_server() @@ -46,14 +46,14 @@ def test_crumb_capture(sentry_init, capture_events): assert crumb["type"] == "http" assert crumb["category"] == "httplib" - assert crumb["data"] == { + assert crumb["data"] == ApproxDict({ "url": url, SPANDATA.HTTP_METHOD: "GET", SPANDATA.HTTP_STATUS_CODE: 200, "reason": "OK", SPANDATA.HTTP_FRAGMENT: "", SPANDATA.HTTP_QUERY: "", - } + }) def test_crumb_capture_hint(sentry_init, capture_events): @@ -73,7 +73,7 @@ def before_breadcrumb(crumb, hint): (crumb,) = event["breadcrumbs"]["values"] assert crumb["type"] == "http" assert crumb["category"] == "httplib" - assert crumb["data"] == { + assert crumb["data"] == ApproxDict({ "url": url, SPANDATA.HTTP_METHOD: "GET", SPANDATA.HTTP_STATUS_CODE: 200, @@ -81,7 +81,7 @@ def before_breadcrumb(crumb, hint): "extra": "foo", SPANDATA.HTTP_FRAGMENT: "", SPANDATA.HTTP_QUERY: "", - } + }) def test_empty_realurl(sentry_init): @@ -131,14 +131,14 @@ def test_httplib_misuse(sentry_init, capture_events, request): assert crumb["type"] == "http" assert crumb["category"] == "httplib" - assert crumb["data"] == { + assert crumb["data"] == ApproxDict({ "url": "http://localhost:{}/200".format(PORT), SPANDATA.HTTP_METHOD: "GET", SPANDATA.HTTP_STATUS_CODE: 200, "reason": "OK", SPANDATA.HTTP_FRAGMENT: "", SPANDATA.HTTP_QUERY: "", - } + }) def test_outgoing_trace_headers(sentry_init, monkeypatch): From 77f1f60688f01e8afbb5ad5b39bb927854ed39f8 Mon Sep 17 00:00:00 2001 From: Tony Xiao Date: Mon, 18 Mar 2024 18:12:26 -0400 Subject: [PATCH 04/20] fix even more tests --- tests/integrations/requests/test_requests.py | 13 +++++++++---- tests/integrations/socket/test_socket.py | 8 ++++---- tests/test_scrubber.py | 3 ++- 3 files changed, 15 insertions(+), 9 deletions(-) diff --git a/tests/integrations/requests/test_requests.py b/tests/integrations/requests/test_requests.py index ed5b273712..c56d1d2314 100644 --- a/tests/integrations/requests/test_requests.py +++ b/tests/integrations/requests/test_requests.py @@ -6,6 +6,7 @@ from sentry_sdk import capture_message from sentry_sdk.consts import SPANDATA from sentry_sdk.integrations.stdlib import StdlibIntegration +from tests.conftest import ApproxDict try: from unittest import mock # python 3.3 and above @@ -28,14 +29,14 @@ def test_crumb_capture(sentry_init, capture_events): (crumb,) = event["breadcrumbs"]["values"] assert crumb["type"] == "http" assert crumb["category"] == "httplib" - assert crumb["data"] == { + assert crumb["data"] == ApproxDict({ "url": url, SPANDATA.HTTP_METHOD: "GET", SPANDATA.HTTP_FRAGMENT: "", SPANDATA.HTTP_QUERY: "", SPANDATA.HTTP_STATUS_CODE: response.status_code, "reason": response.reason, - } + }) @pytest.mark.tests_internal_exceptions @@ -56,9 +57,13 @@ def test_omit_url_data_if_parsing_fails(sentry_init, capture_events): capture_message("Testing!") (event,) = events - assert event["breadcrumbs"]["values"][0]["data"] == { + assert event["breadcrumbs"]["values"][0]["data"] == ApproxDict({ SPANDATA.HTTP_METHOD: "GET", SPANDATA.HTTP_STATUS_CODE: response.status_code, "reason": response.reason, # no url related data - } + }) + + assert "url" not in event["breadcrumbs"]["values"][0]["data"] + assert SPANDATA.HTTP_FRAGMENT not in event["breadcrumbs"]["values"][0]["data"] + assert SPANDATA.HTTP_QUERY not in event["breadcrumbs"]["values"][0]["data"] diff --git a/tests/integrations/socket/test_socket.py b/tests/integrations/socket/test_socket.py index e5f0a6e9c9..77cb80122c 100644 --- a/tests/integrations/socket/test_socket.py +++ b/tests/integrations/socket/test_socket.py @@ -17,10 +17,10 @@ def test_getaddrinfo_trace(sentry_init, capture_events): assert span["op"] == "socket.dns" assert span["description"] == "example.com:443" - assert span["data"] == { + assert span["data"] == ApproxDict({ "host": "example.com", "port": 443, - } + }) def test_create_connection_trace(sentry_init, capture_events): @@ -46,7 +46,7 @@ def test_create_connection_trace(sentry_init, capture_events): assert dns_span["op"] == "socket.dns" assert dns_span["description"] == "example.com:443" - assert dns_span["data"] == { + assert dns_span["data"] == ApproxDict({ "host": "example.com", "port": 443, - } + }) diff --git a/tests/test_scrubber.py b/tests/test_scrubber.py index 126bf158d8..4509bbe37f 100644 --- a/tests/test_scrubber.py +++ b/tests/test_scrubber.py @@ -4,6 +4,7 @@ from sentry_sdk import capture_exception, capture_event, start_transaction, start_span from sentry_sdk.utils import event_from_exception from sentry_sdk.scrubber import EventScrubber +from tests.conftest import ApproxDict logger = logging.getLogger(__name__) @@ -121,7 +122,7 @@ def test_span_data_scrubbing(sentry_init, capture_events): span.set_data("datafoo", "databar") (event,) = events - assert event["spans"][0]["data"] == {"password": "[Filtered]", "datafoo": "databar"} + assert event["spans"][0]["data"] == ApproxDict({"password": "[Filtered]", "datafoo": "databar"}) assert event["_meta"]["spans"] == { "0": {"data": {"password": {"": {"rem": [["!config", "s"]]}}}} } From c39c69205e9007d4fe7ae8b97c06d12324f303d0 Mon Sep 17 00:00:00 2001 From: Tony Xiao Date: Mon, 18 Mar 2024 18:19:41 -0400 Subject: [PATCH 05/20] fix another tests --- tests/integrations/asyncpg/test_asyncpg.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tests/integrations/asyncpg/test_asyncpg.py b/tests/integrations/asyncpg/test_asyncpg.py index a839031c3b..8fe7c526e8 100644 --- a/tests/integrations/asyncpg/test_asyncpg.py +++ b/tests/integrations/asyncpg/test_asyncpg.py @@ -34,6 +34,7 @@ from sentry_sdk.consts import SPANDATA from sentry_sdk.tracing_utils import record_sql_queries from sentry_sdk._compat import contextmanager +from tests.conftest import ApproxDict try: from unittest import mock @@ -46,13 +47,13 @@ ) CRUMBS_CONNECT = { "category": "query", - "data": { + "data": ApproxDict({ "db.name": PG_NAME, "db.system": "postgresql", "db.user": PG_USER, "server.address": PG_HOST, "server.port": PG_PORT, - }, + }), "message": "connect", "type": "default", } From 5c6969a1367a784cdc70e378a6beb23b288132a2 Mon Sep 17 00:00:00 2001 From: Tony Xiao Date: Mon, 18 Mar 2024 18:31:57 -0400 Subject: [PATCH 06/20] fix a few more tests --- tests/integrations/grpc/test_grpc.py | 13 +++++++------ tests/integrations/grpc/test_grpc_aio.py | 9 +++++---- .../strawberry/test_strawberry_py3.py | 17 +++++++++-------- 3 files changed, 21 insertions(+), 18 deletions(-) diff --git a/tests/integrations/grpc/test_grpc.py b/tests/integrations/grpc/test_grpc.py index 0813d655ae..3216049e9c 100644 --- a/tests/integrations/grpc/test_grpc.py +++ b/tests/integrations/grpc/test_grpc.py @@ -11,6 +11,7 @@ from sentry_sdk import Hub, start_transaction from sentry_sdk.consts import OP from sentry_sdk.integrations.grpc import GRPCIntegration +from tests.conftest import ApproxDict from tests.integrations.grpc.grpc_test_service_pb2 import gRPCTestMessage from tests.integrations.grpc.grpc_test_service_pb2_grpc import ( gRPCTestServiceServicer, @@ -151,11 +152,11 @@ def test_grpc_client_starts_span(sentry_init, capture_events_forksafe): span["description"] == "unary unary call to /grpc_test_server.gRPCTestService/TestServe" ) - assert span["data"] == { + assert span["data"] == ApproxDict({ "type": "unary unary", "method": "/grpc_test_server.gRPCTestService/TestServe", "code": "OK", - } + }) @pytest.mark.forked @@ -183,10 +184,10 @@ def test_grpc_client_unary_stream_starts_span(sentry_init, capture_events_forksa span["description"] == "unary stream call to /grpc_test_server.gRPCTestService/TestUnaryStream" ) - assert span["data"] == { + assert span["data"] == ApproxDict({ "type": "unary stream", "method": "/grpc_test_server.gRPCTestService/TestUnaryStream", - } + }) # using unittest.mock.Mock not possible because grpc verifies @@ -229,11 +230,11 @@ def test_grpc_client_other_interceptor(sentry_init, capture_events_forksafe): span["description"] == "unary unary call to /grpc_test_server.gRPCTestService/TestServe" ) - assert span["data"] == { + assert span["data"] == ApproxDict({ "type": "unary unary", "method": "/grpc_test_server.gRPCTestService/TestServe", "code": "OK", - } + }) @pytest.mark.forked diff --git a/tests/integrations/grpc/test_grpc_aio.py b/tests/integrations/grpc/test_grpc_aio.py index 0b8571adca..54e1ba0989 100644 --- a/tests/integrations/grpc/test_grpc_aio.py +++ b/tests/integrations/grpc/test_grpc_aio.py @@ -11,6 +11,7 @@ from sentry_sdk import Hub, start_transaction from sentry_sdk.consts import OP from sentry_sdk.integrations.grpc import GRPCIntegration +from tests.conftests import ApproxDict from tests.integrations.grpc.grpc_test_service_pb2 import gRPCTestMessage from tests.integrations.grpc.grpc_test_service_pb2_grpc import ( gRPCTestServiceServicer, @@ -161,11 +162,11 @@ async def test_grpc_client_starts_span( span["description"] == "unary unary call to /grpc_test_server.gRPCTestService/TestServe" ) - assert span["data"] == { + assert span["data"] == ApproxDict({ "type": "unary unary", "method": "/grpc_test_server.gRPCTestService/TestServe", "code": "OK", - } + }) @pytest.mark.asyncio @@ -190,10 +191,10 @@ async def test_grpc_client_unary_stream_starts_span( span["description"] == "unary stream call to /grpc_test_server.gRPCTestService/TestUnaryStream" ) - assert span["data"] == { + assert span["data"] == ApproxDict({ "type": "unary stream", "method": "/grpc_test_server.gRPCTestService/TestUnaryStream", - } + }) @pytest.mark.asyncio diff --git a/tests/integrations/strawberry/test_strawberry_py3.py b/tests/integrations/strawberry/test_strawberry_py3.py index b357779461..c468423109 100644 --- a/tests/integrations/strawberry/test_strawberry_py3.py +++ b/tests/integrations/strawberry/test_strawberry_py3.py @@ -25,6 +25,7 @@ SentryAsyncExtension, SentrySyncExtension, ) +from tests.conftest import ApproxDict parameterize_strawberry_test = pytest.mark.parametrize( @@ -351,12 +352,12 @@ def test_capture_transaction_on_error( resolve_span = resolve_spans[0] assert resolve_span["parent_span_id"] == query_span["span_id"] assert resolve_span["description"] == "resolving Query.error" - assert resolve_span["data"] == { + assert resolve_span["data"] == ApproxDict({ "graphql.field_name": "error", "graphql.parent_type": "Query", "graphql.field_path": "Query.error", "graphql.path": "error", - } + }) @parameterize_strawberry_test @@ -429,12 +430,12 @@ def test_capture_transaction_on_success( resolve_span = resolve_spans[0] assert resolve_span["parent_span_id"] == query_span["span_id"] assert resolve_span["description"] == "resolving Query.hello" - assert resolve_span["data"] == { + assert resolve_span["data"] == ApproxDict({ "graphql.field_name": "hello", "graphql.parent_type": "Query", "graphql.field_path": "Query.hello", "graphql.path": "hello", - } + }) @parameterize_strawberry_test @@ -507,12 +508,12 @@ def test_transaction_no_operation_name( resolve_span = resolve_spans[0] assert resolve_span["parent_span_id"] == query_span["span_id"] assert resolve_span["description"] == "resolving Query.hello" - assert resolve_span["data"] == { + assert resolve_span["data"] == ApproxDict({ "graphql.field_name": "hello", "graphql.parent_type": "Query", "graphql.field_path": "Query.hello", "graphql.path": "hello", - } + }) @parameterize_strawberry_test @@ -585,9 +586,9 @@ def test_transaction_mutation( resolve_span = resolve_spans[0] assert resolve_span["parent_span_id"] == query_span["span_id"] assert resolve_span["description"] == "resolving Mutation.change" - assert resolve_span["data"] == { + assert resolve_span["data"] == ApproxDict({ "graphql.field_name": "change", "graphql.parent_type": "Mutation", "graphql.field_path": "Mutation.change", "graphql.path": "change", - } + }) From 86d31a151817a60440c0a45a2ab08386ae77ddd5 Mon Sep 17 00:00:00 2001 From: Tony Xiao Date: Mon, 18 Mar 2024 18:37:48 -0400 Subject: [PATCH 07/20] fix even more tests --- tests/integrations/aiohttp/test_aiohttp.py | 5 +++-- tests/integrations/django/test_basic.py | 12 ++++++------ 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/tests/integrations/aiohttp/test_aiohttp.py b/tests/integrations/aiohttp/test_aiohttp.py index de5cf19f44..61c7c520ae 100644 --- a/tests/integrations/aiohttp/test_aiohttp.py +++ b/tests/integrations/aiohttp/test_aiohttp.py @@ -9,6 +9,7 @@ from sentry_sdk import capture_message, start_transaction from sentry_sdk.integrations.aiohttp import AioHttpIntegration +from tests.conftest import ApproxDict try: from unittest import mock # python 3.3 and above @@ -495,7 +496,7 @@ async def handler(request): crumb = event["breadcrumbs"]["values"][0] assert crumb["type"] == "http" assert crumb["category"] == "httplib" - assert crumb["data"] == { + assert crumb["data"] == ApproxDict({ "url": "http://127.0.0.1:{}/".format(raw_server.port), "http.fragment": "", "http.method": "GET", @@ -503,7 +504,7 @@ async def handler(request): "http.response.status_code": 200, "reason": "OK", "extra": "foo", - } + }) @pytest.mark.asyncio diff --git a/tests/integrations/django/test_basic.py b/tests/integrations/django/test_basic.py index 095657fd8a..8c01c71830 100644 --- a/tests/integrations/django/test_basic.py +++ b/tests/integrations/django/test_basic.py @@ -27,7 +27,7 @@ from sentry_sdk.integrations.django.caching import _get_span_description from sentry_sdk.integrations.executing import ExecutingIntegration from sentry_sdk.tracing import Span -from tests.conftest import unpack_werkzeug_response +from tests.conftest import ApproxDict, unpack_werkzeug_response from tests.integrations.django.myapp.wsgi import application from tests.integrations.django.utils import pytest_mark_django_db_decorator @@ -1237,14 +1237,14 @@ def test_cache_spans_middleware( assert first_event["spans"][0]["description"].startswith( "get views.decorators.cache.cache_header." ) - assert first_event["spans"][0]["data"] == {"cache.hit": False} + assert first_event["spans"][0]["data"] == ApproxDict({"cache.hit": False}) assert len(second_event["spans"]) == 2 assert second_event["spans"][0]["op"] == "cache.get_item" assert second_event["spans"][0]["description"].startswith( "get views.decorators.cache.cache_header." ) - assert second_event["spans"][0]["data"] == {"cache.hit": False} + assert second_event["spans"][0]["data"] == ApproxDict({"cache.hit": False}) assert second_event["spans"][1]["op"] == "cache.get_item" assert second_event["spans"][1]["description"].startswith( @@ -1279,14 +1279,14 @@ def test_cache_spans_decorator(sentry_init, client, capture_events, use_django_c assert first_event["spans"][0]["description"].startswith( "get views.decorators.cache.cache_header." ) - assert first_event["spans"][0]["data"] == {"cache.hit": False} + assert first_event["spans"][0]["data"] == ApproxDict({"cache.hit": False}) assert len(second_event["spans"]) == 2 assert second_event["spans"][0]["op"] == "cache.get_item" assert second_event["spans"][0]["description"].startswith( "get views.decorators.cache.cache_header." ) - assert second_event["spans"][0]["data"] == {"cache.hit": False} + assert second_event["spans"][0]["data"] == ApproxDict({"cache.hit": False}) assert second_event["spans"][1]["op"] == "cache.get_item" assert second_event["spans"][1]["description"].startswith( @@ -1323,7 +1323,7 @@ def test_cache_spans_templatetag( assert first_event["spans"][0]["description"].startswith( "get template.cache.some_identifier." ) - assert first_event["spans"][0]["data"] == {"cache.hit": False} + assert first_event["spans"][0]["data"] == ApproxDict({"cache.hit": False}) assert len(second_event["spans"]) == 1 assert second_event["spans"][0]["op"] == "cache.get_item" From e5019c5cc53d825e4bb1d662510c55468e5ed4ea Mon Sep 17 00:00:00 2001 From: Tony Xiao Date: Mon, 18 Mar 2024 18:59:34 -0400 Subject: [PATCH 08/20] fix import --- tests/integrations/grpc/test_grpc_aio.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/integrations/grpc/test_grpc_aio.py b/tests/integrations/grpc/test_grpc_aio.py index 54e1ba0989..41f8abb3fe 100644 --- a/tests/integrations/grpc/test_grpc_aio.py +++ b/tests/integrations/grpc/test_grpc_aio.py @@ -11,7 +11,7 @@ from sentry_sdk import Hub, start_transaction from sentry_sdk.consts import OP from sentry_sdk.integrations.grpc import GRPCIntegration -from tests.conftests import ApproxDict +from tests.conftest import ApproxDict from tests.integrations.grpc.grpc_test_service_pb2 import gRPCTestMessage from tests.integrations.grpc.grpc_test_service_pb2_grpc import ( gRPCTestServiceServicer, From 079945387988d2c4613fd8b616fa194dde6bf2f0 Mon Sep 17 00:00:00 2001 From: Tony Xiao Date: Tue, 19 Mar 2024 10:06:34 -0400 Subject: [PATCH 09/20] fix a few more tests --- .../test_clickhouse_driver.py | 25 +++++++++++++++++++ tests/integrations/httpx/test_httpx.py | 13 +++++++--- 2 files changed, 34 insertions(+), 4 deletions(-) diff --git a/tests/integrations/clickhouse_driver/test_clickhouse_driver.py b/tests/integrations/clickhouse_driver/test_clickhouse_driver.py index 74a04fac44..b39f722c52 100644 --- a/tests/integrations/clickhouse_driver/test_clickhouse_driver.py +++ b/tests/integrations/clickhouse_driver/test_clickhouse_driver.py @@ -10,6 +10,7 @@ from sentry_sdk import start_transaction, capture_message from sentry_sdk.integrations.clickhouse_driver import ClickhouseDriverIntegration +from tests.conftest import ApproxDict EXPECT_PARAMS_IN_SELECT = True if clickhouse_driver.VERSION < (0, 2, 6): @@ -102,6 +103,9 @@ def test_clickhouse_client_breadcrumbs(sentry_init, capture_events) -> None: if not EXPECT_PARAMS_IN_SELECT: expected_breadcrumbs[-1]["data"].pop("db.params", None) + for crumb in expected_breadcrumbs: + crumb["data"] = ApproxDict(crumb["data"]) + for crumb in event["breadcrumbs"]["values"]: crumb.pop("timestamp", None) @@ -201,6 +205,9 @@ def test_clickhouse_client_breadcrumbs_with_pii(sentry_init, capture_events) -> if not EXPECT_PARAMS_IN_SELECT: expected_breadcrumbs[-1]["data"].pop("db.params", None) + for crumb in expected_breadcrumbs: + crumb["data"] = ApproxDict(crumb["data"]) + for crumb in event["breadcrumbs"]["values"]: crumb.pop("timestamp", None) @@ -313,6 +320,9 @@ def test_clickhouse_client_spans( if not EXPECT_PARAMS_IN_SELECT: expected_spans[-1]["data"].pop("db.params", None) + for span in expected_spans: + span["data"] = ApproxDict(span["data"]) + for span in event["spans"]: span.pop("span_id", None) span.pop("start_timestamp", None) @@ -434,6 +444,9 @@ def test_clickhouse_client_spans_with_pii( if not EXPECT_PARAMS_IN_SELECT: expected_spans[-1]["data"].pop("db.params", None) + for span in expected_spans: + span["data"] = ApproxDict(span["data"]) + for span in event["spans"]: span.pop("span_id", None) span.pop("start_timestamp", None) @@ -529,6 +542,9 @@ def test_clickhouse_dbapi_breadcrumbs(sentry_init, capture_events) -> None: if not EXPECT_PARAMS_IN_SELECT: expected_breadcrumbs[-1]["data"].pop("db.params", None) + for crumb in expected_breadcrumbs: + crumb["data"] = ApproxDict(crumb["data"]) + for crumb in event["breadcrumbs"]["values"]: crumb.pop("timestamp", None) @@ -629,6 +645,9 @@ def test_clickhouse_dbapi_breadcrumbs_with_pii(sentry_init, capture_events) -> N if not EXPECT_PARAMS_IN_SELECT: expected_breadcrumbs[-1]["data"].pop("db.params", None) + for crumb in expected_breadcrumbs: + crumb["data"] = ApproxDict(crumb["data"]) + for crumb in event["breadcrumbs"]["values"]: crumb.pop("timestamp", None) @@ -739,6 +758,9 @@ def test_clickhouse_dbapi_spans(sentry_init, capture_events, capture_envelopes) if not EXPECT_PARAMS_IN_SELECT: expected_spans[-1]["data"].pop("db.params", None) + for span in expected_spans: + span["data"] = ApproxDict(span["data"]) + for span in event["spans"]: span.pop("span_id", None) span.pop("start_timestamp", None) @@ -860,6 +882,9 @@ def test_clickhouse_dbapi_spans_with_pii( if not EXPECT_PARAMS_IN_SELECT: expected_spans[-1]["data"].pop("db.params", None) + for span in expected_spans: + span["data"] = ApproxDict(span["data"]) + for span in event["spans"]: span.pop("span_id", None) span.pop("start_timestamp", None) diff --git a/tests/integrations/httpx/test_httpx.py b/tests/integrations/httpx/test_httpx.py index e141faa282..e68d23e052 100644 --- a/tests/integrations/httpx/test_httpx.py +++ b/tests/integrations/httpx/test_httpx.py @@ -7,6 +7,7 @@ from sentry_sdk import capture_message, start_transaction from sentry_sdk.consts import MATCH_ALL, SPANDATA from sentry_sdk.integrations.httpx import HttpxIntegration +from tests.conftest import ApproxDict try: from unittest import mock # python 3.3 and above @@ -46,7 +47,7 @@ def before_breadcrumb(crumb, hint): crumb = event["breadcrumbs"]["values"][0] assert crumb["type"] == "http" assert crumb["category"] == "httplib" - assert crumb["data"] == { + assert crumb["data"] == ApproxDict({ "url": url, SPANDATA.HTTP_METHOD: "GET", SPANDATA.HTTP_FRAGMENT: "", @@ -54,7 +55,7 @@ def before_breadcrumb(crumb, hint): SPANDATA.HTTP_STATUS_CODE: 200, "reason": "OK", "extra": "foo", - } + }) @pytest.mark.parametrize( @@ -291,9 +292,13 @@ def test_omit_url_data_if_parsing_fails(sentry_init, capture_events): capture_message("Testing!") (event,) = events - assert event["breadcrumbs"]["values"][0]["data"] == { + assert event["breadcrumbs"]["values"][0]["data"] == ApproxDict({ SPANDATA.HTTP_METHOD: "GET", SPANDATA.HTTP_STATUS_CODE: 200, "reason": "OK", # no url related data - } + }) + + assert "url" not in event["breadcrumbs"]["values"][0]["data"] + assert SPANDATA.HTTP_FRAGMENT not in event["breadcrumbs"]["values"][0]["data"] + assert SPANDATA.HTTP_QUERY not in event["breadcrumbs"]["values"][0]["data"] From 120eeb70fb06956c5351e010eec024be4e57c354 Mon Sep 17 00:00:00 2001 From: Tony Xiao Date: Tue, 19 Mar 2024 10:19:33 -0400 Subject: [PATCH 10/20] fix another test --- tests/integrations/redis/cluster/test_redis_cluster.py | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/tests/integrations/redis/cluster/test_redis_cluster.py b/tests/integrations/redis/cluster/test_redis_cluster.py index 1e1e59e254..a0c5d7826f 100644 --- a/tests/integrations/redis/cluster/test_redis_cluster.py +++ b/tests/integrations/redis/cluster/test_redis_cluster.py @@ -3,6 +3,7 @@ from sentry_sdk.consts import SPANDATA from sentry_sdk.api import start_transaction from sentry_sdk.integrations.redis import RedisIntegration +from tests.conftest import ApproxDict import redis @@ -82,12 +83,12 @@ def test_rediscluster_basic(sentry_init, capture_events, send_default_pii, descr span = spans[-1] assert span["op"] == "db.redis" assert span["description"] == description - assert span["data"] == { + assert span["data"] == ApproxDict({ SPANDATA.DB_SYSTEM: "redis", # ClusterNode converts localhost to 127.0.0.1 SPANDATA.SERVER_ADDRESS: "127.0.0.1", SPANDATA.SERVER_PORT: 6379, - } + }) assert span["tags"] == { "db.operation": "SET", "redis.command": "SET", @@ -125,7 +126,7 @@ def test_rediscluster_pipeline( (span,) = event["spans"] assert span["op"] == "db.redis" assert span["description"] == "redis.pipeline.execute" - assert span["data"] == { + assert span["data"] == ApproxDict({ "redis.commands": { "count": 3, "first_ten": expected_first_ten, @@ -134,7 +135,7 @@ def test_rediscluster_pipeline( # ClusterNode converts localhost to 127.0.0.1 SPANDATA.SERVER_ADDRESS: "127.0.0.1", SPANDATA.SERVER_PORT: 6379, - } + }) assert span["tags"] == { "redis.transaction": False, # For Cluster, this is always False "redis.is_cluster": True, From f7baf774f4ac0e9d0a7b2978c7b1a8f3db17c1a6 Mon Sep 17 00:00:00 2001 From: Tony Xiao Date: Tue, 19 Mar 2024 10:40:32 -0400 Subject: [PATCH 11/20] fix yet another test --- .../redis/asyncio/test_redis_asyncio.py | 5 +++-- .../cluster_asyncio/test_redis_cluster_asyncio.py | 13 +++++++------ 2 files changed, 10 insertions(+), 8 deletions(-) diff --git a/tests/integrations/redis/asyncio/test_redis_asyncio.py b/tests/integrations/redis/asyncio/test_redis_asyncio.py index 7233b8f908..0ff207d373 100644 --- a/tests/integrations/redis/asyncio/test_redis_asyncio.py +++ b/tests/integrations/redis/asyncio/test_redis_asyncio.py @@ -3,6 +3,7 @@ from sentry_sdk import capture_message, start_transaction from sentry_sdk.consts import SPANDATA from sentry_sdk.integrations.redis import RedisIntegration +from tests.conftest import ApproxDict from fakeredis.aioredis import FakeRedis @@ -64,7 +65,7 @@ async def test_async_redis_pipeline( (span,) = event["spans"] assert span["op"] == "db.redis" assert span["description"] == "redis.pipeline.execute" - assert span["data"] == { + assert span["data"] == ApproxDict({ "redis.commands": { "count": 3, "first_ten": expected_first_ten, @@ -75,7 +76,7 @@ async def test_async_redis_pipeline( "host" ), SPANDATA.SERVER_PORT: 6379, - } + }) assert span["tags"] == { "redis.transaction": is_transaction, "redis.is_cluster": False, diff --git a/tests/integrations/redis/cluster_asyncio/test_redis_cluster_asyncio.py b/tests/integrations/redis/cluster_asyncio/test_redis_cluster_asyncio.py index ad78b79e27..dee164ad51 100644 --- a/tests/integrations/redis/cluster_asyncio/test_redis_cluster_asyncio.py +++ b/tests/integrations/redis/cluster_asyncio/test_redis_cluster_asyncio.py @@ -3,6 +3,7 @@ from sentry_sdk import capture_message, start_transaction from sentry_sdk.consts import SPANDATA from sentry_sdk.integrations.redis import RedisIntegration +from tests.conftest import ApproxDict from redis.asyncio import cluster @@ -47,12 +48,12 @@ async def test_async_breadcrumb(sentry_init, capture_events): assert crumb == { "category": "redis", "message": "GET 'foobar'", - "data": { + "data": ApproxDict({ "db.operation": "GET", "redis.key": "foobar", "redis.command": "GET", "redis.is_cluster": True, - }, + }), "timestamp": crumb["timestamp"], "type": "redis", } @@ -82,12 +83,12 @@ async def test_async_basic(sentry_init, capture_events, send_default_pii, descri (span,) = event["spans"] assert span["op"] == "db.redis" assert span["description"] == description - assert span["data"] == { + assert span["data"] == ApproxDict({ SPANDATA.DB_SYSTEM: "redis", # ClusterNode converts localhost to 127.0.0.1 SPANDATA.SERVER_ADDRESS: "127.0.0.1", SPANDATA.SERVER_PORT: 6379, - } + }) assert span["tags"] == { "redis.is_cluster": True, "db.operation": "SET", @@ -126,7 +127,7 @@ async def test_async_redis_pipeline( (span,) = event["spans"] assert span["op"] == "db.redis" assert span["description"] == "redis.pipeline.execute" - assert span["data"] == { + assert span["data"] == ApproxDict({ "redis.commands": { "count": 3, "first_ten": expected_first_ten, @@ -135,7 +136,7 @@ async def test_async_redis_pipeline( # ClusterNode converts localhost to 127.0.0.1 SPANDATA.SERVER_ADDRESS: "127.0.0.1", SPANDATA.SERVER_PORT: 6379, - } + }) assert span["tags"] == { "redis.transaction": False, "redis.is_cluster": True, From dd584fa0e19f4845c01fa9d0999aaf92e4682ab1 Mon Sep 17 00:00:00 2001 From: Tony Xiao Date: Tue, 19 Mar 2024 10:50:38 -0400 Subject: [PATCH 12/20] fix more tests --- .../rediscluster/test_rediscluster.py | 17 +++++++++-------- tests/test_profiler.py | 1 - 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/tests/integrations/rediscluster/test_rediscluster.py b/tests/integrations/rediscluster/test_rediscluster.py index 14d831a647..762f0ddb76 100644 --- a/tests/integrations/rediscluster/test_rediscluster.py +++ b/tests/integrations/rediscluster/test_rediscluster.py @@ -4,6 +4,7 @@ from sentry_sdk.api import start_transaction from sentry_sdk.consts import SPANDATA from sentry_sdk.integrations.redis import RedisIntegration +from tests.conftest import ApproxDict try: from unittest import mock @@ -56,12 +57,12 @@ def test_rediscluster_basic(rediscluster_cls, sentry_init, capture_events): assert crumb == { "category": "redis", "message": "GET 'foobar'", - "data": { + "data": ApproxDict({ "db.operation": "GET", "redis.key": "foobar", "redis.command": "GET", "redis.is_cluster": True, - }, + }), "timestamp": crumb["timestamp"], "type": "redis", } @@ -96,7 +97,7 @@ def test_rediscluster_pipeline( (span,) = event["spans"] assert span["op"] == "db.redis" assert span["description"] == "redis.pipeline.execute" - assert span["data"] == { + assert span["data"] == ApproxDict({ "redis.commands": { "count": 3, "first_ten": expected_first_ten, @@ -105,7 +106,7 @@ def test_rediscluster_pipeline( SPANDATA.DB_NAME: "1", SPANDATA.SERVER_ADDRESS: "localhost", SPANDATA.SERVER_PORT: 63791, - } + }) assert span["tags"] == { "redis.transaction": False, # For Cluster, this is always False "redis.is_cluster": True, @@ -127,12 +128,12 @@ def test_db_connection_attributes_client(sentry_init, capture_events, redisclust (event,) = events (span,) = event["spans"] - assert span["data"] == { + assert span["data"] == ApproxDict({ SPANDATA.DB_SYSTEM: "redis", SPANDATA.DB_NAME: "1", SPANDATA.SERVER_ADDRESS: "localhost", SPANDATA.SERVER_PORT: 63791, - } + }) @pytest.mark.parametrize("rediscluster_cls", rediscluster_classes) @@ -155,7 +156,7 @@ def test_db_connection_attributes_pipeline( (span,) = event["spans"] assert span["op"] == "db.redis" assert span["description"] == "redis.pipeline.execute" - assert span["data"] == { + assert span["data"] == ApproxDict({ "redis.commands": { "count": 1, "first_ten": ["GET 'foo'"], @@ -164,4 +165,4 @@ def test_db_connection_attributes_pipeline( SPANDATA.DB_NAME: "1", SPANDATA.SERVER_ADDRESS: "localhost", SPANDATA.SERVER_PORT: 63791, - } + }) diff --git a/tests/test_profiler.py b/tests/test_profiler.py index 707b9979b7..495dd3f300 100644 --- a/tests/test_profiler.py +++ b/tests/test_profiler.py @@ -21,7 +21,6 @@ ) from sentry_sdk.tracing import Transaction from sentry_sdk._lru_cache import LRUCache -from sentry_sdk._queue import Queue try: from unittest import mock # python 3.3 and above From 88ed33ea1d691f0cb887fd97eff5ef7893249ae1 Mon Sep 17 00:00:00 2001 From: Tony Xiao Date: Tue, 19 Mar 2024 10:58:35 -0400 Subject: [PATCH 13/20] run black --- sentry_sdk/utils.py | 1 + tests/integrations/aiohttp/test_aiohttp.py | 20 +++--- tests/integrations/asyncpg/test_asyncpg.py | 16 +++-- tests/integrations/boto3/test_s3.py | 24 ++++--- tests/integrations/grpc/test_grpc.py | 34 +++++---- tests/integrations/grpc/test_grpc_aio.py | 22 +++--- tests/integrations/httpx/test_httpx.py | 34 +++++---- .../redis/asyncio/test_redis_asyncio.py | 26 +++---- .../redis/cluster/test_redis_cluster.py | 36 +++++----- .../test_redis_cluster_asyncio.py | 50 +++++++------ .../rediscluster/test_rediscluster.py | 72 ++++++++++--------- tests/integrations/requests/test_requests.py | 32 +++++---- tests/integrations/socket/test_socket.py | 32 +++++---- tests/integrations/stdlib/test_httplib.py | 56 ++++++++------- .../strawberry/test_strawberry_py3.py | 56 ++++++++------- tests/test_scrubber.py | 4 +- tests/test_utils.py | 4 +- 17 files changed, 295 insertions(+), 224 deletions(-) diff --git a/sentry_sdk/utils.py b/sentry_sdk/utils.py index b6e3d8e477..3e25821572 100644 --- a/sentry_sdk/utils.py +++ b/sentry_sdk/utils.py @@ -1749,6 +1749,7 @@ def now(): from gevent import get_hub as get_gevent_hub from gevent.monkey import is_module_patched except ImportError: + def get_gevent_hub(): # type: () -> Any return None diff --git a/tests/integrations/aiohttp/test_aiohttp.py b/tests/integrations/aiohttp/test_aiohttp.py index 61c7c520ae..90ca466175 100644 --- a/tests/integrations/aiohttp/test_aiohttp.py +++ b/tests/integrations/aiohttp/test_aiohttp.py @@ -496,15 +496,17 @@ async def handler(request): crumb = event["breadcrumbs"]["values"][0] assert crumb["type"] == "http" assert crumb["category"] == "httplib" - assert crumb["data"] == ApproxDict({ - "url": "http://127.0.0.1:{}/".format(raw_server.port), - "http.fragment": "", - "http.method": "GET", - "http.query": "", - "http.response.status_code": 200, - "reason": "OK", - "extra": "foo", - }) + assert crumb["data"] == ApproxDict( + { + "url": "http://127.0.0.1:{}/".format(raw_server.port), + "http.fragment": "", + "http.method": "GET", + "http.query": "", + "http.response.status_code": 200, + "reason": "OK", + "extra": "foo", + } + ) @pytest.mark.asyncio diff --git a/tests/integrations/asyncpg/test_asyncpg.py b/tests/integrations/asyncpg/test_asyncpg.py index 8fe7c526e8..611d8ea9d9 100644 --- a/tests/integrations/asyncpg/test_asyncpg.py +++ b/tests/integrations/asyncpg/test_asyncpg.py @@ -47,13 +47,15 @@ ) CRUMBS_CONNECT = { "category": "query", - "data": ApproxDict({ - "db.name": PG_NAME, - "db.system": "postgresql", - "db.user": PG_USER, - "server.address": PG_HOST, - "server.port": PG_PORT, - }), + "data": ApproxDict( + { + "db.name": PG_NAME, + "db.system": "postgresql", + "db.user": PG_USER, + "server.address": PG_HOST, + "server.port": PG_PORT, + } + ), "message": "connect", "type": "default", } diff --git a/tests/integrations/boto3/test_s3.py b/tests/integrations/boto3/test_s3.py index c2b17797e8..8c05b72a3e 100644 --- a/tests/integrations/boto3/test_s3.py +++ b/tests/integrations/boto3/test_s3.py @@ -66,12 +66,14 @@ def test_streaming(sentry_init, capture_events): span1 = event["spans"][0] assert span1["op"] == "http.client" assert span1["description"] == "aws.s3.GetObject" - assert span1["data"] == ApproxDict({ - "http.method": "GET", - "aws.request.url": "https://bucket.s3.amazonaws.com/foo.pdf", - "http.fragment": "", - "http.query": "", - }) + assert span1["data"] == ApproxDict( + { + "http.method": "GET", + "aws.request.url": "https://bucket.s3.amazonaws.com/foo.pdf", + "http.fragment": "", + "http.query": "", + } + ) span2 = event["spans"][1] assert span2["op"] == "http.client.stream" @@ -124,10 +126,12 @@ def test_omit_url_data_if_parsing_fails(sentry_init, capture_events): transaction.finish() (event,) = events - assert event["spans"][0]["data"] == ApproxDict({ - "http.method": "GET", - # no url data - }) + assert event["spans"][0]["data"] == ApproxDict( + { + "http.method": "GET", + # no url data + } + ) assert "aws.request.url" not in event["spans"][0]["data"] assert "http.fragment" not in event["spans"][0]["data"] diff --git a/tests/integrations/grpc/test_grpc.py b/tests/integrations/grpc/test_grpc.py index 3216049e9c..3f49c0a0f4 100644 --- a/tests/integrations/grpc/test_grpc.py +++ b/tests/integrations/grpc/test_grpc.py @@ -152,11 +152,13 @@ def test_grpc_client_starts_span(sentry_init, capture_events_forksafe): span["description"] == "unary unary call to /grpc_test_server.gRPCTestService/TestServe" ) - assert span["data"] == ApproxDict({ - "type": "unary unary", - "method": "/grpc_test_server.gRPCTestService/TestServe", - "code": "OK", - }) + assert span["data"] == ApproxDict( + { + "type": "unary unary", + "method": "/grpc_test_server.gRPCTestService/TestServe", + "code": "OK", + } + ) @pytest.mark.forked @@ -184,10 +186,12 @@ def test_grpc_client_unary_stream_starts_span(sentry_init, capture_events_forksa span["description"] == "unary stream call to /grpc_test_server.gRPCTestService/TestUnaryStream" ) - assert span["data"] == ApproxDict({ - "type": "unary stream", - "method": "/grpc_test_server.gRPCTestService/TestUnaryStream", - }) + assert span["data"] == ApproxDict( + { + "type": "unary stream", + "method": "/grpc_test_server.gRPCTestService/TestUnaryStream", + } + ) # using unittest.mock.Mock not possible because grpc verifies @@ -230,11 +234,13 @@ def test_grpc_client_other_interceptor(sentry_init, capture_events_forksafe): span["description"] == "unary unary call to /grpc_test_server.gRPCTestService/TestServe" ) - assert span["data"] == ApproxDict({ - "type": "unary unary", - "method": "/grpc_test_server.gRPCTestService/TestServe", - "code": "OK", - }) + assert span["data"] == ApproxDict( + { + "type": "unary unary", + "method": "/grpc_test_server.gRPCTestService/TestServe", + "code": "OK", + } + ) @pytest.mark.forked diff --git a/tests/integrations/grpc/test_grpc_aio.py b/tests/integrations/grpc/test_grpc_aio.py index 41f8abb3fe..3e21188ec8 100644 --- a/tests/integrations/grpc/test_grpc_aio.py +++ b/tests/integrations/grpc/test_grpc_aio.py @@ -162,11 +162,13 @@ async def test_grpc_client_starts_span( span["description"] == "unary unary call to /grpc_test_server.gRPCTestService/TestServe" ) - assert span["data"] == ApproxDict({ - "type": "unary unary", - "method": "/grpc_test_server.gRPCTestService/TestServe", - "code": "OK", - }) + assert span["data"] == ApproxDict( + { + "type": "unary unary", + "method": "/grpc_test_server.gRPCTestService/TestServe", + "code": "OK", + } + ) @pytest.mark.asyncio @@ -191,10 +193,12 @@ async def test_grpc_client_unary_stream_starts_span( span["description"] == "unary stream call to /grpc_test_server.gRPCTestService/TestUnaryStream" ) - assert span["data"] == ApproxDict({ - "type": "unary stream", - "method": "/grpc_test_server.gRPCTestService/TestUnaryStream", - }) + assert span["data"] == ApproxDict( + { + "type": "unary stream", + "method": "/grpc_test_server.gRPCTestService/TestUnaryStream", + } + ) @pytest.mark.asyncio diff --git a/tests/integrations/httpx/test_httpx.py b/tests/integrations/httpx/test_httpx.py index e68d23e052..c4ca97321c 100644 --- a/tests/integrations/httpx/test_httpx.py +++ b/tests/integrations/httpx/test_httpx.py @@ -47,15 +47,17 @@ def before_breadcrumb(crumb, hint): crumb = event["breadcrumbs"]["values"][0] assert crumb["type"] == "http" assert crumb["category"] == "httplib" - assert crumb["data"] == ApproxDict({ - "url": url, - SPANDATA.HTTP_METHOD: "GET", - SPANDATA.HTTP_FRAGMENT: "", - SPANDATA.HTTP_QUERY: "", - SPANDATA.HTTP_STATUS_CODE: 200, - "reason": "OK", - "extra": "foo", - }) + assert crumb["data"] == ApproxDict( + { + "url": url, + SPANDATA.HTTP_METHOD: "GET", + SPANDATA.HTTP_FRAGMENT: "", + SPANDATA.HTTP_QUERY: "", + SPANDATA.HTTP_STATUS_CODE: 200, + "reason": "OK", + "extra": "foo", + } + ) @pytest.mark.parametrize( @@ -292,12 +294,14 @@ def test_omit_url_data_if_parsing_fails(sentry_init, capture_events): capture_message("Testing!") (event,) = events - assert event["breadcrumbs"]["values"][0]["data"] == ApproxDict({ - SPANDATA.HTTP_METHOD: "GET", - SPANDATA.HTTP_STATUS_CODE: 200, - "reason": "OK", - # no url related data - }) + assert event["breadcrumbs"]["values"][0]["data"] == ApproxDict( + { + SPANDATA.HTTP_METHOD: "GET", + SPANDATA.HTTP_STATUS_CODE: 200, + "reason": "OK", + # no url related data + } + ) assert "url" not in event["breadcrumbs"]["values"][0]["data"] assert SPANDATA.HTTP_FRAGMENT not in event["breadcrumbs"]["values"][0]["data"] diff --git a/tests/integrations/redis/asyncio/test_redis_asyncio.py b/tests/integrations/redis/asyncio/test_redis_asyncio.py index 0ff207d373..4f024a2824 100644 --- a/tests/integrations/redis/asyncio/test_redis_asyncio.py +++ b/tests/integrations/redis/asyncio/test_redis_asyncio.py @@ -65,18 +65,20 @@ async def test_async_redis_pipeline( (span,) = event["spans"] assert span["op"] == "db.redis" assert span["description"] == "redis.pipeline.execute" - assert span["data"] == ApproxDict({ - "redis.commands": { - "count": 3, - "first_ten": expected_first_ten, - }, - SPANDATA.DB_SYSTEM: "redis", - SPANDATA.DB_NAME: "0", - SPANDATA.SERVER_ADDRESS: connection.connection_pool.connection_kwargs.get( - "host" - ), - SPANDATA.SERVER_PORT: 6379, - }) + assert span["data"] == ApproxDict( + { + "redis.commands": { + "count": 3, + "first_ten": expected_first_ten, + }, + SPANDATA.DB_SYSTEM: "redis", + SPANDATA.DB_NAME: "0", + SPANDATA.SERVER_ADDRESS: connection.connection_pool.connection_kwargs.get( + "host" + ), + SPANDATA.SERVER_PORT: 6379, + } + ) assert span["tags"] == { "redis.transaction": is_transaction, "redis.is_cluster": False, diff --git a/tests/integrations/redis/cluster/test_redis_cluster.py b/tests/integrations/redis/cluster/test_redis_cluster.py index a0c5d7826f..a16d66588c 100644 --- a/tests/integrations/redis/cluster/test_redis_cluster.py +++ b/tests/integrations/redis/cluster/test_redis_cluster.py @@ -83,12 +83,14 @@ def test_rediscluster_basic(sentry_init, capture_events, send_default_pii, descr span = spans[-1] assert span["op"] == "db.redis" assert span["description"] == description - assert span["data"] == ApproxDict({ - SPANDATA.DB_SYSTEM: "redis", - # ClusterNode converts localhost to 127.0.0.1 - SPANDATA.SERVER_ADDRESS: "127.0.0.1", - SPANDATA.SERVER_PORT: 6379, - }) + assert span["data"] == ApproxDict( + { + SPANDATA.DB_SYSTEM: "redis", + # ClusterNode converts localhost to 127.0.0.1 + SPANDATA.SERVER_ADDRESS: "127.0.0.1", + SPANDATA.SERVER_PORT: 6379, + } + ) assert span["tags"] == { "db.operation": "SET", "redis.command": "SET", @@ -126,16 +128,18 @@ def test_rediscluster_pipeline( (span,) = event["spans"] assert span["op"] == "db.redis" assert span["description"] == "redis.pipeline.execute" - assert span["data"] == ApproxDict({ - "redis.commands": { - "count": 3, - "first_ten": expected_first_ten, - }, - SPANDATA.DB_SYSTEM: "redis", - # ClusterNode converts localhost to 127.0.0.1 - SPANDATA.SERVER_ADDRESS: "127.0.0.1", - SPANDATA.SERVER_PORT: 6379, - }) + assert span["data"] == ApproxDict( + { + "redis.commands": { + "count": 3, + "first_ten": expected_first_ten, + }, + SPANDATA.DB_SYSTEM: "redis", + # ClusterNode converts localhost to 127.0.0.1 + SPANDATA.SERVER_ADDRESS: "127.0.0.1", + SPANDATA.SERVER_PORT: 6379, + } + ) assert span["tags"] == { "redis.transaction": False, # For Cluster, this is always False "redis.is_cluster": True, diff --git a/tests/integrations/redis/cluster_asyncio/test_redis_cluster_asyncio.py b/tests/integrations/redis/cluster_asyncio/test_redis_cluster_asyncio.py index dee164ad51..a6d8962afe 100644 --- a/tests/integrations/redis/cluster_asyncio/test_redis_cluster_asyncio.py +++ b/tests/integrations/redis/cluster_asyncio/test_redis_cluster_asyncio.py @@ -48,12 +48,14 @@ async def test_async_breadcrumb(sentry_init, capture_events): assert crumb == { "category": "redis", "message": "GET 'foobar'", - "data": ApproxDict({ - "db.operation": "GET", - "redis.key": "foobar", - "redis.command": "GET", - "redis.is_cluster": True, - }), + "data": ApproxDict( + { + "db.operation": "GET", + "redis.key": "foobar", + "redis.command": "GET", + "redis.is_cluster": True, + } + ), "timestamp": crumb["timestamp"], "type": "redis", } @@ -83,12 +85,14 @@ async def test_async_basic(sentry_init, capture_events, send_default_pii, descri (span,) = event["spans"] assert span["op"] == "db.redis" assert span["description"] == description - assert span["data"] == ApproxDict({ - SPANDATA.DB_SYSTEM: "redis", - # ClusterNode converts localhost to 127.0.0.1 - SPANDATA.SERVER_ADDRESS: "127.0.0.1", - SPANDATA.SERVER_PORT: 6379, - }) + assert span["data"] == ApproxDict( + { + SPANDATA.DB_SYSTEM: "redis", + # ClusterNode converts localhost to 127.0.0.1 + SPANDATA.SERVER_ADDRESS: "127.0.0.1", + SPANDATA.SERVER_PORT: 6379, + } + ) assert span["tags"] == { "redis.is_cluster": True, "db.operation": "SET", @@ -127,16 +131,18 @@ async def test_async_redis_pipeline( (span,) = event["spans"] assert span["op"] == "db.redis" assert span["description"] == "redis.pipeline.execute" - assert span["data"] == ApproxDict({ - "redis.commands": { - "count": 3, - "first_ten": expected_first_ten, - }, - SPANDATA.DB_SYSTEM: "redis", - # ClusterNode converts localhost to 127.0.0.1 - SPANDATA.SERVER_ADDRESS: "127.0.0.1", - SPANDATA.SERVER_PORT: 6379, - }) + assert span["data"] == ApproxDict( + { + "redis.commands": { + "count": 3, + "first_ten": expected_first_ten, + }, + SPANDATA.DB_SYSTEM: "redis", + # ClusterNode converts localhost to 127.0.0.1 + SPANDATA.SERVER_ADDRESS: "127.0.0.1", + SPANDATA.SERVER_PORT: 6379, + } + ) assert span["tags"] == { "redis.transaction": False, "redis.is_cluster": True, diff --git a/tests/integrations/rediscluster/test_rediscluster.py b/tests/integrations/rediscluster/test_rediscluster.py index 762f0ddb76..88f987758b 100644 --- a/tests/integrations/rediscluster/test_rediscluster.py +++ b/tests/integrations/rediscluster/test_rediscluster.py @@ -57,12 +57,14 @@ def test_rediscluster_basic(rediscluster_cls, sentry_init, capture_events): assert crumb == { "category": "redis", "message": "GET 'foobar'", - "data": ApproxDict({ - "db.operation": "GET", - "redis.key": "foobar", - "redis.command": "GET", - "redis.is_cluster": True, - }), + "data": ApproxDict( + { + "db.operation": "GET", + "redis.key": "foobar", + "redis.command": "GET", + "redis.is_cluster": True, + } + ), "timestamp": crumb["timestamp"], "type": "redis", } @@ -97,16 +99,18 @@ def test_rediscluster_pipeline( (span,) = event["spans"] assert span["op"] == "db.redis" assert span["description"] == "redis.pipeline.execute" - assert span["data"] == ApproxDict({ - "redis.commands": { - "count": 3, - "first_ten": expected_first_ten, - }, - SPANDATA.DB_SYSTEM: "redis", - SPANDATA.DB_NAME: "1", - SPANDATA.SERVER_ADDRESS: "localhost", - SPANDATA.SERVER_PORT: 63791, - }) + assert span["data"] == ApproxDict( + { + "redis.commands": { + "count": 3, + "first_ten": expected_first_ten, + }, + SPANDATA.DB_SYSTEM: "redis", + SPANDATA.DB_NAME: "1", + SPANDATA.SERVER_ADDRESS: "localhost", + SPANDATA.SERVER_PORT: 63791, + } + ) assert span["tags"] == { "redis.transaction": False, # For Cluster, this is always False "redis.is_cluster": True, @@ -128,12 +132,14 @@ def test_db_connection_attributes_client(sentry_init, capture_events, redisclust (event,) = events (span,) = event["spans"] - assert span["data"] == ApproxDict({ - SPANDATA.DB_SYSTEM: "redis", - SPANDATA.DB_NAME: "1", - SPANDATA.SERVER_ADDRESS: "localhost", - SPANDATA.SERVER_PORT: 63791, - }) + assert span["data"] == ApproxDict( + { + SPANDATA.DB_SYSTEM: "redis", + SPANDATA.DB_NAME: "1", + SPANDATA.SERVER_ADDRESS: "localhost", + SPANDATA.SERVER_PORT: 63791, + } + ) @pytest.mark.parametrize("rediscluster_cls", rediscluster_classes) @@ -156,13 +162,15 @@ def test_db_connection_attributes_pipeline( (span,) = event["spans"] assert span["op"] == "db.redis" assert span["description"] == "redis.pipeline.execute" - assert span["data"] == ApproxDict({ - "redis.commands": { - "count": 1, - "first_ten": ["GET 'foo'"], - }, - SPANDATA.DB_SYSTEM: "redis", - SPANDATA.DB_NAME: "1", - SPANDATA.SERVER_ADDRESS: "localhost", - SPANDATA.SERVER_PORT: 63791, - }) + assert span["data"] == ApproxDict( + { + "redis.commands": { + "count": 1, + "first_ten": ["GET 'foo'"], + }, + SPANDATA.DB_SYSTEM: "redis", + SPANDATA.DB_NAME: "1", + SPANDATA.SERVER_ADDRESS: "localhost", + SPANDATA.SERVER_PORT: 63791, + } + ) diff --git a/tests/integrations/requests/test_requests.py b/tests/integrations/requests/test_requests.py index c56d1d2314..1f4dd412d7 100644 --- a/tests/integrations/requests/test_requests.py +++ b/tests/integrations/requests/test_requests.py @@ -29,14 +29,16 @@ def test_crumb_capture(sentry_init, capture_events): (crumb,) = event["breadcrumbs"]["values"] assert crumb["type"] == "http" assert crumb["category"] == "httplib" - assert crumb["data"] == ApproxDict({ - "url": url, - SPANDATA.HTTP_METHOD: "GET", - SPANDATA.HTTP_FRAGMENT: "", - SPANDATA.HTTP_QUERY: "", - SPANDATA.HTTP_STATUS_CODE: response.status_code, - "reason": response.reason, - }) + assert crumb["data"] == ApproxDict( + { + "url": url, + SPANDATA.HTTP_METHOD: "GET", + SPANDATA.HTTP_FRAGMENT: "", + SPANDATA.HTTP_QUERY: "", + SPANDATA.HTTP_STATUS_CODE: response.status_code, + "reason": response.reason, + } + ) @pytest.mark.tests_internal_exceptions @@ -57,12 +59,14 @@ def test_omit_url_data_if_parsing_fails(sentry_init, capture_events): capture_message("Testing!") (event,) = events - assert event["breadcrumbs"]["values"][0]["data"] == ApproxDict({ - SPANDATA.HTTP_METHOD: "GET", - SPANDATA.HTTP_STATUS_CODE: response.status_code, - "reason": response.reason, - # no url related data - }) + assert event["breadcrumbs"]["values"][0]["data"] == ApproxDict( + { + SPANDATA.HTTP_METHOD: "GET", + SPANDATA.HTTP_STATUS_CODE: response.status_code, + "reason": response.reason, + # no url related data + } + ) assert "url" not in event["breadcrumbs"]["values"][0]["data"] assert SPANDATA.HTTP_FRAGMENT not in event["breadcrumbs"]["values"][0]["data"] diff --git a/tests/integrations/socket/test_socket.py b/tests/integrations/socket/test_socket.py index 77cb80122c..4f93c1f2a5 100644 --- a/tests/integrations/socket/test_socket.py +++ b/tests/integrations/socket/test_socket.py @@ -17,10 +17,12 @@ def test_getaddrinfo_trace(sentry_init, capture_events): assert span["op"] == "socket.dns" assert span["description"] == "example.com:443" - assert span["data"] == ApproxDict({ - "host": "example.com", - "port": 443, - }) + assert span["data"] == ApproxDict( + { + "host": "example.com", + "port": 443, + } + ) def test_create_connection_trace(sentry_init, capture_events): @@ -38,15 +40,19 @@ def test_create_connection_trace(sentry_init, capture_events): assert connect_span["op"] == "socket.connection" assert connect_span["description"] == "example.com:443" - assert connect_span["data"] == ApproxDict({ - "address": ["example.com", 443], - "timeout": timeout, - "source_address": None, - }) + assert connect_span["data"] == ApproxDict( + { + "address": ["example.com", 443], + "timeout": timeout, + "source_address": None, + } + ) assert dns_span["op"] == "socket.dns" assert dns_span["description"] == "example.com:443" - assert dns_span["data"] == ApproxDict({ - "host": "example.com", - "port": 443, - }) + assert dns_span["data"] == ApproxDict( + { + "host": "example.com", + "port": 443, + } + ) diff --git a/tests/integrations/stdlib/test_httplib.py b/tests/integrations/stdlib/test_httplib.py index 3b5b2736d8..6055b86ab8 100644 --- a/tests/integrations/stdlib/test_httplib.py +++ b/tests/integrations/stdlib/test_httplib.py @@ -46,14 +46,16 @@ def test_crumb_capture(sentry_init, capture_events): assert crumb["type"] == "http" assert crumb["category"] == "httplib" - assert crumb["data"] == ApproxDict({ - "url": url, - SPANDATA.HTTP_METHOD: "GET", - SPANDATA.HTTP_STATUS_CODE: 200, - "reason": "OK", - SPANDATA.HTTP_FRAGMENT: "", - SPANDATA.HTTP_QUERY: "", - }) + assert crumb["data"] == ApproxDict( + { + "url": url, + SPANDATA.HTTP_METHOD: "GET", + SPANDATA.HTTP_STATUS_CODE: 200, + "reason": "OK", + SPANDATA.HTTP_FRAGMENT: "", + SPANDATA.HTTP_QUERY: "", + } + ) def test_crumb_capture_hint(sentry_init, capture_events): @@ -73,15 +75,17 @@ def before_breadcrumb(crumb, hint): (crumb,) = event["breadcrumbs"]["values"] assert crumb["type"] == "http" assert crumb["category"] == "httplib" - assert crumb["data"] == ApproxDict({ - "url": url, - SPANDATA.HTTP_METHOD: "GET", - SPANDATA.HTTP_STATUS_CODE: 200, - "reason": "OK", - "extra": "foo", - SPANDATA.HTTP_FRAGMENT: "", - SPANDATA.HTTP_QUERY: "", - }) + assert crumb["data"] == ApproxDict( + { + "url": url, + SPANDATA.HTTP_METHOD: "GET", + SPANDATA.HTTP_STATUS_CODE: 200, + "reason": "OK", + "extra": "foo", + SPANDATA.HTTP_FRAGMENT: "", + SPANDATA.HTTP_QUERY: "", + } + ) def test_empty_realurl(sentry_init): @@ -131,14 +135,16 @@ def test_httplib_misuse(sentry_init, capture_events, request): assert crumb["type"] == "http" assert crumb["category"] == "httplib" - assert crumb["data"] == ApproxDict({ - "url": "http://localhost:{}/200".format(PORT), - SPANDATA.HTTP_METHOD: "GET", - SPANDATA.HTTP_STATUS_CODE: 200, - "reason": "OK", - SPANDATA.HTTP_FRAGMENT: "", - SPANDATA.HTTP_QUERY: "", - }) + assert crumb["data"] == ApproxDict( + { + "url": "http://localhost:{}/200".format(PORT), + SPANDATA.HTTP_METHOD: "GET", + SPANDATA.HTTP_STATUS_CODE: 200, + "reason": "OK", + SPANDATA.HTTP_FRAGMENT: "", + SPANDATA.HTTP_QUERY: "", + } + ) def test_outgoing_trace_headers(sentry_init, monkeypatch): diff --git a/tests/integrations/strawberry/test_strawberry_py3.py b/tests/integrations/strawberry/test_strawberry_py3.py index c468423109..4911a1b5c3 100644 --- a/tests/integrations/strawberry/test_strawberry_py3.py +++ b/tests/integrations/strawberry/test_strawberry_py3.py @@ -352,12 +352,14 @@ def test_capture_transaction_on_error( resolve_span = resolve_spans[0] assert resolve_span["parent_span_id"] == query_span["span_id"] assert resolve_span["description"] == "resolving Query.error" - assert resolve_span["data"] == ApproxDict({ - "graphql.field_name": "error", - "graphql.parent_type": "Query", - "graphql.field_path": "Query.error", - "graphql.path": "error", - }) + assert resolve_span["data"] == ApproxDict( + { + "graphql.field_name": "error", + "graphql.parent_type": "Query", + "graphql.field_path": "Query.error", + "graphql.path": "error", + } + ) @parameterize_strawberry_test @@ -430,12 +432,14 @@ def test_capture_transaction_on_success( resolve_span = resolve_spans[0] assert resolve_span["parent_span_id"] == query_span["span_id"] assert resolve_span["description"] == "resolving Query.hello" - assert resolve_span["data"] == ApproxDict({ - "graphql.field_name": "hello", - "graphql.parent_type": "Query", - "graphql.field_path": "Query.hello", - "graphql.path": "hello", - }) + assert resolve_span["data"] == ApproxDict( + { + "graphql.field_name": "hello", + "graphql.parent_type": "Query", + "graphql.field_path": "Query.hello", + "graphql.path": "hello", + } + ) @parameterize_strawberry_test @@ -508,12 +512,14 @@ def test_transaction_no_operation_name( resolve_span = resolve_spans[0] assert resolve_span["parent_span_id"] == query_span["span_id"] assert resolve_span["description"] == "resolving Query.hello" - assert resolve_span["data"] == ApproxDict({ - "graphql.field_name": "hello", - "graphql.parent_type": "Query", - "graphql.field_path": "Query.hello", - "graphql.path": "hello", - }) + assert resolve_span["data"] == ApproxDict( + { + "graphql.field_name": "hello", + "graphql.parent_type": "Query", + "graphql.field_path": "Query.hello", + "graphql.path": "hello", + } + ) @parameterize_strawberry_test @@ -586,9 +592,11 @@ def test_transaction_mutation( resolve_span = resolve_spans[0] assert resolve_span["parent_span_id"] == query_span["span_id"] assert resolve_span["description"] == "resolving Mutation.change" - assert resolve_span["data"] == ApproxDict({ - "graphql.field_name": "change", - "graphql.parent_type": "Mutation", - "graphql.field_path": "Mutation.change", - "graphql.path": "change", - }) + assert resolve_span["data"] == ApproxDict( + { + "graphql.field_name": "change", + "graphql.parent_type": "Mutation", + "graphql.field_path": "Mutation.change", + "graphql.path": "change", + } + ) diff --git a/tests/test_scrubber.py b/tests/test_scrubber.py index 4509bbe37f..2c4bd3aa90 100644 --- a/tests/test_scrubber.py +++ b/tests/test_scrubber.py @@ -122,7 +122,9 @@ def test_span_data_scrubbing(sentry_init, capture_events): span.set_data("datafoo", "databar") (event,) = events - assert event["spans"][0]["data"] == ApproxDict({"password": "[Filtered]", "datafoo": "databar"}) + assert event["spans"][0]["data"] == ApproxDict( + {"password": "[Filtered]", "datafoo": "databar"} + ) assert event["_meta"]["spans"] == { "0": {"data": {"password": {"": {"rem": [["!config", "s"]]}}}} } diff --git a/tests/test_utils.py b/tests/test_utils.py index 5ccafa4d23..ab864324ac 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -665,7 +665,9 @@ def target(): assert (thread.ident, thread.name) == results.get(timeout=1) -@pytest.mark.skipif(sys.version_info < (3, 4), reason="threading.main_thread() Not available") +@pytest.mark.skipif( + sys.version_info < (3, 4), reason="threading.main_thread() Not available" +) def test_get_current_thread_meta_main_thread(): results = Queue(maxsize=1) From bb5e9b356ffcdfe2e5355edd8eb0fac2f2b1de84 Mon Sep 17 00:00:00 2001 From: Tony Xiao Date: Tue, 19 Mar 2024 11:54:28 -0400 Subject: [PATCH 14/20] we dont know the name of the gevent threads --- sentry_sdk/utils.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sentry_sdk/utils.py b/sentry_sdk/utils.py index 3e25821572..a64b4b4d98 100644 --- a/sentry_sdk/utils.py +++ b/sentry_sdk/utils.py @@ -1788,7 +1788,7 @@ def get_current_thread_meta(thread=None): if gevent_hub is not None: try: # this is undocumented, so wrap it in try except to be safe - return gevent_hub.thread_ident, gevent_hub.name + return gevent_hub.thread_ident, None except AttributeError: pass From 0daeabde77efd3b6e5272c4a9d7ad48cbea966cd Mon Sep 17 00:00:00 2001 From: Tony Xiao Date: Tue, 19 Mar 2024 12:13:34 -0400 Subject: [PATCH 15/20] thread id should be string --- sentry_sdk/consts.py | 4 ++-- sentry_sdk/tracing.py | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/sentry_sdk/consts.py b/sentry_sdk/consts.py index 4b23caf465..45560b85c9 100644 --- a/sentry_sdk/consts.py +++ b/sentry_sdk/consts.py @@ -193,13 +193,13 @@ class SPANDATA: THREAD_ID = "thread.id" """ - The thread id within which the span was started. This should be a string. + Identifier of a thread from where the span originated. This should be a string. Example: "7972576320" """ THREAD_NAME = "thread.name" """ - The thread name within which the span was started. This should be a string. + Label identifying a thread from where the span originated. This should be a string. Example: "MainThread" """ diff --git a/sentry_sdk/tracing.py b/sentry_sdk/tracing.py index 4383e61df8..7afe7e0944 100644 --- a/sentry_sdk/tracing.py +++ b/sentry_sdk/tracing.py @@ -430,7 +430,7 @@ def set_thread(self, thread_id, thread_name): # type: (Optional[int], Optional[str]) -> None if thread_id is not None: - self.set_data(SPANDATA.THREAD_ID, thread_id) + self.set_data(SPANDATA.THREAD_ID, str(thread_id)) if thread_name is not None: self.set_data(SPANDATA.THREAD_NAME, thread_name) From 6a87a36c69f128dda680768d803dfd5bdbf260bc Mon Sep 17 00:00:00 2001 From: Tony Xiao Date: Wed, 20 Mar 2024 11:47:25 -0400 Subject: [PATCH 16/20] add a few more failure case tests --- tests/test_utils.py | 76 +++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 73 insertions(+), 3 deletions(-) diff --git a/tests/test_utils.py b/tests/test_utils.py index ab864324ac..31db77e448 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -638,14 +638,42 @@ def target2(): assert (thread1.ident, thread1.name) == results.get(timeout=1) +def test_get_current_thread_meta_bad_explicit_thread(): + thread = "fake thread" + + main_thread = threading.main_thread() + + assert (main_thread.ident, main_thread.name) == get_current_thread_meta(thread) + + @pytest.mark.skipif(gevent is None, reason="gevent not enabled") def test_get_current_thread_meta_gevent_in_thread(): results = Queue(maxsize=1) def target(): - job = gevent.spawn(get_current_thread_meta) - job.join() - results.put(job.value) + with mock.patch("sentry_sdk.utils.is_gevent", side_effect=[True]): + job = gevent.spawn(get_current_thread_meta) + job.join() + results.put(job.value) + + thread = threading.Thread(target=target) + thread.start() + thread.join() + assert (thread.ident, None) == results.get(timeout=1) + + +@pytest.mark.skipif(gevent is None, reason="gevent not enabled") +def test_get_current_thread_meta_gevent_in_thread_failed_to_get_hub(): + results = Queue(maxsize=1) + + def target(): + with ( + mock.patch("sentry_sdk.utils.is_gevent", side_effect=[True]), + mock.patch("sentry_sdk.utils.get_gevent_hub", side_effect=["fake hub"]), + ): + job = gevent.spawn(get_current_thread_meta) + job.join() + results.put(job.value) thread = threading.Thread(target=target) thread.start() @@ -665,6 +693,26 @@ def target(): assert (thread.ident, thread.name) == results.get(timeout=1) +@pytest.mark.skipif( + sys.version_info < (3, 4), reason="threading.main_thread() Not available" +) +def test_get_current_thread_meta_bad_running_thread(): + results = Queue(maxsize=1) + + def target(): + with ( + mock.patch("threading.current_thread", side_effect=["fake thread"]), + ): + results.put(get_current_thread_meta()) + + thread = threading.Thread(target=target) + thread.start() + thread.join() + + main_thread = threading.main_thread() + assert (main_thread.ident, main_thread.name) == results.get(timeout=1) + + @pytest.mark.skipif( sys.version_info < (3, 4), reason="threading.main_thread() Not available" ) @@ -682,3 +730,25 @@ def target(): thread.start() thread.join() assert (main_thread.ident, main_thread.name) == results.get(timeout=1) + + +@pytest.mark.skipif( + sys.version_info < (3, 4), reason="threading.main_thread() Not available" +) +def test_get_current_thread_meta_failed_to_get_main_thread(): + results = Queue(maxsize=1) + + def target(): + # mock that somehow the current thread doesn't exist + with ( + mock.patch("threading.current_thread", side_effect=["fake thread"]), + mock.patch("threading.current_thread", side_effect=["fake thread"]), + ): + results.put(get_current_thread_meta()) + + main_thread = threading.main_thread() + + thread = threading.Thread(target=target) + thread.start() + thread.join() + assert (main_thread.ident, main_thread.name) == results.get(timeout=1) From d81a1de0598f037176ce2af09e753f53c75f4f1e Mon Sep 17 00:00:00 2001 From: Tony Xiao Date: Wed, 20 Mar 2024 12:00:59 -0400 Subject: [PATCH 17/20] this syntax fails in older python --- tests/test_utils.py | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/tests/test_utils.py b/tests/test_utils.py index 31db77e448..e07096c219 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -700,9 +700,7 @@ def test_get_current_thread_meta_bad_running_thread(): results = Queue(maxsize=1) def target(): - with ( - mock.patch("threading.current_thread", side_effect=["fake thread"]), - ): + with mock.patch("threading.current_thread", side_effect=["fake thread"]): results.put(get_current_thread_meta()) thread = threading.Thread(target=target) @@ -739,12 +737,9 @@ def test_get_current_thread_meta_failed_to_get_main_thread(): results = Queue(maxsize=1) def target(): - # mock that somehow the current thread doesn't exist - with ( - mock.patch("threading.current_thread", side_effect=["fake thread"]), - mock.patch("threading.current_thread", side_effect=["fake thread"]), - ): - results.put(get_current_thread_meta()) + with mock.patch("threading.current_thread", side_effect=["fake thread"]): + with mock.patch("threading.current_thread", side_effect=["fake thread"]): + results.put(get_current_thread_meta()) main_thread = threading.main_thread() From 1d05db3c7bb49790e1fc11ae939ed401916fb2b8 Mon Sep 17 00:00:00 2001 From: Tony Xiao Date: Wed, 20 Mar 2024 12:24:38 -0400 Subject: [PATCH 18/20] missed one more --- tests/test_utils.py | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/tests/test_utils.py b/tests/test_utils.py index e07096c219..14fe24696f 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -667,13 +667,11 @@ def test_get_current_thread_meta_gevent_in_thread_failed_to_get_hub(): results = Queue(maxsize=1) def target(): - with ( - mock.patch("sentry_sdk.utils.is_gevent", side_effect=[True]), - mock.patch("sentry_sdk.utils.get_gevent_hub", side_effect=["fake hub"]), - ): - job = gevent.spawn(get_current_thread_meta) - job.join() - results.put(job.value) + with mock.patch("sentry_sdk.utils.is_gevent", side_effect=[True]): + with mock.patch("sentry_sdk.utils.get_gevent_hub", side_effect=["fake hub"]): + job = gevent.spawn(get_current_thread_meta) + job.join() + results.put(job.value) thread = threading.Thread(target=target) thread.start() From 33cb135a232aa9b8f99bbd9851a1162120087196 Mon Sep 17 00:00:00 2001 From: Tony Xiao Date: Wed, 20 Mar 2024 12:28:01 -0400 Subject: [PATCH 19/20] run black --- tests/test_utils.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/test_utils.py b/tests/test_utils.py index 14fe24696f..d998fa5f21 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -668,7 +668,9 @@ def test_get_current_thread_meta_gevent_in_thread_failed_to_get_hub(): def target(): with mock.patch("sentry_sdk.utils.is_gevent", side_effect=[True]): - with mock.patch("sentry_sdk.utils.get_gevent_hub", side_effect=["fake hub"]): + with mock.patch( + "sentry_sdk.utils.get_gevent_hub", side_effect=["fake hub"] + ): job = gevent.spawn(get_current_thread_meta) job.join() results.put(job.value) From 5bf2efa468b239bb6348c127001616a525087c10 Mon Sep 17 00:00:00 2001 From: Tony Xiao Date: Wed, 20 Mar 2024 12:30:08 -0400 Subject: [PATCH 20/20] disable test for python<3.4 --- tests/test_utils.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/test_utils.py b/tests/test_utils.py index d998fa5f21..4b8e9087cc 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -638,6 +638,9 @@ def target2(): assert (thread1.ident, thread1.name) == results.get(timeout=1) +@pytest.mark.skipif( + sys.version_info < (3, 4), reason="threading.main_thread() Not available" +) def test_get_current_thread_meta_bad_explicit_thread(): thread = "fake thread"