Skip to content

Commit 3e75908

Browse files
jeremydvossjeremydvoss
committed
instrumentation changes
1 parent b74633a commit 3e75908

File tree

6 files changed

+496
-108
lines changed

6 files changed

+496
-108
lines changed

‎CHANGELOG.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
1717
([#3567](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/3567))
1818
- `opentelemetry-resource-detector-containerid`: make it more quiet on platforms without cgroups
1919
([#3579](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/3579))
20+
- `opentelemetry-instrumentation`: Fix dependency conflict detection when instrumented packages are not installed. Add "instruments_either" feature for instrumentations that target multiple packages.
21+
([#3610](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/3610))
2022

2123
### Added
2224

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

Lines changed: 74 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -12,22 +12,50 @@
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+
get_dist_dependency_conflicts,
21+
)
1922
from opentelemetry.instrumentation.distro import BaseDistro, DefaultDistro
2023
from opentelemetry.instrumentation.environment_variables import (
2124
OTEL_PYTHON_CONFIGURATOR,
2225
OTEL_PYTHON_DISABLED_INSTRUMENTATIONS,
2326
OTEL_PYTHON_DISTRO,
2427
)
2528
from opentelemetry.instrumentation.version import __version__
26-
from opentelemetry.util._importlib_metadata import entry_points
29+
from opentelemetry.util._importlib_metadata import (
30+
EntryPoint,
31+
distributions,
32+
entry_points,
33+
)
2734

2835
_logger = getLogger(__name__)
2936

3037

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

5684
def _load_instrumentors(distro):
5785
package_to_exclude = environ.get(OTEL_PYTHON_DISABLED_INSTRUMENTATIONS, [])
86+
entry_point_finder = _EntryPointDistFinder()
5887
if isinstance(package_to_exclude, str):
5988
package_to_exclude = package_to_exclude.split(",")
6089
# to handle users entering "requests , flask" or "requests, flask" with spaces
@@ -64,44 +93,69 @@ def _load_instrumentors(distro):
6493
entry_point.load()()
6594

6695
for entry_point in entry_points(group="opentelemetry_instrumentor"):
96+
print(entry_point.name)
6797
if entry_point.name in package_to_exclude:
6898
_logger.debug(
6999
"Instrumentation skipped for library %s", entry_point.name
70100
)
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+
print("conflict found %s: %s" % (entry_point.name, conflict))
108+
_logger.debug(
109+
"Skipping instrumentation %s: %s",
110+
entry_point.name,
111+
conflict,
112+
)
113+
continue
114+
115+
# tell instrumentation to not run dep checks again as we already did it above
116+
distro.load_instrumentor(entry_point, skip_dep_check=True)
77117
_logger.debug("Instrumented %s", entry_point.name)
78-
except DependencyConflictError as exc:
79-
_logger.debug(
80-
"Skipping instrumentation %s: %s",
81-
entry_point.name,
82-
exc.conflict,
83-
)
84-
continue
85-
except ModuleNotFoundError as exc:
86-
# ModuleNotFoundError is raised when the library is not installed
87-
# and the instrumentation is not required to be loaded.
88-
# See https://github.com/open-telemetry/opentelemetry-python-contrib/issues/3421
89-
_logger.debug(
90-
"Skipping instrumentation %s: %s", entry_point.name, exc.msg
91-
)
92-
continue
118+
print("Instrumented %s" % entry_point.name)
119+
# except DependencyConflictError as exc:
120+
# print(
121+
# "Skipping instrumentation %s: %s" % (
122+
# entry_point.name,
123+
# exc.conflict,
124+
# )
125+
# )
126+
# _logger.debug(
127+
# "Skipping instrumentation %s: %s",
128+
# entry_point.name,
129+
# exc.conflict,
130+
# )
131+
# continue
132+
# except ModuleNotFoundError as exc:
133+
# # ModuleNotFoundError is raised when the library is not installed
134+
# # and the instrumentation is not required to be loaded.
135+
# # See https://github.com/open-telemetry/opentelemetry-python-contrib/issues/3421
136+
# print(
137+
# "Skipping instrumentation %s: %s" % (entry_point.name, exc.msg)
138+
# )
139+
# _logger.debug(
140+
# "Skipping instrumentation %s: %s", entry_point.name, exc.msg
141+
# )
142+
# continue
93143
except ImportError:
94144
# in scenarios using the kubernetes operator to do autoinstrumentation some
95145
# instrumentors (usually requiring binary extensions) may fail to load
96146
# because the injected autoinstrumentation code does not match the application
97147
# environment regarding python version, libc, etc... In this case it's better
98148
# to skip the single instrumentation rather than failing to load everything
99149
# so treat differently ImportError than the rest of exceptions
150+
print(
151+
"Skipping instrumentation %s: ImportError" % entry_point.name
152+
)
100153
_logger.exception(
101154
"Importing of %s failed, skipping it", entry_point.name
102155
)
103156
continue
104157
except Exception as exc: # pylint: disable=broad-except
158+
print("Skipping instrumentation %s: %s" % (entry_point.name, exc))
105159
_logger.exception("Instrumenting of %s failed", entry_point.name)
106160
raise exc
107161

‎opentelemetry-instrumentation/src/opentelemetry/instrumentation/auto_instrumentation/sitecustomize.py‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,3 +15,4 @@
1515
from opentelemetry.instrumentation.auto_instrumentation import initialize
1616

1717
initialize()
18+
print("OpenTelemetry auto-instrumentation initialized.")

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

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

2222
from opentelemetry.util._importlib_metadata import (
23+
Distribution,
2324
PackageNotFoundError,
2425
version,
2526
)
2627

2728
logger = getLogger(__name__)
2829

30+
# TODO: consider replacing _either with _any or _or
31+
2932

3033
class DependencyConflict:
3134
required: str | None = None
3235
found: str | None = None
33-
34-
def __init__(self, required: str | None, found: str | None = None):
36+
# The following fields are used when an instrumentation requires any of a set of dependencies rather than all.
37+
required_either: Collection[str] = []
38+
found_either: Collection[str] = []
39+
40+
# TODO: No longer requires required field
41+
def __init__(
42+
self,
43+
required: str | None = None,
44+
found: str | None = None,
45+
required_either: Collection[str] = [],
46+
found_either: Collection[str] = [],
47+
):
3548
self.required = required
3649
self.found = found
50+
# The following fields are used when an instrumentation requires any of a set of dependencies rather than all.
51+
self.required_either = required_either
52+
self.found_either = found_either
3753

3854
def __str__(self):
55+
if not self.required and (self.required_either or self.found_either):
56+
print("EITHER STRING")
57+
# TODO: make sure this formats correctly
58+
return f'DependencyConflict: requested any of the following: "{self.required_either}" but found: "{self.found_either}"'
59+
print("AND STRING")
3960
return f'DependencyConflict: requested: "{self.required}" but found: "{self.found}"'
4061

4162

63+
# TODO: Figure out if this should be a subclass of DependencyConflict.
64+
# If now, change functions to return either and then ensure that all the dependents can handle the new value
65+
# class DependencyConflictEither(DependencyConflict):
66+
# required: Collection[str] = []
67+
# found: Collection[str] = []
68+
69+
# def __init__(self, required: Collection[str], found: Collection[str] = []):
70+
# self.required = required
71+
# self.found = found
72+
73+
# def __str__(self):
74+
# return f'DependencyConflictEither: requested: "{self.required}" but found: "{self.found}"'
75+
76+
4277
class DependencyConflictError(Exception):
4378
conflict: DependencyConflict
4479

@@ -49,14 +84,61 @@ def __str__(self):
4984
return str(self.conflict)
5085

5186

87+
def get_dist_dependency_conflicts(
88+
dist: Distribution,
89+
) -> DependencyConflict | None:
90+
instrumentation_deps = []
91+
instrumentation_either_deps = []
92+
extra = "extra"
93+
instruments = "instruments"
94+
instruments_marker = {extra: instruments}
95+
instruments_either = "instruments_either"
96+
instruments_either_marker = {extra: instruments_either}
97+
# print(f"dist: {dist}")
98+
# print(f"dist.requires: {dist.requires}")
99+
if dist.requires:
100+
for dep in dist.requires:
101+
print(f"dep: {dep}")
102+
if extra not in dep:
103+
print(f"Skipping dep: {dep}")
104+
continue
105+
if instruments not in dep and instruments_either not in dep:
106+
print(f"Skipping dep: {dep}")
107+
continue
108+
109+
req = Requirement(dep)
110+
# print(f"req: {req}")
111+
if req.marker.evaluate(instruments_marker): # type: ignore
112+
# print("Evaluated. Append")
113+
instrumentation_deps.append(req) # type: ignore
114+
if req.marker.evaluate(instruments_either_marker): # type: ignore
115+
# print("Evaluated. either. Append")
116+
# Need someway to separate
117+
instrumentation_either_deps.append(req) # type: ignore
118+
dc = get_dependency_conflicts(
119+
instrumentation_deps, instrumentation_either_deps
120+
) # type: ignore
121+
print(f"dep conf: {dc}")
122+
return dc
123+
# return get_dependency_conflicts(instrumentation_deps, instrumentation_either_deps) # type: ignore
124+
125+
52126
def get_dependency_conflicts(
53-
deps: Collection[str | Requirement],
127+
deps: Collection[
128+
str | Requirement
129+
], # Depedencies all of which are required
130+
deps_either: Collection[
131+
str | Requirement
132+
] = [], # Dependencies any of which are required
54133
) -> DependencyConflict | None:
55134
for dep in deps:
135+
# TODO: what is this?
56136
if isinstance(dep, Requirement):
137+
print("REQUIREMENT")
57138
req = dep
58139
else:
59140
try:
141+
print("NOT REQUIREMENT")
60142
req = Requirement(dep)
61143
except InvalidRequirement as exc:
62144
logger.warning(
@@ -69,8 +151,61 @@ def get_dependency_conflicts(
69151
try:
70152
dist_version = version(req.name)
71153
except PackageNotFoundError:
154+
# TODO: Technically this field should allow Requirements type. Tackle this in a separate PR.
72155
return DependencyConflict(dep)
73156

74157
if not req.specifier.contains(dist_version):
158+
# TODO: Technically this field should allow Requirements type
75159
return DependencyConflict(dep, f"{req.name} {dist_version}")
160+
161+
# TODO: add eval of deps_either
162+
if deps_either:
163+
# TODO: change to using DependencyConflict
164+
is_dependency_conflict = True
165+
required_either: Collection[str] = []
166+
found_either: Collection[str] = []
167+
for dep in deps_either:
168+
# TODO: what is this?
169+
if isinstance(dep, Requirement):
170+
print("REQUIREMENT")
171+
req = dep
172+
else:
173+
try:
174+
print("NOT REQUIREMENT")
175+
req = Requirement(dep)
176+
except InvalidRequirement as exc:
177+
logger.warning(
178+
'error parsing dependency, reporting as a conflict: "%s" - %s',
179+
dep,
180+
exc,
181+
)
182+
return DependencyConflict(dep)
183+
184+
try:
185+
dist_version = version(req.name)
186+
except PackageNotFoundError:
187+
# print(f"PackageNotFoundError EITHER: {req.name}")
188+
# TODO: anything here?
189+
# return DependencyConflict(dep)
190+
required_either.append(str(dep))
191+
continue
192+
193+
if req.specifier.contains(dist_version):
194+
is_dependency_conflict = False
195+
# Since only one of the instrumentation_either dependencies is required, there is no depdendency conflict.
196+
break
197+
else:
198+
required_either.append(str(dep))
199+
found_either.append(f"{req.name} {dist_version}")
200+
201+
if is_dependency_conflict:
202+
# return DependencyConflict(dep, f"{req.name} {dist_version}")
203+
# print (f"required_either: {required_either}")
204+
# print (f"found_either: {found_either}")
205+
return DependencyConflict(
206+
required_either=required_either,
207+
found_either=found_either,
208+
)
209+
return None
210+
76211
return None

0 commit comments

Comments
 (0)