clean _load, sitecustomize, dependencies

Author: jeremydvoss

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

@@ -17,6 +17,7 @@
 from os import environ
 
 from opentelemetry.instrumentation.dependencies import (
+    DependencyConflictError,
     get_dist_dependency_conflicts,
 )
 from opentelemetry.instrumentation.distro import BaseDistro, DefaultDistro
@@ -93,7 +94,6 @@ def _load_instrumentors(distro):
         entry_point.load()()
 
     for entry_point in entry_points(group="opentelemetry_instrumentor"):
-        print(entry_point.name)
         if entry_point.name in package_to_exclude:
             _logger.debug(
                 "Instrumentation skipped for library %s", entry_point.name
@@ -104,7 +104,6 @@ def _load_instrumentors(distro):
             entry_point_dist = entry_point_finder.dist_for(entry_point)
             conflict = get_dist_dependency_conflicts(entry_point_dist)
             if conflict:
-                # print("conflict found %s: %s" % (entry_point.name, conflict))
                 _logger.debug(
                     "Skipping instrumentation %s: %s",
                     entry_point.name,
@@ -115,41 +114,29 @@ def _load_instrumentors(distro):
             # tell instrumentation to not run dep checks again as we already did it above
             distro.load_instrumentor(entry_point, skip_dep_check=True)
             _logger.debug("Instrumented %s", entry_point.name)
-            # print("Instrumented %s" % entry_point.name)
-        # except DependencyConflictError as exc:
-        #     print(
-        #         "Skipping instrumentation %s: %s" % (
-        #             entry_point.name,
-        #             exc.conflict,
-        #         )
-        #     )
-        #     _logger.debug(
-        #         "Skipping instrumentation %s: %s",
-        #         entry_point.name,
-        #         exc.conflict,
-        #     )
-        #     continue
-        # except ModuleNotFoundError as exc:
-        #     # ModuleNotFoundError is raised when the library is not installed
-        #     # and the instrumentation is not required to be loaded.
-        #     # See https://github.com/open-telemetry/opentelemetry-python-contrib/issues/3421
-        #     print(
-        #         "Skipping instrumentation %s: %s" % (entry_point.name, exc.msg)
-        #     )
-        #     _logger.debug(
-        #         "Skipping instrumentation %s: %s", entry_point.name, exc.msg
-        #     )
-        #     continue
+        except DependencyConflictError as exc:
+            # DependencyConflicts will generally arise without exception from the get_dist_dependency_conflicts call before instrumentation is attempted. Keeping this exception in case of custom distro and instrumentor behavior. See https://github.com/open-telemetry/opentelemetry-python-contrib/pull/3610 for details.
+            _logger.debug(
+                "Skipping instrumentation %s: %s",
+                entry_point.name,
+                exc.conflict,
+            )
+            continue
+        except ModuleNotFoundError as exc:
+            # ModuleNotFoundError is raised when the library is not installed
+            # and the instrumentation is not required to be loaded.
+            # See https://github.com/open-telemetry/opentelemetry-python-contrib/issues/3421
+            _logger.debug(
+                "Skipping instrumentation %s: %s", entry_point.name, exc.msg
+            )
+            continue
         except ImportError:
             # in scenarios using the kubernetes operator to do autoinstrumentation some
             # instrumentors (usually requiring binary extensions) may fail to load
             # because the injected autoinstrumentation code does not match the application
             # environment regarding python version, libc, etc... In this case it's better
             # to skip the single instrumentation rather than failing to load everything
             # so treat differently ImportError than the rest of exceptions
-            # print(
-            #     "Skipping instrumentation %s: ImportError" % entry_point.name
-            # )
             _logger.exception(
                 "Importing of %s failed, skipping it", entry_point.name
             )

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

@@ -15,4 +15,3 @@
 from opentelemetry.instrumentation.auto_instrumentation import initialize
 
 initialize()
-print("OpenTelemetry auto-instrumentation initialized.")

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

@@ -27,8 +27,6 @@
 
 logger = getLogger(__name__)
 
-# TODO: consider replacing _either with _any or _or
-
 
 class DependencyConflict:
     required: str | None = None
@@ -37,13 +35,10 @@ class DependencyConflict:
     required_either: Collection[str] = []
     found_either: Collection[str] = []
 
-    # TODO: No longer requires required field
     def __init__(
         self,
         required: str | None = None,
         found: str | None = None,
-        # TODO: dangerous
-        # TODO: dangerous
         required_either: Collection[str] = None,
         found_either: Collection[str] = None,
     ):
@@ -55,27 +50,10 @@ def __init__(
 
     def __str__(self):
         if not self.required and (self.required_either or self.found_either):
-            print("EITHER STRING")
-            # TODO: make sure this formats correctly
             return f'DependencyConflict: requested any of the following: "{self.required_either}" but found: "{self.found_either}"'
-        print("AND STRING")
         return f'DependencyConflict: requested: "{self.required}" but found: "{self.found}"'
 
 
-# TODO: Figure out if this should be a subclass of DependencyConflict.
-# If now, change functions to return either and then ensure that all the dependents can handle the new value
-# class DependencyConflictEither(DependencyConflict):
-#     required: Collection[str] = []
-#     found: Collection[str] = []
-
-#     def __init__(self, required: Collection[str], found: Collection[str] = []):
-#         self.required = required
-#         self.found = found
-
-#     def __str__(self):
-#         return f'DependencyConflictEither: requested: "{self.required}" but found: "{self.found}"'
-
-
 class DependencyConflictError(Exception):
     conflict: DependencyConflict
 
@@ -96,59 +74,36 @@ def get_dist_dependency_conflicts(
     instruments_marker = {extra: instruments}
     instruments_either = "instruments_either"
     instruments_either_marker = {extra: instruments_either}
-    # print(f"dist: {dist}")
-    # print(f"dist.requires: {dist.requires}")
     if dist.requires:
         for dep in dist.requires:
-            print(f"dep: {dep}")
             if extra not in dep:
-                print(f"Skipping dep: {dep}")
                 continue
             if instruments not in dep and instruments_either not in dep:
-                print(f"Skipping dep: {dep}")
                 continue
 
             req = Requirement(dep)
-            # print(f"req: {req}")
             if req.marker.evaluate(instruments_marker):  # type: ignore
-                # print("Evaluated. Append")
                 instrumentation_deps.append(req)  # type: ignore
             if req.marker.evaluate(instruments_either_marker):  # type: ignore
-                # print("Evaluated. either. Append")
-                # Need someway to separate
                 instrumentation_either_deps.append(req)  # type: ignore
-    dc = get_dependency_conflicts(
+    return get_dependency_conflicts(
         instrumentation_deps, instrumentation_either_deps
     )  # type: ignore
-    print(f"dep conf: {dc}")
-    return dc
-    # return get_dependency_conflicts(instrumentation_deps, instrumentation_either_deps) # type: ignore
 
 
 def get_dependency_conflicts(
     deps: Collection[
         str | Requirement
-    ],  # Dependenciesall of which are required
+    ],  # Dependencies all of which are required
     deps_either: Collection[
         str | Requirement
-        # TODO: dangerous
     ] = None,  # Dependencies any of which are required
 ) -> DependencyConflict | None:
-    # instruments_conflict = get_dependency_conflicts()
-    # if instruments_conflict:
-    #     return instruments_conflict
-
-    # instruments_either_conflict = get_dependency_conflicts_either()
-    # return instruments_either_conflict
-
     for dep in deps:
-        # TODO: what is this?
         if isinstance(dep, Requirement):
-            print("REQUIREMENT")
             req = dep
         else:
             try:
-                print("NOT REQUIREMENT")
                 req = Requirement(dep)
             except InvalidRequirement as exc:
                 logger.warning(
@@ -161,14 +116,11 @@ def get_dependency_conflicts(
         try:
             dist_version = version(req.name)
         except PackageNotFoundError:
-            # TODO: Technically this field should allow Requirements type. Tackle this in a separate PR.
             return DependencyConflict(dep)
 
         if not req.specifier.contains(dist_version):
-            # TODO: Technically this field should allow Requirements type
             return DependencyConflict(dep, f"{req.name} {dist_version}")
 
-    # TODO: add eval of deps_either
     # If all the dependencies in "instruments" are present, check "instruments_either" for conflicts.
     if deps_either:
         return _get_dependency_conflicts_either(deps_either)
@@ -181,18 +133,14 @@ def _get_dependency_conflicts_either(
 ) -> DependencyConflict | None:
     if not deps_either:
         return None
-    # TODO: change to using DependencyConflict
     is_dependency_conflict = True
     required_either: Collection[str] = []
     found_either: Collection[str] = []
     for dep in deps_either:
-        # TODO: what is this?
         if isinstance(dep, Requirement):
-            print("REQUIREMENT")
             req = dep
         else:
             try:
-                print("NOT REQUIREMENT")
                 req = Requirement(dep)
             except InvalidRequirement as exc:
                 logger.warning(
@@ -205,9 +153,6 @@ def _get_dependency_conflicts_either(
         try:
             dist_version = version(req.name)
         except PackageNotFoundError:
-            # print(f"PackageNotFoundError EITHER: {req.name}")
-            # TODO: anything here?
-            # return DependencyConflict(dep)
             required_either.append(str(dep))
             continue
 
@@ -220,9 +165,6 @@ def _get_dependency_conflicts_either(
         found_either.append(f"{req.name} {dist_version}")
 
     if is_dependency_conflict:
-        # return DependencyConflict(dep, f"{req.name} {dist_version}")
-        # print (f"required_either: {required_either}")
-        # print (f"found_either: {found_either}")
         return DependencyConflict(
             required_either=required_either,
             found_either=found_either,