-
Notifications
You must be signed in to change notification settings - Fork 679
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
🚀 Feature: Make instrumentations compatible with "opentelemetry-instrument" automatic instrumentation #591
Comments
Thanks @torbenw seems like an easy one liner fix! |
@nirga thanks for this update. Any eta to publish a new version so we can use it for auto instrumentation with opentelemetry-instrument? |
@torbenw I’m publishing now |
This does not seem to work for me. Maybe there's more that has to be changed? I'm new to Python, so I don't really know how things work with installing packages. For "opentelemetry-instrumentation-httpx" I have a file "entry_points.txt" in .../venv/Lib/site-packages/opentelemetry_instrumentation_httpx-0.44b0.dist-info:
In .../venv/Lib/site-packages/opentelemetry_instrumentation_langchain-0.14.1.dist-info there is no such file. |
@torbenw I'll try to ask around in the community in circle back. |
@torbenw great, seems that poetry doesn't support PEP-621 :/ |
I think there's more to do even if we can get the instrumentations into the entry_points.txt.
|
@torbenw Hmm, that still seems to be related to the entry point. Looks like it can't find the method that instruments (although its there). |
I think this is missing, too? Example:
I might be misunderstanding this, I'm a Java developer with almost no Python experience. ;) |
Hmm you're right I see that in other instrumentations. But it still won't work since we use Poetry and this attribute won't get bundled (if I understood correctly). So until Poetry supports PEP-621, or we switch to another build system (which will have its own issues since we have a monorepo here) we won't be able to support it :/ |
I think there's a solution! 🤔 |
how does it works? |
@npuichigo just install any of the instrumentations and then initialize the auto instrumentation:
|
Has anybody used this in a custom image used for the open telemetry operator? My naive test to just add this to requirements.txt did not work (error.txt):
|
Hey @kguelzau sorry, this slipped between the cracks. I'll look into this! |
@nirga perhaps there is still a problem in the build process. |
- use optional dependencies, instead of test dependencies - https://python-poetry.org/docs/pyproject/#extras
@nirga / @galkleinman I am also not a python dev, but I think you need to change the dependency to an optional dependency like in the draft PR #2191 (see https://python-poetry.org/docs/pyproject/#extras).
So this should hopefully work within a custom image for the open telemetry operator. |
@kguelzau weird - the package is in the dev dependencies so it shouldn't appear in the resulted pyproject. Have you tried building it with the previous config and see what happens? |
Which component is this feature for?
All Packages
🔖 Feature description
When starting a python app via "opentelemetry-instrument python myapp.py" available instrumentors are automatically discovered and used for tracing. This works with official opentelemetry instrumentors, but not with the instrumentors created by traceloop. Instrumentors created by traceloop should behave the same way as the opentelemetry instrumentors so that they can be used in automatic instrumentation without having to change the code of the python app which is being observed.
🎤 Why is this feature needed ?
Being able to use automatic instrumentation would remove the requirement of changing code to add observability to apps.
✌️ How do you aim to achieve this?
The "official" instrumentations by opentelemetry have the following lines in their pyproject.toml:
Instrumentations with the "opentelemetry_instrumentor" entry-point are automatically loaded by the auto-instrumentation:
🔄️ Additional Information
No response
👀 Have you spent some time to check if this feature request has been raised before?
Are you willing to submit PR?
None
The text was updated successfully, but these errors were encountered: