diff --git a/sentry_sdk/consts.py b/sentry_sdk/consts.py index 23f79ebd63..ce435de36b 100644 --- a/sentry_sdk/consts.py +++ b/sentry_sdk/consts.py @@ -528,6 +528,7 @@ def __init__( profiles_sample_rate=None, # type: Optional[float] profiles_sampler=None, # type: Optional[TracesSampler] profiler_mode=None, # type: Optional[ProfilerMode] + profile_session_sample_rate=None, # type: Optional[float] auto_enabling_integrations=True, # type: bool disabled_integrations=None, # type: Optional[Sequence[sentry_sdk.integrations.Integration]] auto_session_tracking=True, # type: bool diff --git a/sentry_sdk/profiler/continuous_profiler.py b/sentry_sdk/profiler/continuous_profiler.py index 5a76a0696c..b07fbec998 100644 --- a/sentry_sdk/profiler/continuous_profiler.py +++ b/sentry_sdk/profiler/continuous_profiler.py @@ -1,5 +1,6 @@ import atexit import os +import random import sys import threading import time @@ -83,11 +84,15 @@ def setup_continuous_profiler(options, sdk_info, capture_func): else: default_profiler_mode = ThreadContinuousScheduler.mode - experiments = options.get("_experiments", {}) + if options.get("profiler_mode") is not None: + profiler_mode = options["profiler_mode"] + else: + # TODO: deprecate this and just use the existing `profiler_mode` + experiments = options.get("_experiments", {}) - profiler_mode = ( - experiments.get("continuous_profiling_mode") or default_profiler_mode - ) + profiler_mode = ( + experiments.get("continuous_profiling_mode") or default_profiler_mode + ) frequency = DEFAULT_SAMPLING_FREQUENCY @@ -118,19 +123,10 @@ def try_autostart_continuous_profiler(): if _scheduler is None: return - # Ensure that the scheduler only autostarts once per process. - # This is necessary because many web servers use forks to spawn - # additional processes. And the profiler is only spawned on the - # master process, then it often only profiles the main process - # and not the ones where the requests are being handled. - # - # Additionally, we only want this autostart behaviour once per - # process. If the user explicitly calls `stop_profiler`, it should - # be respected and not start the profiler again. - if not _scheduler.should_autostart(): + if not _scheduler.is_auto_start_enabled(): return - _scheduler.ensure_running() + _scheduler.manual_start() def start_profiler(): @@ -138,7 +134,7 @@ def start_profiler(): if _scheduler is None: return - _scheduler.ensure_running() + _scheduler.manual_start() def stop_profiler(): @@ -146,7 +142,7 @@ def stop_profiler(): if _scheduler is None: return - _scheduler.teardown() + _scheduler.manual_stop() def teardown_continuous_profiler(): @@ -164,6 +160,16 @@ def get_profiler_id(): return _scheduler.profiler_id +def determine_profile_session_sampling_decision(sample_rate): + # type: (Union[float, None]) -> bool + + # `None` is treated as `0.0` + if not sample_rate: + return False + + return random.random() < float(sample_rate) + + class ContinuousScheduler: mode = "unknown" # type: ContinuousProfilerMode @@ -175,16 +181,43 @@ def __init__(self, frequency, options, sdk_info, capture_func): self.capture_func = capture_func self.sampler = self.make_sampler() self.buffer = None # type: Optional[ProfileBuffer] + self.pid = None # type: Optional[int] self.running = False - def should_autostart(self): + profile_session_sample_rate = self.options.get("profile_session_sample_rate") + self.sampled = determine_profile_session_sampling_decision( + profile_session_sample_rate + ) + + def is_auto_start_enabled(self): # type: () -> bool + + # Ensure that the scheduler only autostarts once per process. + # This is necessary because many web servers use forks to spawn + # additional processes. And the profiler is only spawned on the + # master process, then it often only profiles the main process + # and not the ones where the requests are being handled. + if self.pid == os.getpid(): + return False + experiments = self.options.get("_experiments") if not experiments: return False + return experiments.get("continuous_profiling_auto_start") + def manual_start(self): + # type: () -> None + if not self.sampled: + return + + self.ensure_running() + + def manual_stop(self): + # type: () -> None + self.teardown() + def ensure_running(self): # type: () -> None raise NotImplementedError @@ -277,15 +310,11 @@ def __init__(self, frequency, options, sdk_info, capture_func): super().__init__(frequency, options, sdk_info, capture_func) self.thread = None # type: Optional[threading.Thread] - self.pid = None # type: Optional[int] self.lock = threading.Lock() - def should_autostart(self): - # type: () -> bool - return super().should_autostart() and self.pid != os.getpid() - def ensure_running(self): # type: () -> None + pid = os.getpid() # is running on the right process @@ -356,13 +385,8 @@ def __init__(self, frequency, options, sdk_info, capture_func): super().__init__(frequency, options, sdk_info, capture_func) self.thread = None # type: Optional[_ThreadPool] - self.pid = None # type: Optional[int] self.lock = threading.Lock() - def should_autostart(self): - # type: () -> bool - return super().should_autostart() and self.pid != os.getpid() - def ensure_running(self): # type: () -> None pid = os.getpid() @@ -393,7 +417,6 @@ def ensure_running(self): # longer allows us to spawn a thread and we have to bail. self.running = False self.thread = None - return def teardown(self): # type: () -> None diff --git a/tests/profiler/test_continuous_profiler.py b/tests/profiler/test_continuous_profiler.py index 32d0e8d0b0..6f4893e59d 100644 --- a/tests/profiler/test_continuous_profiler.py +++ b/tests/profiler/test_continuous_profiler.py @@ -23,13 +23,25 @@ requires_gevent = pytest.mark.skipif(gevent is None, reason="gevent not enabled") -def experimental_options(mode=None, auto_start=None): - return { - "_experiments": { - "continuous_profiling_auto_start": auto_start, - "continuous_profiling_mode": mode, +def get_client_options(use_top_level_profiler_mode): + def client_options(mode=None, auto_start=None, profile_session_sample_rate=1.0): + if use_top_level_profiler_mode: + return { + "profiler_mode": mode, + "profile_session_sample_rate": profile_session_sample_rate, + "_experiments": { + "continuous_profiling_auto_start": auto_start, + }, + } + return { + "profile_session_sample_rate": profile_session_sample_rate, + "_experiments": { + "continuous_profiling_auto_start": auto_start, + "continuous_profiling_mode": mode, + }, } - } + + return client_options mock_sdk_info = { @@ -42,7 +54,10 @@ def experimental_options(mode=None, auto_start=None): @pytest.mark.parametrize("mode", [pytest.param("foo")]) @pytest.mark.parametrize( "make_options", - [pytest.param(experimental_options, id="experiment")], + [ + pytest.param(get_client_options(True), id="non-experiment"), + pytest.param(get_client_options(False), id="experiment"), + ], ) def test_continuous_profiler_invalid_mode(mode, make_options, teardown_profiling): with pytest.raises(ValueError): @@ -62,7 +77,10 @@ def test_continuous_profiler_invalid_mode(mode, make_options, teardown_profiling ) @pytest.mark.parametrize( "make_options", - [pytest.param(experimental_options, id="experiment")], + [ + pytest.param(get_client_options(True), id="non-experiment"), + pytest.param(get_client_options(False), id="experiment"), + ], ) def test_continuous_profiler_valid_mode(mode, make_options, teardown_profiling): options = make_options(mode=mode) @@ -82,7 +100,10 @@ def test_continuous_profiler_valid_mode(mode, make_options, teardown_profiling): ) @pytest.mark.parametrize( "make_options", - [pytest.param(experimental_options, id="experiment")], + [ + pytest.param(get_client_options(True), id="non-experiment"), + pytest.param(get_client_options(False), id="experiment"), + ], ) def test_continuous_profiler_setup_twice(mode, make_options, teardown_profiling): options = make_options(mode=mode) @@ -178,7 +199,10 @@ def assert_single_transaction_without_profile_chunks(envelopes): ) @pytest.mark.parametrize( "make_options", - [pytest.param(experimental_options, id="experiment")], + [ + pytest.param(get_client_options(True), id="non-experiment"), + pytest.param(get_client_options(False), id="experiment"), + ], ) @mock.patch("sentry_sdk.profiler.continuous_profiler.PROFILE_BUFFER_SECONDS", 0.01) def test_continuous_profiler_auto_start_and_manual_stop( @@ -191,7 +215,7 @@ def test_continuous_profiler_auto_start_and_manual_stop( options = make_options(mode=mode, auto_start=True) sentry_init( traces_sample_rate=1.0, - _experiments=options.get("_experiments", {}), + **options, ) envelopes = capture_envelopes() @@ -235,10 +259,13 @@ def test_continuous_profiler_auto_start_and_manual_stop( ) @pytest.mark.parametrize( "make_options", - [pytest.param(experimental_options, id="experiment")], + [ + pytest.param(get_client_options(True), id="non-experiment"), + pytest.param(get_client_options(False), id="experiment"), + ], ) @mock.patch("sentry_sdk.profiler.continuous_profiler.PROFILE_BUFFER_SECONDS", 0.01) -def test_continuous_profiler_manual_start_and_stop( +def test_continuous_profiler_manual_start_and_stop_sampled( sentry_init, capture_envelopes, mode, @@ -248,7 +275,7 @@ def test_continuous_profiler_manual_start_and_stop( options = make_options(mode=mode) sentry_init( traces_sample_rate=1.0, - _experiments=options.get("_experiments", {}), + **options, ) envelopes = capture_envelopes() @@ -275,3 +302,43 @@ def test_continuous_profiler_manual_start_and_stop( time.sleep(0.05) assert_single_transaction_without_profile_chunks(envelopes) + + +@pytest.mark.parametrize( + "mode", + [ + pytest.param("thread"), + pytest.param("gevent", marks=requires_gevent), + ], +) +@pytest.mark.parametrize( + "make_options", + [ + pytest.param(get_client_options(True), id="non-experiment"), + pytest.param(get_client_options(False), id="experiment"), + ], +) +def test_continuous_profiler_manual_start_and_stop_unsampled( + sentry_init, + capture_envelopes, + mode, + make_options, + teardown_profiling, +): + options = make_options(mode=mode, profile_session_sample_rate=0.0) + sentry_init( + traces_sample_rate=1.0, + **options, + ) + + envelopes = capture_envelopes() + + start_profiler() + + with sentry_sdk.start_transaction(name="profiling"): + with sentry_sdk.start_span(op="op"): + time.sleep(0.05) + + assert_single_transaction_without_profile_chunks(envelopes) + + stop_profiler()