From ba2c0cfc4deaac505f39cce450119e617b3362fe Mon Sep 17 00:00:00 2001 From: Riccardo Magliocchetti Date: Fri, 20 Sep 2024 16:55:01 +0200 Subject: [PATCH 1/5] opentelemetry-instrumentation: make it easier to use bootstrap with custom values Pass the available instrumentation libraries and the default libraries from bootstrap_gen to run() (and down to all the functions) instead of relying on the globals. This way it's easier to reuse run with custom values. While at it fix mock handling in tests. --- .../instrumentation/bootstrap.py | 22 ++++++--- .../tests/test_bootstrap.py | 49 +++++++++++++++---- 2 files changed, 53 insertions(+), 18 deletions(-) diff --git a/opentelemetry-instrumentation/src/opentelemetry/instrumentation/bootstrap.py b/opentelemetry-instrumentation/src/opentelemetry/instrumentation/bootstrap.py index 02926ea5c4..8b4bec1c27 100644 --- a/opentelemetry-instrumentation/src/opentelemetry/instrumentation/bootstrap.py +++ b/opentelemetry-instrumentation/src/opentelemetry/instrumentation/bootstrap.py @@ -75,7 +75,7 @@ def _sys_pip_install(package): print(error) -def _pip_check(): +def _pip_check(libraries): """Ensures none of the instrumentations have dependency conflicts. Clean check reported as: 'No broken requirements found.' @@ -113,7 +113,7 @@ def _is_installed(req): return True -def _find_installed_libraries(): +def _find_installed_libraries(default_instrumentations, libraries): for lib in default_instrumentations: yield lib @@ -122,18 +122,24 @@ def _find_installed_libraries(): yield lib["instrumentation"] -def _run_requirements(): +def _run_requirements(default_instrumentations, libraries): logger.setLevel(logging.ERROR) - print("\n".join(_find_installed_libraries())) + print( + "\n".join( + _find_installed_libraries(default_instrumentations, libraries) + ) + ) -def _run_install(): - for lib in _find_installed_libraries(): +def _run_install(default_instrumentations, libraries): + for lib in _find_installed_libraries(default_instrumentations, libraries): _sys_pip_install(lib) _pip_check() -def run() -> None: +def run( + default_instrumentations=default_instrumentations, libraries=libraries +) -> None: action_install = "install" action_requirements = "requirements" @@ -167,4 +173,4 @@ def run() -> None: action_install: _run_install, action_requirements: _run_requirements, }[args.action] - cmd() + cmd(default_instrumentations, libraries) diff --git a/opentelemetry-instrumentation/tests/test_bootstrap.py b/opentelemetry-instrumentation/tests/test_bootstrap.py index 4807f0beb7..ab00effe7c 100644 --- a/opentelemetry-instrumentation/tests/test_bootstrap.py +++ b/opentelemetry-instrumentation/tests/test_bootstrap.py @@ -19,7 +19,10 @@ from unittest.mock import call, patch from opentelemetry.instrumentation import bootstrap -from opentelemetry.instrumentation.bootstrap_gen import libraries +from opentelemetry.instrumentation.bootstrap_gen import ( + default_instrumentations, + libraries, +) def sample_packages(packages, rate): @@ -56,15 +59,17 @@ def setUpClass(cls): "opentelemetry.instrumentation.bootstrap._pip_check", ) - cls.pkg_patcher.start() - cls.mock_pip_install = cls.pip_install_patcher.start() - cls.mock_pip_check = cls.pip_check_patcher.start() + def setUp(self): + super().setUp() + self.mock_pip_check = self.pip_check_patcher.start() + self.mock_pip_install = self.pip_install_patcher.start() + self.pkg_patcher.start() - @classmethod - def tearDownClass(cls): - cls.pip_check_patcher.start() - cls.pip_install_patcher.start() - cls.pkg_patcher.stop() + def tearDown(self): + super().tearDown() + self.pip_check_patcher.stop() + self.pip_install_patcher.stop() + self.pkg_patcher.stop() @patch("sys.argv", ["bootstrap", "-a", "pipenv"]) def test_run_unknown_cmd(self): @@ -87,4 +92,28 @@ def test_run_cmd_install(self): [call(i) for i in self.installed_libraries], any_order=True, ) - self.assertEqual(self.mock_pip_check.call_count, 1) + self.mock_pip_check.assert_called_once() + + @patch("sys.argv", ["bootstrap", "-a", "install"]) + def test_can_override_available_libraries(self): + self.pkg_patcher.stop() + bootstrap.run(libraries=[]) + self.mock_pip_install.assert_has_calls( + [call(i) for i in default_instrumentations], + any_order=True, + ) + self.mock_pip_check.assert_called_once() + + @patch("sys.argv", ["bootstrap", "-a", "install"]) + def test_can_override_available_default_instrumentations(self): + self.pkg_patcher.stop() + with patch( + "opentelemetry.instrumentation.bootstrap._is_installed", + return_value=True, + ): + bootstrap.run(default_instrumentations=[]) + self.mock_pip_install.assert_has_calls( + [call(i) for i in self.installed_libraries], + any_order=True, + ) + self.mock_pip_check.assert_called_once() From 62acd1df6dd5ef2226fc56aaf5f8333ff8d79233 Mon Sep 17 00:00:00 2001 From: Riccardo Magliocchetti Date: Mon, 21 Oct 2024 12:12:13 +0200 Subject: [PATCH 2/5] Fixup pylint warnings --- .../src/opentelemetry/instrumentation/bootstrap.py | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/opentelemetry-instrumentation/src/opentelemetry/instrumentation/bootstrap.py b/opentelemetry-instrumentation/src/opentelemetry/instrumentation/bootstrap.py index 8b4bec1c27..4d10cfc4fb 100644 --- a/opentelemetry-instrumentation/src/opentelemetry/instrumentation/bootstrap.py +++ b/opentelemetry-instrumentation/src/opentelemetry/instrumentation/bootstrap.py @@ -26,8 +26,10 @@ from packaging.requirements import Requirement from opentelemetry.instrumentation.bootstrap_gen import ( - default_instrumentations, - libraries, + default_instrumentations as gen_default_instrumentations, +) +from opentelemetry.instrumentation.bootstrap_gen import ( + libraries as gen_libraries, ) from opentelemetry.instrumentation.version import __version__ from opentelemetry.util._importlib_metadata import ( @@ -138,7 +140,8 @@ def _run_install(default_instrumentations, libraries): def run( - default_instrumentations=default_instrumentations, libraries=libraries + default_instrumentations=gen_default_instrumentations, + libraries=gen_libraries, ) -> None: action_install = "install" action_requirements = "requirements" From 058e03282a5ca6e5be9a7dce526795597b97bbd3 Mon Sep 17 00:00:00 2001 From: Riccardo Magliocchetti Date: Mon, 21 Oct 2024 12:14:00 +0200 Subject: [PATCH 3/5] More pylint fixes --- .../src/opentelemetry/instrumentation/bootstrap.py | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/opentelemetry-instrumentation/src/opentelemetry/instrumentation/bootstrap.py b/opentelemetry-instrumentation/src/opentelemetry/instrumentation/bootstrap.py index 4d10cfc4fb..18e7ef5146 100644 --- a/opentelemetry-instrumentation/src/opentelemetry/instrumentation/bootstrap.py +++ b/opentelemetry-instrumentation/src/opentelemetry/instrumentation/bootstrap.py @@ -22,6 +22,7 @@ SubprocessError, check_call, ) +from typing import Optional from packaging.requirements import Requirement @@ -140,8 +141,8 @@ def _run_install(default_instrumentations, libraries): def run( - default_instrumentations=gen_default_instrumentations, - libraries=gen_libraries, + default_instrumentations: Optional[list] = None, + libraries: Optional[list] = None, ) -> None: action_install = "install" action_requirements = "requirements" @@ -172,6 +173,12 @@ def run( ) args = parser.parse_args() + if libraries is None: + libraries = gen_libraries + + if default_instrumentations is None: + default_instrumentations = gen_default_instrumentations + cmd = { action_install: _run_install, action_requirements: _run_requirements, From 3ba64a4e90b740fc621c78433dc5c3d8f0b7414f Mon Sep 17 00:00:00 2001 From: Riccardo Magliocchetti Date: Mon, 21 Oct 2024 15:52:53 +0200 Subject: [PATCH 4/5] Add missing parameter for _pip_check --- .../src/opentelemetry/instrumentation/bootstrap.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/opentelemetry-instrumentation/src/opentelemetry/instrumentation/bootstrap.py b/opentelemetry-instrumentation/src/opentelemetry/instrumentation/bootstrap.py index 18e7ef5146..cc0ac68f1c 100644 --- a/opentelemetry-instrumentation/src/opentelemetry/instrumentation/bootstrap.py +++ b/opentelemetry-instrumentation/src/opentelemetry/instrumentation/bootstrap.py @@ -137,7 +137,7 @@ def _run_requirements(default_instrumentations, libraries): def _run_install(default_instrumentations, libraries): for lib in _find_installed_libraries(default_instrumentations, libraries): _sys_pip_install(lib) - _pip_check() + _pip_check(libraries) def run( From c25890d302e83f160699e9626f56b6dd13e94bb4 Mon Sep 17 00:00:00 2001 From: Riccardo Magliocchetti Date: Thu, 24 Oct 2024 14:24:58 +0200 Subject: [PATCH 5/5] call pkg_patcher only when it's needed --- opentelemetry-instrumentation/tests/test_bootstrap.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/opentelemetry-instrumentation/tests/test_bootstrap.py b/opentelemetry-instrumentation/tests/test_bootstrap.py index ab00effe7c..0ded8d37b1 100644 --- a/opentelemetry-instrumentation/tests/test_bootstrap.py +++ b/opentelemetry-instrumentation/tests/test_bootstrap.py @@ -63,13 +63,11 @@ def setUp(self): super().setUp() self.mock_pip_check = self.pip_check_patcher.start() self.mock_pip_install = self.pip_install_patcher.start() - self.pkg_patcher.start() def tearDown(self): super().tearDown() self.pip_check_patcher.stop() self.pip_install_patcher.stop() - self.pkg_patcher.stop() @patch("sys.argv", ["bootstrap", "-a", "pipenv"]) def test_run_unknown_cmd(self): @@ -78,25 +76,28 @@ def test_run_unknown_cmd(self): @patch("sys.argv", ["bootstrap", "-a", "requirements"]) def test_run_cmd_print(self): + self.pkg_patcher.start() with patch("sys.stdout", new=StringIO()) as fake_out: bootstrap.run() self.assertEqual( fake_out.getvalue(), "\n".join(self.installed_libraries) + "\n", ) + self.pkg_patcher.stop() @patch("sys.argv", ["bootstrap", "-a", "install"]) def test_run_cmd_install(self): + self.pkg_patcher.start() bootstrap.run() self.mock_pip_install.assert_has_calls( [call(i) for i in self.installed_libraries], any_order=True, ) self.mock_pip_check.assert_called_once() + self.pkg_patcher.stop() @patch("sys.argv", ["bootstrap", "-a", "install"]) def test_can_override_available_libraries(self): - self.pkg_patcher.stop() bootstrap.run(libraries=[]) self.mock_pip_install.assert_has_calls( [call(i) for i in default_instrumentations], @@ -106,7 +107,6 @@ def test_can_override_available_libraries(self): @patch("sys.argv", ["bootstrap", "-a", "install"]) def test_can_override_available_default_instrumentations(self): - self.pkg_patcher.stop() with patch( "opentelemetry.instrumentation.bootstrap._is_installed", return_value=True,