Manual instrumentation falcon #1365
Conversation
…eAnshul756/opentelemetry-python-contrib into manual-instrumentation-falcon
…eAnshul756/opentelemetry-python-contrib into manual-instrumentation-falcon
…eAnshul756/opentelemetry-python-contrib into manual-instrumentation-falcon
| ) | ||
| self._is_instrumented_by_opentelemetry = False | ||
|
|
||
| app = FalconAPI() |
There was a problem hiding this comment.
I didn't understand this. The app arg passed is getting replaced by this; how doe this work?
There was a problem hiding this comment.
So, here we wanted to add more attributes in the original falcon api, but falcon do not let us add anything to already created app because of the use of __slots__. So, what we did is, we created a new child class of Falcon API and copied all the attributes in the new objects with the additional attributes that we wanted to add and returned the new app instead of the original app.
There was a problem hiding this comment.
Does it contain everything? What happens to the rest of the attributes/properties/basically everything other than what __slots__ have on app instance?
There was a problem hiding this comment.
Yes. Falcon can not have additional instance variables because of __slots__ and because the new object is of child class so all the methods of falcon class will be inherited.
There was a problem hiding this comment.
I am a little skeptical of this. I will try to find an example where this is not true.
There was a problem hiding this comment.
Yes, I understand your point. What are the other possibilities? Do you have any suggestion? I can not think of anything else.
There was a problem hiding this comment.
I would move the tracer/meter/instruments/config state to _TraceMiddleware since it's the one that's actually using them and then add it to middleware list.
There was a problem hiding this comment.
Not just _TraceMiddleware but __call__ function from _InstrumentedFalconAPI also uses those states. If we move all these states to _TraceMiddleware then we'll have to move all the instrumentation logic to middleware. Are you suggesting that?
There was a problem hiding this comment.
I think _InstrumentedFalconAPI is a simple WSGI middleware. It's not necessary they maintain separate tracer/meter/instruments on it's own. It can use what will be used for the TraceMiddleware.
There was a problem hiding this comment.
Sorry I'm not able to understand. Can you please elaborate?
…eAnshul756/opentelemetry-python-contrib into manual-instrumentation-falcon
|
@ocelotl : I guess you have mentioned that in wrong issue. Please delete your comment from here and paste it on this issue |
Right, moved ✌️ |
Description
Manual Instrumentation in Falcon
Fixes #1342
Type of change
Please delete options that are not relevant.
How Has This Been Tested?
Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration
tox -e test-instrumentation-falcon1tox -e test-instrumentation-falcon2tox -e test-instrumentation-falcon3Does This PR Require a Core Repo Change?
Checklist: