Skip to content
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.

Sign up for GitHub

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

Fix http_methods_to_capture #3596

Merged
merged 6 commits into from
Oct 4, 2024
Merged

Fix http_methods_to_capture #3596

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
098569f
Fix http_methods_to_capture
sentrivana Oct 2, 2024
d29a64c
fix tests
sentrivana Oct 3, 2024
4309812
bla
sentrivana Oct 3, 2024
ff7d3b8
formatting
sentrivana Oct 3, 2024
935c12e
also test asgi middleware
sentrivana Oct 3, 2024
45ef6d0
remove asgi tests, will add this to master
sentrivana Oct 3, 2024
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
9 changes: 9 additions & 0 deletions sentry_sdk/integrations/_wsgi_common.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import json
from contextlib import contextmanager
from copy import deepcopy

import sentry_sdk
Expand All @@ -15,6 +16,7 @@
if TYPE_CHECKING:
from typing import Any
from typing import Dict
from typing import Iterator
from typing import Mapping
from typing import MutableMapping
from typing import Optional
Expand Down Expand Up @@ -50,6 +52,13 @@
)


# This noop context manager can be replaced with "from contextlib import nullcontext" when we drop Python 3.6 support
@contextmanager
def nullcontext():
# type: () -> Iterator[None]
yield


def request_body_within_bounds(client, content_length):
# type: (Optional[sentry_sdk.client.BaseClient], int) -> bool
if client is None:
Expand Down
65 changes: 36 additions & 29 deletions sentry_sdk/integrations/asgi.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
)
from sentry_sdk.integrations._wsgi_common import (
DEFAULT_HTTP_METHODS_TO_CAPTURE,
nullcontext,
)
from sentry_sdk.sessions import track_session
from sentry_sdk.tracing import (
Expand Down Expand Up @@ -190,9 +191,10 @@ async def _run_app(self, scope, receive, send, asgi_version):
)

method = scope.get("method", "").upper()
if method in self.http_methods_to_capture:
with sentry_sdk.continue_trace(_get_headers(scope)):
with sentry_sdk.start_transaction(
should_trace = method in self.http_methods_to_capture
with sentry_sdk.continue_trace(_get_headers(scope)):
with (
sentry_sdk.start_transaction(
op=(
OP.WEBSOCKET_SERVER
if ty == "websocket"
Expand All @@ -202,37 +204,42 @@ async def _run_app(self, scope, receive, send, asgi_version):
source=transaction_source,
origin=self.span_origin,
custom_sampling_context={"asgi_scope": scope},
) as transaction:
)
if should_trace
else nullcontext()
) as transaction:
if transaction is not None:
logger.debug(
"[ASGI] Started transaction: %s", transaction
)
transaction.set_tag("asgi.type", ty)
try:

async def _sentry_wrapped_send(event):
# type: (Dict[str, Any]) -> Any
is_http_response = (
event.get("type") == "http.response.start"
and "status" in event
)
if is_http_response:
transaction.set_http_status(event["status"])

return await send(event)

if asgi_version == 2:
return await self.app(scope)(
receive, _sentry_wrapped_send
)
else:
return await self.app(
scope, receive, _sentry_wrapped_send
)
except Exception as exc:
_capture_exception(
exc, mechanism_type=self.mechanism_type
try:

async def _sentry_wrapped_send(event):
# type: (Dict[str, Any]) -> Any
is_http_response = (
event.get("type") == "http.response.start"
and transaction is not None
and "status" in event
)
raise exc from None
if is_http_response:
transaction.set_http_status(event["status"])

return await send(event)

if asgi_version == 2:
return await self.app(scope)(
receive, _sentry_wrapped_send
)
else:
return await self.app(
scope, receive, _sentry_wrapped_send
)
except Exception as exc:
_capture_exception(
exc, mechanism_type=self.mechanism_type
)
raise exc from None
finally:
_asgi_middleware_applied.set(False)

Expand Down
35 changes: 20 additions & 15 deletions sentry_sdk/integrations/wsgi.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
from sentry_sdk.integrations._wsgi_common import (
DEFAULT_HTTP_METHODS_TO_CAPTURE,
_filter_headers,
nullcontext,
)
from sentry_sdk.sessions import track_session
from sentry_sdk.tracing import Transaction, TRANSACTION_SOURCE_ROUTE
Expand Down Expand Up @@ -106,27 +107,31 @@ def __call__(self, environ, start_response):
)

method = environ.get("REQUEST_METHOD", "").upper()
if method in self.http_methods_to_capture:
with sentry_sdk.continue_trace(environ):
with sentry_sdk.start_transaction(
should_trace = method in self.http_methods_to_capture
with sentry_sdk.continue_trace(environ):
with (
sentry_sdk.start_transaction(
environ,
op=OP.HTTP_SERVER,
name="generic WSGI request",
source=TRANSACTION_SOURCE_ROUTE,
origin=self.span_origin,
custom_sampling_context={"wsgi_environ": environ},
) as transaction:
try:
response = self.app(
environ,
partial(
_sentry_start_response,
start_response,
transaction,
),
)
except BaseException:
reraise(*_capture_exception())
)
if should_trace
else nullcontext()
) as transaction:
try:
response = self.app(
environ,
partial(
_sentry_start_response,
start_response,
transaction,
),
)
except BaseException:
reraise(*_capture_exception())

finally:
_wsgi_middleware_applied.set(False)
Expand Down
24 changes: 17 additions & 7 deletions tests/integrations/django/test_basic.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1113,6 +1113,9 @@ def test_csrf(sentry_init, client):
assert content == b"ok"


# This test is forked because it doesn't clean up after itself properly and makes
# other tests fail to resolve routes
@pytest.mark.forked
@pytest.mark.skipif(DJANGO_VERSION < (2, 0), reason="Requires Django > 2.0")
def test_custom_urlconf_middleware(
settings, sentry_init, client, capture_events, render_span_tree
Expand Down Expand Up @@ -1202,14 +1205,19 @@ def test_transaction_http_method_default(sentry_init, client, capture_events):
By default OPTIONS and HEAD requests do not create a transaction.
"""
sentry_init(
integrations=[DjangoIntegration()],
integrations=[
DjangoIntegration(
middleware_spans=False,
signals_spans=False,
)
],
traces_sample_rate=1.0,
)
events = capture_events()

client.get("/nomessage")
client.options("/nomessage")
client.head("/nomessage")
client.get(reverse("nomessage"))
client.options(reverse("nomessage"))
client.head(reverse("nomessage"))

(event,) = events

Expand All @@ -1225,15 +1233,17 @@ def test_transaction_http_method_custom(sentry_init, client, capture_events):
"OPTIONS",
"head",
), # capitalization does not matter
middleware_spans=False,
signals_spans=False,
)
],
traces_sample_rate=1.0,
)
events = capture_events()

client.get("/nomessage")
client.options("/nomessage")
client.head("/nomessage")
client.get(reverse("nomessage"))
client.options(reverse("nomessage"))
client.head(reverse("nomessage"))

assert len(events) == 2

Expand Down
Loading