From 1b839c5a0c256f2295a9f7b74d50fcbc341c5969 Mon Sep 17 00:00:00 2001 From: Neel Shah Date: Tue, 3 Jun 2025 16:47:25 +0200 Subject: [PATCH 1/2] Update trace decorator to not use start_child --- sentry_sdk/tracing_utils.py | 6 +- tests/conftest.py | 25 -------- tests/test_basics.py | 44 +++++++------ tests/tracing/test_decorator.py | 105 ++++++++++++++++++-------------- 4 files changed, 86 insertions(+), 94 deletions(-) diff --git a/sentry_sdk/tracing_utils.py b/sentry_sdk/tracing_utils.py index 20b88f1e32..140ce57139 100644 --- a/sentry_sdk/tracing_utils.py +++ b/sentry_sdk/tracing_utils.py @@ -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) @@ -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) diff --git a/tests/conftest.py b/tests/conftest.py index 5987265e32..d64652c4e9 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -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 @@ -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" @@ -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 diff --git a/tests/test_basics.py b/tests/test_basics.py index f728eafdbf..17a0e218ad 100644 --- a/tests/test_basics.py +++ b/tests/test_basics.py @@ -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: diff --git a/tests/tracing/test_decorator.py b/tests/tracing/test_decorator.py index ed6dedb26f..c8a384f42f 100644 --- a/tests/tracing/test_decorator.py +++ b/tests/tracing/test_decorator.py @@ -3,10 +3,9 @@ import pytest +import sentry_sdk from sentry_sdk.tracing import trace -from sentry_sdk.tracing_utils import start_child_span_decorator from sentry_sdk.utils import logger -from tests.conftest import patch_start_tracing_child def my_example_function(): @@ -17,68 +16,80 @@ async def my_async_example_function(): return "return_of_async_function" -@pytest.mark.forked -def test_trace_decorator(): - with patch_start_tracing_child() as fake_start_child: +def test_trace_decorator(sentry_init, capture_events): + sentry_init(traces_sample_rate=1.0) + events = capture_events() + + with sentry_sdk.start_span(name="test"): result = my_example_function() - fake_start_child.assert_not_called() assert result == "return_of_sync_function" - result2 = start_child_span_decorator(my_example_function)() - fake_start_child.assert_called_once_with( - op="function", name="test_decorator.my_example_function" - ) + result2 = trace(my_example_function)() assert result2 == "return_of_sync_function" + (event,) = events + (span,) = event["spans"] + assert span["op"] == "function" + assert span["description"] == "test_decorator.my_example_function" + + +def test_trace_decorator_no_trx(sentry_init, capture_events): + sentry_init(traces_sample_rate=1.0) + events = capture_events() + + with mock.patch.object(logger, "debug", mock.Mock()) as fake_debug: + result = my_example_function() + assert result == "return_of_sync_function" + fake_debug.assert_not_called() -@pytest.mark.forked -def test_trace_decorator_no_trx(): - with patch_start_tracing_child(fake_transaction_is_none=True): - with mock.patch.object(logger, "debug", mock.Mock()) as fake_debug: - result = my_example_function() - fake_debug.assert_not_called() - assert result == "return_of_sync_function" + result2 = trace(my_example_function)() + assert result2 == "return_of_sync_function" + fake_debug.assert_called_once_with( + "Cannot create a child span for %s. " + "Please start a Sentry transaction before calling this function.", + "test_decorator.my_example_function", + ) - result2 = start_child_span_decorator(my_example_function)() - fake_debug.assert_called_once_with( - "Cannot create a child span for %s. " - "Please start a Sentry transaction before calling this function.", - "test_decorator.my_example_function", - ) - assert result2 == "return_of_sync_function" + assert len(events) == 0 -@pytest.mark.forked @pytest.mark.asyncio -async def test_trace_decorator_async(): - with patch_start_tracing_child() as fake_start_child: +async def test_trace_decorator_async(sentry_init, capture_events): + sentry_init(traces_sample_rate=1.0) + events = capture_events() + + with sentry_sdk.start_span(name="test"): result = await my_async_example_function() - fake_start_child.assert_not_called() assert result == "return_of_async_function" - result2 = await start_child_span_decorator(my_async_example_function)() - fake_start_child.assert_called_once_with( - op="function", - name="test_decorator.my_async_example_function", - ) + result2 = await trace(my_async_example_function)() assert result2 == "return_of_async_function" + (event,) = events + (span,) = event["spans"] + assert span["op"] == "function" + assert span["description"] == "test_decorator.my_async_example_function" + @pytest.mark.asyncio -async def test_trace_decorator_async_no_trx(): - with patch_start_tracing_child(fake_transaction_is_none=True): - with mock.patch.object(logger, "debug", mock.Mock()) as fake_debug: - result = await my_async_example_function() - fake_debug.assert_not_called() - assert result == "return_of_async_function" - - result2 = await start_child_span_decorator(my_async_example_function)() - fake_debug.assert_called_once_with( - "Cannot create a child span for %s. " - "Please start a Sentry transaction before calling this function.", - "test_decorator.my_async_example_function", - ) - assert result2 == "return_of_async_function" +async def test_trace_decorator_async_no_trx(sentry_init, capture_events): + sentry_init(traces_sample_rate=1.0) + events = capture_events() + + with mock.patch.object(logger, "debug", mock.Mock()) as fake_debug: + result = await my_async_example_function() + fake_debug.assert_not_called() + assert result == "return_of_async_function" + + result2 = await trace(my_async_example_function)() + fake_debug.assert_called_once_with( + "Cannot create a child span for %s. " + "Please start a Sentry transaction before calling this function.", + "test_decorator.my_async_example_function", + ) + assert result2 == "return_of_async_function" + + assert len(events) == 0 def test_functions_to_trace_signature_unchanged_sync(sentry_init): From 5dba74b96f4a39b0c530e64e89a9684c3b6e0f1b Mon Sep 17 00:00:00 2001 From: Neel Shah Date: Tue, 3 Jun 2025 15:31:56 +0200 Subject: [PATCH 2/2] Remove `scope.span =` setter and make sure `scope.span` reference is correct in context manager regardless of source of span. --- MIGRATION_GUIDE.md | 1 + sentry_sdk/integrations/celery/__init__.py | 6 ++--- .../opentelemetry/contextvars_context.py | 10 +++++++- sentry_sdk/scope.py | 6 ----- sentry_sdk/tracing.py | 5 ---- .../rust_tracing/test_rust_tracing.py | 6 ++--- .../test_context_scope_management.py | 24 +++++++++++++++++++ tests/opentelemetry/test_potel.py | 5 +--- tests/test_api.py | 24 ------------------- 9 files changed, 40 insertions(+), 47 deletions(-) create mode 100644 tests/opentelemetry/test_context_scope_management.py diff --git a/MIGRATION_GUIDE.md b/MIGRATION_GUIDE.md index 5c2402e07f..b92c493cea 100644 --- a/MIGRATION_GUIDE.md +++ b/MIGRATION_GUIDE.md @@ -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. diff --git a/sentry_sdk/integrations/celery/__init__.py b/sentry_sdk/integrations/celery/__init__.py index f8cde88682..f078f629da 100644 --- a/sentry_sdk/integrations/celery/__init__.py +++ b/sentry_sdk/integrations/celery/__init__.py @@ -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): diff --git a/sentry_sdk/opentelemetry/contextvars_context.py b/sentry_sdk/opentelemetry/contextvars_context.py index 51d450af82..abd4c60d3f 100644 --- a/sentry_sdk/opentelemetry/contextvars_context.py +++ b/sentry_sdk/opentelemetry/contextvars_context.py @@ -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, @@ -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: diff --git a/sentry_sdk/scope.py b/sentry_sdk/scope.py index dec8e70e22..94b779317b 100644 --- a/sentry_sdk/scope.py +++ b/sentry_sdk/scope.py @@ -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] diff --git a/sentry_sdk/tracing.py b/sentry_sdk/tracing.py index 20c3f3ffac..ea5e2eee88 100644 --- a/sentry_sdk/tracing.py +++ b/sentry_sdk/tracing.py @@ -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, @@ -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 - self.scope = sentry_sdk.get_current_scope() - self.scope.span = self - return self def __exit__(self, ty, value, tb): diff --git a/tests/integrations/rust_tracing/test_rust_tracing.py b/tests/integrations/rust_tracing/test_rust_tracing.py index 9ab64843c4..a7ddbfa6ec 100644 --- a/tests/integrations/rust_tracing/test_rust_tracing.py +++ b/tests/integrations/rust_tracing/test_rust_tracing.py @@ -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 @@ -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 - assert current_span.root_span is None + assert current_span is None def test_on_event_exception(sentry_init, capture_events): diff --git a/tests/opentelemetry/test_context_scope_management.py b/tests/opentelemetry/test_context_scope_management.py new file mode 100644 index 0000000000..a4872ef858 --- /dev/null +++ b/tests/opentelemetry/test_context_scope_management.py @@ -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 diff --git a/tests/opentelemetry/test_potel.py b/tests/opentelemetry/test_potel.py index 753f2b4cf2..ce333142fd 100644 --- a/tests/opentelemetry/test_potel.py +++ b/tests/opentelemetry/test_potel.py @@ -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): diff --git a/tests/test_api.py b/tests/test_api.py index ae88791f96..55415d6444 100644 --- a/tests/test_api.py +++ b/tests/test_api.py @@ -13,7 +13,6 @@ start_span, set_tags, get_global_scope, - get_current_scope, get_isolation_scope, ) @@ -21,29 +20,6 @@ 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()