Skip to content

Commit

Permalink
Fix auto-instrumentation dependency conflict detection (#530)
Browse files Browse the repository at this point in the history
  • Loading branch information
owais authored Jun 4, 2021
1 parent 9c834f0 commit 5b125b1
Show file tree
Hide file tree
Showing 3 changed files with 54 additions and 9 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -25,22 +29,36 @@ 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
instrumentation_deps.append(str(dep))

return get_dependency_conflicts(instrumentation_deps)


def get_dependency_conflicts(
deps: Collection[str],
) -> 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
27 changes: 25 additions & 2 deletions opentelemetry-instrumentation/tests/test_dependencies.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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"',
Expand All @@ -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"',
)

0 comments on commit 5b125b1

Please sign in to comment.