Skip to content

Commit

Permalink
Move scope context init outside integration (#3850)
Browse files Browse the repository at this point in the history
* Move scope context init outside integration

* 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
  • Loading branch information
sl0thentr0py authored Dec 5, 2024
1 parent abd4baa commit 65fbd50
Show file tree
Hide file tree
Showing 11 changed files with 72 additions and 53 deletions.
1 change: 1 addition & 0 deletions MIGRATION_GUIDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,7 @@ Looking to upgrade from Sentry SDK 2.x to 3.x? Here's a comprehensive list of wh
- `span.containing_transaction` has been removed. Use `span.root_span` instead.
- `continue_from_headers`, `continue_from_environ` and `from_traceparent` have been removed, please use top-level API `sentry_sdk.continue_trace` instead.
- `PropagationContext` constructor no longer takes a `dynamic_sampling_context` but takes a `baggage` object instead.
- `ThreadingIntegration` no longer takes the `propagate_hub` argument.

### Deprecated

Expand Down
2 changes: 2 additions & 0 deletions sentry_sdk/_init_implementation.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
from typing import TYPE_CHECKING

import sentry_sdk
from sentry_sdk.integrations.opentelemetry.scope import setup_scope_context_management

if TYPE_CHECKING:
from typing import Any, Optional
Expand All @@ -24,6 +25,7 @@ def _init(*args, **kwargs):
"""
client = sentry_sdk.Client(*args, **kwargs)
sentry_sdk.get_global_scope().set_client(client)
setup_scope_context_management()
_check_python_deprecations()


Expand Down
32 changes: 30 additions & 2 deletions sentry_sdk/integrations/opentelemetry/contextvars_context.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
from typing import cast, TYPE_CHECKING

from opentelemetry.trace import set_span_in_context
from opentelemetry.context import Context, get_value, set_value
from opentelemetry.context.contextvars_context import ContextVarsRuntimeContext

Expand All @@ -9,25 +12,50 @@
SENTRY_USE_ISOLATION_SCOPE_KEY,
)

if TYPE_CHECKING:
from typing import Optional
from sentry_sdk.integrations.opentelemetry.scope import PotelScope


class SentryContextVarsRuntimeContext(ContextVarsRuntimeContext):
def attach(self, context):
# type: (Context) -> object
scopes = get_value(SENTRY_SCOPES_KEY, context)

should_fork_isolation_scope = context.pop(
SENTRY_FORK_ISOLATION_SCOPE_KEY, False
)
should_fork_isolation_scope = cast("bool", should_fork_isolation_scope)

should_use_isolation_scope = context.pop(SENTRY_USE_ISOLATION_SCOPE_KEY, None)
should_use_isolation_scope = cast(
"Optional[PotelScope]", should_use_isolation_scope
)

should_use_current_scope = context.pop(SENTRY_USE_CURRENT_SCOPE_KEY, None)
should_use_current_scope = cast(
"Optional[PotelScope]", should_use_current_scope
)

if scopes and isinstance(scopes, tuple):
if scopes:
scopes = cast("tuple[PotelScope, PotelScope]", scopes)
(current_scope, isolation_scope) = scopes
else:
current_scope = sentry_sdk.get_current_scope()
isolation_scope = sentry_sdk.get_isolation_scope()

new_context = context

if should_use_current_scope:
new_scope = should_use_current_scope

# the main case where we use use_scope is for
# scope propagation in the ThreadingIntegration
# so we need to carry forward the span reference explicitly too
span = should_use_current_scope.span
if span:
new_context = set_span_in_context(span._otel_span, new_context)

else:
new_scope = current_scope.fork()

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

new_scopes = (new_scope, new_isolation_scope)

new_context = set_value(SENTRY_SCOPES_KEY, new_scopes, context)
new_context = set_value(SENTRY_SCOPES_KEY, new_scopes, new_context)
return super().attach(new_context)
13 changes: 0 additions & 13 deletions sentry_sdk/integrations/opentelemetry/integration.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,10 @@
"""

from sentry_sdk.integrations import DidNotEnable, Integration
from sentry_sdk.integrations.opentelemetry.scope import setup_initial_scopes
from sentry_sdk.integrations.opentelemetry.propagator import SentryPropagator
from sentry_sdk.integrations.opentelemetry.span_processor import (
SentrySpanProcessor,
)
from sentry_sdk.integrations.opentelemetry.contextvars_context import (
SentryContextVarsRuntimeContext,
)
from sentry_sdk.integrations.opentelemetry.sampler import SentrySampler
from sentry_sdk.utils import logger

Expand Down Expand Up @@ -45,7 +41,6 @@ def setup_once():
"Use at your own risk."
)

_setup_scope_context_management()
_setup_sentry_tracing()
_patch_readable_span()
# _setup_instrumentors()
Expand All @@ -70,14 +65,6 @@ def sentry_patched_readable_span(self):
Span._readable_span = sentry_patched_readable_span


def _setup_scope_context_management():
# type: () -> None
import opentelemetry.context

opentelemetry.context._RUNTIME_CONTEXT = SentryContextVarsRuntimeContext()
setup_initial_scopes()


def _setup_sentry_tracing():
# type: () -> None
provider = TracerProvider(sampler=SentrySampler())
Expand Down
12 changes: 11 additions & 1 deletion sentry_sdk/integrations/opentelemetry/scope.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
from contextlib import contextmanager

from opentelemetry.context import (
Context,
get_value,
set_value,
attach,
Expand All @@ -24,6 +23,9 @@
SENTRY_USE_ISOLATION_SCOPE_KEY,
TRACESTATE_SAMPLED_KEY,
)
from sentry_sdk.integrations.opentelemetry.contextvars_context import (
SentryContextVarsRuntimeContext,
)
from sentry_sdk.integrations.opentelemetry.utils import trace_state_from_baggage
from sentry_sdk.scope import Scope, ScopeType
from sentry_sdk.tracing import POTelSpan
Expand Down Expand Up @@ -152,6 +154,14 @@ def setup_initial_scopes():
attach(set_value(SENTRY_SCOPES_KEY, scopes))


def setup_scope_context_management():
# type: () -> None
import opentelemetry.context

opentelemetry.context._RUNTIME_CONTEXT = SentryContextVarsRuntimeContext()
setup_initial_scopes()


@contextmanager
def isolation_scope():
# type: () -> Generator[Scope, None, None]
Expand Down
20 changes: 4 additions & 16 deletions sentry_sdk/integrations/threading.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
from sentry_sdk.utils import (
event_from_exception,
capture_internal_exceptions,
logger,
reraise,
)

Expand All @@ -27,22 +26,10 @@
class ThreadingIntegration(Integration):
identifier = "threading"

def __init__(self, propagate_hub=None, propagate_scope=True):
# type: (Optional[bool], bool) -> None
if propagate_hub is not None:
logger.warning(
"Deprecated: propagate_hub is deprecated. This will be removed in the future."
)

# Note: propagate_hub did not have any effect on propagation of scope data
# scope data was always propagated no matter what the value of propagate_hub was
# This is why the default for propagate_scope is True

def __init__(self, propagate_scope=True):
# type: (bool) -> None
self.propagate_scope = propagate_scope

if propagate_hub is not None:
self.propagate_scope = propagate_hub

@staticmethod
def setup_once():
# type: () -> None
Expand Down Expand Up @@ -99,7 +86,8 @@ def _run_old_run_func():
with sentry_sdk.use_scope(current_scope_to_use):
return _run_old_run_func()
else:
return _run_old_run_func()
with sentry_sdk.isolation_scope():
return _run_old_run_func()

return run # type: ignore

Expand Down
7 changes: 4 additions & 3 deletions sentry_sdk/scope.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
SENTRY_TRACE_HEADER_NAME,
NoOpSpan,
Span,
POTelSpan,
Transaction,
)
from sentry_sdk.utils import (
Expand Down Expand Up @@ -669,7 +670,7 @@ def clear(self):
self.clear_breadcrumbs()
self._should_capture = True # type: bool

self._span = None # type: Optional[Span]
self._span = None # type: Optional[POTelSpan]
self._session = None # type: Optional[Session]
self._force_auto_session_tracking = None # type: Optional[bool]

Expand Down Expand Up @@ -777,13 +778,13 @@ def set_user(self, value):

@property
def span(self):
# type: () -> Optional[Span]
# type: () -> Optional[POTelSpan]
"""Get current tracing span."""
return self._span

@span.setter
def span(self, span):
# type: (Optional[Span]) -> None
# type: (Optional[POTelSpan]) -> None
"""Set current tracing span."""
self._span = span

Expand Down
4 changes: 2 additions & 2 deletions sentry_sdk/tracing.py
Original file line number Diff line number Diff line change
Expand Up @@ -1280,11 +1280,11 @@ def __eq__(self, other):
def __repr__(self):
# type: () -> str
return (
"<%s(op=%r, description:%r, trace_id=%r, span_id=%r, parent_span_id=%r, sampled=%r, origin=%r)>"
"<%s(op=%r, name:%r, trace_id=%r, span_id=%r, parent_span_id=%r, sampled=%r, origin=%r)>"
% (
self.__class__.__name__,
self.op,
self.description,
self.name,
self.trace_id,
self.span_id,
self.parent_span_id,
Expand Down
8 changes: 6 additions & 2 deletions tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,10 @@ def benchmark():


from sentry_sdk import scope
import sentry_sdk.integrations.opentelemetry.scope as potel_scope
from sentry_sdk.integrations.opentelemetry.scope import (
setup_scope_context_management,
setup_initial_scopes,
)


@pytest.fixture(autouse=True)
Expand All @@ -74,7 +77,7 @@ def clean_scopes():
scope._isolation_scope.set(None)
scope._current_scope.set(None)

potel_scope.setup_initial_scopes()
setup_initial_scopes()


@pytest.fixture(autouse=True)
Expand Down Expand Up @@ -187,6 +190,7 @@ def inner(*a, **kw):
kw.setdefault("transport", TestTransport())
client = sentry_sdk.Client(*a, **kw)
sentry_sdk.get_global_scope().set_client(client)
setup_scope_context_management()

if request.node.get_closest_marker("forked"):
# Do not run isolation if the test is already running in
Expand Down
20 changes: 10 additions & 10 deletions tests/integrations/threading/test_threading.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,11 +35,11 @@ def crash():
assert not events


@pytest.mark.parametrize("propagate_hub", (True, False))
def test_propagates_hub(sentry_init, capture_events, propagate_hub):
@pytest.mark.parametrize("propagate_scope", (True, False))
def test_propagates_scope(sentry_init, capture_events, propagate_scope):
sentry_init(
default_integrations=False,
integrations=[ThreadingIntegration(propagate_hub=propagate_hub)],
integrations=[ThreadingIntegration(propagate_scope=propagate_scope)],
)
events = capture_events()

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

if propagate_hub:
if propagate_scope:
assert event["tags"]["stage1"] == "true"
else:
assert "stage1" not in event.get("tags", {})


@pytest.mark.parametrize("propagate_hub", (True, False))
def test_propagates_threadpool_hub(sentry_init, capture_events, propagate_hub):
@pytest.mark.parametrize("propagate_scope", (True, False))
def test_propagates_threadpool_scope(sentry_init, capture_events, propagate_scope):
sentry_init(
traces_sample_rate=1.0,
integrations=[ThreadingIntegration(propagate_hub=propagate_hub)],
integrations=[ThreadingIntegration(propagate_scope=propagate_scope)],
)
events = capture_events()

def double(number):
with sentry_sdk.start_span(op="task", name=str(number)):
with sentry_sdk.start_span(op="task", name=str(number), only_if_parent=True):
return number * 2

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

sentry_sdk.flush()

if propagate_hub:
if propagate_scope:
assert len(events) == 1
(event,) = events
assert event["spans"][0]["trace_id"] == event["spans"][1]["trace_id"]
Expand Down
6 changes: 2 additions & 4 deletions tests/test_scope.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,11 @@
ScopeType,
should_send_default_pii,
)
from sentry_sdk.integrations.opentelemetry.integration import (
_setup_scope_context_management,
)
from sentry_sdk.integrations.opentelemetry.scope import (
PotelScope as Scope,
use_scope,
use_isolation_scope,
setup_scope_context_management,
)


Expand All @@ -31,7 +29,7 @@

@pytest.fixture(autouse=True)
def setup_otel_scope_management():
_setup_scope_context_management()
setup_scope_context_management()


def test_copying():
Expand Down

0 comments on commit 65fbd50

Please sign in to comment.