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

feat(profiling): Set active thread id for quart #1830

Merged
merged 1 commit into from Mar 6, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
68 changes: 59 additions & 9 deletions sentry_sdk/integrations/quart.py
@@ -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
Expand All @@ -11,6 +14,7 @@
event_from_exception,
)

from sentry_sdk._functools import wraps
from sentry_sdk._types import TYPE_CHECKING

if TYPE_CHECKING:
Expand All @@ -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")

Expand Down Expand Up @@ -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
Copy link
Contributor

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 also add_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?

Copy link
Member Author

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.


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 = (
Copy link
Contributor

Choose a reason for hiding this comment

The 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.

Copy link
Member Author

Choose a reason for hiding this comment

The 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):
Expand Down
44 changes: 44 additions & 0 deletions tests/integrations/quart/test_quart.py
@@ -1,3 +1,6 @@
import json
import threading

import pytest
import pytest_asyncio

Expand Down Expand Up @@ -41,6 +44,20 @@ async def hi_with_id(message_id):
capture_message("hi with id")
return "ok with id"

@app.get("/sync/thread_ids")
def _thread_ids_sync():
return {
"main": str(threading.main_thread().ident),
"active": str(threading.current_thread().ident),
}

@app.get("/async/thread_ids")
async def _thread_ids_async():
return {
"main": str(threading.main_thread().ident),
"active": str(threading.current_thread().ident),
}

return app


Expand Down Expand Up @@ -523,3 +540,30 @@ async def dispatch_request(self):

assert event["message"] == "hi"
assert event["transaction"] == "hello_class"


@pytest.mark.parametrize("endpoint", ["/sync/thread_ids", "/async/thread_ids"])
async def test_active_thread_id(sentry_init, capture_envelopes, endpoint, app):
sentry_init(
traces_sample_rate=1.0,
_experiments={"profiles_sample_rate": 1.0},
)

envelopes = capture_envelopes()

async with app.test_client() as client:
response = await client.get(endpoint)
assert response.status_code == 200

data = json.loads(response.content)

envelopes = [envelope for envelope in envelopes]
assert len(envelopes) == 1

profiles = [item for item in envelopes[0].items if item.type == "profile"]
assert len(profiles) == 1

for profile in profiles:
transactions = profile.payload.json["transactions"]
assert len(transactions) == 1
assert str(data["active"]) == transactions[0]["active_thread_id"]