feat(profiling): Continuous profiling sample rate (#4002)

This introduces a new top level setting for the continuous profiling
session sample rate. The sample rate is evaluated once at the beginning
and is used to determine whether or not the profiler will be run for the
remainder of the process.

Author: Zylphrex

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

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,35 +123,26 @@ 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():
     # type: () -> None
     if _scheduler is None:
         return
 
-    _scheduler.ensure_running()
+    _scheduler.manual_start()
 
 
 def stop_profiler():
     # type: () -> None
     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

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()