Skip to content
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

importlib-metadata breaks code #3234

Open
lzchen opened this issue Mar 23, 2023 · 18 comments
Open

importlib-metadata breaks code #3234

lzchen opened this issue Mar 23, 2023 · 18 comments
Assignees

Comments

@lzchen
Copy link
Contributor

lzchen commented Mar 23, 2023

          Was there a need to unconditionally use `importlib-metadata`, even on Python >= 3.10? This change broke my application framework (Asphalt) because it does `isinstance(value, EntryPoint)`, checking against `importlib.metadata.EntryPoint`, but after `importlib_metadata` is imported, iterating entry points (even through the stdlib facilities) yields `EntryPoint` instances from that third party library, which are NOT compatible with the stdlib ones!

EDIT: I see that importlib.metadata.EntryPoint is not directly documented in the API docs.

Originally posted by @agronholm in #3167 (comment)

@lzchen lzchen changed the title Was there a need to unconditionally use importlib-metadata, even on Python >= 3.10? This change broke my application framework (Asphalt) because it does isinstance(value, EntryPoint), checking against importlib.metadata.EntryPoint, but after importlib_metadata is imported, iterating entry points (even through the stdlib facilities) yields EntryPoint instances from that third party library, which are NOT compatible with the stdlib ones! importlib-metadata breaks code Mar 23, 2023
@agronholm
Copy link

Given that importlib.metadata is no longer provisional in Python 3.10, its API is stable, so the workaround should only be used for Python 3.9 and earlier.

@mvilanova
Copy link

Pinning to 1.16.0 temporarily fixed the issue for us in Python 3.11.

@aabmass
Copy link
Member

aabmass commented Mar 23, 2023

Given that importlib.metadata is no longer provisional in Python 3.10, its API is stable, so the workaround should only be used for Python 3.9 and earlier.

That makes sense to me 👍

Just to make sure I understand @agronholm, your original description of the issue does not sound like an issue with OpenTelemetry's usage of the library. It kind of sounds like an issue in either Asphalt (you mentioned EntryPoint is not documented looks fixed in this commit) in combination with some weird side effects between importlib.metadata and importlib-metadata.

Is that right?

@agronholm
Copy link

Just to make sure I understand @agronholm, your original description of the issue does not sound like an issue with OpenTelemetry's usage of the library. It kind of sounds like an issue in either Asphalt (you mentioned EntryPoint is not documented looks fixed in this commit) in combination with some weird side effects between importlib.metadata and importlib-metadata.

Yeah, it was only after upgrading the opentelemetry libs to 0.38b0 that I encountered this issue. Asphalt itself only uses the backport on Python 3.9 and earlier. It's not needed on 3.10 or later, and is just extra baggage there. I don't know if anybody is doing isinstance() with EntryPoint elsewhere, but if they do, they'll encounter the same issue.

@agronholm
Copy link

At any rate, the latest Asphalt no longer uses the EntryPoint class for comparisons so it's not an issue for me any longer.

@aabmass
Copy link
Member

aabmass commented Mar 23, 2023

@mvilanova did you encounter this outside of Asphalt? If not I think we should close this bug and open a separate one to make it conditional import for python<3.10

Arnatious pushed a commit to Arnatious/opentelemetry-python that referenced this issue May 23, 2023
Arnatious added a commit to Arnatious/opentelemetry-python that referenced this issue May 23, 2023
jenshnielsen added a commit to jenshnielsen/opentelemetry-python that referenced this issue Jul 6, 2023
This api is no longer provisional in 3.10 and this reduces
the number of dependencies for any instrumented code
on >= 3.10

Fixes open-telemetry#3234
@jsjeannotte
Copy link

Hi folks! Any update on this? We're still stuck with pinning to 1.16.0 and would love to un-pin.

@lzchen
Copy link
Contributor Author

lzchen commented Oct 11, 2023

@jsjeannotte

There was a pr that was closed so this issue still exists. @ocelotl for visibility. Seems like import-metadata still causing headaches :D

@lzchen
Copy link
Contributor Author

lzchen commented Oct 12, 2023

@jsjeannotte

Can you elaborate if you are experiencing breaking changes? Can you try upgrading to the latest opentelemetry to see if importlib-metadata is causing you issues?

@jsjeannotte
Copy link

I'm reaching out internally for the details, but a colleague using one of my python library reported this issue again when I unpinned opentelemetry-api on Tuesday (and picked up 1.20.0). They're running on Python 3.11. This is the stack trace they sent me:

  File "/apps/sirtdispatch/src/dispatch/extensions.py", line 3, in <module>
    import nflxlog
  File "/apps/sirtdispatch/lib/python3.11/site-packages/nflxlog/__init__.py", line 14, in <module>
    from opentelemetry import trace
  File "/apps/sirtdispatch/lib/python3.11/site-packages/opentelemetry/trace/__init__.py", line 87, in <module>
    from opentelemetry import context as context_api
  File "/apps/sirtdispatch/lib/python3.11/site-packages/opentelemetry/context/__init__.py", line 25, in <module>
    from opentelemetry.util._importlib_metadata import entry_points
  File "/apps/sirtdispatch/lib/python3.11/site-packages/opentelemetry/util/_importlib_metadata.py", line 17, in <module>
    from importlib_metadata import (  # type: ignore
ImportError: cannot import name 'EntryPoints' from 'importlib_metadata' (/apps/sirtdispatch/lib/python3.11/site-packages/importlib_metadata/__init__.py)

My guess is another dependency in their project is pinning to an incompatible version of importlib_metadata? Anyhow, I'm wondering if proactively fixing opentelemetry-api by using importlib.metadata for Python > 3.7 and conditionally including importlib_metadata for Python 3.7 would remove the issue completely.

@agronholm
Copy link

Is there a reason for continuing to support a Python release (3.7) well past its EOL date?

@aabmass
Copy link
Member

aabmass commented Oct 19, 2023

My guess is another dependency in their project is pinning to an incompatible version of importlib_metadata?

It would be helpful to figure out what is going on in their dependency tree and what version is actually being installed. We are not pinning to a single version but to 6.x

"importlib-metadata >= 6.0, < 7.0",

Anyhow, I'm wondering if proactively fixing opentelemetry-api by using importlib.metadata for Python > 3.7 and conditionally including importlib_metadata for Python 3.7 would remove the issue completely.

I don't think dropping Python 3.7 would actually fix things because importlib.metadata has breaking changes between python versions, which is exactly the reason that people are pinning to a major version of the PyPI package. See this note and the compatibility matrix here. IIRC @ocelotl was running into a lot of issues with missing imports and features. By using importlib-metadata we can just program against one API regardless of python version.

That said, I just noticed https://pypi.org/project/backports.entry-points-selectable/ which might support the feature set we need and be less invasive.

@mlasch
Copy link

mlasch commented May 22, 2024

I am also experiencing issues with the pinned version of importlib-metadata. Because the opentelemetry packages depend on importlib-metadata, it makes it impossible to use the importlib.metadata library from the standard library in my projects.

I work with a recent Yocto build with Python 3.11 and I cannot easily use the external importlib-metadata package.

The build package from pypa has a good solution which might work here as well: https://github.com/pypa/build/blob/main/src/build/_compat/importlib.py
They use the correct metadata module depending on the Python version and package availability.

@xrmx
Copy link
Contributor

xrmx commented Sep 6, 2024

@mlasch Is this python/importlib_metadata#489 the issue (fixed in 8.1.0) that wouldn't let you use importlib-metadata in yocto?

@mlasch
Copy link

mlasch commented Oct 20, 2024

@xrmx No, python/importlib_metadata#489 is not the issue. The problem is not that I am unable to use the backported package importlib-metadata at all in Yocto.
The problem is that I'm building an embedded system and unnecessary dependencies are generally not preferred. importlib is already available from the standard library.
I think the correct solution would be to use importlib.metadata for Python versions where it is available and use the package importlib_metadata for older Python versions.

@mlasch
Copy link

mlasch commented Oct 20, 2024

Actually #4177 could be a solution. I agree it makes the code more complex, but modern Python versions would not have to use importlib_metadata.

@xrmx
Copy link
Contributor

xrmx commented Oct 21, 2024

@mlasch we decided to use importlib-metadata on all python versions to simplify things on our side. From the next release importlib-metadata is used instead of pkg-resources.

@mlasch
Copy link

mlasch commented Oct 22, 2024

@xrmx That's unfortunate. However, nice to see that https://github.com/open-telemetry/opentelemetry-python/blob/main/opentelemetry-api/src/opentelemetry/util/_importlib_metadata.py is the single place which imports importlib_metadata that makes patching much easier.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
8 participants