diff --git a/CHANGELOG.md b/CHANGELOG.md index f1193f685b..f04f8647af 100644 --- a/CHANGELOG.md +++ b/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)) diff --git a/instrumentation/opentelemetry-instrumentation-threading/src/opentelemetry/instrumentation/threading/__init__.py b/instrumentation/opentelemetry-instrumentation-threading/src/opentelemetry/instrumentation/threading/__init__.py index 0befa26165..6352197465 100644 --- a/instrumentation/opentelemetry-instrumentation-threading/src/opentelemetry/instrumentation/threading/__init__.py +++ b/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:] diff --git a/instrumentation/opentelemetry-instrumentation-threading/tests/test_threading.py b/instrumentation/opentelemetry-instrumentation-threading/tests/test_threading.py index 15f67b8d61..06fcc54029 100644 --- a/instrumentation/opentelemetry-instrumentation-threading/tests/test_threading.py +++ b/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()