From 8582da5b8decd99f3780e820b5652d4c72b7a953 Mon Sep 17 00:00:00 2001
From: Riccardo Magliocchetti <riccardo.magliocchetti@gmail.com>
Date: Fri, 25 Oct 2024 21:25:51 +0200
Subject: [PATCH] opentelemetry-instrumentation: don't fail if an instrumentor
 raises ImportError (#2923)

---
 CHANGELOG.md                                  |  2 +
 .../auto_instrumentation/_load.py             | 11 ++++
 .../tests/auto_instrumentation/test_load.py   | 64 +++++++++++++++++++
 3 files changed, 77 insertions(+)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index f3682d4a70..867f30afbb 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -37,6 +37,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 - Deprecation of pkg_resource in favor of importlib.metadata
   ([#2871](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2871))
+- `opentelemetry-instrumentation` Don't fail distro loading if instrumentor raises ImportError, instead skip them
+  ([#2923](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2923))
 
 ## Version 1.27.0/0.48b0 (2024-08-28)
 
diff --git a/opentelemetry-instrumentation/src/opentelemetry/instrumentation/auto_instrumentation/_load.py b/opentelemetry-instrumentation/src/opentelemetry/instrumentation/auto_instrumentation/_load.py
index acc81c701c..3d602b2a1d 100644
--- a/opentelemetry-instrumentation/src/opentelemetry/instrumentation/auto_instrumentation/_load.py
+++ b/opentelemetry-instrumentation/src/opentelemetry/instrumentation/auto_instrumentation/_load.py
@@ -113,6 +113,17 @@ 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)
+        except ImportError:
+            # in scenarios using the kubernetes operator to do autoinstrumentation some
+            # instrumentors (usually requiring binary extensions) may fail to load
+            # because the injected autoinstrumentation code does not match the application
+            # environment regarding python version, libc, etc... In this case it's better
+            # to skip the single instrumentation rather than failing to load everything
+            # so treat differently ImportError than the rest of exceptions
+            _logger.exception(
+                "Importing of %s failed, skipping it", entry_point.name
+            )
+            continue
         except Exception as exc:  # pylint: disable=broad-except
             _logger.exception("Instrumenting 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 2d8538b5b3..ce9abe1365 100644
--- a/opentelemetry-instrumentation/tests/auto_instrumentation/test_load.py
+++ b/opentelemetry-instrumentation/tests/auto_instrumentation/test_load.py
@@ -310,6 +310,70 @@ def test_load_instrumentors_dep_conflict(self, iter_mock, dep_mock):  # pylint:
         )
         distro_mock.load_instrumentor.assert_called_once()
 
+    @patch(
+        "opentelemetry.instrumentation.auto_instrumentation._load.get_dist_dependency_conflicts"
+    )
+    @patch(
+        "opentelemetry.instrumentation.auto_instrumentation._load.entry_points"
+    )
+    def test_load_instrumentors_import_error_does_not_stop_everything(
+        self, iter_mock, dep_mock
+    ):
+        ep_mock1 = Mock(name="instr1")
+        ep_mock2 = Mock(name="instr2")
+
+        distro_mock = Mock()
+        distro_mock.load_instrumentor.side_effect = [ImportError, None]
+
+        # Mock entry points in order
+        iter_mock.side_effect = [
+            (),
+            (ep_mock1, ep_mock2),
+            (),
+        ]
+        dep_mock.return_value = None
+
+        _load._load_instrumentors(distro_mock)
+
+        distro_mock.load_instrumentor.assert_has_calls(
+            [
+                call(ep_mock1, skip_dep_check=True),
+                call(ep_mock2, skip_dep_check=True),
+            ]
+        )
+        self.assertEqual(distro_mock.load_instrumentor.call_count, 2)
+
+    @patch(
+        "opentelemetry.instrumentation.auto_instrumentation._load.get_dist_dependency_conflicts"
+    )
+    @patch(
+        "opentelemetry.instrumentation.auto_instrumentation._load.entry_points"
+    )
+    def test_load_instrumentors_raises_exception(self, iter_mock, dep_mock):
+        ep_mock1 = Mock(name="instr1")
+        ep_mock2 = Mock(name="instr2")
+
+        distro_mock = Mock()
+        distro_mock.load_instrumentor.side_effect = [ValueError, None]
+
+        # Mock entry points in order
+        iter_mock.side_effect = [
+            (),
+            (ep_mock1, ep_mock2),
+            (),
+        ]
+        dep_mock.return_value = None
+
+        with self.assertRaises(ValueError):
+            _load._load_instrumentors(distro_mock)
+
+        distro_mock.load_instrumentor.assert_has_calls(
+            [
+                call(ep_mock1, skip_dep_check=True),
+            ]
+        )
+        self.assertEqual(distro_mock.load_instrumentor.call_count, 1)
+
     def test_load_instrumentors_no_entry_point_mocks(self):
         distro_mock = Mock()
         _load._load_instrumentors(distro_mock)