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 ASGI frameworks #1778

Closed
Closed
3 changes: 2 additions & 1 deletion sentry_sdk/integrations/asgi.py
Expand Up @@ -14,6 +14,7 @@
from sentry_sdk.hub import Hub, _should_send_default_pii
from sentry_sdk.integrations._wsgi_common import _filter_headers
from sentry_sdk.integrations.modules import _get_installed_modules
from sentry_sdk.profiler import start_profiling
from sentry_sdk.sessions import auto_session_tracking
from sentry_sdk.tracing import (
SOURCE_FOR_STYLE,
Expand Down Expand Up @@ -175,7 +176,7 @@ async def _run_app(self, scope, callback):

with hub.start_transaction(
transaction, custom_sampling_context={"asgi_scope": scope}
):
), start_profiling(transaction, hub):
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will enable profiling for all the ASGI frameworks as well. This should be removed in #1797 as that will enable it for all transactions.

# XXX: Would be cool to have correct span status, but we
# would have to wrap send(). That is a bit hard to do with
# the current abstraction over ASGI 2/3.
Expand Down
14 changes: 10 additions & 4 deletions sentry_sdk/integrations/django/asgi.py
Expand Up @@ -7,6 +7,7 @@
"""

import asyncio
import threading

from sentry_sdk import Hub, _functools
from sentry_sdk._types import MYPY
Expand Down Expand Up @@ -89,10 +90,15 @@ def wrap_async_view(hub, callback):
async def sentry_wrapped_callback(request, *args, **kwargs):
# type: (Any, *Any, **Any) -> Any

with hub.start_span(
op=OP.VIEW_RENDER, description=request.resolver_match.view_name
):
return await callback(request, *args, **kwargs)
with hub.configure_scope() as sentry_scope:
# set the active thread id to the handler thread for sync views
# this isn't necessary for async views since that runs on main
sentry_scope.set_active_thread_id(threading.current_thread().ident)

with hub.start_span(
op=OP.VIEW_RENDER, description=request.resolver_match.view_name
):
return await callback(request, *args, **kwargs)

return sentry_wrapped_callback

Expand Down
15 changes: 11 additions & 4 deletions sentry_sdk/integrations/django/views.py
@@ -1,3 +1,5 @@
import threading

from sentry_sdk.consts import OP
from sentry_sdk.hub import Hub
from sentry_sdk._types import MYPY
Expand Down Expand Up @@ -73,9 +75,14 @@ def _wrap_sync_view(hub, callback):
@_functools.wraps(callback)
def sentry_wrapped_callback(request, *args, **kwargs):
# type: (Any, *Any, **Any) -> Any
with hub.start_span(
op=OP.VIEW_RENDER, description=request.resolver_match.view_name
):
return callback(request, *args, **kwargs)
with hub.configure_scope() as sentry_scope:
# set the active thread id to the handler thread for sync views
# this isn't necessary for async views since that runs on main
sentry_scope.set_active_thread_id(threading.current_thread().ident)

with hub.start_span(
op=OP.VIEW_RENDER, description=request.resolver_match.view_name
):
return callback(request, *args, **kwargs)

return sentry_wrapped_callback
18 changes: 18 additions & 0 deletions sentry_sdk/integrations/fastapi.py
@@ -1,3 +1,6 @@
import asyncio
import threading

from sentry_sdk._types import MYPY
from sentry_sdk.hub import Hub, _should_send_default_pii
from sentry_sdk.integrations import DidNotEnable
Expand Down Expand Up @@ -62,6 +65,21 @@ def patch_get_request_handler():

def _sentry_get_request_handler(*args, **kwargs):
# type: (*Any, **Any) -> Any
dependant = kwargs.get("dependant")
if dependant and not asyncio.iscoroutinefunction(dependant.call):
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it safe to use asyncio here for this?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes I think so.
We vendored this from starlette for doing these coroutine checks.

@antonpirker knows this stuff better since he worked on it.

old_call = dependant.call

def _sentry_call(*args, **kwargs):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is setting the thread in fastapi still necessary if we already do it in request_response in starlette?

# type: (*Any, **Any) -> Any
hub = Hub.current
with hub.configure_scope() as sentry_scope:
# set the active thread id to the handler thread for sync views
# this isn't necessary for async views since that runs on main
sentry_scope.set_active_thread_id(threading.current_thread().ident)
return old_call(*args, **kwargs)

dependant.call = _sentry_call

old_app = old_get_request_handler(*args, **kwargs)

async def _sentry_app(*args, **kwargs):
Expand Down
69 changes: 60 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 MYPY

if MYPY:
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
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was introduced in quart==0.11.1, do we need to support older versions?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nope, tox says min 0.16.1, so all good

except ImportError:
raise DidNotEnable("Quart is not installed")

Expand Down Expand Up @@ -71,18 +77,63 @@ 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:
# set the active thread id to the handler thread for sync views
# this isn't necessary for async views since that runs on main
sentry_scope.set_active_thread_id(
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
5 changes: 5 additions & 0 deletions sentry_sdk/integrations/starlette.py
Expand Up @@ -2,6 +2,7 @@

import asyncio
import functools
import threading

from sentry_sdk._compat import iteritems
from sentry_sdk._types import MYPY
Expand Down Expand Up @@ -403,6 +404,10 @@ def _sentry_sync_func(*args, **kwargs):
return old_func(*args, **kwargs)

with hub.configure_scope() as sentry_scope:
# set the active thread id to the handler thread for sync views
# this isn't necessary for async views since that runs on main
sentry_scope.set_active_thread_id(threading.current_thread().ident)

request = args[0]

_set_transaction_name_and_source(
Expand Down
37 changes: 37 additions & 0 deletions tests/integrations/django/asgi/test_asgi.py
@@ -1,3 +1,5 @@
import json

import django
import pytest
from channels.testing import HttpCommunicator
Expand Down Expand Up @@ -70,6 +72,41 @@ async def test_async_views(sentry_init, capture_events, application):
}


@pytest.mark.parametrize("application", APPS)
@pytest.mark.parametrize("endpoint", ["/sync/thread_ids", "/async/thread_ids"])
@pytest.mark.asyncio
@pytest.mark.skipif(
django.VERSION < (3, 1), reason="async views have been introduced in Django 3.1"
)
async def test_active_thread_id(sentry_init, capture_envelopes, endpoint, application):
sentry_init(
integrations=[DjangoIntegration()],
traces_sample_rate=1.0,
_experiments={"profiles_sample_rate": 1.0},
)

envelopes = capture_envelopes()

comm = HttpCommunicator(application, "GET", endpoint)
response = await comm.get_response()
assert response["status"] == 200, response["body"]

await comm.wait()

data = json.loads(response["body"])

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"]


@pytest.mark.asyncio
@pytest.mark.skipif(
django.VERSION < (3, 1), reason="async views have been introduced in Django 3.1"
Expand Down
6 changes: 6 additions & 0 deletions tests/integrations/django/myapp/urls.py
Expand Up @@ -58,6 +58,7 @@ def path(path, *args, **kwargs):
views.csrf_hello_not_exempt,
name="csrf_hello_not_exempt",
),
path("sync/thread_ids", views.thread_ids_sync, name="thread_ids_sync"),
]

# async views
Expand All @@ -67,6 +68,11 @@ def path(path, *args, **kwargs):
if views.my_async_view is not None:
urlpatterns.append(path("my_async_view", views.my_async_view, name="my_async_view"))

if views.thread_ids_async is not None:
urlpatterns.append(
path("async/thread_ids", views.thread_ids_async, name="thread_ids_async")
)

# rest framework
try:
urlpatterns.append(
Expand Down
23 changes: 23 additions & 0 deletions tests/integrations/django/myapp/views.py
@@ -1,3 +1,6 @@
import json
import threading

from django import VERSION
from django.contrib.auth import login
from django.contrib.auth.models import User
Expand Down Expand Up @@ -159,6 +162,16 @@ def csrf_hello_not_exempt(*args, **kwargs):
return HttpResponse("ok")


def thread_ids_sync(*args, **kwargs):
response = json.dumps(
{
"main": threading.main_thread().ident,
"active": threading.current_thread().ident,
}
)
return HttpResponse(response)


if VERSION >= (3, 1):
# Use exec to produce valid Python 2
exec(
Expand All @@ -173,6 +186,16 @@ def csrf_hello_not_exempt(*args, **kwargs):
await asyncio.sleep(1)
return HttpResponse('Hello World')"""
)

exec(
"""async def thread_ids_async(request):
response = json.dumps({
"main": threading.main_thread().ident,
"active": threading.current_thread().ident,
})
return HttpResponse(response)"""
)
else:
async_message = None
my_async_view = None
thread_ids_async = None
46 changes: 46 additions & 0 deletions tests/integrations/fastapi/test_fastapi.py
@@ -1,3 +1,6 @@
import json
import threading

import pytest
from sentry_sdk.integrations.fastapi import FastApiIntegration

Expand All @@ -23,6 +26,20 @@ async def _message_with_id(message_id):
capture_message("Hi")
return {"message": "Hi"}

@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 @@ -135,3 +152,32 @@ def test_legacy_setup(

(event,) = events
assert event["transaction"] == "/message/{message_id}"


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

envelopes = capture_envelopes()

client = TestClient(asgi_app)
response = 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"]