You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
This could be considered a bug, poorly documented behavior or just fragility of monkey patching.
importfalcon# in app.py or somethingclassMyApp(falcon.App):
...
# in main.py or somethingimportfalconfromfalconimportAppfromopentelemetry.instrumentation.falconimportFalconInstrumentorfromappimportMyAppFalconInstrumentor.instrument()
App() # won't be instrumentedMyApp() # won't be instrumentedfalcon.App() # will be instrumented
This can be fixed by instrumenting before from falcon import App and from app import MyApp:
importfalcon# in app.py or somethingclassMyApp(falcon.App):
...
# in main.py or somethingimportfalconfromopentelemetry.instrumentation.falconimportFalconInstrumentorFalconInstrumentor.instrument()
fromfalconimportAppfromappimportMyAppApp() # won't be instrumentedMyApp() # won't be instrumentedfalcon.App() # will be instrumented
But:
Linters won't be happy
This is super ugly
The only way to figure this out is to dig deep into the OpenTelemetry source code, understand where/how the monkey patching is happening and then apply the fix. It's not exactly documented.
I understand the desire to provide users with "auto-instrumentation" by means a single function call / even running opentelemetry-instrument on unmodified code. It's super cool! But it can't be the only way to do things. As demonstrated here there are a lot of rough edges to this.
It would be nice if the API was presented in a layered fashion:
opentelemetry-instrument
FalconInstrumentor.instrument()
Falcon Middleware + WSGI middleware that just calls some public methods / uses a public context manager.
Falcon Middleware + the public context manager
This way, users can just go with opentelemtry-instrument if that works for them. But if that doesn't work, they have the option to peel back the onion, read a bit deeper into the docs and understand how to add the middleware themselves and such. This also decreases the burden of functionality of auto-instrumentation: if in certain situations it is too hard / fragile / hacky to get something working via auto-instrumentation, but it is possible with a bit more work (e.g. a WSGI wrapper), this can be documented as "if you want XYZ advanced feature, you can instrument yourself and configure it following this example".
The text was updated successfully, but these errors were encountered:
You've correctly discovered that instrumentation must be applied/activated before importing any the libraries that are to be instrumented. It may work for some apps even if it is applied post-import but by design it is supposed to be applied pre-import.
There are a few things being raised here.
Document that instrumentation should be applied before imported the instrumented library.
Make the Falcon middleware part of the public API so users can use that directly without the instrumentation ceremony.
Perhaps add an instrument_app() method like Flask so that users can use the auto-inject mechanism on a single app/api instance.
My only concern is about 2. I think it can be useful to make the middleware available for direct use but using the middleware directly might not always result in the same telemetry data as generated by auto-instrumentation but I suppose if that is documented, it should be fine.
I don't see why we cannot do these but I'd suggest to open a different issue for each topic.
This could be considered a bug, poorly documented behavior or just fragility of monkey patching.
This can be fixed by instrumenting before
from falcon import App
andfrom app import MyApp
:But:
I understand the desire to provide users with "auto-instrumentation" by means a single function call / even running
opentelemetry-instrument
on unmodified code. It's super cool! But it can't be the only way to do things. As demonstrated here there are a lot of rough edges to this.It would be nice if the API was presented in a layered fashion:
opentelemetry-instrument
FalconInstrumentor.instrument()
This way, users can just go with
opentelemtry-instrument
if that works for them. But if that doesn't work, they have the option to peel back the onion, read a bit deeper into the docs and understand how to add the middleware themselves and such. This also decreases the burden of functionality of auto-instrumentation: if in certain situations it is too hard / fragile / hacky to get something working via auto-instrumentation, but it is possible with a bit more work (e.g. a WSGI wrapper), this can be documented as "if you want XYZ advanced feature, you can instrument yourself and configure it following this example".The text was updated successfully, but these errors were encountered: