From f32d28cb861e8a33f84861969219fa767c8f08e5 Mon Sep 17 00:00:00 2001 From: Ivana Kellyerova Date: Thu, 22 Feb 2024 12:48:24 +0100 Subject: [PATCH 01/10] wip --- sentry_sdk/metrics.py | 24 ++++-------------------- 1 file changed, 4 insertions(+), 20 deletions(-) diff --git a/sentry_sdk/metrics.py b/sentry_sdk/metrics.py index da2df222da..f1902a396e 100644 --- a/sentry_sdk/metrics.py +++ b/sentry_sdk/metrics.py @@ -11,7 +11,7 @@ from functools import wraps, partial import sentry_sdk -from sentry_sdk._compat import PY2, text_type, utc_from_timestamp, iteritems +from sentry_sdk._compat import text_type, utc_from_timestamp, iteritems from sentry_sdk.utils import ( ContextVar, now, @@ -19,7 +19,6 @@ to_timestamp, serialize_frame, json_dumps, - is_gevent, ) from sentry_sdk.envelope import Envelope, Item from sentry_sdk.tracing import ( @@ -55,7 +54,6 @@ try: - from gevent.monkey import get_original # type: ignore from gevent.threadpool import ThreadPool # type: ignore except ImportError: import importlib @@ -423,15 +421,7 @@ def __init__( self._running = True self._lock = threading.Lock() - if is_gevent() and PY2: - # get_original on threading.Event in Python 2 incorrectly returns - # the gevent-patched class. Luckily, threading.Event is just an alias - # for threading._Event in Python 2, and get_original on - # threading._Event correctly gets us the stdlib original. - event_cls = get_original("threading", "_Event") - else: - event_cls = get_original("threading", "Event") - self._flush_event = event_cls() # type: threading.Event + self._flush_event = threading.Event() # type: threading.Event self._force_flush = False @@ -466,16 +456,10 @@ def _ensure_thread(self): self._flusher_pid = pid - if not is_gevent(): - self._flusher = threading.Thread(target=self._flush_loop) - self._flusher.daemon = True - start_flusher = self._flusher.start - else: - self._flusher = ThreadPool(1) - start_flusher = partial(self._flusher.spawn, func=self._flush_loop) + self._flusher = threading.Thread(target=self._flush_loop, daemon=True) try: - start_flusher() + self._flusher.start() except RuntimeError: # Unfortunately at this point the interpreter is in a state that no # longer allows us to spawn a thread and we have to bail. From 4315209701a74aaaec4217d3f0fd5f32c3875d15 Mon Sep 17 00:00:00 2001 From: Ivana Kellyerova Date: Thu, 22 Feb 2024 12:54:51 +0100 Subject: [PATCH 02/10] python 2 compat --- sentry_sdk/metrics.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/sentry_sdk/metrics.py b/sentry_sdk/metrics.py index f1902a396e..73720e16aa 100644 --- a/sentry_sdk/metrics.py +++ b/sentry_sdk/metrics.py @@ -456,7 +456,8 @@ def _ensure_thread(self): self._flusher_pid = pid - self._flusher = threading.Thread(target=self._flush_loop, daemon=True) + self._flusher = threading.Thread(target=self._flush_loop) + self._flusher.daemon = True try: self._flusher.start() From 1bc8dcd0c89e433400cb09c169201bf9a7356770 Mon Sep 17 00:00:00 2001 From: Ivana Kellyerova Date: Thu, 22 Feb 2024 13:47:26 +0100 Subject: [PATCH 03/10] wip --- sentry_sdk/metrics.py | 13 +------------ tests/test_metrics.py | 2 +- tox.ini | 6 ------ 3 files changed, 2 insertions(+), 19 deletions(-) diff --git a/sentry_sdk/metrics.py b/sentry_sdk/metrics.py index 73720e16aa..04f45a9e1a 100644 --- a/sentry_sdk/metrics.py +++ b/sentry_sdk/metrics.py @@ -53,16 +53,6 @@ from sentry_sdk._types import MetricValue -try: - from gevent.threadpool import ThreadPool # type: ignore -except ImportError: - import importlib - - def get_original(module, name): - # type: (str, str) -> Any - return getattr(importlib.import_module(module), name) - - _in_metrics = ContextVar("in_metrics") _sanitize_key = partial(re.compile(r"[^a-zA-Z0-9_/.-]+").sub, "_") _sanitize_value = partial(re.compile(r"[^\w\d_:/@\.{}\[\]$-]+", re.UNICODE).sub, "_") @@ -422,7 +412,6 @@ def __init__( self._lock = threading.Lock() self._flush_event = threading.Event() # type: threading.Event - self._force_flush = False # The aggregator shifts its flushing by up to an entire rollup window to @@ -433,7 +422,7 @@ def __init__( # jittering. self._flush_shift = random.random() * self.ROLLUP_IN_SECONDS - self._flusher = None # type: Optional[Union[threading.Thread, ThreadPool]] + self._flusher = None # type: Optional[threading.Thread] self._flusher_pid = None # type: Optional[int] def _ensure_thread(self): diff --git a/tests/test_metrics.py b/tests/test_metrics.py index e78802f7e6..ce0122ace4 100644 --- a/tests/test_metrics.py +++ b/tests/test_metrics.py @@ -957,7 +957,7 @@ def bad_capture_envelope(*args, **kwargs): def test_flush_recursion_protection_background_flush( sentry_init, capture_envelopes, monkeypatch, maybe_monkeypatched_threading ): - monkeypatch.setattr(metrics.MetricsAggregator, "FLUSHER_SLEEP_TIME", 0.1) + monkeypatch.setattr(metrics.MetricsAggregator, "FLUSHER_SLEEP_TIME", 0.01) sentry_init( release="fun-release", environment="not-fun-env", diff --git a/tox.ini b/tox.ini index 34870b1ada..a23251f186 100644 --- a/tox.ini +++ b/tox.ini @@ -247,12 +247,6 @@ deps = {py3.6,py3.7,py3.8,py3.9,py3.10,py3.11,py3.12}-common: pytest<7.0.0 # === Gevent === - # See http://www.gevent.org/install.html#older-versions-of-python - # for justification of the versions pinned below - py3.5-gevent: gevent==20.9.0 - # See https://stackoverflow.com/questions/51496550/runtime-warning-greenlet-greenlet-size-changed - # for justification why greenlet is pinned here - py3.5-gevent: greenlet==0.4.17 {py2.7,py3.6,py3.7,py3.8,py3.9,py3.10,py3.11,py3.12}-gevent: gevent>=22.10.0, <22.11.0 # See https://github.com/pytest-dev/pytest/issues/9621 # and https://github.com/pytest-dev/pytest-forked/issues/67 From a5c56009dbd2a84825a5b9c4ef344fbe65a297a4 Mon Sep 17 00:00:00 2001 From: Ivana Kellyerova Date: Thu, 22 Feb 2024 19:45:02 +0100 Subject: [PATCH 04/10] initialize contextvar as false --- sentry_sdk/metrics.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/sentry_sdk/metrics.py b/sentry_sdk/metrics.py index 04f45a9e1a..b52e30b6b9 100644 --- a/sentry_sdk/metrics.py +++ b/sentry_sdk/metrics.py @@ -53,7 +53,7 @@ from sentry_sdk._types import MetricValue -_in_metrics = ContextVar("in_metrics") +_in_metrics = ContextVar("in_metrics", default=False) _sanitize_key = partial(re.compile(r"[^a-zA-Z0-9_/.-]+").sub, "_") _sanitize_value = partial(re.compile(r"[^\w\d_:/@\.{}\[\]$-]+", re.UNICODE).sub, "_") _set = set # set is shadowed below @@ -84,7 +84,7 @@ def get_code_location(stacklevel): def recursion_protection(): # type: () -> Generator[bool, None, None] """Enters recursion protection and returns the old flag.""" - old_in_metrics = _in_metrics.get(False) + old_in_metrics = _in_metrics.get() _in_metrics.set(True) try: yield old_in_metrics From aceeee9629ab0ac960c1297e387aebaa6a943753 Mon Sep 17 00:00:00 2001 From: Ivana Kellyerova Date: Mon, 26 Feb 2024 10:37:26 +0100 Subject: [PATCH 05/10] disable for <=3.6 --- sentry_sdk/client.py | 30 ++++++++++++++++++++++-------- tests/test_metrics.py | 29 +++++++++++++++++++++++++++++ 2 files changed, 51 insertions(+), 8 deletions(-) diff --git a/sentry_sdk/client.py b/sentry_sdk/client.py index 18eb2eab14..bb6681b096 100644 --- a/sentry_sdk/client.py +++ b/sentry_sdk/client.py @@ -5,6 +5,7 @@ import socket from sentry_sdk._compat import ( + PY37, datetime_utcnow, string_types, text_type, @@ -20,6 +21,7 @@ get_type_name, get_default_release, handle_in_app, + is_gevent, logger, ) from sentry_sdk.serializer import serialize @@ -256,14 +258,26 @@ def _capture_envelope(envelope): self.metrics_aggregator = None # type: Optional[MetricsAggregator] experiments = self.options.get("_experiments", {}) if experiments.get("enable_metrics", True): - from sentry_sdk.metrics import MetricsAggregator - - self.metrics_aggregator = MetricsAggregator( - capture_func=_capture_envelope, - enable_code_locations=bool( - experiments.get("metric_code_locations", True) - ), - ) + if is_gevent(): + # Context vars are not working correctly even with + # gevent-patched Python 3.6 and lower. + metrics_supported = PY37 + else: + metrics_supported = True + + if metrics_supported: + from sentry_sdk.metrics import MetricsAggregator + + self.metrics_aggregator = MetricsAggregator( + capture_func=_capture_envelope, + enable_code_locations=bool( + experiments.get("metric_code_locations", True) + ), + ) + else: + logger.info( + "Metrics not supported on Python 3.6 and lower with gevent." + ) max_request_body_size = ("always", "never", "small", "medium") if self.options["max_request_body_size"] not in max_request_body_size: diff --git a/tests/test_metrics.py b/tests/test_metrics.py index ce0122ace4..5a8364c0b3 100644 --- a/tests/test_metrics.py +++ b/tests/test_metrics.py @@ -13,6 +13,17 @@ except ImportError: import mock # python < 3.3 +try: + import gevent +except ImportError: + gevent = None + + +minimum_python_37_with_gevent = pytest.mark.skipif( + gevent and sys.version_info < (3, 7), + reason="Require Python 3.7 or higher with gevent", +) + def parse_metrics(bytes): rv = [] @@ -45,6 +56,7 @@ def parse_metrics(bytes): return rv +@minimum_python_37_with_gevent @pytest.mark.forked def test_incr(sentry_init, capture_envelopes, maybe_monkeypatched_threading): sentry_init( @@ -97,6 +109,7 @@ def test_incr(sentry_init, capture_envelopes, maybe_monkeypatched_threading): } +@minimum_python_37_with_gevent @pytest.mark.forked def test_timing(sentry_init, capture_envelopes, maybe_monkeypatched_threading): sentry_init( @@ -157,6 +170,7 @@ def test_timing(sentry_init, capture_envelopes, maybe_monkeypatched_threading): ) +@minimum_python_37_with_gevent @pytest.mark.forked def test_timing_decorator( sentry_init, capture_envelopes, maybe_monkeypatched_threading @@ -252,6 +266,7 @@ def amazing_nano(): assert line.strip() == "assert amazing() == 42" +@minimum_python_37_with_gevent @pytest.mark.forked def test_timing_basic(sentry_init, capture_envelopes, maybe_monkeypatched_threading): sentry_init( @@ -306,6 +321,7 @@ def test_timing_basic(sentry_init, capture_envelopes, maybe_monkeypatched_thread } +@minimum_python_37_with_gevent @pytest.mark.forked def test_distribution(sentry_init, capture_envelopes, maybe_monkeypatched_threading): sentry_init( @@ -368,6 +384,7 @@ def test_distribution(sentry_init, capture_envelopes, maybe_monkeypatched_thread ) +@minimum_python_37_with_gevent @pytest.mark.forked def test_set(sentry_init, capture_envelopes, maybe_monkeypatched_threading): sentry_init( @@ -421,6 +438,7 @@ def test_set(sentry_init, capture_envelopes, maybe_monkeypatched_threading): } +@minimum_python_37_with_gevent @pytest.mark.forked def test_gauge(sentry_init, capture_envelopes, maybe_monkeypatched_threading): sentry_init( @@ -454,6 +472,7 @@ def test_gauge(sentry_init, capture_envelopes, maybe_monkeypatched_threading): } +@minimum_python_37_with_gevent @pytest.mark.forked def test_multiple(sentry_init, capture_envelopes): sentry_init( @@ -508,6 +527,7 @@ def test_multiple(sentry_init, capture_envelopes): } +@minimum_python_37_with_gevent @pytest.mark.forked def test_transaction_name( sentry_init, capture_envelopes, maybe_monkeypatched_threading @@ -548,6 +568,7 @@ def test_transaction_name( } +@minimum_python_37_with_gevent @pytest.mark.forked @pytest.mark.parametrize("sample_rate", [1.0, None]) def test_metric_summaries( @@ -658,6 +679,7 @@ def test_metric_summaries( } +@minimum_python_37_with_gevent @pytest.mark.forked def test_metrics_summary_disabled( sentry_init, capture_envelopes, maybe_monkeypatched_threading @@ -702,6 +724,7 @@ def test_metrics_summary_disabled( assert "_metrics_summary" not in t["spans"][0] +@minimum_python_37_with_gevent @pytest.mark.forked def test_metrics_summary_filtered( sentry_init, capture_envelopes, maybe_monkeypatched_threading @@ -771,6 +794,7 @@ def should_summarize_metric(key, tags): } in t["d:foo@second"] +@minimum_python_37_with_gevent @pytest.mark.forked def test_tag_normalization( sentry_init, capture_envelopes, maybe_monkeypatched_threading @@ -818,6 +842,7 @@ def test_tag_normalization( # fmt: on +@minimum_python_37_with_gevent @pytest.mark.forked def test_before_emit_metric( sentry_init, capture_envelopes, maybe_monkeypatched_threading @@ -861,6 +886,7 @@ def before_emit(key, tags): } +@minimum_python_37_with_gevent @pytest.mark.forked def test_aggregator_flush( sentry_init, capture_envelopes, maybe_monkeypatched_threading @@ -881,6 +907,7 @@ def test_aggregator_flush( assert Hub.current.client.metrics_aggregator.buckets == {} +@minimum_python_37_with_gevent @pytest.mark.forked def test_tag_serialization( sentry_init, capture_envelopes, maybe_monkeypatched_threading @@ -921,6 +948,7 @@ def test_tag_serialization( } +@minimum_python_37_with_gevent @pytest.mark.forked def test_flush_recursion_protection( sentry_init, capture_envelopes, monkeypatch, maybe_monkeypatched_threading @@ -953,6 +981,7 @@ def bad_capture_envelope(*args, **kwargs): assert m[0][1] == "counter@none" +@minimum_python_37_with_gevent @pytest.mark.forked def test_flush_recursion_protection_background_flush( sentry_init, capture_envelopes, monkeypatch, maybe_monkeypatched_threading From ae6fcb7408aca0c3b44eeb767bcb1f1fdf806acd Mon Sep 17 00:00:00 2001 From: Ivana Kellyerova Date: Mon, 26 Feb 2024 10:40:12 +0100 Subject: [PATCH 06/10] wording --- sentry_sdk/client.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/sentry_sdk/client.py b/sentry_sdk/client.py index bb6681b096..2554a2a177 100644 --- a/sentry_sdk/client.py +++ b/sentry_sdk/client.py @@ -259,8 +259,8 @@ def _capture_envelope(envelope): experiments = self.options.get("_experiments", {}) if experiments.get("enable_metrics", True): if is_gevent(): - # Context vars are not working correctly even with - # gevent-patched Python 3.6 and lower. + # Context vars are not working correctly on Python <=3.6 + # with gevent. metrics_supported = PY37 else: metrics_supported = True From 475a161e6b5d8a8817000f5ff50f63fd893ca9c4 Mon Sep 17 00:00:00 2001 From: Ivana Kellyerova Date: Mon, 26 Feb 2024 11:48:40 +0100 Subject: [PATCH 07/10] one more test --- tests/test_metrics.py | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/tests/test_metrics.py b/tests/test_metrics.py index 5a8364c0b3..fdb5628806 100644 --- a/tests/test_metrics.py +++ b/tests/test_metrics.py @@ -1013,3 +1013,28 @@ def bad_capture_envelope(*args, **kwargs): m = parse_metrics(envelope.items[0].payload.get_bytes()) assert len(m) == 1 assert m[0][1] == "counter@none" + + +@pytest.mark.skipif( + not gevent or sys.version_info >= (3, 7), + reason="Python 3.6 or lower and gevent required", +) +def test_disable_metrics_for_old_python_with_gevent( + sentry_init, capture_envelopes, maybe_monkeypatched_threading +): + if maybe_monkeypatched_threading != "greenlet": + pytest.skip() + + sentry_init( + release="fun-release", + environment="not-fun-env", + _experiments={"enable_metrics": True}, + ) + envelopes = capture_envelopes() + + metrics.incr("counter") + + Hub.current.flush() + + assert Hub.current.client.metrics_aggregator is None + assert not envelopes From 096bd3f2797b32e05583ed1eb3bf3727f17bc60c Mon Sep 17 00:00:00 2001 From: Ivana Kellyerova Date: Mon, 26 Feb 2024 11:49:49 +0100 Subject: [PATCH 08/10] more concise --- sentry_sdk/client.py | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/sentry_sdk/client.py b/sentry_sdk/client.py index 2554a2a177..270d814bfe 100644 --- a/sentry_sdk/client.py +++ b/sentry_sdk/client.py @@ -258,13 +258,9 @@ def _capture_envelope(envelope): self.metrics_aggregator = None # type: Optional[MetricsAggregator] experiments = self.options.get("_experiments", {}) if experiments.get("enable_metrics", True): - if is_gevent(): - # Context vars are not working correctly on Python <=3.6 - # with gevent. - metrics_supported = PY37 - else: - metrics_supported = True - + # Context vars are not working correctly on Python <=3.6 + # with gevent. + metrics_supported = not is_gevent() or PY37 if metrics_supported: from sentry_sdk.metrics import MetricsAggregator From ca04762c1b8344552c768c3f5a277255ee52670c Mon Sep 17 00:00:00 2001 From: Ivana Kellyerova Date: Mon, 26 Feb 2024 11:55:52 +0100 Subject: [PATCH 09/10] note --- tests/test_metrics.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_metrics.py b/tests/test_metrics.py index fdb5628806..85ca1a7cb2 100644 --- a/tests/test_metrics.py +++ b/tests/test_metrics.py @@ -1023,7 +1023,7 @@ def test_disable_metrics_for_old_python_with_gevent( sentry_init, capture_envelopes, maybe_monkeypatched_threading ): if maybe_monkeypatched_threading != "greenlet": - pytest.skip() + pytest.skip("Test specifically for gevent/greenlet") sentry_init( release="fun-release", From 15c67ecfac525d4c15952abc2693d3ddc82ba3a2 Mon Sep 17 00:00:00 2001 From: Ivana Kellyerova Date: Mon, 26 Feb 2024 12:05:11 +0100 Subject: [PATCH 10/10] fork --- tests/test_metrics.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/test_metrics.py b/tests/test_metrics.py index 85ca1a7cb2..d3cfd659d1 100644 --- a/tests/test_metrics.py +++ b/tests/test_metrics.py @@ -1019,6 +1019,7 @@ def bad_capture_envelope(*args, **kwargs): not gevent or sys.version_info >= (3, 7), reason="Python 3.6 or lower and gevent required", ) +@pytest.mark.forked def test_disable_metrics_for_old_python_with_gevent( sentry_init, capture_envelopes, maybe_monkeypatched_threading ):