Skip to content

Commit 80cd1f1

Browse files
antonpirkerantonpirker
authored
Fix transaction name in Starlette and FastAPI (getsentry#2341)
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 getsentry#2262 Closes getsentry#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)
1 parent ba6de38 commit 80cd1f1

5 files changed

Lines changed: 564 additions & 67 deletions

File tree

‎sentry_sdk/integrations/asgi.py‎

Lines changed: 58 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -17,12 +17,15 @@
1717
from sentry_sdk.integrations._asgi_common import (
1818
_get_headers,
1919
_get_request_data,
20+
_get_url,
2021
)
2122
from sentry_sdk.integrations.modules import _get_installed_modules
2223
from sentry_sdk.sessions import auto_session_tracking
2324
from sentry_sdk.tracing import (
2425
SOURCE_FOR_STYLE,
2526
TRANSACTION_SOURCE_ROUTE,
27+
TRANSACTION_SOURCE_URL,
28+
TRANSACTION_SOURCE_COMPONENT,
2629
)
2730
from sentry_sdk.utils import (
2831
ContextVar,
@@ -35,10 +38,11 @@
3538
from sentry_sdk.tracing import Transaction
3639

3740
if TYPE_CHECKING:
38-
from typing import Dict
3941
from typing import Any
40-
from typing import Optional
4142
from typing import Callable
43+
from typing import Dict
44+
from typing import Optional
45+
from typing import Tuple
4246

4347
from sentry_sdk._types import Event, Hint
4448

@@ -144,7 +148,8 @@ async def _run_asgi3(self, scope, receive, send):
144148
async def _run_app(self, scope, receive, send, asgi_version):
145149
# type: (Any, Any, Any, Any, int) -> Any
146150
is_recursive_asgi_middleware = _asgi_middleware_applied.get(False)
147-
if is_recursive_asgi_middleware:
151+
is_lifespan = scope["type"] == "lifespan"
152+
if is_recursive_asgi_middleware or is_lifespan:
148153
try:
149154
if asgi_version == 2:
150155
return await self.app(scope)(receive, send)
@@ -167,24 +172,35 @@ async def _run_app(self, scope, receive, send, asgi_version):
167172
sentry_scope.add_event_processor(processor)
168173

169174
ty = scope["type"]
175+
(
176+
transaction_name,
177+
transaction_source,
178+
) = self._get_transaction_name_and_source(
179+
self.transaction_style,
180+
scope,
181+
)
170182

171183
if ty in ("http", "websocket"):
172184
transaction = continue_trace(
173185
_get_headers(scope),
174186
op="{}.server".format(ty),
187+
name=transaction_name,
188+
source=transaction_source,
175189
)
176190
logger.debug(
177191
"[ASGI] Created transaction (continuing trace): %s",
178192
transaction,
179193
)
180194
else:
181-
transaction = Transaction(op=OP.HTTP_SERVER)
195+
transaction = Transaction(
196+
op=OP.HTTP_SERVER,
197+
name=transaction_name,
198+
source=transaction_source,
199+
)
182200
logger.debug(
183201
"[ASGI] Created transaction (new): %s", transaction
184202
)
185203

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

235-
self._set_transaction_name_and_source(event, self.transaction_style, asgi_scope)
251+
# Only set transaction name if not already set by Starlette or FastAPI (or other frameworks)
252+
already_set = event["transaction"] != _DEFAULT_TRANSACTION_NAME and event[
253+
"transaction_info"
254+
].get("source") in [
255+
TRANSACTION_SOURCE_COMPONENT,
256+
TRANSACTION_SOURCE_ROUTE,
257+
]
258+
if not already_set:
259+
name, source = self._get_transaction_name_and_source(
260+
self.transaction_style, asgi_scope
261+
)
262+
event["transaction"] = name
263+
event["transaction_info"] = {"source": source}
264+
265+
logger.debug(
266+
"[ASGI] Set transaction name and source in event_processor: '%s' / '%s'",
267+
event["transaction"],
268+
event["transaction_info"]["source"],
269+
)
236270

237271
return event
238272

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

245-
def _set_transaction_name_and_source(self, event, transaction_style, asgi_scope):
246-
# type: (Event, str, Any) -> None
247-
transaction_name_already_set = (
248-
event.get("transaction", _DEFAULT_TRANSACTION_NAME)
249-
!= _DEFAULT_TRANSACTION_NAME
250-
)
251-
if transaction_name_already_set:
252-
return
253-
254-
name = ""
279+
def _get_transaction_name_and_source(self, transaction_style, asgi_scope):
280+
# type: (SentryAsgiMiddleware, str, Any) -> Tuple[str, str]
281+
name = None
282+
source = SOURCE_FOR_STYLE[transaction_style]
283+
ty = asgi_scope.get("type")
255284

256285
if transaction_style == "endpoint":
257286
endpoint = asgi_scope.get("endpoint")
@@ -260,6 +289,9 @@ def _set_transaction_name_and_source(self, event, transaction_style, asgi_scope)
260289
# an endpoint, overwrite our generic transaction name.
261290
if endpoint:
262291
name = transaction_from_function(endpoint) or ""
292+
else:
293+
name = _get_url(asgi_scope, "http" if ty == "http" else "ws", host=None)
294+
source = TRANSACTION_SOURCE_URL
263295

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

273-
if not name:
274-
event["transaction"] = _DEFAULT_TRANSACTION_NAME
275-
event["transaction_info"] = {"source": TRANSACTION_SOURCE_ROUTE}
276-
logger.debug(
277-
"[ASGI] Set default transaction name and source on event: '%s' / '%s'",
278-
event["transaction"],
279-
event["transaction_info"]["source"],
280-
)
281-
return
282-
283-
event["transaction"] = name
284-
event["transaction_info"] = {"source": SOURCE_FOR_STYLE[transaction_style]}
285-
logger.debug(
286-
"[ASGI] Set transaction name and source on event: '%s' / '%s'",
287-
event["transaction"],
288-
event["transaction_info"]["source"],
289-
)
308+
if name is None:
309+
name = _DEFAULT_TRANSACTION_NAME
310+
source = TRANSACTION_SOURCE_ROUTE
311+
return name, source
312+
313+
return name, source

‎sentry_sdk/integrations/starlette.py‎

Lines changed: 53 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,11 @@
1414
request_body_within_bounds,
1515
)
1616
from sentry_sdk.integrations.asgi import SentryAsgiMiddleware
17-
from sentry_sdk.tracing import SOURCE_FOR_STYLE, TRANSACTION_SOURCE_ROUTE
17+
from sentry_sdk.tracing import (
18+
SOURCE_FOR_STYLE,
19+
TRANSACTION_SOURCE_COMPONENT,
20+
TRANSACTION_SOURCE_ROUTE,
21+
)
1822
from sentry_sdk.utils import (
1923
AnnotatedValue,
2024
capture_internal_exceptions,
@@ -25,7 +29,7 @@
2529
)
2630

2731
if TYPE_CHECKING:
28-
from typing import Any, Awaitable, Callable, Dict, Optional
32+
from typing import Any, Awaitable, Callable, Dict, Optional, Tuple
2933

3034
from sentry_sdk.scope import Scope as SentryScope
3135

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

113+
# Update transaction name with middleware name
114+
with hub.configure_scope() as sentry_scope:
115+
name, source = _get_transaction_from_middleware(app, scope, integration)
116+
if name is not None:
117+
sentry_scope.set_transaction_name(
118+
name,
119+
source=source,
120+
)
121+
109122
with hub.start_span(
110123
op=OP.MIDDLEWARE_STARLETTE, description=middleware_name
111124
) as middleware_span:
@@ -337,12 +350,14 @@ def patch_asgi_app():
337350

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

343357
middleware = SentryAsgiMiddleware(
344358
lambda *a, **kw: old_app(self, *a, **kw),
345359
mechanism_type=StarletteIntegration.identifier,
360+
transaction_style=integration.transaction_style,
346361
)
347362

348363
middleware.__call__ = middleware._run_asgi3
@@ -620,35 +635,53 @@ async def json(self):
620635
return await self.request.json()
621636

622637

638+
def _transaction_name_from_router(scope):
639+
# type: (StarletteScope) -> Optional[str]
640+
router = scope.get("router")
641+
if not router:
642+
return None
643+
644+
for route in router.routes:
645+
match = route.matches(scope)
646+
if match[0] == Match.FULL:
647+
return route.path
648+
649+
return None
650+
651+
623652
def _set_transaction_name_and_source(scope, transaction_style, request):
624653
# type: (SentryScope, str, Any) -> None
625-
name = ""
654+
name = None
655+
source = SOURCE_FOR_STYLE[transaction_style]
626656

627657
if transaction_style == "endpoint":
628658
endpoint = request.scope.get("endpoint")
629659
if endpoint:
630-
name = transaction_from_function(endpoint) or ""
660+
name = transaction_from_function(endpoint) or None
631661

632662
elif transaction_style == "url":
633-
router = request.scope["router"]
634-
for route in router.routes:
635-
match = route.matches(request.scope)
636-
637-
if match[0] == Match.FULL:
638-
if transaction_style == "endpoint":
639-
name = transaction_from_function(match[1]["endpoint"]) or ""
640-
break
641-
elif transaction_style == "url":
642-
name = route.path
643-
break
644-
645-
if not name:
663+
name = _transaction_name_from_router(request.scope)
664+
665+
if name is None:
646666
name = _DEFAULT_TRANSACTION_NAME
647667
source = TRANSACTION_SOURCE_ROUTE
648-
else:
649-
source = SOURCE_FOR_STYLE[transaction_style]
650668

651669
scope.set_transaction_name(name, source=source)
652670
logger.debug(
653671
"[Starlette] Set transaction name and source on scope: %s / %s", name, source
654672
)
673+
674+
675+
def _get_transaction_from_middleware(app, asgi_scope, integration):
676+
# type: (Any, Dict[str, Any], StarletteIntegration) -> Tuple[Optional[str], Optional[str]]
677+
name = None
678+
source = None
679+
680+
if integration.transaction_style == "endpoint":
681+
name = transaction_from_function(app.__class__)
682+
source = TRANSACTION_SOURCE_COMPONENT
683+
elif integration.transaction_style == "url":
684+
name = _transaction_name_from_router(asgi_scope)
685+
source = TRANSACTION_SOURCE_ROUTE
686+
687+
return name, source

0 commit comments

Comments
 (0)