diff --git a/sentry_sdk/_compat.py b/sentry_sdk/_compat.py index 8c1bf9711f..38872051ff 100644 --- a/sentry_sdk/_compat.py +++ b/sentry_sdk/_compat.py @@ -140,29 +140,74 @@ def __new__(metacls, name, this_bases, d): return type.__new__(MetaClass, "temporary_class", (), {}) -def check_thread_support(): - # type: () -> None +def check_uwsgi_thread_support(): + # type: () -> bool + # We check two things here: + # + # 1. uWSGI doesn't run in threaded mode by default -- issue a warning if + # that's the case. + # + # 2. Additionally, if uWSGI is running in preforking mode (default), it needs + # the --py-call-uwsgi-fork-hooks option for the SDK to work properly. This + # is because any background threads spawned before the main process is + # forked are NOT CLEANED UP IN THE CHILDREN BY DEFAULT even if + # --enable-threads is on. One has to explicitly provide + # --py-call-uwsgi-fork-hooks to force uWSGI to run regular cpython + # after-fork hooks that take care of cleaning up stale thread data. try: from uwsgi import opt # type: ignore except ImportError: - return + return True + + from sentry_sdk.consts import FALSE_VALUES + + def enabled(option): + # type: (str) -> bool + value = opt.get(option, False) + if isinstance(value, bool): + return value + + if isinstance(value, bytes): + try: + value = value.decode() + except Exception: + pass + + return value and str(value).lower() not in FALSE_VALUES # When `threads` is passed in as a uwsgi option, # `enable-threads` is implied on. - if "threads" in opt: - return + threads_enabled = "threads" in opt or enabled("enable-threads") + fork_hooks_on = enabled("py-call-uwsgi-fork-hooks") + lazy_mode = enabled("lazy-apps") or enabled("lazy") - # put here because of circular import - from sentry_sdk.consts import FALSE_VALUES + if lazy_mode and not threads_enabled: + from warnings import warn - if str(opt.get("enable-threads", "0")).lower() in FALSE_VALUES: + warn( + Warning( + "IMPORTANT: " + "We detected the use of uWSGI without thread support. " + "This might lead to unexpected issues. " + 'Please run uWSGI with "--enable-threads" for full support.' + ) + ) + + return False + + elif not lazy_mode and (not threads_enabled or not fork_hooks_on): from warnings import warn warn( Warning( - "We detected the use of uwsgi with disabled threads. " - "This will cause issues with the transport you are " - "trying to use. Please enable threading for uwsgi. " - '(Add the "enable-threads" flag).' + "IMPORTANT: " + "We detected the use of uWSGI in preforking mode without " + "thread support. This might lead to crashing workers. " + 'Please run uWSGI with both "--enable-threads" and ' + '"--py-call-uwsgi-fork-hooks" for full support.' ) ) + + return False + + return True diff --git a/sentry_sdk/client.py b/sentry_sdk/client.py index 7e2659810d..18eb2eab14 100644 --- a/sentry_sdk/client.py +++ b/sentry_sdk/client.py @@ -4,7 +4,13 @@ import random import socket -from sentry_sdk._compat import datetime_utcnow, string_types, text_type, iteritems +from sentry_sdk._compat import ( + datetime_utcnow, + string_types, + text_type, + iteritems, + check_uwsgi_thread_support, +) from sentry_sdk.utils import ( capture_internal_exceptions, current_stacktrace, @@ -18,7 +24,7 @@ ) from sentry_sdk.serializer import serialize from sentry_sdk.tracing import trace, has_tracing_enabled -from sentry_sdk.transport import make_transport +from sentry_sdk.transport import HttpTransport, make_transport from sentry_sdk.consts import ( DEFAULT_MAX_VALUE_LENGTH, DEFAULT_OPTIONS, @@ -249,28 +255,15 @@ def _capture_envelope(envelope): self.metrics_aggregator = None # type: Optional[MetricsAggregator] experiments = self.options.get("_experiments", {}) - if experiments.get("enable_metrics", True) or experiments.get( - "force_enable_metrics", False - ): - try: - import uwsgi # type: ignore - except ImportError: - uwsgi = None - - if uwsgi is not None and not experiments.get( - "force_enable_metrics", False - ): - logger.warning("Metrics currently not supported with uWSGI.") - - else: - 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 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) + ), + ) max_request_body_size = ("always", "never", "small", "medium") if self.options["max_request_body_size"] not in max_request_body_size: @@ -316,6 +309,16 @@ def _capture_envelope(envelope): self._setup_instrumentation(self.options.get("functions_to_trace", [])) + if ( + self.monitor + or self.metrics_aggregator + or has_profiling_enabled(self.options) + or isinstance(self.transport, HttpTransport) + ): + # If we have anything on that could spawn a background thread, we + # need to check if it's safe to use them. + check_uwsgi_thread_support() + @property def dsn(self): # type: () -> Optional[str] diff --git a/sentry_sdk/consts.py b/sentry_sdk/consts.py index 64e2cdf521..ad7b1099ae 100644 --- a/sentry_sdk/consts.py +++ b/sentry_sdk/consts.py @@ -46,7 +46,6 @@ "transport_zlib_compression_level": Optional[int], "transport_num_pools": Optional[int], "enable_metrics": Optional[bool], - "force_enable_metrics": Optional[bool], "metrics_summary_sample_rate": Optional[float], "should_summarize_metric": Optional[Callable[[str, MetricTags], bool]], "before_emit_metric": Optional[Callable[[str, MetricTags], bool]], diff --git a/sentry_sdk/hub.py b/sentry_sdk/hub.py index 45afb56cc9..21b59283aa 100644 --- a/sentry_sdk/hub.py +++ b/sentry_sdk/hub.py @@ -1,6 +1,5 @@ import copy import sys - from contextlib import contextmanager from sentry_sdk._compat import with_metaclass diff --git a/sentry_sdk/worker.py b/sentry_sdk/worker.py index 02628b9b29..27b2f2f69c 100644 --- a/sentry_sdk/worker.py +++ b/sentry_sdk/worker.py @@ -2,7 +2,6 @@ import threading from time import sleep, time -from sentry_sdk._compat import check_thread_support from sentry_sdk._queue import Queue, FullError from sentry_sdk.utils import logger from sentry_sdk.consts import DEFAULT_QUEUE_SIZE @@ -21,7 +20,6 @@ class BackgroundWorker(object): def __init__(self, queue_size=DEFAULT_QUEUE_SIZE): # type: (int) -> None - check_thread_support() self._queue = Queue(queue_size) # type: Queue self._lock = threading.Lock() self._thread = None # type: Optional[threading.Thread] diff --git a/tests/test_client.py b/tests/test_client.py index fa55c1111a..0954a8c5e8 100644 --- a/tests/test_client.py +++ b/tests/test_client.py @@ -5,8 +5,8 @@ import subprocess import sys import time - from textwrap import dedent + from sentry_sdk import ( Hub, Client, @@ -1316,3 +1316,43 @@ def test_error_sampler(_, sentry_init, capture_events, test_config): # Ensure two arguments (the event and hint) were passed to the sampler function assert len(test_config.sampler_function_mock.call_args[0]) == 2 + + +@pytest.mark.forked +@pytest.mark.parametrize( + "opt,missing_flags", + [ + # lazy mode with enable-threads, no warning + [{"enable-threads": True, "lazy-apps": True}, []], + [{"enable-threads": "true", "lazy-apps": b"1"}, []], + # preforking mode with enable-threads and py-call-uwsgi-fork-hooks, no warning + [{"enable-threads": True, "py-call-uwsgi-fork-hooks": True}, []], + [{"enable-threads": b"true", "py-call-uwsgi-fork-hooks": b"on"}, []], + # lazy mode, no enable-threads, warning + [{"lazy-apps": True}, ["--enable-threads"]], + [{"enable-threads": b"false", "lazy-apps": True}, ["--enable-threads"]], + [{"enable-threads": b"0", "lazy": True}, ["--enable-threads"]], + # preforking mode, no enable-threads or py-call-uwsgi-fork-hooks, warning + [{}, ["--enable-threads", "--py-call-uwsgi-fork-hooks"]], + [{"processes": b"2"}, ["--enable-threads", "--py-call-uwsgi-fork-hooks"]], + [{"enable-threads": True}, ["--py-call-uwsgi-fork-hooks"]], + [{"enable-threads": b"1"}, ["--py-call-uwsgi-fork-hooks"]], + [ + {"enable-threads": b"false"}, + ["--enable-threads", "--py-call-uwsgi-fork-hooks"], + ], + [{"py-call-uwsgi-fork-hooks": True}, ["--enable-threads"]], + ], +) +def test_uwsgi_warnings(sentry_init, recwarn, opt, missing_flags): + uwsgi = mock.MagicMock() + uwsgi.opt = opt + with mock.patch.dict("sys.modules", uwsgi=uwsgi): + sentry_init(profiles_sample_rate=1.0) + if missing_flags: + assert len(recwarn) == 1 + record = recwarn.pop() + for flag in missing_flags: + assert flag in str(record.message) + else: + assert not recwarn