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

Auto instrumentation and manual instrumentation should both leverage package.py inside the instrumentation for dependency checking #3201

Open
rjduffner opened this issue Jan 22, 2025 · 1 comment · May be fixed by #3202

Comments

@rjduffner
Copy link
Contributor

Right now, auto instrumentation does a slightly different dependency check than the manual instrumentation does. auto instrumentation checks the pyproject.toml instruments and extras dependencies while the instrumentation checks the _instruments variable in the package.py.

For all but two of the instrumentations, this is not an issue as those instrumentations are tied 1 instrumentation to 1 package. However, there are two instrumentations where 1 instrumentation can instrument 2 python packages. In this case, auto instrumentation will fail because the auto instrumentation's dependency check fails because both packages are not installed.

Lets show this more in depth in the opentelemetry-instrumentation-kafka-python.

This instrumentation can instrument either the package, kafka-python or the package kafka-python-ng

Because of this, the pyproject.toml specifies both packages in the instruments dependencies list

The package.py also specifies both packages and the dependent versions as well

The difference comes in during the dependency check

Inside the instrumentation, the two package requirements are treated as an OR (this package or that package). However inside the auto instrumentation, they are treated as an AND. Because, both packages can't install we end up with an error like,

2025-01-21 12:52:28,342 DEBUG [opentelemetry.instrumentation.auto_instrumentation._load:_load_instrumentors:105][pid=92229][tname=MainThread] [trace_id=0 span_id=0] Skipping instrumentation kafka: DependencyConflict: requested: "kafka-python<3.0,>=2.0; extra == "instruments"" but found: "None"

During auto_instrumentation, the kafka-python instrumentation is run through https://github.com/open-telemetry/opentelemetry-python-contrib/blob/main/opentelemetry-instrumentation/src/opentelemetry/instrumentation/dependencies.py#L43-L59 which looks at each package in the instruments or extras list and makes sure they are installed. (The AND)

During manual instrumentation, the dependencies are loaded from the instrumentation https://github.com/open-telemetry/opentelemetry-python-contrib/blob/main/opentelemetry-instrumentation/src/opentelemetry/instrumentation/instrumentor.py#L87 with this function https://github.com/open-telemetry/opentelemetry-python-contrib/blob/main/instrumentation/opentelemetry-instrumentation-kafka-python/src/opentelemetry/instrumentation/kafka/__init__.py#L95-L113 which checks for either distribution but not both (The OR).

I believe the best solution is to ditch the dist dependency check found during auto instrumentation and only use the the instrumentation dependency check as this allows far more flexibility in non standard cases.

There might be some additional context in the slack thread I started to make sure I fully understood this behavior before filing this issue.. https://cloud-native.slack.com/archives/C01PD4HUVBL/p1737494014736559

Thanks @xrmx for confirming.

@rjduffner
Copy link
Contributor Author

Working on this issue in #3202

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