fixed either logic. new tests pass

Author: jeremydvoss

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

@@ -147,7 +147,7 @@ def get_dependency_conflicts(
     # TODO: add eval of deps_either
     if deps_either:
         # TODO: change to using DependencyConflict
-        # is_dependency_conflict = True
+        is_dependency_conflict = True
         required_either: Collection[str] = []
         found_either: Collection[str] = []
         for dep in deps_either:
@@ -177,19 +177,21 @@ def get_dependency_conflicts(
                 continue
 
             if req.specifier.contains(dist_version):
-                # is_dependency_conflict = False
+                is_dependency_conflict = False
                 # Since only one of the instrumentation_either dependencies is required, there is no depdendency conflict.
                 break
             else:
                 required_either.append(str(dep))
                 found_either.append(f"{req.name} {dist_version}")
 
-        # 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,
-        )
+        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,
+            )
+        return None
 
     return None

opentelemetry-instrumentation/tests/auto_instrumentation/test_load.py

@@ -28,7 +28,7 @@
 )
 from opentelemetry.instrumentation.version import __version__
 
-
+# TODO: Revert changes from 6d5a5149d02fdec50d8053767b55bc91d80a9228
 class TestLoad(TestCase):
     @patch.dict(
         "os.environ", {OTEL_PYTHON_CONFIGURATOR: "custom_configurator2"}

opentelemetry-instrumentation/tests/test_dependencies.py

@@ -16,14 +16,18 @@
 
 import pytest
 from packaging.requirements import Requirement
+from unittest.mock import patch
 
 from opentelemetry.instrumentation.dependencies import (
     DependencyConflict,
     get_dependency_conflicts,
     get_dist_dependency_conflicts,
 )
 from opentelemetry.test.test_base import TestBase
-from opentelemetry.util._importlib_metadata import Distribution
+from opentelemetry.util._importlib_metadata import (
+    Distribution,
+    PackageNotFoundError,
+)
 
 
 class TestDependencyConflicts(TestBase):
@@ -97,8 +101,69 @@ def read_text(self, filename):
 
             @property
             def requires(self):
+                # TODO: make another test for returning something with a blank list for both and and or
                 return None
 
         dist = MockDistribution()
         conflict = get_dist_dependency_conflicts(dist)
         self.assertTrue(conflict is None)
+
+    @patch("opentelemetry.instrumentation.dependencies.version")
+    def test_get_dist_dependency_conflicts_either(self, version_mock):
+        class MockDistribution(Distribution):
+            def locate_file(self, path):
+                pass
+
+            def read_text(self, filename):
+                pass
+
+            @property
+            def requires(self):
+                return [
+                    'foo ~= 1.0; extra == "instruments_either"',
+                    'bar ~= 1.0; extra == "instruments_either"'
+                ]
+
+        dist = MockDistribution()
+        
+        def version_side_effect(package_name):
+            if package_name == "foo":
+                raise PackageNotFoundError("foo not found")
+            elif package_name == "bar":
+                return "1.0.0"
+            else:
+                raise PackageNotFoundError(f"{package_name} not found")
+        
+        version_mock.side_effect = version_side_effect
+        conflict = get_dist_dependency_conflicts(dist)
+        print("CONFLICT", conflict)
+        self.assertIsNone(conflict)
+
+    @patch("opentelemetry.instrumentation.dependencies.version")
+    def test_get_dist_dependency_conflicts_neither(self, version_mock):
+        class MockDistribution(Distribution):
+            def locate_file(self, path):
+                pass
+
+            def read_text(self, filename):
+                pass
+
+            @property
+            def requires(self):
+                return [
+                    'foo ~= 1.0; extra == "instruments_either"',
+                    'bar ~= 1.0; extra == "instruments_either"'
+                ]
+
+        dist = MockDistribution()
+        # version_mock.side_effect = lambda x: "1.0.0" if x == "foo" else "2.0.0"
+        # version_mock("foo").return_value = "2.0.0"
+        version_mock.side_effect = PackageNotFoundError("not found")
+        conflict = get_dist_dependency_conflicts(dist)
+        self.assertTrue(conflict is not None)
+        self.assertTrue(isinstance(conflict, DependencyConflict))
+        self.assertEqual(
+            str(conflict),
+            # TODO: fix tests of either conflict
+            '''DependencyConflict: requested any of the following: "['foo~=1.0; extra == "instruments-either"', 'bar~=1.0; extra == "instruments-either"']" but found: "[]"''',
+        )