starlette: use wrapt for patching instead of class replacement#3530
starlette: use wrapt for patching instead of class replacement#3530SrdjanLL wants to merge 2 commits into
Conversation
| from starlette import applications | ||
| from starlette.routing import Match |
There was a problem hiding this comment.
So if we don't have an ordering issue with other instrumentations then the value of switching to wrapt would be the removal of these top level imports and push them at a later time or move them under type checking. Do you see this achievable?
|
Would love a fix to the issue, if it is this, so that we don't need to manually do Right now, you have to manually instrument the app in order to use "zero code instrumentation" (and successfully join agents in the same trace). The combination of "zero code" and also needing to patch your code doesn't make any sense. cc @aabmass |
|
This PR has been automatically marked as stale because it has not had any activity for 14 days. It will be closed if no further activity occurs within 14 days of this comment. |
|
This PR has been closed due to inactivity. Please reopen if you would like to continue working on it. |
Description
Moving starlette instrumentation to patch
Starlette.__init__method usingwraptinstead of doing the class replacement to prevent potential ordering issues.Left the behaviour of manual instrumentation methods of
StarletteInstrumentorto work as it did before.The core issue where this surfaced (#3476) has already been closed, but this was still a relevant improvement
Type of change
Please delete options that are not relevant.
How Has This Been Tested?
Does This PR Require a Core Repo Change?
Checklist:
See contributing.md for styleguide, changelog guidelines, and more.
addedupdated