Skip to content

Remove sampled setter and fix sanic behavior with an event processor #3779

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 1 commit into from
Nov 14, 2024
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 @@ -11,6 +11,7 @@ Looking to upgrade from Sentry SDK 2.x to 3.x? Here's a comprehensive list of wh
- The SDK now supports Python 3.7 and higher.
- `sentry_sdk.start_span` now only takes keyword arguments.
- `sentry_sdk.start_transaction`/`sentry_sdk.start_span` no longer takes the following arguments: `span`, `parent_sampled`, `trace_id`, `span_id` or `parent_span_id`.
- You can no longer change the sampled status of a span with `span.sampled = False` after starting it.
- The `Span()` constructor does not accept a `hub` parameter anymore.
- `Span.finish()` does not accept a `hub` parameter anymore.
- The `Profile()` constructor does not accept a `hub` parameter anymore.
Expand Down
23 changes: 16 additions & 7 deletions sentry_sdk/integrations/sanic.py
Original file line number Diff line number Diff line change
Expand Up @@ -187,8 +187,11 @@ async def _context_enter(request):
return

weak_request = weakref.ref(request)
request.ctx._sentry_scope = sentry_sdk.isolation_scope()
scope = request.ctx._sentry_scope.__enter__()
request.ctx._sentry_scope_manager = sentry_sdk.isolation_scope()
scope = request.ctx._sentry_scope_manager.__enter__()
request.ctx._sentry_scope = scope

scope.set_transaction_name(request.path, TRANSACTION_SOURCE_URL)
scope.clear_breadcrumbs()
scope.add_event_processor(_make_request_processor(weak_request))

Expand All @@ -197,7 +200,7 @@ async def _context_enter(request):
dict(request.headers)
)
request.ctx._sentry_continue_trace.__enter__()
request.ctx._sentry_transaction = sentry_sdk.start_transaction(
request.ctx._sentry_transaction = sentry_sdk.start_span(
op=OP.HTTP_SERVER,
# Unless the request results in a 404 error, the name and source will get overwritten in _set_transaction
name=request.path,
Expand All @@ -220,14 +223,20 @@ async def _context_exit(request, response=None):
# happens while trying to end the transaction, we still attempt to exit the scope.
with capture_internal_exceptions():
request.ctx._sentry_transaction.set_http_status(response_status)
request.ctx._sentry_transaction.sampled &= (

if (
isinstance(integration, SanicIntegration)
and response_status not in integration._unsampled_statuses
)
and response_status in integration._unsampled_statuses
):
# drop the event in an event processor
request.ctx._sentry_scope.add_event_processor(
lambda _event, _hint: None
)

request.ctx._sentry_transaction.__exit__(None, None, None)
request.ctx._sentry_continue_trace.__exit__(None, None, None)

request.ctx._sentry_scope.__exit__(None, None, None)
request.ctx._sentry_scope_manager.__exit__(None, None, None)


async def _set_transaction(request, route, **_):
Expand Down
9 changes: 3 additions & 6 deletions sentry_sdk/tracing.py
Original file line number Diff line number Diff line change
Expand Up @@ -1231,7 +1231,9 @@ def __init__(
skip_span = False
if only_if_parent:
parent_span_context = get_current_span().get_span_context()
skip_span = not parent_span_context.is_valid or parent_span_context.is_remote
skip_span = (
not parent_span_context.is_valid or parent_span_context.is_remote
)

if skip_span:
self._otel_span = INVALID_SPAN
Expand Down Expand Up @@ -1401,11 +1403,6 @@ def sampled(self):
# type: () -> Optional[bool]
return self._otel_span.get_span_context().trace_flags.sampled

@sampled.setter
def sampled(self, value):
# type: (Optional[bool]) -> None
pass

@property
def op(self):
# type: () -> Optional[str]
Expand Down
10 changes: 6 additions & 4 deletions tests/integrations/sanic/test_sanic.py
Original file line number Diff line number Diff line change
Expand Up @@ -346,8 +346,9 @@ def __init__(
expected_status,
expected_transaction_name,
expected_source=None,
has_transaction_event=True,
):
# type: (Iterable[Optional[Container[int]]], str, int, Optional[str], Optional[str]) -> None
# type: (Iterable[Optional[Container[int]]], str, int, Optional[str], Optional[str], bool) -> None
"""
expected_transaction_name of None indicates we expect to not receive a transaction
"""
Expand All @@ -356,6 +357,7 @@ def __init__(
self.expected_status = expected_status
self.expected_transaction_name = expected_transaction_name
self.expected_source = expected_source
self.has_transaction_event = has_transaction_event


@pytest.mark.skipif(
Expand Down Expand Up @@ -386,6 +388,7 @@ def __init__(
url="/404",
expected_status=404,
expected_transaction_name=None,
has_transaction_event=False,
),
TransactionTestConfig(
# With no ignored HTTP statuses, we should get transactions for 404 errors
Expand All @@ -401,6 +404,7 @@ def __init__(
url="/message",
expected_status=200,
expected_transaction_name=None,
has_transaction_event=False,
),
],
)
Expand Down Expand Up @@ -430,9 +434,7 @@ def test_transactions(test_config, sentry_init, app, capture_events):
(transaction_event, *_) = [*transaction_events, None]

# We should have no transaction event if and only if we expect no transactions
assert (transaction_event is None) == (
test_config.expected_transaction_name is None
)
assert bool(transaction_event) == test_config.has_transaction_event

# If a transaction was expected, ensure it is correct
assert (
Expand Down
Loading