Fix threading instrumentation context types (#3322)

Add None check to context handling in threading instrumentation
Add testcases for None context

Co-authored-by: Emídio Neto <[email protected]>
Co-authored-by: Riccardo Magliocchetti <[email protected]>

Author: 3 people

CHANGELOG.md

@@ -41,6 +41,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
   ([#3247](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/3247))
 - `opentelemetry-instrumentation-asyncpg` Fix fallback for empty queries.
   ([#3253](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/3253))
+- `opentelemetry-instrumentation-threading` Fix broken context typehints
+  ([#3322](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/3322))
 - `opentelemetry-instrumentation-requests` always record span status code in duration metric
   ([#3323](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/3323))
 

instrumentation/opentelemetry-instrumentation-threading/src/opentelemetry/instrumentation/threading/__init__.py

@@ -150,7 +150,8 @@ def __wrap_threading_run(
             token = context.attach(instance._otel_context)
             return call_wrapped(*args, **kwargs)
         finally:
-            context.detach(token)  # type: ignore[reportArgumentType] remove with https://github.com/open-telemetry/opentelemetry-python-contrib/issues/3321
+            if token is not None:
+                context.detach(token)
 
     @staticmethod
     def __wrap_thread_pool_submit(
@@ -169,7 +170,8 @@ def wrapped_func(*func_args: Any, **func_kwargs: Any) -> R:
                 token = context.attach(otel_context)
                 return original_func(*func_args, **func_kwargs)
             finally:
-                context.detach(token)  # type: ignore[reportArgumentType] remove with https://github.com/open-telemetry/opentelemetry-python-contrib/issues/3321
+                if token is not None:
+                    context.detach(token)
 
         # replace the original function with the wrapped function
         new_args: tuple[Callable[..., Any], ...] = (wrapped_func,) + args[1:]

instrumentation/opentelemetry-instrumentation-threading/tests/test_threading.py

@@ -15,12 +15,14 @@
 import threading
 from concurrent.futures import ThreadPoolExecutor
 from typing import List
+from unittest.mock import MagicMock, patch
 
 from opentelemetry import trace
 from opentelemetry.instrumentation.threading import ThreadingInstrumentor
 from opentelemetry.test.test_base import TestBase
 
 
+# pylint: disable=too-many-public-methods
 class TestThreading(TestBase):
     def setUp(self):
         super().setUp()
@@ -224,3 +226,67 @@ def test_uninstrumented(self):
         self.assertEqual(len(spans), 1)
 
         ThreadingInstrumentor().instrument()
+
+    @patch(
+        "opentelemetry.context.attach",
+        new=MagicMock(return_value=None),
+    )
+    @patch(
+        "opentelemetry.context.detach",
+        autospec=True,
+    )
+    def test_threading_with_none_context_token(self, mock_detach: MagicMock):
+        with self.get_root_span():
+            thread = threading.Thread(target=self.fake_func)
+            thread.start()
+            thread.join()
+            mock_detach.assert_not_called()
+
+    @patch(
+        "opentelemetry.context._RUNTIME_CONTEXT.attach",
+        new=MagicMock(return_value=MagicMock()),
+    )
+    @patch(
+        "opentelemetry.context._RUNTIME_CONTEXT.detach",
+        new=MagicMock(return_value=None),
+    )
+    @patch("opentelemetry.context.detach", autospec=True)
+    def test_threading_with_valid_context_token(self, mock_detach: MagicMock):
+        with self.get_root_span():
+            thread = threading.Thread(target=self.fake_func)
+            thread.start()
+            thread.join()
+            mock_detach.assert_called_once()
+
+    @patch(
+        "opentelemetry.context.attach",
+        new=MagicMock(return_value=None),
+    )
+    @patch(
+        "opentelemetry.context.detach",
+        autospec=True,
+    )
+    def test_thread_pool_with_none_context_token(self, mock_detach: MagicMock):
+        with self.get_root_span(), ThreadPoolExecutor(
+            max_workers=1
+        ) as executor:
+            future = executor.submit(self.get_current_span_context_for_test)
+            future.result()
+            mock_detach.assert_not_called()
+
+    @patch(
+        "opentelemetry.context._RUNTIME_CONTEXT.attach",
+        new=MagicMock(return_value=MagicMock()),
+    )
+    @patch(
+        "opentelemetry.context._RUNTIME_CONTEXT.detach",
+        new=MagicMock(return_value=None),
+    )
+    @patch("opentelemetry.context.detach", autospec=True)
+    def test_threadpool_with_valid_context_token(self, mock_detach: MagicMock):
+        with self.get_root_span(), ThreadPoolExecutor(
+            max_workers=1
+        ) as executor:
+            future = executor.submit(self.get_current_span_context_for_test)
+            future.result()
+            mock_detach.assert_called_once()