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.

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

Sanic integration performance #2419

Merged
merged 13 commits into from
Oct 13, 2023
55 changes: 49 additions & 6 deletions sentry_sdk/integrations/sanic.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,11 @@
import weakref
from inspect import isawaitable

from sentry_sdk import continue_trace
from sentry_sdk._compat import urlparse, reraise
from sentry_sdk.consts import OP
from sentry_sdk.hub import Hub
from sentry_sdk.tracing import TRANSACTION_SOURCE_COMPONENT
from sentry_sdk.tracing import TRANSACTION_SOURCE_COMPONENT, TRANSACTION_SOURCE_URL
from sentry_sdk.utils import (
capture_internal_exceptions,
event_from_exception,
Expand All @@ -19,6 +21,7 @@
from sentry_sdk._types import TYPE_CHECKING

if TYPE_CHECKING:
from collections.abc import Container
from typing import Any
from typing import Callable
from typing import Optional
Expand All @@ -27,6 +30,7 @@
from typing import Dict

from sanic.request import Request, RequestParameters
from sanic.response import BaseHTTPResponse

from sentry_sdk._types import Event, EventProcessor, Hint
from sanic.router import Route
Expand Down Expand Up @@ -54,6 +58,16 @@ class SanicIntegration(Integration):
identifier = "sanic"
version = None

def __init__(self, unsampled_statuses=frozenset({404})):
Copy link
Contributor

Choose a reason for hiding this comment

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

Please also add the new option to the Sanic docs if you haven't already.

# type: (Optional[Container[int]]) -> None
"""
The unsampled_statuses parameter can be used to specify for which HTTP statuses the
transactions should not be sent to Sentry. By default, transactions are sent for all
HTTP statuses, except 404. Set unsampled_statuses to None to send transactions for all
HTTP statuses, including 404.
"""
self._unsampled_statuses = unsampled_statuses or set()

@staticmethod
def setup_once():
# type: () -> None
Expand Down Expand Up @@ -180,16 +194,45 @@ async def _hub_enter(request):
scope.clear_breadcrumbs()
scope.add_event_processor(_make_request_processor(weak_request))

transaction = continue_trace(
dict(request.headers),
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,
source=TRANSACTION_SOURCE_URL,
)
request.ctx._sentry_transaction = request.ctx._sentry_hub.start_transaction(
transaction
).__enter__()


async def _hub_exit(request, response=None):
# type: (Request, Optional[BaseHTTPResponse]) -> None
with capture_internal_exceptions():
if not request.ctx._sentry_do_integration:
return

integration = Hub.current.get_integration(SanicIntegration) # type: Integration

response_status = None if response is None else response.status

# This capture_internal_exceptions block has been intentionally nested here, so that in case an exception
# happens while trying to end the transaction, we still attempt to exit the hub.
with capture_internal_exceptions():
request.ctx._sentry_transaction.set_http_status(response_status)
request.ctx._sentry_transaction.sampled &= (
isinstance(integration, SanicIntegration)
sentrivana marked this conversation as resolved.
Show resolved Hide resolved
and response_status not in integration._unsampled_statuses
)
request.ctx._sentry_transaction.__exit__(None, None, None)

async def _hub_exit(request, **_):
# type: (Request, **Any) -> None
request.ctx._sentry_hub.__exit__(None, None, None)
request.ctx._sentry_hub.__exit__(None, None, None)


async def _set_transaction(request, route, **kwargs):
async def _set_transaction(request, route, **_):
# type: (Request, Route, **Any) -> None
hub = Hub.current
if hub.get_integration(SanicIntegration) is not None:
if request.ctx._sentry_do_integration:
with capture_internal_exceptions():
with hub.configure_scope() as scope:
route_name = route.name.replace(request.app.name, "").strip(".")
Expand Down
125 changes: 124 additions & 1 deletion tests/integrations/sanic/test_sanic.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,20 @@

from sentry_sdk import capture_message, configure_scope
from sentry_sdk.integrations.sanic import SanicIntegration
from sentry_sdk.tracing import TRANSACTION_SOURCE_COMPONENT, TRANSACTION_SOURCE_URL

from sanic import Sanic, request, response, __version__ as SANIC_VERSION_RAW
from sanic.response import HTTPResponse
from sanic.exceptions import SanicException

from sentry_sdk._types import TYPE_CHECKING

if TYPE_CHECKING:
from collections.abc import Iterable, Container
from typing import Any, Optional
sentrivana marked this conversation as resolved.
Show resolved Hide resolved

SANIC_VERSION = tuple(map(int, SANIC_VERSION_RAW.split(".")))
PERFORMANCE_SUPPORTED = SANIC_VERSION >= (21, 9)


@pytest.fixture
Expand Down Expand Up @@ -49,6 +57,10 @@ def hi_with_id(request, message_id):
capture_message("hi with id")
return response.text("ok with id")

@app.route("/500")
def fivehundred(_):
1 / 0

return app


Expand Down Expand Up @@ -88,7 +100,7 @@ def test_request_data(sentry_init, app, capture_events):
("/message/123456", "hi_with_id", "component"),
],
)
def test_transaction(
def test_transaction_name(
sentry_init, app, capture_events, url, expected_transaction, expected_source
):
sentry_init(integrations=[SanicIntegration()])
Expand Down Expand Up @@ -284,3 +296,114 @@ async def runner():

with configure_scope() as scope:
assert not scope._tags


class TransactionTestConfig:
sentrivana marked this conversation as resolved.
Show resolved Hide resolved
"""
Data class to store configurations for each performance transaction test run, including
both the inputs and relevant expected results.
"""

def __init__(
self,
integration_args,
url,
expected_status,
expected_transaction_name,
expected_source=None,
):
# type: (Iterable[Optional[Container[int]]], str, int, Optional[str], Optional[str]) -> None
"""
expected_transaction_name of None indicates we expect to not receive a transaction
"""
self.integration_args = integration_args
self.url = url
self.expected_status = expected_status
self.expected_transaction_name = expected_transaction_name
self.expected_source = expected_source


@pytest.mark.skipif(
not PERFORMANCE_SUPPORTED, reason="Performance not supported on this Sanic version"
)
@pytest.mark.parametrize(
"test_config",
[
TransactionTestConfig(
# Transaction for successful page load
integration_args=(),
url="/message",
expected_status=200,
expected_transaction_name="hi",
expected_source=TRANSACTION_SOURCE_COMPONENT,
),
TransactionTestConfig(
# Transaction still recorded when we have an internal server error
integration_args=(),
url="/500",
expected_status=500,
expected_transaction_name="fivehundred",
expected_source=TRANSACTION_SOURCE_COMPONENT,
),
TransactionTestConfig(
# By default, no transaction when we have a 404 error
integration_args=(),
url="/404",
expected_status=404,
expected_transaction_name=None,
),
TransactionTestConfig(
# With no ignored HTTP statuses, we should get transactions for 404 errors
integration_args=(None,),
url="/404",
expected_status=404,
expected_transaction_name="/404",
expected_source=TRANSACTION_SOURCE_URL,
),
TransactionTestConfig(
# Transaction can be suppressed for other HTTP statuses, too, by passing config to the integration
integration_args=({200},),
url="/message",
expected_status=200,
expected_transaction_name=None,
),
],
)
def test_transactions(test_config, sentry_init, app, capture_events):
# type: (TransactionTestConfig, Any, Any, Any) -> None

# Init the SanicIntegration with the desired arguments
sentry_init(
integrations=[SanicIntegration(*test_config.integration_args)],
traces_sample_rate=1.0,
)
events = capture_events()

# Make request to the desired URL
_, response = app.test_client.get(test_config.url)
assert response.status == test_config.expected_status

# Extract the transaction events by inspecting the event types. We should at most have 1 transaction event.
transaction_events = [
e for e in events if "type" in e and e["type"] == "transaction"
]
assert len(transaction_events) <= 1

# Get the only transaction event, or set to None if there are no transaction 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
)

# If a transaction was expected, ensure it is correct
assert (
transaction_event is None
or transaction_event["transaction"] == test_config.expected_transaction_name
)
assert (
transaction_event is None
or transaction_event["transaction_info"]["source"]
== test_config.expected_source
)
9 changes: 9 additions & 0 deletions tox.ini
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,7 @@ envlist =
{py3.6,py3.7,py3.8}-sanic-v{20}
{py3.7,py3.8,py3.9,py3.10,py3.11}-sanic-v{21}
{py3.7,py3.8,py3.9,py3.10,py3.11}-sanic-v{22}
{py3.8,py3.9,py3.10,py3.11}-sanic-latest

# Starlette
{py3.7,py3.8,py3.9,py3.10,py3.11}-starlette-v{0.20,0.22,0.24,0.26,0.28}
Expand Down Expand Up @@ -452,10 +453,18 @@ deps =
sanic-v21: sanic>=21.0,<22.0
sanic-v22: sanic>=22.0,<22.9.0

# Sanic is not using semver, so here we check the current latest version of Sanic. When this test breaks, we should
# determine whether it is because we need to fix something in our integration, or whether Sanic has simply dropped
# support for an older Python version. If Sanic has dropped support for an older python version, we should add a new
# line above to test for the newest Sanic version still supporting the old Python version, and we should update the
# line below so we test the latest Sanic version only using the Python versions that are supported.
sanic-latest: sanic>=23.6

sanic: websockets<11.0
sanic: aiohttp
sanic-v21: sanic_testing<22
sanic-v22: sanic_testing<22.9.0
sanic-latest: sanic_testing>=23.6
{py3.5,py3.6}-sanic: aiocontextvars==0.2.1
{py3.5}-sanic: ujson<4

Expand Down