diff --git a/CHANGELOG.md b/CHANGELOG.md index 9bbaae300f..3ede5cbf0a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -115,6 +115,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ([#4175](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/4175)) - `opentelemetry-docker-tests` Fix docker-tests assumption by Postgres-Sqlalchemy case about scope of metrics ([#4258](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/4258)) +- `opentelemetry-instrumentation-threading`: fix AttributeError when Thread is run without starting + ([#4246](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/4246)) ### Breaking changes 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 6352197465..baf50ce970 100644 --- a/instrumentation/opentelemetry-instrumentation-threading/src/opentelemetry/instrumentation/threading/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-threading/src/opentelemetry/instrumentation/threading/__init__.py @@ -147,7 +147,8 @@ def __wrap_threading_run( ) -> R: token = None try: - token = context.attach(instance._otel_context) + if hasattr(instance, "_otel_context"): + token = context.attach(instance._otel_context) return call_wrapped(*args, **kwargs) finally: if token is not None: diff --git a/instrumentation/opentelemetry-instrumentation-threading/tests/test_threading.py b/instrumentation/opentelemetry-instrumentation-threading/tests/test_threading.py index ad4bcaf019..3a06969b26 100644 --- a/instrumentation/opentelemetry-instrumentation-threading/tests/test_threading.py +++ b/instrumentation/opentelemetry-instrumentation-threading/tests/test_threading.py @@ -12,8 +12,11 @@ # See the License for the specific language governing permissions and # limitations under the License. +from __future__ import annotations + import threading from concurrent.futures import ( # pylint: disable=no-name-in-module; TODO #4199 + Future, ThreadPoolExecutor, ) from typing import List @@ -66,7 +69,7 @@ def test_trace_context_propagation_in_thread_pool_with_multiple_workers( executor = ThreadPoolExecutor(max_workers=max_workers) expected_span_contexts: List[trace.SpanContext] = [] - futures_list = [] + futures_list: List[Future[trace.SpanContext]] = [] for num in range(max_workers): with self._tracer.start_as_current_span(f"trace_{num}") as span: expected_span_context = span.get_span_context() @@ -125,15 +128,15 @@ def fake_func(self): def get_current_span_context_for_test() -> trace.SpanContext: return trace.get_current_span().get_span_context() - def print_square(self, num): + def print_square(self, num: int | float) -> int | float: with self._tracer.start_as_current_span("square"): return num * num - def print_cube(self, num): + def print_cube(self, num: int | float) -> int | float: with self._tracer.start_as_current_span("cube"): return num * num * num - def print_square_with_thread(self, num): + def print_square_with_thread(self, num: int | float) -> int | float: with self._tracer.start_as_current_span("square"): cube_thread = threading.Thread(target=self.print_cube, args=(10,)) @@ -141,7 +144,7 @@ def print_square_with_thread(self, num): cube_thread.join() return num * num - def calculate(self, num): + def calculate(self, num: int | float) -> None: with self._tracer.start_as_current_span("calculate"): square_thread = threading.Thread( target=self.print_square, args=(num,) @@ -294,3 +297,48 @@ def test_threadpool_with_valid_context_token(self, mock_detach: MagicMock): future = executor.submit(self.get_current_span_context_for_test) future.result() mock_detach.assert_called_once() + + def test_threading_run_without_start(self): + square_thread = threading.Thread(target=self.print_square, args=(10,)) + with self._tracer.start_as_current_span("root"): + square_thread.run() + + spans = self.memory_exporter.get_finished_spans() + self.assertEqual(len(spans), 2) + root_span = next(span for span in spans if span.name == "root") + self.assertIsNotNone(root_span) + self.assertIsNone(root_span.parent) + square_span = next(span for span in spans if span.name == "square") + self.assertIsNotNone(square_span) + self.assertIs(square_span.parent, root_span.get_span_context()) + + def test_threading_run_with_custom_run(self): + _tracer = self._tracer + + class ThreadWithCustomRun(threading.Thread): + def run(self): + # don't call super().run() on purpose + # Thread.run() cannot be called twice + with _tracer.start_as_current_span("square"): + pass + + square_thread = ThreadWithCustomRun( + target=self.print_square, args=(10,) + ) + with self._tracer.start_as_current_span("run_1"): + square_thread.run() + with self._tracer.start_as_current_span("run_2"): + square_thread.run() + + spans = self.memory_exporter.get_finished_spans() + self.assertEqual(len(spans), 4) + run_1_span = next(span for span in spans if span.name == "run_1") + run_2_span = next(span for span in spans if span.name == "run_2") + square_spans = [span for span in spans if span.name == "square"] + square_spans.sort(key=lambda x: x.start_time or 0) + run_1_child_span = square_spans[0] + run_2_child_span = square_spans[1] + self.assertIs(run_1_child_span.parent, run_1_span.get_span_context()) + self.assertIs(run_2_child_span.parent, run_2_span.get_span_context()) + self.assertIsNone(run_1_span.parent) + self.assertIsNone(run_2_span.parent)