Fix propagate_scope=False in ThreadingIntegration (#4310)

`ThreadingIntegration` can optionally **NOT** propagate scope data to
threads (`propagate_scope=False`). In that case, in POTel we were
wrapping the thread's task in an `isolation_scope()`:

```python
with sentry_sdk.isolation_scope() as scope:
    return _run_old_run_func()
```

But as this forks the currently active isolation scope, the thread
effectively gets all scope data from the parent isolation scope -- so
the scope is actually propagated to the thread, even though it shouldn't
be since `propagate_scope=False`.

~We effectively need some way to give the thread a clear isolation scope
instead. In this PR, I'm just clearing the forked iso scope, but I'm not
sure if this is good enough and if something doesn't need to be done on
the OTel side too.~

~Another option would be to set the iso/current scopes to the initial,
empty iso/current scopes instead, before running the thread's target
function.~

UPDATE: we're just instantiating new scopes now


Another change is that in OTel, the spans in the threads, now without a
parent, automatically get promoted to transactions. (On master they'd
just be orphaned spans, so they wouldn't be taken into account at all.)
We probably need to instruct folks to add `only_if_parent` if they don't
want this to happen.

---------

Co-authored-by: Neel Shah <[email protected]>

Author: sentrivana

sentry_sdk/__init__.py

@@ -1,4 +1,6 @@
-from sentry_sdk.scope import Scope
+# TODO-neel scope switch
+# TODO-neel avoid duplication between api and __init__
+from sentry_sdk.opentelemetry.scope import PotelScope as Scope
 from sentry_sdk.transport import Transport, HttpTransport
 from sentry_sdk.client import Client
 

sentry_sdk/integrations/threading.py

@@ -4,6 +4,8 @@
 from threading import Thread, current_thread
 
 import sentry_sdk
+from sentry_sdk import Scope
+from sentry_sdk.scope import ScopeType
 from sentry_sdk.integrations import Integration
 from sentry_sdk.utils import (
     event_from_exception,
@@ -17,7 +19,6 @@
     from typing import Any
     from typing import TypeVar
     from typing import Callable
-    from typing import Optional
 
     from sentry_sdk._types import ExcInfo
 
@@ -75,8 +76,8 @@ def sentry_start(self, *a, **kw):
                     isolation_scope = sentry_sdk.get_isolation_scope().fork()
                     current_scope = sentry_sdk.get_current_scope().fork()
             else:
-                isolation_scope = None
-                current_scope = None
+                isolation_scope = Scope(ty=ScopeType.ISOLATION)
+                current_scope = Scope(ty=ScopeType.CURRENT)
 
             # Patching instance methods in `start()` creates a reference cycle if
             # done in a naive way. See
@@ -98,7 +99,7 @@ def sentry_start(self, *a, **kw):
 
 
 def _wrap_run(isolation_scope_to_use, current_scope_to_use, old_run_func):
-    # type: (Optional[sentry_sdk.Scope], Optional[sentry_sdk.Scope], F) -> F
+    # type: (sentry_sdk.Scope, sentry_sdk.Scope, F) -> F
     @wraps(old_run_func)
     def run(*a, **kw):
         # type: (*Any, **Any) -> Any
@@ -110,12 +111,8 @@ def _run_old_run_func():
             except Exception:
                 reraise(*_capture_exception())
 
-        if isolation_scope_to_use is not None and current_scope_to_use is not None:
-            with sentry_sdk.use_isolation_scope(isolation_scope_to_use):
-                with sentry_sdk.use_scope(current_scope_to_use):
-                    return _run_old_run_func()
-        else:
-            with sentry_sdk.isolation_scope():
+        with sentry_sdk.use_isolation_scope(isolation_scope_to_use):
+            with sentry_sdk.use_scope(current_scope_to_use):
                 return _run_old_run_func()
 
     return run  # type: ignore

sentry_sdk/integrations/wsgi.py

@@ -254,7 +254,7 @@ class _ScopedResponse:
     __slots__ = ("_response", "_scope")
 
     def __init__(self, scope, response):
-        # type: (sentry_sdk.scope.Scope, Iterator[bytes]) -> None
+        # type: (sentry_sdk.Scope, Iterator[bytes]) -> None
         self._scope = scope
         self._response = response
 

sentry_sdk/opentelemetry/scope.py

@@ -175,7 +175,7 @@ def setup_scope_context_management():
 
 @contextmanager
 def isolation_scope():
-    # type: () -> Generator[Scope, None, None]
+    # type: () -> Generator[PotelScope, None, None]
     context = set_value(SENTRY_FORK_ISOLATION_SCOPE_KEY, True)
     token = attach(context)
     try:
@@ -186,7 +186,7 @@ def isolation_scope():
 
 @contextmanager
 def new_scope():
-    # type: () -> Generator[Scope, None, None]
+    # type: () -> Generator[PotelScope, None, None]
     token = attach(get_current())
     try:
         yield PotelScope.get_current_scope()
@@ -196,7 +196,7 @@ def new_scope():
 
 @contextmanager
 def use_scope(scope):
-    # type: (Scope) -> Generator[Scope, None, None]
+    # type: (PotelScope) -> Generator[PotelScope, None, None]
     context = set_value(SENTRY_USE_CURRENT_SCOPE_KEY, scope)
     token = attach(context)
 
@@ -208,7 +208,7 @@ def use_scope(scope):
 
 @contextmanager
 def use_isolation_scope(isolation_scope):
-    # type: (Scope) -> Generator[Scope, None, None]
+    # type: (PotelScope) -> Generator[PotelScope, None, None]
     context = set_value(SENTRY_USE_ISOLATION_SCOPE_KEY, isolation_scope)
     token = attach(context)
 

tests/integrations/threading/test_threading.py

@@ -232,7 +232,7 @@ def do_some_work(number):
     threads = []
 
     with sentry_sdk.start_span(op="outer-trx"):
-        for number in range(5):
+        for number in range(2):
             with sentry_sdk.start_span(
                 op=f"outer-submit-{number}", name="Thread: main"
             ):
@@ -243,32 +243,44 @@ def do_some_work(number):
         for t in threads:
             t.join()
 
-    (event,) = events
     if propagate_scope:
+        # The children spans from the threads become parts of the existing span
+        # tree since we propagated the scope
+        assert len(events) == 1
+        (event,) = events
+
         assert render_span_tree(event) == dedent(
             """\
             - op="outer-trx": description=null
               - op="outer-submit-0": description="Thread: main"
                 - op="inner-run-0": description="Thread: child-0"
               - op="outer-submit-1": description="Thread: main"
-                - op="inner-run-1": description="Thread: child-1"
-              - op="outer-submit-2": description="Thread: main"
-                - op="inner-run-2": description="Thread: child-2"
-              - op="outer-submit-3": description="Thread: main"
-                - op="inner-run-3": description="Thread: child-3"
-              - op="outer-submit-4": description="Thread: main"
-                - op="inner-run-4": description="Thread: child-4"\
+                - op="inner-run-1": description="Thread: child-1"\
 """
         )
 
     elif not propagate_scope:
-        assert render_span_tree(event) == dedent(
+        # The spans from the threads become their own root spans/transactions
+        # as the connection to the parent span was severed when the scope was
+        # cleared
+        assert len(events) == 3
+        (event1, event2, event3) = sorted(events, key=render_span_tree)
+
+        assert render_span_tree(event1) == dedent(
+            """\
+            - op="inner-run-0": description=null\
+"""
+        )
+        assert render_span_tree(event2) == dedent(
+            """\
+            - op="inner-run-1": description=null\
+"""
+        )
+
+        assert render_span_tree(event3) == dedent(
             """\
             - op="outer-trx": description=null
               - op="outer-submit-0": description="Thread: main"
-              - op="outer-submit-1": description="Thread: main"
-              - op="outer-submit-2": description="Thread: main"
-              - op="outer-submit-3": description="Thread: main"
-              - op="outer-submit-4": description="Thread: main"\
+              - op="outer-submit-1": description="Thread: main"\
 """
         )