Make auto instrumentation use the same dependency resolver as manual instrumentation does

Author: rjduffner

CHANGELOG.md

@@ -11,6 +11,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## Unreleased
 
+- Make auto instrumentation use the same dependency resolver as manual instrumentation does
+  ([#3202](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/3202))
+
 ### Added
 
 - `opentelemetry-instrumentation-confluent-kafka` Add support for confluent-kafka <=2.7.0

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

@@ -16,7 +16,7 @@
 
 import unittest
 from timeit import default_timer
-from unittest.mock import Mock, patch
+from unittest.mock import Mock, call, patch
 
 import fastapi
 from fastapi.middleware.httpsredirect import HTTPSRedirectMiddleware
@@ -37,6 +37,10 @@
 from opentelemetry.instrumentation.auto_instrumentation._load import (
     _load_instrumentors,
 )
+from opentelemetry.instrumentation.dependencies import (
+    DependencyConflict,
+    DependencyConflictError,
+)
 from opentelemetry.sdk.metrics.export import (
     HistogramDataPoint,
     NumberDataPoint,
@@ -54,10 +58,7 @@
 from opentelemetry.semconv.trace import SpanAttributes
 from opentelemetry.test.globals_test import reset_trace_globals
 from opentelemetry.test.test_base import TestBase
-from opentelemetry.util._importlib_metadata import (
-    PackageNotFoundError,
-    entry_points,
-)
+from opentelemetry.util._importlib_metadata import entry_points
 from opentelemetry.util.http import (
     OTEL_INSTRUMENTATION_HTTP_CAPTURE_HEADERS_SANITIZE_FIELDS,
     OTEL_INSTRUMENTATION_HTTP_CAPTURE_HEADERS_SERVER_REQUEST,
@@ -1031,26 +1032,6 @@ def client_response_hook(send_span, scope, message):
             )
 
 
-def mock_version_with_fastapi(*args, **kwargs):
-    req_name = args[0]
-    if req_name == "fastapi":
-        # TODO: Value now matters
-        return "0.58"
-    raise PackageNotFoundError()
-
-
-def mock_version_with_old_fastapi(*args, **kwargs):
-    req_name = args[0]
-    if req_name == "fastapi":
-        # TODO: Value now matters
-        return "0.57"
-    raise PackageNotFoundError()
-
-
-def mock_version_without_fastapi(*args, **kwargs):
-    raise PackageNotFoundError()
-
-
 class TestAutoInstrumentation(TestBaseAutoFastAPI):
     """Test the auto-instrumented variant
 
@@ -1062,31 +1043,65 @@ def test_entry_point_exists(self):
         (ep,) = entry_points(group="opentelemetry_instrumentor")
         self.assertEqual(ep.name, "fastapi")
 
-    @patch("opentelemetry.instrumentation.dependencies.version")
-    def test_instruments_with_fastapi_installed(self, mock_version):
-        mock_version.side_effect = mock_version_with_fastapi
+    @staticmethod
+    def _instrumentation_loaded_successfully_call():
+        return call("Instrumented %s", "fastapi")
+
+    @staticmethod
+    def _instrumentation_failed_to_load_call(dependency_conflict):
+        return call(
+            "Skipping instrumentation %s: %s", "fastapi", dependency_conflict
+        )
+
+    @patch("opentelemetry.instrumentation.auto_instrumentation._load._logger")
+    def test_instruments_with_fastapi_installed(self, mock_logger):
         mock_distro = Mock()
+        mock_distro.load_instrumentor.return_value = None
         _load_instrumentors(mock_distro)
-        mock_version.assert_called_once_with("fastapi")
         self.assertEqual(len(mock_distro.load_instrumentor.call_args_list), 1)
         (ep,) = mock_distro.load_instrumentor.call_args.args
         self.assertEqual(ep.name, "fastapi")
+        mock_logger.debug.assert_has_calls(
+            [self._instrumentation_loaded_successfully_call()]
+        )
 
-    @patch("opentelemetry.instrumentation.dependencies.version")
-    def test_instruments_with_old_fastapi_installed(self, mock_version):  # pylint: disable=no-self-use
-        mock_version.side_effect = mock_version_with_old_fastapi
+    @patch("opentelemetry.instrumentation.auto_instrumentation._load._logger")
+    def test_instruments_with_old_fastapi_installed(self, mock_logger):  # pylint: disable=no-self-use
+        dependency_conflict = DependencyConflict("0.58", "0.57")
         mock_distro = Mock()
+        mock_distro.load_instrumentor.side_effect = DependencyConflictError(
+            dependency_conflict
+        )
         _load_instrumentors(mock_distro)
-        mock_version.assert_called_once_with("fastapi")
-        mock_distro.load_instrumentor.assert_not_called()
+        self.assertEqual(len(mock_distro.load_instrumentor.call_args_list), 1)
+        (ep,) = mock_distro.load_instrumentor.call_args.args
+        self.assertEqual(ep.name, "fastapi")
+        assert (
+            self._instrumentation_loaded_successfully_call()
+            not in mock_logger.debug.call_args_list
+        )
+        mock_logger.debug.assert_has_calls(
+            [self._instrumentation_failed_to_load_call(dependency_conflict)]
+        )
 
-    @patch("opentelemetry.instrumentation.dependencies.version")
-    def test_instruments_without_fastapi_installed(self, mock_version):  # pylint: disable=no-self-use
-        mock_version.side_effect = mock_version_without_fastapi
+    @patch("opentelemetry.instrumentation.auto_instrumentation._load._logger")
+    def test_instruments_without_fastapi_installed(self, mock_logger):  # pylint: disable=no-self-use
+        dependency_conflict = DependencyConflict("0.58", None)
         mock_distro = Mock()
+        mock_distro.load_instrumentor.side_effect = DependencyConflictError(
+            dependency_conflict
+        )
         _load_instrumentors(mock_distro)
-        mock_version.assert_called_once_with("fastapi")
-        mock_distro.load_instrumentor.assert_not_called()
+        self.assertEqual(len(mock_distro.load_instrumentor.call_args_list), 1)
+        (ep,) = mock_distro.load_instrumentor.call_args.args
+        self.assertEqual(ep.name, "fastapi")
+        assert (
+            self._instrumentation_loaded_successfully_call()
+            not in mock_logger.debug.call_args_list
+        )
+        mock_logger.debug.assert_has_calls(
+            [self._instrumentation_failed_to_load_call(dependency_conflict)]
+        )
 
     def _create_app(self):
         # instrumentation is handled by the instrument call

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

@@ -12,50 +12,22 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-from functools import cached_property
 from logging import getLogger
 from os import environ
 
-from opentelemetry.instrumentation.dependencies import (
-    get_dist_dependency_conflicts,
-)
+from opentelemetry.instrumentation.dependencies import DependencyConflictError
 from opentelemetry.instrumentation.distro import BaseDistro, DefaultDistro
 from opentelemetry.instrumentation.environment_variables import (
     OTEL_PYTHON_CONFIGURATOR,
     OTEL_PYTHON_DISABLED_INSTRUMENTATIONS,
     OTEL_PYTHON_DISTRO,
 )
 from opentelemetry.instrumentation.version import __version__
-from opentelemetry.util._importlib_metadata import (
-    EntryPoint,
-    distributions,
-    entry_points,
-)
+from opentelemetry.util._importlib_metadata import entry_points
 
 _logger = getLogger(__name__)
 
 
-class _EntryPointDistFinder:
-    @cached_property
-    def _mapping(self):
-        return {
-            self._key_for(ep): dist
-            for dist in distributions()
-            for ep in dist.entry_points
-        }
-
-    def dist_for(self, entry_point: EntryPoint):
-        dist = getattr(entry_point, "dist", None)
-        if dist:
-            return dist
-
-        return self._mapping.get(self._key_for(entry_point))
-
-    @staticmethod
-    def _key_for(entry_point: EntryPoint):
-        return f"{entry_point.group}:{entry_point.name}:{entry_point.value}"
-
-
 def _load_distro() -> BaseDistro:
     distro_name = environ.get(OTEL_PYTHON_DISTRO, None)
     for entry_point in entry_points(group="opentelemetry_distro"):
@@ -83,7 +55,6 @@ def _load_distro() -> BaseDistro:
 
 def _load_instrumentors(distro):
     package_to_exclude = environ.get(OTEL_PYTHON_DISABLED_INSTRUMENTATIONS, [])
-    entry_point_finder = _EntryPointDistFinder()
     if isinstance(package_to_exclude, str):
         package_to_exclude = package_to_exclude.split(",")
         # to handle users entering "requests , flask" or "requests, flask" with spaces
@@ -100,19 +71,17 @@ def _load_instrumentors(distro):
             continue
 
         try:
-            entry_point_dist = entry_point_finder.dist_for(entry_point)
-            conflict = get_dist_dependency_conflicts(entry_point_dist)
-            if conflict:
-                _logger.debug(
-                    "Skipping instrumentation %s: %s",
-                    entry_point.name,
-                    conflict,
-                )
-                continue
-
-            # tell instrumentation to not run dep checks again as we already did it above
-            distro.load_instrumentor(entry_point, skip_dep_check=True)
+            distro.load_instrumentor(
+                entry_point, raise_exception_on_conflict=True
+            )
             _logger.debug("Instrumented %s", entry_point.name)
+        except DependencyConflictError as exc:
+            _logger.debug(
+                "Skipping instrumentation %s: %s",
+                entry_point.name,
+                exc.conflict,
+            )
+            continue
         except ImportError:
             # in scenarios using the kubernetes operator to do autoinstrumentation some
             # instrumentors (usually requiring binary extensions) may fail to load

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

@@ -40,6 +40,14 @@ def __str__(self):
         return f'DependencyConflict: requested: "{self.required}" but found: "{self.found}"'
 
 
+class DependencyConflictError(Exception):
+    def __init__(self, conflict: DependencyConflict):
+        self.conflict = conflict
+
+    def __str__(self):
+        return str(self.conflict)
+
+
 def get_dist_dependency_conflicts(
     dist: Distribution,
 ) -> DependencyConflict | None:

opentelemetry-instrumentation/src/opentelemetry/instrumentation/instrumentor.py

@@ -28,6 +28,7 @@
 )
 from opentelemetry.instrumentation.dependencies import (
     DependencyConflict,
+    DependencyConflictError,
     get_dependency_conflicts,
 )
 
@@ -104,10 +105,15 @@ def instrument(self, **kwargs: Any):
 
         # check if instrumentor has any missing or conflicting dependencies
         skip_dep_check = kwargs.pop("skip_dep_check", False)
+        raise_exception_on_conflict = kwargs.pop(
+            "raise_exception_on_conflict", False
+        )
         if not skip_dep_check:
             conflict = self._check_dependency_conflicts()
             if conflict:
                 _LOG.error(conflict)
+                if raise_exception_on_conflict:
+                    raise DependencyConflictError(conflict)
                 return None
 
         # initialize semantic conventions opt-in if needed