Skip to content

Commit 92f5f0a

Browse files
authored
Fix ThreadingIntegration by carrying forward span reference in (#3851)
`use_scope` Since the otel context span reference is the source of truth for the current active span, we need to explicitly pass the span reference on the scope through when we use `use_scope` since we are changing context variables in the other thread. Also, * fixes some typing in the original scope * adds more types to the `contextvars_context` manager
1 parent 8ce7510 commit 92f5f0a

File tree

6 files changed

+51
-33
lines changed

6 files changed

+51
-33
lines changed

MIGRATION_GUIDE.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -138,6 +138,7 @@ Looking to upgrade from Sentry SDK 2.x to 3.x? Here's a comprehensive list of wh
138138
- `span.containing_transaction` has been removed. Use `span.root_span` instead.
139139
- `continue_from_headers`, `continue_from_environ` and `from_traceparent` have been removed, please use top-level API `sentry_sdk.continue_trace` instead.
140140
- `PropagationContext` constructor no longer takes a `dynamic_sampling_context` but takes a `baggage` object instead.
141+
- `ThreadingIntegration` no longer takes the `propagate_hub` argument.
141142

142143
### Deprecated
143144

sentry_sdk/integrations/opentelemetry/contextvars_context.py

Lines changed: 30 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,6 @@
1+
from typing import cast, TYPE_CHECKING
2+
3+
from opentelemetry.trace import set_span_in_context
14
from opentelemetry.context import Context, get_value, set_value
25
from opentelemetry.context.contextvars_context import ContextVarsRuntimeContext
36

@@ -9,25 +12,50 @@
912
SENTRY_USE_ISOLATION_SCOPE_KEY,
1013
)
1114

15+
if TYPE_CHECKING:
16+
from typing import Optional
17+
from sentry_sdk.integrations.opentelemetry.scope import PotelScope
18+
1219

1320
class SentryContextVarsRuntimeContext(ContextVarsRuntimeContext):
1421
def attach(self, context):
1522
# type: (Context) -> object
1623
scopes = get_value(SENTRY_SCOPES_KEY, context)
24+
1725
should_fork_isolation_scope = context.pop(
1826
SENTRY_FORK_ISOLATION_SCOPE_KEY, False
1927
)
28+
should_fork_isolation_scope = cast("bool", should_fork_isolation_scope)
29+
2030
should_use_isolation_scope = context.pop(SENTRY_USE_ISOLATION_SCOPE_KEY, None)
31+
should_use_isolation_scope = cast(
32+
"Optional[PotelScope]", should_use_isolation_scope
33+
)
34+
2135
should_use_current_scope = context.pop(SENTRY_USE_CURRENT_SCOPE_KEY, None)
36+
should_use_current_scope = cast(
37+
"Optional[PotelScope]", should_use_current_scope
38+
)
2239

23-
if scopes and isinstance(scopes, tuple):
40+
if scopes:
41+
scopes = cast("tuple[PotelScope, PotelScope]", scopes)
2442
(current_scope, isolation_scope) = scopes
2543
else:
2644
current_scope = sentry_sdk.get_current_scope()
2745
isolation_scope = sentry_sdk.get_isolation_scope()
2846

47+
new_context = context
48+
2949
if should_use_current_scope:
3050
new_scope = should_use_current_scope
51+
52+
# the main case where we use use_scope is for
53+
# scope propagation in the ThreadingIntegration
54+
# so we need to carry forward the span reference explicitly too
55+
span = should_use_current_scope.span
56+
if span:
57+
new_context = set_span_in_context(span._otel_span, new_context)
58+
3159
else:
3260
new_scope = current_scope.fork()
3361

@@ -40,5 +68,5 @@ def attach(self, context):
4068

4169
new_scopes = (new_scope, new_isolation_scope)
4270

43-
new_context = set_value(SENTRY_SCOPES_KEY, new_scopes, context)
71+
new_context = set_value(SENTRY_SCOPES_KEY, new_scopes, new_context)
4472
return super().attach(new_context)

sentry_sdk/integrations/threading.py

Lines changed: 4 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@
77
from sentry_sdk.utils import (
88
event_from_exception,
99
capture_internal_exceptions,
10-
logger,
1110
reraise,
1211
)
1312

@@ -27,22 +26,10 @@
2726
class ThreadingIntegration(Integration):
2827
identifier = "threading"
2928

30-
def __init__(self, propagate_hub=None, propagate_scope=True):
31-
# type: (Optional[bool], bool) -> None
32-
if propagate_hub is not None:
33-
logger.warning(
34-
"Deprecated: propagate_hub is deprecated. This will be removed in the future."
35-
)
36-
37-
# Note: propagate_hub did not have any effect on propagation of scope data
38-
# scope data was always propagated no matter what the value of propagate_hub was
39-
# This is why the default for propagate_scope is True
40-
29+
def __init__(self, propagate_scope=True):
30+
# type: (bool) -> None
4131
self.propagate_scope = propagate_scope
4232

43-
if propagate_hub is not None:
44-
self.propagate_scope = propagate_hub
45-
4633
@staticmethod
4734
def setup_once():
4835
# type: () -> None
@@ -99,7 +86,8 @@ def _run_old_run_func():
9986
with sentry_sdk.use_scope(current_scope_to_use):
10087
return _run_old_run_func()
10188
else:
102-
return _run_old_run_func()
89+
with sentry_sdk.isolation_scope():
90+
return _run_old_run_func()
10391

10492
return run # type: ignore
10593

sentry_sdk/scope.py

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
SENTRY_TRACE_HEADER_NAME,
2727
NoOpSpan,
2828
Span,
29+
POTelSpan,
2930
Transaction,
3031
)
3132
from sentry_sdk.utils import (
@@ -669,7 +670,7 @@ def clear(self):
669670
self.clear_breadcrumbs()
670671
self._should_capture = True # type: bool
671672

672-
self._span = None # type: Optional[Span]
673+
self._span = None # type: Optional[POTelSpan]
673674
self._session = None # type: Optional[Session]
674675
self._force_auto_session_tracking = None # type: Optional[bool]
675676

@@ -777,13 +778,13 @@ def set_user(self, value):
777778

778779
@property
779780
def span(self):
780-
# type: () -> Optional[Span]
781+
# type: () -> Optional[POTelSpan]
781782
"""Get current tracing span."""
782783
return self._span
783784

784785
@span.setter
785786
def span(self, span):
786-
# type: (Optional[Span]) -> None
787+
# type: (Optional[POTelSpan]) -> None
787788
"""Set current tracing span."""
788789
self._span = span
789790

sentry_sdk/tracing.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1280,11 +1280,11 @@ def __eq__(self, other):
12801280
def __repr__(self):
12811281
# type: () -> str
12821282
return (
1283-
"<%s(op=%r, description:%r, trace_id=%r, span_id=%r, parent_span_id=%r, sampled=%r, origin=%r)>"
1283+
"<%s(op=%r, name:%r, trace_id=%r, span_id=%r, parent_span_id=%r, sampled=%r, origin=%r)>"
12841284
% (
12851285
self.__class__.__name__,
12861286
self.op,
1287-
self.description,
1287+
self.name,
12881288
self.trace_id,
12891289
self.span_id,
12901290
self.parent_span_id,

tests/integrations/threading/test_threading.py

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -35,11 +35,11 @@ def crash():
3535
assert not events
3636

3737

38-
@pytest.mark.parametrize("propagate_hub", (True, False))
39-
def test_propagates_hub(sentry_init, capture_events, propagate_hub):
38+
@pytest.mark.parametrize("propagate_scope", (True, False))
39+
def test_propagates_scope(sentry_init, capture_events, propagate_scope):
4040
sentry_init(
4141
default_integrations=False,
42-
integrations=[ThreadingIntegration(propagate_hub=propagate_hub)],
42+
integrations=[ThreadingIntegration(propagate_scope=propagate_scope)],
4343
)
4444
events = capture_events()
4545

@@ -65,33 +65,33 @@ def stage2():
6565
assert exception["mechanism"]["type"] == "threading"
6666
assert not exception["mechanism"]["handled"]
6767

68-
if propagate_hub:
68+
if propagate_scope:
6969
assert event["tags"]["stage1"] == "true"
7070
else:
7171
assert "stage1" not in event.get("tags", {})
7272

7373

74-
@pytest.mark.parametrize("propagate_hub", (True, False))
75-
def test_propagates_threadpool_hub(sentry_init, capture_events, propagate_hub):
74+
@pytest.mark.parametrize("propagate_scope", (True, False))
75+
def test_propagates_threadpool_scope(sentry_init, capture_events, propagate_scope):
7676
sentry_init(
7777
traces_sample_rate=1.0,
78-
integrations=[ThreadingIntegration(propagate_hub=propagate_hub)],
78+
integrations=[ThreadingIntegration(propagate_scope=propagate_scope)],
7979
)
8080
events = capture_events()
8181

8282
def double(number):
83-
with sentry_sdk.start_span(op="task", name=str(number)):
83+
with sentry_sdk.start_span(op="task", name=str(number), only_if_parent=True):
8484
return number * 2
8585

86-
with sentry_sdk.start_transaction(name="test_handles_threadpool"):
86+
with sentry_sdk.start_span(name="test_handles_threadpool"):
8787
with futures.ThreadPoolExecutor(max_workers=1) as executor:
8888
tasks = [executor.submit(double, number) for number in [1, 2, 3, 4]]
8989
for future in futures.as_completed(tasks):
9090
print("Getting future value!", future.result())
9191

9292
sentry_sdk.flush()
9393

94-
if propagate_hub:
94+
if propagate_scope:
9595
assert len(events) == 1
9696
(event,) = events
9797
assert event["spans"][0]["trace_id"] == event["spans"][1]["trace_id"]

0 commit comments

Comments
 (0)