use old logic for tracking transactions

Author: Zylphrex

sentry_sdk/consts.py

@@ -529,7 +529,7 @@ def __init__(
         profiles_sample_rate=None,  # type: Optional[float]
         profiles_sampler=None,  # type: Optional[TracesSampler]
         profiler_mode=None,  # type: Optional[ProfilerMode]
-        profile_lifecycle="manual",  # type: Literal["manual", "auto"]
+        profile_lifecycle="manual",  # type: Literal["manual", "trace"]
         profile_session_sample_rate=None,  # type: Optional[float]
         auto_enabling_integrations=True,  # type: bool
         disabled_integrations=None,  # type: Optional[Sequence[sentry_sdk.integrations.Integration]]

sentry_sdk/profiler/continuous_profiler.py

@@ -32,6 +32,7 @@
     from typing import Dict
     from typing import List
     from typing import Optional
+    from typing import Set
     from typing import Type
     from typing import Union
     from typing_extensions import TypedDict
@@ -134,15 +135,15 @@ def try_autostart_continuous_profiler():
     _scheduler.manual_start()
 
 
-def try_profile_lifecycle_auto_start():
-    # type: () -> bool
+def try_profile_lifecycle_trace_start():
+    # type: () -> Union[ContinuousProfile, None]
     if _scheduler is None:
-        return False
+        return None
 
     return _scheduler.auto_start()
 
 
-def try_profile_lifecycle_auto_stop():
+def try_profile_lifecycle_trace_stop():
     # type: () -> None
     if _scheduler is None:
         return
@@ -191,6 +192,14 @@ def determine_profile_session_sampling_decision(sample_rate):
     return random.random() < float(sample_rate)
 
 
+class ContinuousProfile:
+    active: bool = True
+
+    def stop(self):
+        # type: () -> None
+        self.active = False
+
+
 class ContinuousScheduler:
     mode = "unknown"  # type: ContinuousProfilerMode
 
@@ -213,9 +222,8 @@ def __init__(self, frequency, options, sdk_info, capture_func):
 
         self.running = False
 
-        self.active_spans = 0
-        self.started_spans = deque(maxlen=128)  # type: Deque[None]
-        self.finished_spans = deque(maxlen=128)  # type: Deque[None]
+        self.new_profiles = deque(maxlen=128)  # type: Deque[ContinuousProfile]
+        self.active_profiles = set()  # type: Set[ContinuousProfile]
 
     def is_auto_start_enabled(self):
         # type: () -> bool
@@ -235,28 +243,21 @@ def is_auto_start_enabled(self):
         return experiments.get("continuous_profiling_auto_start")
 
     def auto_start(self):
-        # type: () -> bool
+        # type: () -> Union[ContinuousProfile, None]
         if not self.sampled:
-            return False
+            return None
 
-        if self.lifecycle != "auto":
-            return False
+        if self.lifecycle != "trace":
+            return None
 
         logger.debug("[Profiling] Auto starting profiler")
 
-        self.started_spans.append(None)
-        self.ensure_running()
-
-        return True
+        profile = ContinuousProfile()
 
-    def auto_stop(self):
-        # type: () -> None
-        if self.lifecycle != "auto":
-            return
-
-        logger.debug("[Profiling] Auto stopping profiler")
+        self.new_profiles.append(profile)
+        self.ensure_running()
 
-        self.finished_spans.append(None)
+        return profile
 
     def manual_start(self):
         # type: () -> None
@@ -306,7 +307,7 @@ def make_sampler(self):
 
         cache = LRUCache(max_size=256)
 
-        if self.lifecycle == "auto":
+        if self.lifecycle == "trace":
 
             def _sample_stack(*args, **kwargs):
                 # type: (*Any, **Any) -> None
@@ -315,16 +316,20 @@ def _sample_stack(*args, **kwargs):
                 This should be called at a regular interval to collect samples.
                 """
 
-                if (
-                    not self.active_spans
-                    and not self.started_spans
-                    and not self.finished_spans
-                ):
+                # no profiles taking place, so we can stop early
+                if not self.new_profiles and not self.active_profiles:
                     self.running = False
                     return
 
-                started_spans = len(self.started_spans)
-                finished_spans = len(self.finished_spans)
+                # This is the number of profiles we want to pop off.
+                # It's possible another thread adds a new profile to
+                # the list and we spend longer than we want inside
+                # the loop below.
+                #
+                # Also make sure to set this value before extracting
+                # frames so we do not write to any new profiles that
+                # were started after this point.
+                new_profiles = len(self.new_profiles)
 
                 ts = now()
 
@@ -339,13 +344,32 @@ def _sample_stack(*args, **kwargs):
                     capture_internal_exception(sys.exc_info())
                     return
 
-                for _ in range(started_spans):
-                    self.started_spans.popleft()
-
-                for _ in range(finished_spans):
-                    self.finished_spans.popleft()
-
-                self.active_spans = self.active_spans + started_spans - finished_spans
+                # Move the new profiles into the active_profiles set.
+                #
+                # We cannot directly add the to active_profiles set
+                # in `start_profiling` because it is called from other
+                # threads which can cause a RuntimeError when it the
+                # set sizes changes during iteration without a lock.
+                #
+                # We also want to avoid using a lock here so threads
+                # that are starting profiles are not blocked until it
+                # can acquire the lock.
+                for _ in range(new_profiles):
+                    self.active_profiles.add(self.new_profiles.popleft())
+                inactive_profiles = []
+
+                for profile in self.active_profiles:
+                    if profile.active:
+                        pass
+                    else:
+                        # 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
+                        # in a RuntimeError.
+                        inactive_profiles.append(profile)
+
+                for profile in inactive_profiles:
+                    self.active_profiles.remove(profile)
 
                 if self.buffer is None:
                     self.reset_buffer()

sentry_sdk/profiler/transaction_profiler.py

@@ -644,7 +644,7 @@ def _sample_stack(*args, **kwargs):
                 if profile.active:
                     profile.write(now, sample)
                 else:
-                    # If a thread is marked inactive, we buffer it
+                    # 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
                     # in a RuntimeError.

sentry_sdk/scope.py

@@ -15,7 +15,7 @@
 from sentry_sdk.profiler.continuous_profiler import (
     get_profiler_id,
     try_autostart_continuous_profiler,
-    try_profile_lifecycle_auto_start,
+    try_profile_lifecycle_trace_start,
 )
 from sentry_sdk.profiler.transaction_profiler import Profile
 from sentry_sdk.session import Session
@@ -1055,12 +1055,12 @@ def start_transaction(
 
             transaction._profile = profile
 
-            transaction._started_profile_lifecycle = try_profile_lifecycle_auto_start()
+            transaction._continuous_profile = try_profile_lifecycle_trace_start()
 
             # Typically, the profiler is set when the transaction is created. But when
             # using the auto lifecycle, the profiler isn't running when the first
             # transaction is started. So make sure we update the profiler id on it.
-            if transaction._started_profile_lifecycle:
+            if transaction._continuous_profile is not None:
                 transaction.set_profiler_id(get_profiler_id())
 
             # we don't bother to keep spans if we already know we're not going to

sentry_sdk/tracing.py

@@ -5,10 +5,7 @@
 
 import sentry_sdk
 from sentry_sdk.consts import INSTRUMENTER, SPANSTATUS, SPANDATA
-from sentry_sdk.profiler.continuous_profiler import (
-    get_profiler_id,
-    try_profile_lifecycle_auto_stop,
-)
+from sentry_sdk.profiler.continuous_profiler import get_profiler_id
 from sentry_sdk.utils import (
     get_current_thread_meta,
     is_valid_sample_rate,
@@ -37,7 +34,8 @@
     P = ParamSpec("P")
     R = TypeVar("R")
 
-    import sentry_sdk.profiler
+    from sentry_sdk.profiler.continuous_profiler import ContinuousProfile
+    from sentry_sdk.profiler.transaction_profiler import Profile
     from sentry_sdk._types import (
         Event,
         MeasurementUnit,
@@ -272,7 +270,6 @@ class Span:
         "scope",
         "origin",
         "name",
-        "_started_profile_lifecycle",
     )
 
     def __init__(
@@ -771,6 +768,7 @@ class Transaction(Span):
         "_measurements",
         "_contexts",
         "_profile",
+        "_continuous_profile",
         "_baggage",
     )
 
@@ -792,10 +790,8 @@ def __init__(  # type: ignore[misc]
         self.parent_sampled = parent_sampled
         self._measurements = {}  # type: Dict[str, MeasurementValue]
         self._contexts = {}  # type: Dict[str, Any]
-        self._profile = (
-            None
-        )  # type: Optional[sentry_sdk.profiler.transaction_profiler.Profile]
-        self._started_profile_lifecycle = False  # type: bool
+        self._profile = None  # type: Optional[Profile]
+        self._continuous_profile = None  # type: Optional[ContinuousProfile]
         self._baggage = baggage
 
     def __repr__(self):
@@ -848,8 +844,8 @@ def __exit__(self, ty, value, tb):
         if self._profile is not None:
             self._profile.__exit__(ty, value, tb)
 
-        if self._started_profile_lifecycle:
-            try_profile_lifecycle_auto_stop()
+        if self._continuous_profile is not None:
+            self._continuous_profile.stop()
 
         super().__exit__(ty, value, tb)
 

tests/profiler/test_continuous_profiler.py

@@ -126,14 +126,17 @@ def test_continuous_profiler_setup_twice(mode, make_options, teardown_profiling)
     )
 
 
-def assert_single_transaction_with_profile_chunks(envelopes, thread):
+def assert_single_transaction_with_profile_chunks(
+    envelopes, thread, max_chunks, transactions=1
+):
     items = defaultdict(list)
     for envelope in envelopes:
         for item in envelope.items:
             items[item.type].append(item)
 
-    assert len(items["transaction"]) == 1
+    assert len(items["transaction"]) == transactions
     assert len(items["profile_chunk"]) > 0
+    assert len(items["profile_chunk"]) <= max_chunks
 
     transaction = items["transaction"][0].payload.json
 
@@ -231,7 +234,7 @@ def test_continuous_profiler_auto_start_and_manual_stop(
         with sentry_sdk.start_span(op="op"):
             time.sleep(0.05)
 
-    assert_single_transaction_with_profile_chunks(envelopes, thread)
+    assert_single_transaction_with_profile_chunks(envelopes, thread, max_chunks=10)
 
     for _ in range(3):
         stop_profiler()
@@ -252,7 +255,7 @@ def test_continuous_profiler_auto_start_and_manual_stop(
             with sentry_sdk.start_span(op="op"):
                 time.sleep(0.05)
 
-        assert_single_transaction_with_profile_chunks(envelopes, thread)
+        assert_single_transaction_with_profile_chunks(envelopes, thread, max_chunks=10)
 
 
 @pytest.mark.parametrize(
@@ -298,7 +301,7 @@ def test_continuous_profiler_manual_start_and_stop_sampled(
             with sentry_sdk.start_span(op="op"):
                 time.sleep(0.05)
 
-        assert_single_transaction_with_profile_chunks(envelopes, thread)
+        assert_single_transaction_with_profile_chunks(envelopes, thread, max_chunks=10)
 
         stop_profiler()
 
@@ -367,15 +370,17 @@ def test_continuous_profiler_manual_start_and_stop_unsampled(
         pytest.param(get_client_options(False), id="experiment"),
     ],
 )
-@mock.patch("sentry_sdk.profiler.continuous_profiler.PROFILE_BUFFER_SECONDS", 0.01)
+@mock.patch("sentry_sdk.profiler.continuous_profiler.PROFILE_BUFFER_SECONDS", 0.1)
 def test_continuous_profiler_auto_start_and_stop_sampled(
     sentry_init,
     capture_envelopes,
     mode,
     make_options,
     teardown_profiling,
 ):
-    options = make_options(mode=mode, profile_session_sample_rate=1.0, lifecycle="auto")
+    options = make_options(
+        mode=mode, profile_session_sample_rate=1.0, lifecycle="trace"
+    )
     sentry_init(
         traces_sample_rate=1.0,
         **options,
@@ -391,13 +396,26 @@ def test_continuous_profiler_auto_start_and_stop_sampled(
         with sentry_sdk.start_transaction(name="profiling"):
             assert get_profiler_id() is not None, "profiler should be running"
             with sentry_sdk.start_span(op="op"):
-                time.sleep(0.05)
+                time.sleep(0.03)
+            assert get_profiler_id() is not None, "profiler should be running"
+
+        # the profiler takes a while to stop 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"
+
+        with sentry_sdk.start_transaction(name="profiling"):
+            assert get_profiler_id() is not None, "profiler should be running"
+            with sentry_sdk.start_span(op="op"):
+                time.sleep(0.03)
             assert get_profiler_id() is not None, "profiler should be running"
 
         # wait at least 1 cycle for the profiler to stop
         time.sleep(0.05)
         assert get_profiler_id() is None, "profiler should not be running"
-        assert_single_transaction_with_profile_chunks(envelopes, thread)
+
+        assert_single_transaction_with_profile_chunks(
+            envelopes, thread, max_chunks=1, transactions=2
+        )
 
 
 @pytest.mark.parametrize(
@@ -422,7 +440,9 @@ def test_continuous_profiler_auto_start_and_stop_unsampled(
     make_options,
     teardown_profiling,
 ):
-    options = make_options(mode=mode, profile_session_sample_rate=0.0, lifecycle="auto")
+    options = make_options(
+        mode=mode, profile_session_sample_rate=0.0, lifecycle="trace"
+    )
     sentry_init(
         traces_sample_rate=1.0,
         **options,