-
Notifications
You must be signed in to change notification settings - Fork 565
fix: Replace asyncio.iscoroutinefunction with compatibility shim for Python 3.16 #4915
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
base: master
Are you sure you want to change the base?
Changes from 7 commits
cb84e92
0b84014
ad2c56d
81b4b80
1c41672
f6996fa
d81d3bc
24e2fdc
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -10,6 +10,7 @@ | |
from functools import partial | ||
|
||
import sentry_sdk | ||
from sentry_sdk._compat import iscoroutinefunction | ||
from sentry_sdk.api import continue_trace | ||
from sentry_sdk.consts import OP | ||
from sentry_sdk.integrations._asgi_common import ( | ||
|
@@ -76,10 +77,10 @@ def _looks_like_asgi3(app): | |
if inspect.isclass(app): | ||
return hasattr(app, "__await__") | ||
elif inspect.isfunction(app): | ||
return asyncio.iscoroutinefunction(app) | ||
return iscoroutinefunction(app) | ||
|
||
else: | ||
call = getattr(app, "__call__", None) # noqa | ||
return asyncio.iscoroutinefunction(call) | ||
return iscoroutinefunction(call) | ||
|
||
|
||
class SentryAsgiMiddleware: | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,55 @@ | ||
from contextlib import contextmanager | ||
from typing import Generator, Optional | ||
|
||
# Import the helper that returns the current active span/transaction without | ||
# importing the top-level package to avoid circular imports. | ||
from sentry_sdk.tracing_utils import get_current_span | ||
|
||
|
||
@contextmanager | ||
def allow_n_plus_one(reason: Optional[str] = None) -> Generator[None, None, None]: | ||
"""Context manager to mark the current span and its root transaction as | ||
intentionally allowed N+1. | ||
This sets tags on the active span and its containing transaction so that | ||
server-side N+1 detectors (if updated to honor these tags) can ignore the | ||
transaction. This helper is best-effort and will not raise if there is no | ||
active span/transaction. | ||
Usage: | ||
with allow_n_plus_one("expected loop"): | ||
for x in queryset: | ||
... | ||
""" | ||
span = get_current_span() | ||
if span is not None: | ||
try: | ||
# Tag the active span | ||
span.set_tag("sentry.n_plus_one.ignore", True) | ||
if reason: | ||
span.set_tag("sentry.n_plus_one.reason", reason) | ||
|
||
# Also tag the containing transaction if available | ||
try: | ||
tx = span.containing_transaction | ||
except Exception: | ||
tx = None | ||
|
||
if tx is not None: | ||
try: | ||
tx.set_tag("sentry.n_plus_one.ignore", True) | ||
if reason: | ||
tx.set_tag("sentry.n_plus_one.reason", reason) | ||
except Exception: | ||
# best-effort: do not fail if transaction tagging fails | ||
pass | ||
except Exception: | ||
# best-effort: silence any unexpected errors | ||
pass | ||
|
||
try: | ||
yield | ||
finally: | ||
# keep tags; no cleanup required | ||
pass | ||
|
||
cursor[bot] marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Switching
asyncio.iscoroutinefunction
forinspect.iscoroutinefunction
is tricky because of historical incompatibility. Since we support Python versions 3.6 and up we have to care about this unfortunately.We should use the most conservative cutoff to not break any users. So while CPython deprecated
asyncio.iscoroutinefunction
in version 3.12, we should probably mirror Starlette who use 3.13 because of a bug in standard libraries which have not been backported. See Kludex/starlette#2983Also, since we vendor
_is_async_callable
, the change below would keep the vendor up to date. We can therefore be more confident that we won't run into edge cases, since users ofstarlette
would have run into them already.