Added comments to tests

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,15 +68,15 @@ 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
 
         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

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,23 +250,28 @@ 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),
                 call(ep_mock4, skip_dep_check=True),
             ]
         )
         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(