diff --git a/sentry_sdk/profiler/continuous_profiler.py b/sentry_sdk/profiler/continuous_profiler.py index 77ba60dbda..00dd29e36c 100644 --- a/sentry_sdk/profiler/continuous_profiler.py +++ b/sentry_sdk/profiler/continuous_profiler.py @@ -236,6 +236,7 @@ def __init__(self, frequency, options, sdk_info, capture_func): self.pid = None # type: Optional[int] self.running = False + self.soft_shutdown = False self.new_profiles = deque(maxlen=128) # type: Deque[ContinuousProfile] self.active_profiles = set() # type: Set[ContinuousProfile] @@ -317,7 +318,7 @@ def profiler_id(self): return self.buffer.profiler_id def make_sampler(self): - # type: () -> Callable[..., None] + # type: () -> Callable[..., bool] cwd = os.getcwd() cache = LRUCache(max_size=256) @@ -325,7 +326,7 @@ def make_sampler(self): if self.lifecycle == "trace": def _sample_stack(*args, **kwargs): - # type: (*Any, **Any) -> None + # type: (*Any, **Any) -> bool """ Take a sample of the stack on all the threads in the process. This should be called at a regular interval to collect samples. @@ -333,8 +334,7 @@ def _sample_stack(*args, **kwargs): # no profiles taking place, so we can stop early if not self.new_profiles and not self.active_profiles: - self.running = False - return + return True # This is the number of profiles we want to pop off. # It's possible another thread adds a new profile to @@ -357,7 +357,7 @@ def _sample_stack(*args, **kwargs): # For some reason, the frame we get doesn't have certain attributes. # When this happens, we abandon the current sample as it's bad. capture_internal_exception(sys.exc_info()) - return + return False # Move the new profiles into the active_profiles set. # @@ -374,9 +374,7 @@ def _sample_stack(*args, **kwargs): inactive_profiles = [] for profile in self.active_profiles: - if profile.active: - pass - else: + if not profile.active: # If a profile is marked inactive, we buffer it # to `inactive_profiles` so it can be removed. # We cannot remove it here as it would result @@ -389,10 +387,12 @@ def _sample_stack(*args, **kwargs): if self.buffer is not None: self.buffer.write(ts, sample) + return False + else: def _sample_stack(*args, **kwargs): - # type: (*Any, **Any) -> None + # type: (*Any, **Any) -> bool """ Take a sample of the stack on all the threads in the process. This should be called at a regular interval to collect samples. @@ -409,11 +409,13 @@ def _sample_stack(*args, **kwargs): # For some reason, the frame we get doesn't have certain attributes. # When this happens, we abandon the current sample as it's bad. capture_internal_exception(sys.exc_info()) - return + return False if self.buffer is not None: self.buffer.write(ts, sample) + return False + return _sample_stack def run(self): @@ -421,7 +423,7 @@ def run(self): last = time.perf_counter() while self.running: - self.sampler() + self.soft_shutdown = self.sampler() # some time may have elapsed since the last time # we sampled, so we need to account for that and @@ -430,6 +432,15 @@ def run(self): if elapsed < self.interval: thread_sleep(self.interval - elapsed) + # the soft shutdown happens here to give it a chance + # for the profiler to be reused + if self.soft_shutdown: + self.running = False + + # make sure to explicitly exit the profiler here or there might + # be multiple profilers at once + break + # after sleeping, make sure to take the current # timestamp so we can use it next iteration last = time.perf_counter() @@ -458,6 +469,8 @@ def __init__(self, frequency, options, sdk_info, capture_func): def ensure_running(self): # type: () -> None + self.soft_shutdown = False + pid = os.getpid() # is running on the right process @@ -532,6 +545,9 @@ def __init__(self, frequency, options, sdk_info, capture_func): def ensure_running(self): # type: () -> None + + self.soft_shutdown = False + pid = os.getpid() # is running on the right process diff --git a/tests/profiler/test_continuous_profiler.py b/tests/profiler/test_continuous_profiler.py index 991f8bda5d..7283ec7164 100644 --- a/tests/profiler/test_continuous_profiler.py +++ b/tests/profiler/test_continuous_profiler.py @@ -459,33 +459,54 @@ def test_continuous_profiler_auto_start_and_stop_sampled( thread = threading.current_thread() + all_profiler_ids = set() + for _ in range(3): envelopes.clear() + profiler_ids = set() + with sentry_sdk.start_transaction(name="profiling 1"): - assert get_profiler_id() is not None, "profiler should be running" + profiler_id = get_profiler_id() + assert profiler_id is not None, "profiler should be running" + profiler_ids.add(profiler_id) with sentry_sdk.start_span(op="op"): time.sleep(0.1) - assert get_profiler_id() is not None, "profiler should be running" + profiler_id = get_profiler_id() + assert profiler_id is not None, "profiler should be running" + profiler_ids.add(profiler_id) + + time.sleep(0.03) # the profiler takes a while to stop in auto mode so if we start # a transaction immediately, it'll be part of the same chunk - assert get_profiler_id() is not None, "profiler should be running" + profiler_id = get_profiler_id() + assert profiler_id is not None, "profiler should be running" + profiler_ids.add(profiler_id) with sentry_sdk.start_transaction(name="profiling 2"): - assert get_profiler_id() is not None, "profiler should be running" + profiler_id = get_profiler_id() + assert profiler_id is not None, "profiler should be running" + profiler_ids.add(profiler_id) with sentry_sdk.start_span(op="op"): time.sleep(0.1) - assert get_profiler_id() is not None, "profiler should be running" + profiler_id = get_profiler_id() + assert profiler_id is not None, "profiler should be running" + profiler_ids.add(profiler_id) # wait at least 1 cycle for the profiler to stop time.sleep(0.2) assert get_profiler_id() is None, "profiler should not be running" + assert len(profiler_ids) == 1 + all_profiler_ids.add(profiler_ids.pop()) + assert_single_transaction_with_profile_chunks( envelopes, thread, max_chunks=1, transactions=2 ) + assert len(all_profiler_ids) == 3 + @pytest.mark.parametrize( "mode",