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"', + )