Skip to content

Commit 283f999

Browse files
committed
Add "instruments-any" feature: unblock multi-target instrumentations
while fixing dependency conflict breakage (SQUASH)
1 parent f9453b9 commit 283f999

File tree

17 files changed

+549
-83
lines changed

17 files changed

+549
-83
lines changed

CHANGELOG.md

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,18 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
1111
1212
## Unreleased
1313

14+
### Fixed
15+
16+
- `opentelemetry-instrumentation`: Fix dependency conflict detection when instrumented packages are not installed by moving check back to before instrumentors are loaded. Add "instruments-any" feature for instrumentations that target multiple packages.
17+
([#3610](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/3610))
18+
19+
### Added
20+
21+
- `opentelemetry-instrumentation-psycopg2` Utilize instruments-any functionality.
22+
([#3610](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/3612))
23+
- `opentelemetry-instrumentation-kafka-python` Utilize instruments-any functionality.
24+
([#3610](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/3612))
25+
1426
## Version 1.35.0/0.56b0 (2025-07-11)
1527

1628
### Added

CONTRIBUTING.md

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -325,10 +325,14 @@ Below is a checklist of things to be mindful of when implementing a new instrume
325325
### Update supported instrumentation package versions
326326

327327
- Navigate to the **instrumentation package directory:**
328-
- Update **`pyproject.toml`** file by modifying _instruments_ entry in the `[project.optional-dependencies]` section with the new version constraint
329-
- Update `_instruments` variable in instrumentation **`package.py`** file with the new version constraint
328+
- Update **`pyproject.toml`** file by modifying `instruments` or `instruments-any` entry in the `[project.optional-dependencies]` section with the new version constraint
329+
- Update `_instruments` or `_instruments_any` variable in instrumentation **`package.py`** file with the new version constraint
330330
- At the **root of the project directory**, run `tox -e generate` to regenerate necessary files
331331

332+
Please note that `instruments-any` is an optional field that can be used instead of or in addition to `instruments`. While `instruments` is a list of dependencies, _all_ of which are expected by the instrumentation, `instruments-any` is a list _any_ of which but not all are expected.
333+
334+
<!-- See https://github.com/open-telemetry/opentelemetry-python-contrib/pull/3610 for details on instruments-any -->
335+
332336
If you're adding support for a new version of the instrumentation package, follow these additional steps:
333337

334338
- At the **instrumentation package directory:** Add new test-requirements.txt file with the respective package version required for testing

instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py

Lines changed: 16 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,6 @@
4040
)
4141
from opentelemetry.instrumentation.dependencies import (
4242
DependencyConflict,
43-
DependencyConflictError,
4443
)
4544
from opentelemetry.sdk.metrics.export import (
4645
HistogramDataPoint,
@@ -1102,40 +1101,34 @@ def test_instruments_with_fastapi_installed(self, mock_logger):
11021101
[self._instrumentation_loaded_successfully_call()]
11031102
)
11041103

1104+
@patch(
1105+
"opentelemetry.instrumentation.auto_instrumentation._load.get_dist_dependency_conflicts"
1106+
)
11051107
@patch("opentelemetry.instrumentation.auto_instrumentation._load._logger")
1106-
def test_instruments_with_old_fastapi_installed(self, mock_logger): # pylint: disable=no-self-use
1108+
def test_instruments_with_old_fastapi_installed(
1109+
self, mock_logger, mock_dep
1110+
): # pylint: disable=no-self-use
11071111
dependency_conflict = DependencyConflict("0.58", "0.57")
11081112
mock_distro = Mock()
1109-
mock_distro.load_instrumentor.side_effect = DependencyConflictError(
1110-
dependency_conflict
1111-
)
1113+
mock_dep.return_value = dependency_conflict
11121114
_load_instrumentors(mock_distro)
1113-
self.assertEqual(len(mock_distro.load_instrumentor.call_args_list), 1)
1114-
(ep,) = mock_distro.load_instrumentor.call_args.args
1115-
self.assertEqual(ep.name, "fastapi")
1116-
assert (
1117-
self._instrumentation_loaded_successfully_call()
1118-
not in mock_logger.debug.call_args_list
1119-
)
1115+
mock_distro.load_instrumentor.assert_not_called()
11201116
mock_logger.debug.assert_has_calls(
11211117
[self._instrumentation_failed_to_load_call(dependency_conflict)]
11221118
)
11231119

1120+
@patch(
1121+
"opentelemetry.instrumentation.auto_instrumentation._load.get_dist_dependency_conflicts"
1122+
)
11241123
@patch("opentelemetry.instrumentation.auto_instrumentation._load._logger")
1125-
def test_instruments_without_fastapi_installed(self, mock_logger): # pylint: disable=no-self-use
1124+
def test_instruments_without_fastapi_installed(
1125+
self, mock_logger, mock_dep
1126+
): # pylint: disable=no-self-use
11261127
dependency_conflict = DependencyConflict("0.58", None)
11271128
mock_distro = Mock()
1128-
mock_distro.load_instrumentor.side_effect = DependencyConflictError(
1129-
dependency_conflict
1130-
)
1129+
mock_dep.return_value = dependency_conflict
11311130
_load_instrumentors(mock_distro)
1132-
self.assertEqual(len(mock_distro.load_instrumentor.call_args_list), 1)
1133-
(ep,) = mock_distro.load_instrumentor.call_args.args
1134-
self.assertEqual(ep.name, "fastapi")
1135-
assert (
1136-
self._instrumentation_loaded_successfully_call()
1137-
not in mock_logger.debug.call_args_list
1138-
)
1131+
mock_distro.load_instrumentor.assert_not_called()
11391132
mock_logger.debug.assert_has_calls(
11401133
[self._instrumentation_failed_to_load_call(dependency_conflict)]
11411134
)

instrumentation/opentelemetry-instrumentation-kafka-python/pyproject.toml

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,8 @@ dependencies = [
3131
]
3232

3333
[project.optional-dependencies]
34-
instruments = [
34+
instruments = []
35+
instruments-any = [
3536
"kafka-python >= 2.0, < 3.0",
3637
"kafka-python-ng >= 2.0, < 3.0"
3738
]

instrumentation/opentelemetry-instrumentation-kafka-python/src/opentelemetry/instrumentation/kafka/package.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,4 +16,5 @@
1616
_instruments_kafka_python = "kafka-python >= 2.0, < 3.0"
1717
_instruments_kafka_python_ng = "kafka-python-ng >= 2.0, < 3.0"
1818

19-
_instruments = (_instruments_kafka_python, _instruments_kafka_python_ng)
19+
_instruments = ()
20+
_instruments_any = (_instruments_kafka_python, _instruments_kafka_python_ng)

instrumentation/opentelemetry-instrumentation-psycopg2/pyproject.toml

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,8 @@ dependencies = [
3131
]
3232

3333
[project.optional-dependencies]
34-
instruments = [
34+
instruments = []
35+
instruments-any = [
3536
"psycopg2 >= 2.7.3.1",
3637
"psycopg2-binary >= 2.7.3.1",
3738
]

instrumentation/opentelemetry-instrumentation-psycopg2/src/opentelemetry/instrumentation/psycopg2/package.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,8 @@
1616
_instruments_psycopg2 = "psycopg2 >= 2.7.3.1"
1717
_instruments_psycopg2_binary = "psycopg2-binary >= 2.7.3.1"
1818

19-
_instruments = (
19+
_instruments = ()
20+
_instruments_any = (
2021
_instruments_psycopg2,
2122
_instruments_psycopg2_binary,
2223
)

instrumentation/opentelemetry-instrumentation-psycopg2/tests/test_psycopg2_instrumentation.py

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -137,12 +137,16 @@ def _distribution(name):
137137
# Note there is only one test here but it is run twice in tox
138138
# once with the psycopg2 package installed and once with
139139
# psycopg2-binary installed.
140+
@patch(
141+
"opentelemetry.instrumentation.auto_instrumentation._load.get_dist_dependency_conflicts"
142+
)
140143
@patch("opentelemetry.instrumentation.auto_instrumentation._load._logger")
141-
def test_instruments_with_psycopg2_installed(self, mock_logger):
144+
def test_instruments_with_psycopg2_installed(self, mock_logger, mock_dep):
142145
def _instrumentation_loaded_successfully_call():
143146
return call("Instrumented %s", "psycopg2")
144147

145148
mock_distro = Mock()
149+
mock_dep.return_value = None
146150
mock_distro.load_instrumentor.return_value = None
147151
_load_instrumentors(mock_distro)
148152
self.assertEqual(len(mock_distro.load_instrumentor.call_args_list), 1)

opentelemetry-instrumentation/src/opentelemetry/instrumentation/auto_instrumentation/_load.py

Lines changed: 48 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -12,22 +12,51 @@
1212
# See the License for the specific language governing permissions and
1313
# limitations under the License.
1414

15+
from functools import cached_property
1516
from logging import getLogger
1617
from os import environ
1718

18-
from opentelemetry.instrumentation.dependencies import DependencyConflictError
19+
from opentelemetry.instrumentation.dependencies import (
20+
DependencyConflictError,
21+
get_dist_dependency_conflicts,
22+
)
1923
from opentelemetry.instrumentation.distro import BaseDistro, DefaultDistro
2024
from opentelemetry.instrumentation.environment_variables import (
2125
OTEL_PYTHON_CONFIGURATOR,
2226
OTEL_PYTHON_DISABLED_INSTRUMENTATIONS,
2327
OTEL_PYTHON_DISTRO,
2428
)
2529
from opentelemetry.instrumentation.version import __version__
26-
from opentelemetry.util._importlib_metadata import entry_points
30+
from opentelemetry.util._importlib_metadata import (
31+
EntryPoint,
32+
distributions,
33+
entry_points,
34+
)
2735

2836
_logger = getLogger(__name__)
2937

3038

39+
class _EntryPointDistFinder:
40+
@cached_property
41+
def _mapping(self):
42+
return {
43+
self._key_for(ep): dist
44+
for dist in distributions()
45+
for ep in dist.entry_points
46+
}
47+
48+
def dist_for(self, entry_point: EntryPoint):
49+
dist = getattr(entry_point, "dist", None)
50+
if dist:
51+
return dist
52+
53+
return self._mapping.get(self._key_for(entry_point))
54+
55+
@staticmethod
56+
def _key_for(entry_point: EntryPoint):
57+
return f"{entry_point.group}:{entry_point.name}:{entry_point.value}"
58+
59+
3160
def _load_distro() -> BaseDistro:
3261
distro_name = environ.get(OTEL_PYTHON_DISTRO, None)
3362
for entry_point in entry_points(group="opentelemetry_distro"):
@@ -55,6 +84,7 @@ def _load_distro() -> BaseDistro:
5584

5685
def _load_instrumentors(distro):
5786
package_to_exclude = environ.get(OTEL_PYTHON_DISABLED_INSTRUMENTATIONS, [])
87+
entry_point_finder = _EntryPointDistFinder()
5888
if isinstance(package_to_exclude, str):
5989
package_to_exclude = package_to_exclude.split(",")
6090
# to handle users entering "requests , flask" or "requests, flask" with spaces
@@ -71,11 +101,24 @@ def _load_instrumentors(distro):
71101
continue
72102

73103
try:
74-
distro.load_instrumentor(
75-
entry_point, raise_exception_on_conflict=True
76-
)
104+
entry_point_dist = entry_point_finder.dist_for(entry_point)
105+
conflict = get_dist_dependency_conflicts(entry_point_dist)
106+
if conflict:
107+
_logger.debug(
108+
"Skipping instrumentation %s: %s",
109+
entry_point.name,
110+
conflict,
111+
)
112+
continue
113+
114+
# tell instrumentation to not run dep checks again as we already did it above
115+
distro.load_instrumentor(entry_point, skip_dep_check=True)
77116
_logger.debug("Instrumented %s", entry_point.name)
78117
except DependencyConflictError as exc:
118+
# Dependency conflicts are generally caught from get_dist_dependency_conflicts
119+
# returning a DependencyConflict. Keeping this error handling in case custom
120+
# distro and instrumentor behavior raises a DependencyConflictError later.
121+
# See https://github.com/open-telemetry/opentelemetry-python-contrib/pull/3610
79122
_logger.debug(
80123
"Skipping instrumentation %s: %s",
81124
entry_point.name,

opentelemetry-instrumentation/src/opentelemetry/instrumentation/dependencies.py

Lines changed: 114 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
from packaging.requirements import InvalidRequirement, Requirement
2121

2222
from opentelemetry.util._importlib_metadata import (
23+
Distribution,
2324
PackageNotFoundError,
2425
version,
2526
)
@@ -28,14 +29,44 @@
2829

2930

3031
class DependencyConflict:
32+
"""Represents a dependency conflict in OpenTelemetry instrumentation.
33+
34+
This class is used to track conflicts between required dependencies and the
35+
actual installed packages. It supports two scenarios:
36+
37+
1. Standard conflicts where all dependencies are required
38+
2. Either/or conflicts where only one of a set of dependencies is required
39+
40+
Attributes:
41+
required: The required dependency specification that conflicts with what's installed.
42+
found: The actual dependency that was found installed (if any).
43+
required_any: Collection of dependency specifications where any one would satisfy
44+
the requirement (for either/or scenarios).
45+
found_any: Collection of actual dependencies found for either/or scenarios.
46+
"""
47+
3148
required: str | None = None
3249
found: str | None = None
33-
34-
def __init__(self, required: str | None, found: str | None = None):
50+
# The following fields are used when an instrumentation requires any of a set of dependencies rather than all.
51+
required_any: Collection[str] = None
52+
found_any: Collection[str] = None
53+
54+
def __init__(
55+
self,
56+
required: str | None = None,
57+
found: str | None = None,
58+
required_any: Collection[str] = None,
59+
found_any: Collection[str] = None,
60+
):
3561
self.required = required
3662
self.found = found
63+
# The following fields are used when an instrumentation requires any of a set of dependencies rather than all.
64+
self.required_any = required_any
65+
self.found_any = found_any
3766

3867
def __str__(self):
68+
if not self.required and (self.required_any or self.found_any):
69+
return f'DependencyConflict: requested any of the following: "{self.required_any}" but found: "{self.found_any}"'
3970
return f'DependencyConflict: requested: "{self.required}" but found: "{self.found}"'
4071

4172

@@ -49,8 +80,39 @@ def __str__(self):
4980
return str(self.conflict)
5081

5182

83+
def get_dist_dependency_conflicts(
84+
dist: Distribution,
85+
) -> DependencyConflict | None:
86+
instrumentation_deps = []
87+
instrumentation_any_deps = []
88+
extra = "extra"
89+
instruments = "instruments"
90+
instruments_marker = {extra: instruments}
91+
instruments_any = "instruments-any"
92+
instruments_any_marker = {extra: instruments_any}
93+
if dist.requires:
94+
for dep in dist.requires:
95+
if extra not in dep:
96+
continue
97+
if instruments not in dep and instruments_any not in dep:
98+
continue
99+
100+
req = Requirement(dep)
101+
if req.marker.evaluate(instruments_marker): # type: ignore
102+
instrumentation_deps.append(req) # type: ignore
103+
if req.marker.evaluate(instruments_any_marker): # type: ignore
104+
instrumentation_any_deps.append(req) # type: ignore
105+
return get_dependency_conflicts(
106+
instrumentation_deps, instrumentation_any_deps
107+
) # type: ignore
108+
109+
52110
def get_dependency_conflicts(
53-
deps: Collection[str | Requirement],
111+
deps: Collection[
112+
str | Requirement
113+
], # Dependencies all of which are required
114+
deps_any: Collection[str | Requirement]
115+
| None = None, # Dependencies any of which are required
54116
) -> DependencyConflict | None:
55117
for dep in deps:
56118
if isinstance(dep, Requirement):
@@ -73,4 +135,53 @@ def get_dependency_conflicts(
73135

74136
if not req.specifier.contains(dist_version):
75137
return DependencyConflict(dep, f"{req.name} {dist_version}")
138+
139+
# If all the dependencies in "instruments" are present, check "instruments-any" for conflicts.
140+
if deps_any:
141+
return _get_dependency_conflicts_any(deps_any)
142+
return None
143+
144+
145+
# This is a helper functions designed to ease reading and meet linting requirements.
146+
def _get_dependency_conflicts_any(
147+
deps_any: Collection[str | Requirement],
148+
) -> DependencyConflict | None:
149+
if not deps_any:
150+
return None
151+
is_dependency_conflict = True
152+
required_any: Collection[str] = []
153+
found_any: Collection[str] = []
154+
for dep in deps_any:
155+
if isinstance(dep, Requirement):
156+
req = dep
157+
else:
158+
try:
159+
req = Requirement(dep)
160+
except InvalidRequirement as exc:
161+
logger.warning(
162+
'error parsing dependency, reporting as a conflict: "%s" - %s',
163+
dep,
164+
exc,
165+
)
166+
return DependencyConflict(dep)
167+
168+
try:
169+
dist_version = version(req.name)
170+
except PackageNotFoundError:
171+
required_any.append(str(dep))
172+
continue
173+
174+
if req.specifier.contains(dist_version):
175+
# Since only one of the instrumentation_any dependencies is required, there is no dependency conflict.
176+
is_dependency_conflict = False
177+
break
178+
# If the version does not match, add it to the list of unfulfilled requirement options.
179+
required_any.append(str(dep))
180+
found_any.append(f"{req.name} {dist_version}")
181+
182+
if is_dependency_conflict:
183+
return DependencyConflict(
184+
required_any=required_any,
185+
found_any=found_any,
186+
)
76187
return None

0 commit comments

Comments
 (0)