-
-
Notifications
You must be signed in to change notification settings - Fork 944
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
OpenTelemetry Falcon 4.x integration #1828
Comments
Falcon 3.0 WSGI-only (it seems ASGI is unsupported?) integration might be resolved by open-telemetry/opentelemetry-python-contrib#644 by @adriangb. |
Well we would love your help over there! Specifically it seems that catching tracebacks is very different in two and three. I also think it would be great if you could take an overall look at the package, I'm sure you could find improvements 😃 |
Now that we have brought this up, I'd like to point out that a number of well-known projects have added or decided to add support for OpenTelemetry out of the box. I'm one of the maintainers of OpenTelemetry Python and I'd be glad to help Falcon do the same if that is something of interest to Falcon maintainers. Falcon itself would only take dependency only on Some other OSS projects I know of that have decided to support OpenTelemetry officially: Spring Sleuth: https://github.com/spring-cloud-incubator/spring-cloud-sleuth-otel |
Hi @owais! OTOH, we could probably make it optional, and host the plugin package under the Falconry umbrella, while shipping some very basic instrumentation affordances in the framework itself (if needed). We could also add an install "extra" dependency pulling in the supported instrumentation plugins. @owais Do you know any Python frameworks that could serve as an example how to properly ship and integrate it? |
Maybe just adding well designed hooks for telemetry/metrics/logging + fixing issues like exception capture (#1942 as well as open-telemetry/opentelemetry-python-contrib#644) might be enough? Basically making it dead-simple for users (or the OpenTelemetry team) to hook into Falcon. Perhaps even write documentation and have tests around hooking |
@vytas7 The dependency is very small both in terms of size and functionality but I see your concern. It can be integrated into the framework in a way that does not require the dependency to run but works automatically when it is available. We'd essentially be creating a tiny wrapper for OpenTelemetry in Falcon that uses the real module when available and is noop when not although technically that is already what opentelemetry-api does for the actual library (opentelemetry-sdk).
Google PubSub Python client is in the process of adding this. PR: googleapis/python-pubsub#149 Happy to help whichever direction you'd like to take. |
That sounds good, yeah, that's what we are doing for certain optional media de/serialization features such as We'll need to take a closer look at all this, but I do like the idea of adding some optional instrumentation affordances directly to the framework, and OpenTelemetry could be the first such plugin. This could hopefully allow us to reduce the instrumentation overhead even further, and make sure Falcon is well supported. See also: #10 & #1908. |
Hi again @owais & @srikanthccv, |
@vytas7 I am eager to take on this development, are we still looking to work on this task. |
That's awesome @gmittal22 😎 Are you by chance in Prague EuroPython, and would like to Sprint on this on Saturday, or is it unrelated? |
No, I am based in Asia. Would want to sprint but how does next week sound? |
It's yet to be seen how much traction OpenTelemetry gains, but it seems to be getting embraced by large actors.
OpenTelemetry Python instrumentation actually has a Falcon 2.0 instrumentation plugin!
It would be great to extend this instrumentor for the upcoming Falcon 3.x series.
Some things that I expect to break or be missing:
falcon.API
➡️falcon.App
renaming might not get picked by the plugin's monkey patching unless one is using the deprectatedAPI
aliasRelated/a subset of: #10
The text was updated successfully, but these errors were encountered: