-
Notifications
You must be signed in to change notification settings - Fork 535
feat(profiling): Set active thread id for quart #1830
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
Changes from all commits
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 |
---|---|---|
@@ -1,5 +1,8 @@ | ||
from __future__ import absolute_import | ||
|
||
import inspect | ||
import threading | ||
|
||
from sentry_sdk.hub import _should_send_default_pii, Hub | ||
from sentry_sdk.integrations import DidNotEnable, Integration | ||
from sentry_sdk.integrations._wsgi_common import _filter_headers | ||
|
@@ -11,6 +14,7 @@ | |
event_from_exception, | ||
) | ||
|
||
from sentry_sdk._functools import wraps | ||
from sentry_sdk._types import TYPE_CHECKING | ||
|
||
if TYPE_CHECKING: | ||
|
@@ -34,13 +38,15 @@ | |
request, | ||
websocket, | ||
) | ||
from quart.scaffold import Scaffold # type: ignore | ||
from quart.signals import ( # type: ignore | ||
got_background_exception, | ||
got_request_exception, | ||
got_websocket_exception, | ||
request_started, | ||
websocket_started, | ||
) | ||
from quart.utils import is_coroutine_function # type: ignore | ||
except ImportError: | ||
raise DidNotEnable("Quart is not installed") | ||
|
||
|
@@ -71,18 +77,62 @@ def setup_once(): | |
got_request_exception.connect(_capture_exception) | ||
got_websocket_exception.connect(_capture_exception) | ||
|
||
old_app = Quart.__call__ | ||
patch_asgi_app() | ||
patch_scaffold_route() | ||
|
||
|
||
def patch_asgi_app(): | ||
# type: () -> None | ||
old_app = Quart.__call__ | ||
|
||
async def sentry_patched_asgi_app(self, scope, receive, send): | ||
# type: (Any, Any, Any, Any) -> Any | ||
if Hub.current.get_integration(QuartIntegration) is None: | ||
return await old_app(self, scope, receive, send) | ||
|
||
middleware = SentryAsgiMiddleware(lambda *a, **kw: old_app(self, *a, **kw)) | ||
middleware.__call__ = middleware._run_asgi3 | ||
return await middleware(scope, receive, send) | ||
|
||
Quart.__call__ = sentry_patched_asgi_app | ||
|
||
|
||
def patch_scaffold_route(): | ||
# type: () -> None | ||
old_route = Scaffold.route | ||
|
||
def _sentry_route(*args, **kwargs): | ||
# type: (*Any, **Any) -> Any | ||
old_decorator = old_route(*args, **kwargs) | ||
|
||
def decorator(old_func): | ||
# type: (Any) -> Any | ||
|
||
if inspect.isfunction(old_func) and not is_coroutine_function(old_func): | ||
|
||
@wraps(old_func) | ||
def _sentry_func(*args, **kwargs): | ||
# type: (*Any, **Any) -> Any | ||
hub = Hub.current | ||
integration = hub.get_integration(QuartIntegration) | ||
if integration is None: | ||
return old_func(*args, **kwargs) | ||
|
||
with hub.configure_scope() as sentry_scope: | ||
if sentry_scope.profile is not None: | ||
sentry_scope.profile.active_thread_id = ( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What is the active_thread_id used for? Quart doesn't really use threads, so I'm interested to learn what the need is. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For context, we're working on adding profiling support for the various ASGI frameworks, and when profiling, we have the concept of an active profile which for the purpose of a web server, it is the thread on which the request is handled. In this case, we're specifically targeting synchronous request handlers because the way they're handled is by dispatching the handler to another thread. So what we're trying to do here is after it's been dispatched, we want to determine the thread id the handler is running on and update the profile metadata to be aware of that. |
||
threading.current_thread().ident | ||
) | ||
|
||
return old_func(*args, **kwargs) | ||
|
||
return old_decorator(_sentry_func) | ||
|
||
async def sentry_patched_asgi_app(self, scope, receive, send): | ||
# type: (Any, Any, Any, Any) -> Any | ||
if Hub.current.get_integration(QuartIntegration) is None: | ||
return await old_app(self, scope, receive, send) | ||
return old_decorator(old_func) | ||
|
||
middleware = SentryAsgiMiddleware(lambda *a, **kw: old_app(self, *a, **kw)) | ||
middleware.__call__ = middleware._run_asgi3 | ||
return await middleware(scope, receive, send) | ||
return decorator | ||
|
||
Quart.__call__ = sentry_patched_asgi_app | ||
Scaffold.route = _sentry_route | ||
|
||
|
||
def _set_transaction_name_and_source(scope, transaction_style, request): | ||
|
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.
route
isn't the only way to create a handler, there is alsoadd_url_rule
and the corresponding ones for websockets.Did you consider using a
before_request
function as a place to run some code for every request?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.
Looking at
before_request
, I'm not sure it'll serve the use case we want here because it we specifically want to read the thread id of the thread running the request handler and from what I can see,before_request
is called from the main thread and potentially dispatched to another thread independently from the request handler.