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

Fix transaction name in Starlette and FastAPI #2341

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
9074cf9
Extract functions in separate module
antonpirker Aug 31, 2023
951fe67
Updated tests
antonpirker Aug 31, 2023
1118ca5
Update tests
antonpirker Aug 31, 2023
84638d1
Pinned dep
antonpirker Sep 1, 2023
0ec247f
Pin anyio, because new major breaks something.
antonpirker Sep 1, 2023
eeb6d7c
Merge branch 'antonpirker/pin-anyio-for-tests' into antonpirker/clean…
antonpirker Sep 1, 2023
e6221a5
Merge branch 'master' into antonpirker/cleanup-asgi-starlette
antonpirker Sep 1, 2023
69bf709
Add debug logging
antonpirker Sep 1, 2023
0e084fb
Added tests for better transaction naming.
antonpirker Sep 1, 2023
7cc9047
Set name/source in asgi to something meaningful
antonpirker Sep 1, 2023
1b2d8fb
Fixed some tests
antonpirker Sep 1, 2023
d126f81
More tests
antonpirker Sep 1, 2023
d472521
Cleanup
antonpirker Sep 1, 2023
face789
Reactivated missing test
antonpirker Sep 1, 2023
9696d7b
Fixed typing
antonpirker Sep 1, 2023
227f5a9
Propper url getting
antonpirker Sep 1, 2023
10e2a7d
Merge branch 'master' into antonpirker/fix-transaction-name-in-starle…
antonpirker Sep 1, 2023
390f1c6
Fixed merge problems
antonpirker Sep 1, 2023
e4de8ac
Cleanup
antonpirker Sep 1, 2023
22d0146
Cleanup
antonpirker Sep 1, 2023
e4fd8b1
Added middleware test
antonpirker Sep 4, 2023
13e9381
Fixed transaction name in fastapi middleware
antonpirker Sep 4, 2023
31dc662
Added test for middleware in starlette
antonpirker Sep 4, 2023
a38525c
Cleanup
antonpirker Sep 4, 2023
6971ee6
Make tests to be more like documentation
antonpirker Sep 4, 2023
2a9c8bc
Cleanup
antonpirker Sep 4, 2023
4bf7c29
Better transaction names for middleware
antonpirker Sep 4, 2023
b755919
Better readability
antonpirker Sep 4, 2023
1ff4ea2
Merge branch 'master' into antonpirker/fix-transaction-name-in-starle…
antonpirker Sep 5, 2023
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
92 changes: 58 additions & 34 deletions sentry_sdk/integrations/asgi.py
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
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