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 <9735060+emdneto@users.noreply.github.com>
Co-authored-by: Riccardo Magliocchetti <riccardo.magliocchetti@gmail.com>
This commit is contained in:
Joe McGinley
2025-03-06 17:44:44 +00:00
committed by GitHub
parent fde1ef84c7
commit ad2fe813ab
3 changed files with 72 additions and 2 deletions

View File

@ -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))

View File

@ -150,7 +150,8 @@ class ThreadingInstrumentor(BaseInstrumentor):
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 @@ class ThreadingInstrumentor(BaseInstrumentor):
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:]

View File

@ -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 @@ class TestThreading(TestBase):
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()