Skip to content

Commit 257c8dc

Browse files
committed
Fix threading instrumentation context types
Add None check to context handling in threading instrumentation Add testcases for None context
1 parent c6c0162 commit 257c8dc

File tree

3 files changed

+72
-2
lines changed

3 files changed

+72
-2
lines changed

CHANGELOG.md

+2
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
3939
([#3247](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/3247))
4040
- `opentelemetry-instrumentation-asyncpg` Fix fallback for empty queries.
4141
([#3253](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/3253))
42+
- `opentelemetry-instrumentation-threading` Fix broken context typehints
43+
([#3322](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/3322))
4244

4345
## Version 1.30.0/0.51b0 (2025-02-03)
4446

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

+4-2
Original file line numberDiff line numberDiff line change
@@ -150,7 +150,8 @@ def __wrap_threading_run(
150150
token = context.attach(instance._otel_context)
151151
return call_wrapped(*args, **kwargs)
152152
finally:
153-
context.detach(token) # type: ignore[reportArgumentType] remove with https://github.com/open-telemetry/opentelemetry-python-contrib/issues/3321
153+
if token is not None:
154+
context.detach(token)
154155

155156
@staticmethod
156157
def __wrap_thread_pool_submit(
@@ -169,7 +170,8 @@ def wrapped_func(*func_args: Any, **func_kwargs: Any) -> R:
169170
token = context.attach(otel_context)
170171
return original_func(*func_args, **func_kwargs)
171172
finally:
172-
context.detach(token) # type: ignore[reportArgumentType] remove with https://github.com/open-telemetry/opentelemetry-python-contrib/issues/3321
173+
if token is not None:
174+
context.detach(token)
173175

174176
# replace the original function with the wrapped function
175177
new_args: tuple[Callable[..., Any], ...] = (wrapped_func,) + args[1:]

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

+66
Original file line numberDiff line numberDiff line change
@@ -15,12 +15,14 @@
1515
import threading
1616
from concurrent.futures import ThreadPoolExecutor
1717
from typing import List
18+
from unittest.mock import MagicMock, patch
1819

1920
from opentelemetry import trace
2021
from opentelemetry.instrumentation.threading import ThreadingInstrumentor
2122
from opentelemetry.test.test_base import TestBase
2223

2324

25+
# pylint: disable=too-many-public-methods
2426
class TestThreading(TestBase):
2527
def setUp(self):
2628
super().setUp()
@@ -224,3 +226,67 @@ def test_uninstrumented(self):
224226
self.assertEqual(len(spans), 1)
225227

226228
ThreadingInstrumentor().instrument()
229+
230+
@patch(
231+
"opentelemetry.context.attach",
232+
new=MagicMock(return_value=None),
233+
)
234+
@patch(
235+
"opentelemetry.context.detach",
236+
autospec=True,
237+
)
238+
def test_threading_with_none_context_token(self, mock_detach: MagicMock):
239+
with self.get_root_span():
240+
thread = threading.Thread(target=self.fake_func)
241+
thread.start()
242+
thread.join()
243+
mock_detach.assert_not_called()
244+
245+
@patch(
246+
"opentelemetry.context._RUNTIME_CONTEXT.attach",
247+
new=MagicMock(return_value=MagicMock()),
248+
)
249+
@patch(
250+
"opentelemetry.context._RUNTIME_CONTEXT.detach",
251+
new=MagicMock(return_value=None),
252+
)
253+
@patch("opentelemetry.context.detach", autospec=True)
254+
def test_threading_with_valid_context_token(self, mock_detach: MagicMock):
255+
with self.get_root_span():
256+
thread = threading.Thread(target=self.fake_func)
257+
thread.start()
258+
thread.join()
259+
mock_detach.assert_called_once()
260+
261+
@patch(
262+
"opentelemetry.context.attach",
263+
new=MagicMock(return_value=None),
264+
)
265+
@patch(
266+
"opentelemetry.context.detach",
267+
autospec=True,
268+
)
269+
def test_thread_pool_with_none_context_token(self, mock_detach: MagicMock):
270+
with self.get_root_span(), ThreadPoolExecutor(
271+
max_workers=1
272+
) as executor:
273+
future = executor.submit(self.get_current_span_context_for_test)
274+
future.result()
275+
mock_detach.assert_not_called()
276+
277+
@patch(
278+
"opentelemetry.context._RUNTIME_CONTEXT.attach",
279+
new=MagicMock(return_value=MagicMock()),
280+
)
281+
@patch(
282+
"opentelemetry.context._RUNTIME_CONTEXT.detach",
283+
new=MagicMock(return_value=None),
284+
)
285+
@patch("opentelemetry.context.detach", autospec=True)
286+
def test_threadpool_with_valid_context_token(self, mock_detach: MagicMock):
287+
with self.get_root_span(), ThreadPoolExecutor(
288+
max_workers=1
289+
) as executor:
290+
future = executor.submit(self.get_current_span_context_for_test)
291+
future.result()
292+
mock_detach.assert_called_once()

0 commit comments

Comments
 (0)