From eef7a1324c4cc399c4930ddfacb04b9e8e34e59f Mon Sep 17 00:00:00 2001
From: Owais Lone <owais@lone.pw>
Date: Thu, 3 Jun 2021 00:08:54 +0530
Subject: [PATCH] Fix auto-instrumentation dependency conflict detection

---
 CHANGELOG.md                                  |  4 +++
 .../instrumentation/dependencies.py           | 33 +++++++++++++++----
 .../tests/test_dependencies.py                | 27 +++++++++++++--
 3 files changed, 55 insertions(+), 9 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 272482b72c..66d75fa655 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -6,6 +6,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased](https://github.com/open-telemetry/opentelemetry-python/compare/v1.3.0-0.22b0...HEAD)
 
+- `opentelemetry-instrumentation` Fixed cases where trying to use an instrumentation package without the
+  target library was crashing auto instrumentation agent.
+  ([#530](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/530))
+
 ## [0.22b0](https://github.com/open-telemetry/opentelemetry-python/releases/tag/v1.3.0-0.22b0) - 2021-06-01
 
 ### Changed
diff --git a/opentelemetry-instrumentation/src/opentelemetry/instrumentation/dependencies.py b/opentelemetry-instrumentation/src/opentelemetry/instrumentation/dependencies.py
index 69cf6b3354..0ae5e02db4 100644
--- a/opentelemetry-instrumentation/src/opentelemetry/instrumentation/dependencies.py
+++ b/opentelemetry-instrumentation/src/opentelemetry/instrumentation/dependencies.py
@@ -1,12 +1,16 @@
+from logging import getLogger
 from typing import Collection, Optional
 
 from pkg_resources import (
     Distribution,
     DistributionNotFound,
+    RequirementParseError,
     VersionConflict,
     get_distribution,
 )
 
+logger = getLogger(__file__)
+
 
 class DependencyConflict:
     required: str = None
@@ -25,12 +29,20 @@ def __str__(self):
 def get_dist_dependency_conflicts(
     dist: Distribution,
 ) -> Optional[DependencyConflict]:
-    deps = [
-        dep
-        for dep in dist.requires(("instruments",))
-        if dep not in dist.requires()
-    ]
-    return get_dependency_conflicts(deps)
+    main_deps = dist.requires()
+    instrumentation_deps = []
+    for dep in dist.requires(("instruments",)):
+        if dep not in main_deps:
+            # we set marker to none so string representation of the dependency looks like
+            #    requests ~= 1.0
+            # instead of
+            #    requests ~= 1.0; extra = "instruments"
+            # which does not work with `get_distribution()`
+            dep.marker = None
+            dep.marker = 'extra = "instruments"'
+            instrumentation_deps.append(str(dep))
+
+    return get_dependency_conflicts(instrumentation_deps)
 
 
 def get_dependency_conflicts(
@@ -38,9 +50,16 @@ def get_dependency_conflicts(
 ) -> Optional[DependencyConflict]:
     for dep in deps:
         try:
-            get_distribution(str(dep))
+            get_distribution(dep)
         except VersionConflict as exc:
             return DependencyConflict(dep, exc.dist)
         except DistributionNotFound:
             return DependencyConflict(dep)
+        except RequirementParseError as exc:
+            logger.warning(
+                'error parsing dependency, reporting as a conflict: "%s" - %s',
+                dep,
+                exc,
+            )
+            return DependencyConflict(dep)
     return None
diff --git a/opentelemetry-instrumentation/tests/test_dependencies.py b/opentelemetry-instrumentation/tests/test_dependencies.py
index 778781ee44..8b2f2e9b39 100644
--- a/opentelemetry-instrumentation/tests/test_dependencies.py
+++ b/opentelemetry-instrumentation/tests/test_dependencies.py
@@ -14,11 +14,13 @@
 
 # pylint: disable=protected-access
 
+import pkg_resources
 import pytest
 
 from opentelemetry.instrumentation.dependencies import (
     DependencyConflict,
     get_dependency_conflicts,
+    get_dist_dependency_conflicts,
 )
 from opentelemetry.test.test_base import TestBase
 
@@ -37,7 +39,6 @@ def test_get_dependency_conflicts_not_installed(self):
         conflict = get_dependency_conflicts(["this-package-does-not-exist"])
         self.assertTrue(conflict is not None)
         self.assertTrue(isinstance(conflict, DependencyConflict))
-        print(conflict)
         self.assertEqual(
             str(conflict),
             'DependencyConflict: requested: "this-package-does-not-exist" but found: "None"',
@@ -47,10 +48,32 @@ def test_get_dependency_conflicts_mismatched_version(self):
         conflict = get_dependency_conflicts(["pytest == 5000"])
         self.assertTrue(conflict is not None)
         self.assertTrue(isinstance(conflict, DependencyConflict))
-        print(conflict)
         self.assertEqual(
             str(conflict),
             'DependencyConflict: requested: "pytest == 5000" but found: "pytest {0}"'.format(
                 pytest.__version__
             ),
         )
+
+    def test_get_dist_dependency_conflicts(self):
+        def mock_requires(extras=()):
+            if "instruments" in extras:
+                return [
+                    pkg_resources.Requirement(
+                        'test-pkg ~= 1.0; extra == "instruments"'
+                    )
+                ]
+            return []
+
+        dist = pkg_resources.Distribution(
+            project_name="test-instrumentation", version="1.0"
+        )
+        dist.requires = mock_requires
+
+        conflict = get_dist_dependency_conflicts(dist)
+        self.assertTrue(conflict is not None)
+        self.assertTrue(isinstance(conflict, DependencyConflict))
+        self.assertEqual(
+            str(conflict),
+            'DependencyConflict: requested: "test-pkg~=1.0" but found: "None"',
+        )