From f9d48edc129793f27fde22755a0904070fa57c12 Mon Sep 17 00:00:00 2001 From: jeremydvoss Date: Wed, 7 Jun 2023 11:12:18 -0700 Subject: [PATCH] Added comments to tests --- .../auto_instrumentation/_load.py | 23 +++++++++-------- .../tests/auto_instrumentation/test_load.py | 25 ++++++++++++++----- 2 files changed, 31 insertions(+), 17 deletions(-) diff --git a/opentelemetry-instrumentation/src/opentelemetry/instrumentation/auto_instrumentation/_load.py b/opentelemetry-instrumentation/src/opentelemetry/instrumentation/auto_instrumentation/_load.py index c7a8da04c7..9dbaa8877b 100644 --- a/opentelemetry-instrumentation/src/opentelemetry/instrumentation/auto_instrumentation/_load.py +++ b/opentelemetry-instrumentation/src/opentelemetry/instrumentation/auto_instrumentation/_load.py @@ -28,27 +28,28 @@ ) from opentelemetry.instrumentation.version import __version__ -logger = getLogger(__name__) +_logger = getLogger(__name__) def _load_distros() -> BaseDistro: distro_name = environ.get(OTEL_PYTHON_DISTRO, None) for entry_point in iter_entry_points("opentelemetry_distro"): try: + # If no distro is specified, use first to come up. if distro_name is None or distro_name == entry_point.name: distro = entry_point.load()() if not isinstance(distro, BaseDistro): - logger.debug( + _logger.debug( "%s is not an OpenTelemetry Distro. Skipping", entry_point.name, ) continue - logger.debug( + _logger.debug( "Distribution %s will be configured", entry_point.name ) return distro except Exception as exc: # pylint: disable=broad-except - logger.exception( + _logger.exception( "Distribution %s configuration failed", entry_point.name ) raise exc @@ -67,7 +68,7 @@ def _load_instrumentors(distro): for entry_point in iter_entry_points("opentelemetry_instrumentor"): if entry_point.name in package_to_exclude: - logger.debug( + _logger.debug( "Instrumentation skipped for library %s", entry_point.name ) continue @@ -75,7 +76,7 @@ def _load_instrumentors(distro): try: conflict = get_dist_dependency_conflicts(entry_point.dist) if conflict: - logger.debug( + _logger.debug( "Skipping instrumentation %s: %s", entry_point.name, conflict, @@ -84,9 +85,9 @@ def _load_instrumentors(distro): # tell instrumentation to not run dep checks again as we already did it above distro.load_instrumentor(entry_point, skip_dep_check=True) - logger.debug("Instrumented %s", entry_point.name) + _logger.debug("Instrumented %s", entry_point.name) except Exception as exc: # pylint: disable=broad-except - logger.exception("Instrumenting of %s failed", entry_point.name) + _logger.exception("Instrumenting of %s failed", entry_point.name) raise exc for entry_point in iter_entry_points("opentelemetry_post_instrument"): @@ -98,7 +99,7 @@ def _load_configurators(): configured = None for entry_point in iter_entry_points("opentelemetry_configurator"): if configured is not None: - logger.warning( + _logger.warning( "Configuration of %s not loaded, %s already loaded", entry_point.name, configured, @@ -112,12 +113,12 @@ def _load_configurators(): entry_point.load()().configure(auto_instrumentation_version=__version__) # type: ignore configured = entry_point.name else: - logger.warning( + _logger.warning( "Configuration of %s not loaded because %s is set by %s", entry_point.name, configurator_name, OTEL_PYTHON_CONFIGURATOR, ) except Exception as exc: # pylint: disable=broad-except - logger.exception("Configuration of %s failed", entry_point.name) + _logger.exception("Configuration of %s failed", entry_point.name) raise exc diff --git a/opentelemetry-instrumentation/tests/auto_instrumentation/test_load.py b/opentelemetry-instrumentation/tests/auto_instrumentation/test_load.py index 6dfeb219f0..02b8d33f6b 100644 --- a/opentelemetry-instrumentation/tests/auto_instrumentation/test_load.py +++ b/opentelemetry-instrumentation/tests/auto_instrumentation/test_load.py @@ -33,6 +33,7 @@ class TestLoad(TestCase): "opentelemetry.instrumentation.auto_instrumentation._load.iter_entry_points" ) def test_load_configurators(self, iter_mock): + # Add multiple entry points but only specify the 2nd in the environment variable. ep_mock1 = Mock() ep_mock1.name = "custom_configurator1" configurator_mock1 = Mock() @@ -65,6 +66,7 @@ def test_load_configurators_no_ep( iter_mock, ): iter_mock.return_value = () + # Confirm method does not crash if not entry points exist. _load._load_configurators() @patch.dict( @@ -74,6 +76,7 @@ def test_load_configurators_no_ep( "opentelemetry.instrumentation.auto_instrumentation._load.iter_entry_points" ) def test_load_configurators_error(self, iter_mock): + # Add multiple entry points but only specify the 2nd in the environment variable. ep_mock1 = Mock() ep_mock1.name = "custom_configurator1" configurator_mock1 = Mock() @@ -89,6 +92,7 @@ def test_load_configurators_error(self, iter_mock): ep_mock3.load.return_value = configurator_mock3 iter_mock.return_value = (ep_mock1, ep_mock2, ep_mock3) + # Confirm failed configuration raises exception. self.assertRaises(Exception, _load._load_configurators) @patch.dict("os.environ", {OTEL_PYTHON_DISTRO: "custom_distro2"}) @@ -99,6 +103,7 @@ def test_load_configurators_error(self, iter_mock): "opentelemetry.instrumentation.auto_instrumentation._load.iter_entry_points" ) def test_load_distros(self, iter_mock, isinstance_mock): + # Add multiple entry points but only specify the 2nd in the environment variable. ep_mock1 = Mock() ep_mock1.name = "custom_distro1" distro_mock1 = Mock() @@ -113,6 +118,7 @@ def test_load_distros(self, iter_mock, isinstance_mock): ep_mock3.load.return_value = distro_mock3 iter_mock.return_value = (ep_mock1, ep_mock2, ep_mock3) + # Mock entry points to be instances of BaseDistro. isinstance_mock.return_value = True self.assertEqual( _load._load_distros(), @@ -132,6 +138,7 @@ def test_load_distros(self, iter_mock, isinstance_mock): def test_load_distros_not_distro( self, iter_mock, default_distro_mock, isinstance_mock ): + # Add multiple entry points but only specify the 2nd in the environment variable. ep_mock1 = Mock() ep_mock1.name = "custom_distro1" distro_mock1 = Mock() @@ -146,6 +153,7 @@ def test_load_distros_not_distro( ep_mock3.load.return_value = distro_mock3 iter_mock.return_value = (ep_mock1, ep_mock2, ep_mock3) + # Confirm default distro is used if specified entry point is not a BaseDistro isinstance_mock.return_value = False self.assertEqual( _load._load_distros(), @@ -161,6 +169,7 @@ def test_load_distros_not_distro( ) def test_load_distros_no_ep(self, iter_mock, default_distro_mock): iter_mock.return_value = () + # Confirm default distro is used if there are no entry points. self.assertEqual( _load._load_distros(), default_distro_mock(), @@ -190,6 +199,7 @@ def test_load_distros_error(self, iter_mock, isinstance_mock): iter_mock.return_value = (ep_mock1, ep_mock2, ep_mock3) isinstance_mock.return_value = True + # Confirm method raises exception if it fails to load a distro. self.assertRaises(Exception, _load._load_distros) @patch.dict( @@ -203,6 +213,7 @@ def test_load_distros_error(self, iter_mock, isinstance_mock): "opentelemetry.instrumentation.auto_instrumentation._load.iter_entry_points" ) def test_load_instrumentors(self, iter_mock, dep_mock): + # Mock opentelemetry_pre_instrument entry points pre_ep_mock1 = Mock() pre_ep_mock1.name = "pre1" pre_mock1 = Mock() @@ -213,12 +224,7 @@ def test_load_instrumentors(self, iter_mock, dep_mock): pre_mock2 = Mock() pre_ep_mock2.load.return_value = pre_mock2 - ep_mock3 = Mock() - ep_mock3.name = "instr3" - - ep_mock4 = Mock() - ep_mock4.name = "instr4" - + # Mock opentelemetry_instrumentor entry points ep_mock1 = Mock() ep_mock1.name = "instr1" @@ -231,6 +237,7 @@ def test_load_instrumentors(self, iter_mock, dep_mock): ep_mock4 = Mock() ep_mock4.name = "instr4" + # Mock opentelemetry_instrumentor entry points post_ep_mock1 = Mock() post_ep_mock1.name = "post1" post_mock1 = Mock() @@ -243,16 +250,20 @@ def test_load_instrumentors(self, iter_mock, dep_mock): distro_mock = Mock() + # Mock entry points in order iter_mock.side_effect = [ (pre_ep_mock1, pre_ep_mock2), (ep_mock1, ep_mock2, ep_mock3, ep_mock4), (post_ep_mock1, post_ep_mock2), ] + # No dependency conflict dep_mock.return_value = None _load._load_instrumentors(distro_mock) + # All opentelemetry_pre_instrument entry points should be loaded pre_mock1.assert_called_once() pre_mock2.assert_called_once() self.assertEqual(iter_mock.call_count, 3) + # Only non-disabled instrumentations should be loaded distro_mock.load_instrumentor.assert_has_calls( [ call(ep_mock2, skip_dep_check=True), @@ -260,6 +271,7 @@ def test_load_instrumentors(self, iter_mock, dep_mock): ] ) self.assertEqual(distro_mock.load_instrumentor.call_count, 2) + # All opentelemetry_post_instrument entry points should be loaded post_mock1.assert_called_once() post_mock2.assert_called_once() @@ -289,6 +301,7 @@ def test_load_instrumentors_dep_conflict(self, iter_mock, dep_mock): distro_mock = Mock() iter_mock.return_value = (ep_mock1, ep_mock2, ep_mock3, ep_mock4) + # If a dependency conflict is raised, that instrumentation should not be loaded, but others still should. dep_mock.side_effect = [None, "DependencyConflict"] _load._load_instrumentors(distro_mock) distro_mock.load_instrumentor.assert_has_calls(