8000 Fix `propagate_scope=False` in `ThreadingIntegration` (#4310) · getsentry/sentry-python@a7e24d9 · GitHub
[go: up one dir, main page]
Skip to content

Commit a7e24d9

Browse files
sentrivanasl0thentr0py
sentrivana
and
sl0thentr0py
authored
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 <neel.shah@sentry.io>
1 parent d4b9d50 commit a7e24d9

File tree

5 files changed

+41
-30
lines changed

5 files changed

+41
-30
lines changed

‎sentry_sdk/__init__.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,6 @@
1-
from sentry_sdk.scope import Scope
1+
# TODO-neel scope switch
2+
# TODO-neel avoid duplication between api and __init__
3+
from sentry_sdk.opentelemetry.scope import PotelScope as Scope
24
from sentry_sdk.transport import Transport, HttpTransport
35
from sentry_sdk.client import Client
46

‎sentry_sdk/integrations/threading.py

Lines changed: 7 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@
44
from threading import Thread, current_thread
55

66
import sentry_sdk
7+
from sentry_sdk import Scope
8+
from sentry_sdk.scope import ScopeType
79
from sentry_sdk.integrations import Integration
810
from sentry_sdk.utils import (
911
event_from_exception,
@@ -17,7 +19,6 @@
1719
from typing import Any
1820
from typing import TypeVar
1921
from typing import Callable
20-
from typing import Optional
2122

2223
from sentry_sdk._types import ExcInfo
2324

@@ -75,8 +76,8 @@ def sentry_start(self, *a, **kw):
7576
isolation_scope = sentry_sdk.get_isolation_scope().fork()
7677
current_scope = sentry_sdk.get_current_scope().fork()
7778
else:
78-
isolation_scope = None
79-
current_scope = None
79+
isolation_scope = Scope(ty=ScopeType.ISOLATION)
80+
current_scope = Scope(ty=ScopeType.CURRENT)
8081

8182
# Patching instance methods in `start()` creates a reference cycle if
8283
# done in a naive way. See
@@ -98,7 +99,7 @@ def sentry_start(self, *a, **kw):
9899

99100

100101
def _wrap_run(isolation_scope_to_use, current_scope_to_use, old_run_func):
101-
# type: (Optional[sentry_sdk.Scope], Optional[sentry_sdk.Scope], F) -> F
102+
# type: (sentry_sdk.Scope, sentry_sdk.Scope, F) -> F
102103
@wraps(old_run_func)
103104
def run(*a, **kw):
104105
# type: (*Any, **Any) -> Any
@@ -110,12 +111,8 @@ def _run_old_run_func():
110111
except Exception:
111112
reraise(*_capture_exception())
112113

113-
if isolation_scope_to_use is not None and current_scope_to_use is not None:
114-
with sentry_sdk.use_isolation_scope(isolation_scope_to_use):
115-
with sentry_sdk.use_scope(current_scope_to_use):
116-
return _run_old_run_func()
117-
else:
118-
with sentry_sdk.isolation_scope():
114+
with sentry_sdk.use_isolation_scope(isolation_scope_to_use):
115+
with sentry_sdk.use_scope(current_scope_to_use):
119116
return _run_old_run_func()
120117

121118
return run # type: ignore

‎sentry_sdk/integrations/wsgi.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -254,7 +254,7 @@ class _ScopedResponse:
254254
__slots__ = ("_response", "_scope")
255255

256256
def __init__(self, scope, response):
257-
# type: (sentry_sdk.scope.Scope, Iterator[bytes]) -> None
257+
# type: (sentry_sdk.Scope, Iterator[bytes]) -> None
258258
self._scope = scope
259259
self._response = response
260260

‎sentry_sdk/opentelemetry/scope.py

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -175,7 +175,7 @@ def setup_scope_context_management():
175175

176176
@contextmanager
177177
def isolation_scope():
178-
# type: () -> Generator[Scope, None, None]
178+
# type: () -> Generator[PotelScope, None, None]
179179
context = set_value(SENTRY_FORK_ISOLATION_SCOPE_KEY, True)
180180
token = attach(context)
181181
try:
@@ -186,7 +186,7 @@ def isolation_scope():
186186

187187
@contextmanager
188188
def new_scope():
189-
# type: () -> Generator[Scope, None, None]
189+
# type: () -> Generator[PotelScope, None, None]
190190
token = attach(get_current())
191191
try:
192192
yield PotelScope.get_current_scope()
@@ -196,7 +196,7 @@ def new_scope():
196196

197197
@contextmanager
198198
def use_scope(scope):
199-
# type: (Scope) -> Generator[Scope, None, None]
199+
# type: (PotelScope) -> Generator[PotelScope, None, None]
200200
context = set_value(SENTRY_USE_CURRENT_SCOPE_KEY, scope)
201201
token = attach(context)
202202

@@ -208,7 +208,7 @@ def use_scope(scope):
208208

209209
@contextmanager
210210
def use_isolation_scope(isolation_scope):
211-
# type: (Scope) -> Generator[Scope, None, None]
211+
# type: (PotelScope) -> Generator[PotelScope, None, None]
212212
context = set_value(SENTRY_USE_ISOLATION_SCOPE_KEY, isolation_scope)
213213
token = attach(context)
214214

‎tests/integrations/threading/test_threading.py

Lines changed: 26 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -232,7 +232,7 @@ def do_some_work(number):
232232
threads = []
233233

234234
with sentry_sdk.start_span(op="outer-trx"):
235-
for number in range(5):
235+
for number in range(2):
236236
with sentry_sdk.start_span(
237237
op=f"outer-submit-{number}", name="Thread: main"
238238
):
@@ -243,32 +243,44 @@ def do_some_work(number):
243243
for t in threads:
244244
t.join()
245245

246-
(event,) = events
247246
if propagate_scope:
247+
# The children spans from the threads become parts of the existing span
248+
# tree since we propagated the scope
249+
assert len(events) == 1
250+
(event,) = events
251+
248252
assert render_span_tree(event) == dedent(
249253
"""\
250254
- op="outer-trx": description=null
251255
- op="outer-submit-0": description="Thread: main"
252256
- op="inner-run-0": description="Thread: child-0"
253257
- op="outer-submit-1": description="Thread: main"
254-
- op="inner-run-1": description="Thread: child-1"
255-
- op="outer-submit-2": description="Thread: main"
256-
- op="inner-run-2": description="Thread: child-2"
257-
- op="outer-submit-3": description="Thread: main"
258-
- op="inner-run-3": description="Thread: child-3"
259-
- op="outer-submit-4": description="Thread: main"
260-
- op="inner-run-4": description="Thread: child-4"\
258+
- op="inner-run-1": description="Thread: child-1"\
261259
"""
262260
)
263261

264262
elif not propagate_scope:
265-
assert render_span_tree(event) == dedent(
263+
# The spans from the threads become their own root spans/transactions
264+
# as the connection to the parent span was severed when the scope was
265+
# cleared
266+
assert len(events) == 3
267+
(event1, event2, event3) = sorted(events, key=render_span_tree)
268+
269+
assert render_span_tree(event1) == dedent(
270+
"""\
271+
- op="inner-run-0": description=null\
272+
"""
273+
)
274+
assert render_span_tree(event2) == dedent(
275+
"""\
276+
- op="inner-run-1": description=null\
277+
"""
278+
)
279+
280+
assert render_span_tree(event3) == dedent(
266281
"""\
267282
- op="outer-trx": description=null
268283
- op="outer-submit-0": description=" 1C6A ;Thread: main"
269-
- op="outer-submit-1": description="Thread: main"
270-
- op="outer-submit-2": description="Thread: main"
271-
- op="outer-submit-3": description="Thread: main"
272-
- op="outer-submit-4": descrip 47DE tion="Thread: main"\
284+
- op="outer-submit-1": description="Thread: main"\
273285
"""
274286
)

0 commit comments

Comments
 (0)
0