feat(profiling): Set active thread id for quart#1830
Conversation
|
@pgjones we're making some changes to the Quart integration to enable our (beta) profiler on ASGI frameworks, in case you want to take a quick look at the changes! |
|
|
||
| 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.
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.
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.
|
|
||
| def patch_scaffold_route(): | ||
| # type: () -> None | ||
| old_route = Scaffold.route |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
|
I understand. Had you considered patching the |
I did consider that at one point but I noticed that |
|
Yes, I think that is correct. However, the other methods could be a before/after function which I think would also ideally be recorded in the transaction? (As the total time to respond will include these). If not I think what is in this PR does make the most sense. |
So even if the other calls to |
03ff178 to
073e658
Compare
|
This pull request has gone three weeks without activity. In another week, I will close it. But! If you comment or otherwise update it, I will reset the clock, and if you label it "A weed is but an unloved flower." ― Ella Wheeler Wilcox 🥀 |
Following up to #1824 to set the active thread id for quart.
896fcb7 to
1f60e7b
Compare
|
sry this flew under the radar |
Following up to #1824 to set the active thread id for quart.