Skip to content

Commit

Permalink
Fix transaction name in Starlette and FastAPI (#2341)
Browse files Browse the repository at this point in the history 
Set the url as a transaction name instead of 'generic ASGI request' in the beginning, so traces_sampler has something to work with that is more meaningful than 'generic ASGI request'.

Closes #2262
Closes #2263
New Behaviour:

Note: transaction names can be two styles, "url" or "endpoint". (set by the transaction_style parameter of the Integrations)

Note 2: See also @pytest.mark.parametrize decorator in the new tests as reference.

    vanilla ASGI: set URL instead of always "generic ASGI request"
    Starlette:
        normal request: transaction name is function name or route (depending on transaction_style setting)
        traces_sampler: always receives the raw URL as the transaction name (no matter the transaction_style setting. because we do not know more at the time the traces_sampler is called.)
        requests that end in a middleware (like 404, CORS): the functions name or the raw URL (depending on transaction_style setting)
    FastAPI
        normal request: transaction name is function name or route (depending on transaction_style setting)
        traces_sampler: always receives the raw URL as the transaction name (no matter the transaction_style setting. because we do not know more at the time the traces_sampler is called.)
        requests that end in a middleware (like 404, CORS): the functions name or the raw URL (depending on transaction_style setting)
    There used to be "generic ASGI request" transactions being created at the server startup (when a "lifespan" ASGI message was received.) Those transactions are not created anymore. (we can think of creating propper "Server was started/stopped" transactions in the future)
  • Loading branch information
@antonpirker @sentrivana
antonpirker authored and sentrivana committed Sep 18, 2023
1 parent e0614c0 commit d34ddb3
Show file tree
Hide file tree
Showing 5 changed files with 564 additions and 67 deletions.
92 changes: 58 additions & 34 deletions sentry_sdk/integrations/asgi.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,15 @@
from sentry_sdk.integrations._asgi_common import (
_get_headers,
_get_request_data,
_get_url,
)
from sentry_sdk.integrations.modules import _get_installed_modules
from sentry_sdk.sessions import auto_session_tracking
from sentry_sdk.tracing import (
SOURCE_FOR_STYLE,
TRANSACTION_SOURCE_ROUTE,
TRANSACTION_SOURCE_URL,
TRANSACTION_SOURCE_COMPONENT,
)
from sentry_sdk.utils import (
ContextVar,
Expand All @@ -35,10 +38,11 @@
from sentry_sdk.tracing import Transaction

if TYPE_CHECKING:
from typing import Dict
from typing import Any
from typing import Optional
from typing import Callable
from typing import Dict
from typing import Optional
from typing import Tuple

from sentry_sdk._types import Event, Hint

Expand Down Expand Up @@ -144,7 +148,8 @@ async def _run_asgi3(self, scope, receive, send):
async def _run_app(self, scope, receive, send, asgi_version):
# type: (Any, Any, Any, Any, int) -> Any
is_recursive_asgi_middleware = _asgi_middleware_applied.get(False)
if is_recursive_asgi_middleware:
is_lifespan = scope["type"] == "lifespan"
if is_recursive_asgi_middleware or is_lifespan:
try:
if asgi_version == 2:
return await self.app(scope)(receive, send)
Expand All @@ -167,24 +172,35 @@ async def _run_app(self, scope, receive, send, asgi_version):
sentry_scope.add_event_processor(processor)

ty = scope["type"]
(
transaction_name,
transaction_source,
) = self._get_transaction_name_and_source(
self.transaction_style,
scope,
)

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

transaction.name = _DEFAULT_TRANSACTION_NAME
transaction.source = TRANSACTION_SOURCE_ROUTE
transaction.set_tag("asgi.type", ty)
logger.debug(
"[ASGI] Set transaction name and source on transaction: '%s' / '%s'",
Expand Down Expand Up @@ -232,7 +248,25 @@ def event_processor(self, event, hint, asgi_scope):
request_data.update(_get_request_data(asgi_scope))
event["request"] = deepcopy(request_data)

self._set_transaction_name_and_source(event, self.transaction_style, asgi_scope)
# Only set transaction name if not already set by Starlette or FastAPI (or other frameworks)
already_set = event["transaction"] != _DEFAULT_TRANSACTION_NAME and event[
"transaction_info"
].get("source") in [
TRANSACTION_SOURCE_COMPONENT,
TRANSACTION_SOURCE_ROUTE,
]
if not already_set:
name, source = self._get_transaction_name_and_source(
self.transaction_style, asgi_scope
)
event["transaction"] = name
event["transaction_info"] = {"source": source}

logger.debug(
"[ASGI] Set transaction name and source in event_processor: '%s' / '%s'",
event["transaction"],
event["transaction_info"]["source"],
)

return event

Expand All @@ -242,16 +276,11 @@ def event_processor(self, event, hint, asgi_scope):
# data to your liking it's recommended to use the `before_send` callback
# for that.

def _set_transaction_name_and_source(self, event, transaction_style, asgi_scope):
# type: (Event, str, Any) -> None
transaction_name_already_set = (
event.get("transaction", _DEFAULT_TRANSACTION_NAME)
!= _DEFAULT_TRANSACTION_NAME
)
if transaction_name_already_set:
return

name = ""
def _get_transaction_name_and_source(self, transaction_style, asgi_scope):
# type: (SentryAsgiMiddleware, str, Any) -> Tuple[str, str]
name = None
source = SOURCE_FOR_STYLE[transaction_style]
ty = asgi_scope.get("type")

if transaction_style == "endpoint":
endpoint = asgi_scope.get("endpoint")
Expand All @@ -260,6 +289,9 @@ def _set_transaction_name_and_source(self, event, transaction_style, asgi_scope)
# an endpoint, overwrite our generic transaction name.
if endpoint:
name = transaction_from_function(endpoint) or ""
else:
name = _get_url(asgi_scope, "http" if ty == "http" else "ws", host=None)
source = TRANSACTION_SOURCE_URL

elif transaction_style == "url":
# FastAPI includes the route object in the scope to let Sentry extract the
Expand All @@ -269,21 +301,13 @@ def _set_transaction_name_and_source(self, event, transaction_style, asgi_scope)
path = getattr(route, "path", None)
if path is not None:
name = path
else:
name = _get_url(asgi_scope, "http" if ty == "http" else "ws", host=None)
source = TRANSACTION_SOURCE_URL

if not name:
event["transaction"] = _DEFAULT_TRANSACTION_NAME
event["transaction_info"] = {"source": TRANSACTION_SOURCE_ROUTE}
logger.debug(
"[ASGI] Set default transaction name and source on event: '%s' / '%s'",
event["transaction"],
event["transaction_info"]["source"],
)
return

event["transaction"] = name
event["transaction_info"] = {"source": SOURCE_FOR_STYLE[transaction_style]}
logger.debug(
"[ASGI] Set transaction name and source on event: '%s' / '%s'",
event["transaction"],
event["transaction_info"]["source"],
)
if name is None:
name = _DEFAULT_TRANSACTION_NAME
source = TRANSACTION_SOURCE_ROUTE
return name, source

return name, source
73 changes: 53 additions & 20 deletions sentry_sdk/integrations/starlette.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,11 @@
request_body_within_bounds,
)
from sentry_sdk.integrations.asgi import SentryAsgiMiddleware
from sentry_sdk.tracing import SOURCE_FOR_STYLE, TRANSACTION_SOURCE_ROUTE
from sentry_sdk.tracing import (
SOURCE_FOR_STYLE,
TRANSACTION_SOURCE_COMPONENT,
TRANSACTION_SOURCE_ROUTE,
)
from sentry_sdk.utils import (
AnnotatedValue,
capture_internal_exceptions,
Expand All @@ -25,7 +29,7 @@
)

if TYPE_CHECKING:
from typing import Any, Awaitable, Callable, Dict, Optional
from typing import Any, Awaitable, Callable, Dict, Optional, Tuple

from sentry_sdk.scope import Scope as SentryScope

Expand Down Expand Up @@ -106,6 +110,15 @@ async def _create_span_call(app, scope, receive, send, **kwargs):
if integration is not None:
middleware_name = app.__class__.__name__

# Update transaction name with middleware name
with hub.configure_scope() as sentry_scope:
name, source = _get_transaction_from_middleware(app, scope, integration)
if name is not None:
sentry_scope.set_transaction_name(
name,
source=source,
)

with hub.start_span(
op=OP.MIDDLEWARE_STARLETTE, description=middleware_name
) as middleware_span:
Expand Down Expand Up @@ -337,12 +350,14 @@ def patch_asgi_app():

async def _sentry_patched_asgi_app(self, scope, receive, send):
# type: (Starlette, StarletteScope, Receive, Send) -> None
if Hub.current.get_integration(StarletteIntegration) is None:
integration = Hub.current.get_integration(StarletteIntegration)
if integration is None:
return await old_app(self, scope, receive, send)

middleware = SentryAsgiMiddleware(
lambda *a, **kw: old_app(self, *a, **kw),
mechanism_type=StarletteIntegration.identifier,
transaction_style=integration.transaction_style,
)

middleware.__call__ = middleware._run_asgi3
Expand Down Expand Up @@ -620,35 +635,53 @@ async def json(self):
return await self.request.json()


def _transaction_name_from_router(scope):
# type: (StarletteScope) -> Optional[str]
router = scope.get("router")
if not router:
return None

for route in router.routes:
match = route.matches(scope)
if match[0] == Match.FULL:
return route.path

return None


def _set_transaction_name_and_source(scope, transaction_style, request):
# type: (SentryScope, str, Any) -> None
name = ""
name = None
source = SOURCE_FOR_STYLE[transaction_style]

if transaction_style == "endpoint":
endpoint = request.scope.get("endpoint")
if endpoint:
name = transaction_from_function(endpoint) or ""
name = transaction_from_function(endpoint) or None

elif transaction_style == "url":
router = request.scope["router"]
for route in router.routes:
match = route.matches(request.scope)

if match[0] == Match.FULL:
if transaction_style == "endpoint":
name = transaction_from_function(match[1]["endpoint"]) or ""
break
elif transaction_style == "url":
name = route.path
break

if not name:
name = _transaction_name_from_router(request.scope)

if name is None:
name = _DEFAULT_TRANSACTION_NAME
source = TRANSACTION_SOURCE_ROUTE
else:
source = SOURCE_FOR_STYLE[transaction_style]

scope.set_transaction_name(name, source=source)
logger.debug(
"[Starlette] Set transaction name and source on scope: %s / %s", name, source
)


def _get_transaction_from_middleware(app, asgi_scope, integration):
# type: (Any, Dict[str, Any], StarletteIntegration) -> Tuple[Optional[str], Optional[str]]
name = None
source = None

if integration.transaction_style == "endpoint":
name = transaction_from_function(app.__class__)
source = TRANSACTION_SOURCE_COMPONENT
elif integration.transaction_style == "url":
name = _transaction_name_from_router(asgi_scope)
source = TRANSACTION_SOURCE_ROUTE

return name, source
Loading

0 comments on commit d34ddb3

Please sign in to comment.