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

Jump to bottom

Configure HTTP methods to capture in ASGI middleware and frameworks #3533

Merged
merged 8 commits into from
Oct 1, 2024
Merged

Configure HTTP methods to capture in ASGI middleware and frameworks #3533

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
1518f5d
Added http_methods_to_capture to starlette
antonpirker Sep 13, 2024
583c09b
Added tests for http_methods_to_capture to Fastapi
antonpirker Sep 13, 2024
7cc7bb5
Linting
antonpirker Sep 13, 2024
b86e57a
Skip test in old Starlette that does not support HEAD requests
antonpirker Sep 13, 2024
b45a349
Cleanup
antonpirker Sep 13, 2024
2136871
Skip tests in oldest FastAPI because it can not handle HEAD requests
antonpirker Sep 13, 2024
c39f042
Bumping skipped version
antonpirker Sep 13, 2024
8e63cc7
Merge branch 'master' into antonpirker/http_methods_to_capture_asgi
antonpirker Sep 30, 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
13 changes: 13 additions & 0 deletions sentry_sdk/integrations/_asgi_common.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,19 @@
from sentry_sdk.utils import AnnotatedValue


DEFAULT_HTTP_METHODS_TO_CAPTURE = (
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we maybe just import this from _wsgi_common here? Or directly use the _wsgi_common constant in the integrations?

"CONNECT",
"DELETE",
"GET",
# "HEAD", # do not capture HEAD requests by default
# "OPTIONS", # do not capture OPTIONS requests by default
"PATCH",
"POST",
"PUT",
"TRACE",
)


def _get_headers(asgi_scope):
# type: (Any) -> Dict[str, str]
"""
Expand Down
106 changes: 63 additions & 43 deletions sentry_sdk/integrations/asgi.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
"""

import asyncio
from contextlib import contextmanager
import inspect
from copy import deepcopy
from functools import partial
Expand All @@ -14,6 +15,7 @@
from sentry_sdk.consts import OP

from sentry_sdk.integrations._asgi_common import (
DEFAULT_HTTP_METHODS_TO_CAPTURE,
_get_headers,
_get_request_data,
_get_url,
Expand Down Expand Up @@ -42,6 +44,7 @@
from typing import Any
from typing import Callable
from typing import Dict
from typing import Iterator
from typing import Optional
from typing import Tuple

Expand All @@ -55,6 +58,13 @@
TRANSACTION_STYLE_VALUES = ("endpoint", "url")


# 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
Comment on lines +61 to +65
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe move this to utils.py and use it from there? Also for wsgi.py



def _capture_exception(exc, mechanism_type="asgi"):
# type: (Any, str) -> None

Expand Down Expand Up @@ -89,17 +99,19 @@ class SentryAsgiMiddleware:
"transaction_style",
"mechanism_type",
"span_origin",
"http_methods_to_capture",
)

def __init__(
self,
app,
unsafe_context_data=False,
transaction_style="endpoint",
mechanism_type="asgi",
span_origin="manual",
app, # type: Any
unsafe_context_data=False, # type: bool
transaction_style="endpoint", # type: str
mechanism_type="asgi", # type: str
span_origin="manual", # type: str
http_methods_to_capture=DEFAULT_HTTP_METHODS_TO_CAPTURE, # type: Tuple[str, ...]
):
# type: (Any, bool, str, str, str) -> None
# type: (...) -> None
"""
Instrument an ASGI application with Sentry. Provides HTTP/websocket
data to sent events and basic handling for exceptions bubbling up
Expand Down Expand Up @@ -134,6 +146,7 @@ def __init__(
self.mechanism_type = mechanism_type
self.span_origin = span_origin
self.app = app
self.http_methods_to_capture = http_methods_to_capture

if _looks_like_asgi3(app):
self.__call__ = self._run_asgi3 # type: Callable[..., Any]
Expand Down Expand Up @@ -185,52 +198,59 @@ async def _run_app(self, scope, receive, send, asgi_version):
scope,
)

if ty in ("http", "websocket"):
transaction = continue_trace(
_get_headers(scope),
op="{}.server".format(ty),
name=transaction_name,
source=transaction_source,
origin=self.span_origin,
)
logger.debug(
"[ASGI] Created transaction (continuing trace): %s",
transaction,
)
else:
transaction = Transaction(
op=OP.HTTP_SERVER,
name=transaction_name,
source=transaction_source,
origin=self.span_origin,
)
method = scope.get("method", "").upper()
transaction = None
if method in self.http_methods_to_capture:
if ty in ("http", "websocket"):
transaction = continue_trace(
_get_headers(scope),
op="{}.server".format(ty),
name=transaction_name,
source=transaction_source,
origin=self.span_origin,
)
logger.debug(
"[ASGI] Created transaction (continuing trace): %s",
transaction,
)
else:
transaction = Transaction(
op=OP.HTTP_SERVER,
name=transaction_name,
source=transaction_source,
origin=self.span_origin,
)
logger.debug(
"[ASGI] Created transaction (new): %s", transaction
)

transaction.set_tag("asgi.type", ty)
logger.debug(
"[ASGI] Created transaction (new): %s", transaction
"[ASGI] Set transaction name and source on transaction: '%s' / '%s'",
transaction.name,
transaction.source,
)

transaction.set_tag("asgi.type", ty)
logger.debug(
"[ASGI] Set transaction name and source on transaction: '%s' / '%s'",
transaction.name,
transaction.source,
)

with sentry_sdk.start_transaction(
transaction,
custom_sampling_context={"asgi_scope": scope},
with (
sentry_sdk.start_transaction(
transaction,
custom_sampling_context={"asgi_scope": scope},
)
if transaction is not None
else nullcontext()
):
logger.debug("[ASGI] Started transaction: %s", transaction)
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
)
if is_http_response:
transaction.set_http_status(event["status"])
if transaction is not None:
is_http_response = (
event.get("type") == "http.response.start"
and "status" in event
)
if is_http_response:
transaction.set_http_status(event["status"])
Comment on lines +247 to +253
Copy link
Member Author

Choose a reason for hiding this comment

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

I did it this way to make mypy happy

Copy link
Contributor

Choose a reason for hiding this comment

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

mypy also made me do things I'm not proud of. We need mypy anonymous


return await send(event)

Expand Down
8 changes: 8 additions & 0 deletions sentry_sdk/integrations/starlette.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
Integration,
_DEFAULT_FAILED_REQUEST_STATUS_CODES,
)
from sentry_sdk.integrations._asgi_common import DEFAULT_HTTP_METHODS_TO_CAPTURE
from sentry_sdk.integrations._wsgi_common import (
HttpCodeRangeContainer,
_is_json_content_type,
Expand Down Expand Up @@ -85,6 +86,7 @@ def __init__(
transaction_style="url", # type: str
failed_request_status_codes=_DEFAULT_FAILED_REQUEST_STATUS_CODES, # type: Union[Set[int], list[HttpStatusCodeRange], None]
middleware_spans=True, # type: bool
http_methods_to_capture=DEFAULT_HTTP_METHODS_TO_CAPTURE, # type: tuple[str, ...]
):
# type: (...) -> None
if transaction_style not in TRANSACTION_STYLE_VALUES:
Expand All @@ -94,6 +96,7 @@ def __init__(
)
self.transaction_style = transaction_style
self.middleware_spans = middleware_spans
self.http_methods_to_capture = tuple(map(str.upper, http_methods_to_capture))

if isinstance(failed_request_status_codes, Set):
self.failed_request_status_codes = (
Expand Down Expand Up @@ -390,6 +393,11 @@ async def _sentry_patched_asgi_app(self, scope, receive, send):
mechanism_type=StarletteIntegration.identifier,
transaction_style=integration.transaction_style,
span_origin=StarletteIntegration.origin,
http_methods_to_capture=(
integration.http_methods_to_capture
if integration
else DEFAULT_HTTP_METHODS_TO_CAPTURE
),
)

middleware.__call__ = middleware._run_asgi3
Expand Down
96 changes: 95 additions & 1 deletion tests/integrations/fastapi/test_fastapi.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
import json
import logging
import pytest
import threading
import warnings
from unittest import mock

import pytest
import fastapi
from fastapi import FastAPI, HTTPException, Request
from fastapi.testclient import TestClient
from fastapi.middleware.trustedhost import TrustedHostMiddleware
Expand All @@ -13,6 +14,10 @@
from sentry_sdk.integrations.asgi import SentryAsgiMiddleware
from sentry_sdk.integrations.fastapi import FastApiIntegration
from sentry_sdk.integrations.starlette import StarletteIntegration
from sentry_sdk.utils import parse_version


FASTAPI_VERSION = parse_version(fastapi.__version__)

from tests.integrations.starlette import test_starlette

Expand All @@ -31,6 +36,17 @@ async def _message():
capture_message("Hi")
return {"message": "Hi"}

@app.delete("/nomessage")
@app.get("/nomessage")
@app.head("/nomessage")
@app.options("/nomessage")
@app.patch("/nomessage")
@app.post("/nomessage")
@app.put("/nomessage")
@app.trace("/nomessage")
async def _nomessage():
return {"message": "nothing here..."}

@app.get("/message/{message_id}")
async def _message_with_id(message_id):
capture_message("Hi")
Expand Down Expand Up @@ -548,6 +564,84 @@ async def _error():
assert not events


@pytest.mark.skipif(
FASTAPI_VERSION < (0, 80),
reason="Requires FastAPI >= 0.80, because earlier versions do not support HTTP 'HEAD' requests",
)
def test_transaction_http_method_default(sentry_init, capture_events):
"""
By default OPTIONS and HEAD requests do not create a transaction.
"""
# FastAPI is heavily based on Starlette so we also need
# to enable StarletteIntegration.
# In the future this will be auto enabled.
sentry_init(
traces_sample_rate=1.0,
integrations=[
StarletteIntegration(),
FastApiIntegration(),
],
)

app = fastapi_app_factory()

events = capture_events()

client = TestClient(app)
client.get("/nomessage")
client.options("/nomessage")
client.head("/nomessage")

assert len(events) == 1

(event,) = events

assert event["request"]["method"] == "GET"


@pytest.mark.skipif(
FASTAPI_VERSION < (0, 80),
reason="Requires FastAPI >= 0.80, because earlier versions do not support HTTP 'HEAD' requests",
)
def test_transaction_http_method_custom(sentry_init, capture_events):
# FastAPI is heavily based on Starlette so we also need
# to enable StarletteIntegration.
# In the future this will be auto enabled.
sentry_init(
traces_sample_rate=1.0,
integrations=[
StarletteIntegration(
http_methods_to_capture=(
"OPTIONS",
"head",
), # capitalization does not matter
),
FastApiIntegration(
http_methods_to_capture=(
"OPTIONS",
"head",
), # capitalization does not matter
),
],
)

app = fastapi_app_factory()

events = capture_events()

client = TestClient(app)
client.get("/nomessage")
client.options("/nomessage")
client.head("/nomessage")

assert len(events) == 2

(event1, event2) = events

assert event1["request"]["method"] == "OPTIONS"
assert event2["request"]["method"] == "HEAD"


@test_starlette.parametrize_test_configurable_status_codes
def test_configurable_status_codes(
sentry_init,
Expand Down
Loading
Loading