Skip to content

Remove scope.span = setter and make sure scope.span reference is correct in context manager regardless of source of span #4439

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jun 4, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions MIGRATION_GUIDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,7 @@ Looking to upgrade from Sentry SDK 2.x to 3.x? Here's a comprehensive list of wh
- `span_id`
- `parent_span_id`: you can supply a `parent_span` instead
- The `Scope.transaction` property has been removed. To obtain the root span (previously transaction), use `Scope.root_span`. To set the root span's (transaction's) name, use `Scope.set_transaction_name()`.
- The `Scope.span =` setter has been removed.
- Passing a list or `None` for `failed_request_status_codes` in the Starlette integration is no longer supported. Pass a set of integers instead.
- The `span` argument of `Scope.trace_propagation_meta` is no longer supported.
- Setting `Scope.user` directly is no longer supported. Use `Scope.set_user()` instead.
Expand Down
6 changes: 3 additions & 3 deletions sentry_sdk/integrations/celery/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -100,9 +100,9 @@ def setup_once():
def _set_status(status):
# type: (str) -> None
with capture_internal_exceptions():
scope = sentry_sdk.get_current_scope()
if scope.span is not None:
scope.span.set_status(status)
span = sentry_sdk.get_current_span()
if span is not None:
span.set_status(status)


def _capture_exception(task, exc_info):
Expand Down
10 changes: 9 additions & 1 deletion sentry_sdk/opentelemetry/contextvars_context.py
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
from typing import cast, TYPE_CHECKING

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

import sentry_sdk
from sentry_sdk.tracing import Span
from sentry_sdk.opentelemetry.consts import (
SENTRY_SCOPES_KEY,
SENTRY_FORK_ISOLATION_SCOPE_KEY,
Expand Down Expand Up @@ -60,6 +62,12 @@ def attach(self, context):
else:
new_scope = current_scope.fork()

# carry forward a wrapped span reference since the otel context is always the
# source of truth for the active span
current_span = get_current_span(context)
if current_span != INVALID_SPAN:
new_scope._span = Span(otel_span=get_current_span(context))

if should_use_isolation_scope:
new_isolation_scope = should_use_isolation_scope
elif should_fork_isolation_scope:
Expand Down
6 changes: 0 additions & 6 deletions sentry_sdk/scope.py
Original file line number Diff line number Diff line change
Expand Up @@ -732,12 +732,6 @@ def span(self):
"""Get current tracing span."""
return self._span

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

@property
def profile(self):
# type: () -> Optional[Profile]
Expand Down
5 changes: 0 additions & 5 deletions sentry_sdk/tracing.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
from opentelemetry.sdk.trace import ReadableSpan
from opentelemetry.version import __version__ as otel_version

import sentry_sdk
from sentry_sdk.consts import (
DEFAULT_SPAN_NAME,
DEFAULT_SPAN_ORIGIN,
Expand Down Expand Up @@ -280,10 +279,6 @@ def __enter__(self):
# set as the implicit current context
self._ctx_token = context.attach(ctx)

# get the new scope that was forked on context.attach
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is now done universally in the context manager instead of here which would just affect the sentry created spans

self.scope = sentry_sdk.get_current_scope()
self.scope.span = self

return self

def __exit__(self, ty, value, tb):
Expand Down
6 changes: 4 additions & 2 deletions sentry_sdk/tracing_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -729,9 +729,10 @@ async def func_with_tracing(*args, **kwargs):
)
return await func(*args, **kwargs)

with span.start_child(
with sentry_sdk.start_span(
op=OP.FUNCTION,
name=qualname_from_function(func),
only_if_parent=True,
):
return await func(*args, **kwargs)

Expand All @@ -757,9 +758,10 @@ def func_with_tracing(*args, **kwargs):
)
return func(*args, **kwargs)

with span.start_child(
with sentry_sdk.start_span(
op=OP.FUNCTION,
name=qualname_from_function(func),
only_if_parent=True,
):
return func(*args, **kwargs)

Expand Down
25 changes: 0 additions & 25 deletions tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,7 @@
import socket
import warnings
from threading import Thread
from contextlib import contextmanager
from http.server import BaseHTTPRequestHandler, HTTPServer
from unittest import mock

import pytest
import jsonschema
Expand Down Expand Up @@ -35,12 +33,6 @@

from tests import _warning_recorder, _warning_recorder_mgr

from typing import TYPE_CHECKING

if TYPE_CHECKING:
from typing import Optional
from collections.abc import Iterator


SENTRY_EVENT_SCHEMA = "./checkouts/data-schemas/relay/event.schema.json"

Expand Down Expand Up @@ -637,23 +629,6 @@ def werkzeug_set_cookie(client, servername, key, value):
client.set_cookie(key, value)


@contextmanager
def patch_start_tracing_child(fake_transaction_is_none=False):
# type: (bool) -> Iterator[Optional[mock.MagicMock]]
if not fake_transaction_is_none:
fake_transaction = mock.MagicMock()
fake_start_child = mock.MagicMock()
fake_transaction.start_child = fake_start_child
else:
fake_transaction = None
fake_start_child = None

with mock.patch(
"sentry_sdk.tracing_utils.get_current_span", return_value=fake_transaction
):
yield fake_start_child


class ApproxDict(dict):
def __eq__(self, other):
# For an ApproxDict to equal another dict, the other dict just needs to contain
Expand Down
6 changes: 2 additions & 4 deletions tests/integrations/rust_tracing/test_rust_tracing.py
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,7 @@ def test_nested_on_new_span_on_close(sentry_init, capture_events):
assert "version" not in second_span_data


def test_on_new_span_without_transaction(sentry_init):
def test_no_spans_without_transaction(sentry_init):
rust_tracing = FakeRustTracing()
integration = RustTracingIntegration(
"test_on_new_span_without_transaction", rust_tracing.set_layer_impl
Expand All @@ -185,11 +185,9 @@ def test_on_new_span_without_transaction(sentry_init):

assert sentry_sdk.get_current_span() is None

# Should still create a span hierarchy, it just will not be under a txn
rust_tracing.new_span(RustTracingLevel.Info, 3)
current_span = sentry_sdk.get_current_span()
assert current_span is not None
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this was just INVALID_SPAN before so this test wasn't correct anyway

assert current_span.root_span is None
assert current_span is None


def test_on_event_exception(sentry_init, capture_events):
Expand Down
24 changes: 24 additions & 0 deletions tests/opentelemetry/test_context_scope_management.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
from opentelemetry import trace

import sentry_sdk
from sentry_sdk.tracing import Span


tracer = trace.get_tracer(__name__)


def test_scope_span_reference_started_with_sentry(sentry_init):
sentry_init(traces_sample_rate=1.0)

with sentry_sdk.start_span(name="test") as span:
assert sentry_sdk.get_current_span() == span
assert sentry_sdk.get_current_scope().span == span


def test_scope_span_reference_started_with_otel(sentry_init):
sentry_init(traces_sample_rate=1.0)

with tracer.start_as_current_span("test") as otel_span:
wrapped_span = Span(otel_span=otel_span)
assert sentry_sdk.get_current_span() == wrapped_span
assert sentry_sdk.get_current_scope().span == wrapped_span
5 changes: 1 addition & 4 deletions tests/opentelemetry/test_potel.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,7 @@

@pytest.fixture(autouse=True)
def sentry_init_potel(sentry_init):
sentry_init(
traces_sample_rate=1.0,
_experiments={"otel_powered_performance": True},
)
sentry_init(traces_sample_rate=1.0)


def test_root_span_transaction_payload_started_with_otel_only(capture_envelopes):
Expand Down
24 changes: 0 additions & 24 deletions tests/test_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,37 +13,13 @@
start_span,
set_tags,
get_global_scope,
get_current_scope,
get_isolation_scope,
)

from sentry_sdk.client import Client, NonRecordingClient
from tests.conftest import SortedBaggage


@pytest.mark.forked
def test_get_current_span():
fake_scope = mock.MagicMock()
fake_scope.span = mock.MagicMock()
assert get_current_span(fake_scope) == fake_scope.span

fake_scope.span = None
assert get_current_span(fake_scope) is None


@pytest.mark.forked
def test_get_current_span_current_scope(sentry_init):
sentry_init()

assert get_current_span() is None

scope = get_current_scope()
fake_span = mock.MagicMock()
scope.span = fake_span

assert get_current_span() == fake_span


@pytest.mark.forked
def test_get_current_span_current_scope_with_span(sentry_init):
sentry_init()
Expand Down
44 changes: 24 additions & 20 deletions tests/test_basics.py
Original file line number Diff line number Diff line change
Expand Up @@ -748,32 +748,36 @@ def _hello_world(word):


def test_functions_to_trace(sentry_init, capture_events):
functions_to_trace = [
{"qualified_name": "tests.test_basics._hello_world"},
{"qualified_name": "time.sleep"},
]

sentry_init(
traces_sample_rate=1.0,
functions_to_trace=functions_to_trace,
)
original_sleep = time.sleep
try:
functions_to_trace = [
{"qualified_name": "tests.test_basics._hello_world"},
{"qualified_name": "time.sleep"},
]

sentry_init(
traces_sample_rate=1.0,
functions_to_trace=functions_to_trace,
)

events = capture_events()
events = capture_events()

with start_span(name="something"):
time.sleep(0)
with start_span(name="something"):
time.sleep(0)

for word in ["World", "You"]:
_hello_world(word)
for word in ["World", "You"]:
_hello_world(word)

assert len(events) == 1
assert len(events) == 1

(event,) = events
(event,) = events

assert len(event["spans"]) == 3
assert event["spans"][0]["description"] == "time.sleep"
assert event["spans"][1]["description"] == "tests.test_basics._hello_world"
assert event["spans"][2]["description"] == "tests.test_basics._hello_world"
assert len(event["spans"]) == 3
assert event["spans"][0]["description"] == "time.sleep"
assert event["spans"][1]["description"] == "tests.test_basics._hello_world"
assert event["spans"][2]["description"] == "tests.test_basics._hello_world"
finally:
time.sleep = original_sleep


class WorldGreeter:
Expand Down
Loading
Loading