Skip to content

Commit

Permalink
Fix http_methods_to_capture (#3596)
Browse files Browse the repository at this point in the history 
I messed up the merge. If an error happens, we should still report it, whatever the HTTP method. Fixing here.
  • Loading branch information
@sentrivana
sentrivana authored Oct 4, 2024
1 parent 5ecd9c5 commit 29aeebb
Show file tree
Hide file tree
Showing 4 changed files with 82 additions and 51 deletions.
9 changes: 9 additions & 0 deletions sentry_sdk/integrations/_wsgi_common.py
View file
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
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
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
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

0 comments on commit 29aeebb

Please sign in to comment.