Skip to content

Commit 8fec6b6

Browse files
authored
fix(threading): attribute error when run is called w/o start (#4246)
* fix(threading): attribute error when run is called w/o start * update changelog * don't initialize context on run, fix tests * address styling comments * remove no-op context attachment
1 parent 7326faa commit 8fec6b6

File tree

3 files changed

+57
-6
lines changed

3 files changed

+57
-6
lines changed

CHANGELOG.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -115,6 +115,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
115115
([#4175](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/4175))
116116
- `opentelemetry-docker-tests` Fix docker-tests assumption by Postgres-Sqlalchemy case about scope of metrics
117117
([#4258](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/4258))
118+
- `opentelemetry-instrumentation-threading`: fix AttributeError when Thread is run without starting
119+
([#4246](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/4246))
118120

119121
### Breaking changes
120122

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -147,7 +147,8 @@ def __wrap_threading_run(
147147
) -> R:
148148
token = None
149149
try:
150-
token = context.attach(instance._otel_context)
150+
if hasattr(instance, "_otel_context"):
151+
token = context.attach(instance._otel_context)
151152
return call_wrapped(*args, **kwargs)
152153
finally:
153154
if token is not None:

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

Lines changed: 53 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,11 @@
1212
# See the License for the specific language governing permissions and
1313
# limitations under the License.
1414

15+
from __future__ import annotations
16+
1517
import threading
1618
from concurrent.futures import ( # pylint: disable=no-name-in-module; TODO #4199
19+
Future,
1720
ThreadPoolExecutor,
1821
)
1922
from typing import List
@@ -66,7 +69,7 @@ def test_trace_context_propagation_in_thread_pool_with_multiple_workers(
6669
executor = ThreadPoolExecutor(max_workers=max_workers)
6770

6871
expected_span_contexts: List[trace.SpanContext] = []
69-
futures_list = []
72+
futures_list: List[Future[trace.SpanContext]] = []
7073
for num in range(max_workers):
7174
with self._tracer.start_as_current_span(f"trace_{num}") as span:
7275
expected_span_context = span.get_span_context()
@@ -125,23 +128,23 @@ def fake_func(self):
125128
def get_current_span_context_for_test() -> trace.SpanContext:
126129
return trace.get_current_span().get_span_context()
127130

128-
def print_square(self, num):
131+
def print_square(self, num: int | float) -> int | float:
129132
with self._tracer.start_as_current_span("square"):
130133
return num * num
131134

132-
def print_cube(self, num):
135+
def print_cube(self, num: int | float) -> int | float:
133136
with self._tracer.start_as_current_span("cube"):
134137
return num * num * num
135138

136-
def print_square_with_thread(self, num):
139+
def print_square_with_thread(self, num: int | float) -> int | float:
137140
with self._tracer.start_as_current_span("square"):
138141
cube_thread = threading.Thread(target=self.print_cube, args=(10,))
139142

140143
cube_thread.start()
141144
cube_thread.join()
142145
return num * num
143146

144-
def calculate(self, num):
147+
def calculate(self, num: int | float) -> None:
145148
with self._tracer.start_as_current_span("calculate"):
146149
square_thread = threading.Thread(
147150
target=self.print_square, args=(num,)
@@ -294,3 +297,48 @@ def test_threadpool_with_valid_context_token(self, mock_detach: MagicMock):
294297
future = executor.submit(self.get_current_span_context_for_test)
295298
future.result()
296299
mock_detach.assert_called_once()
300+
301+
def test_threading_run_without_start(self):
302+
square_thread = threading.Thread(target=self.print_square, args=(10,))
303+
with self._tracer.start_as_current_span("root"):
304+
square_thread.run()
305+
306+
spans = self.memory_exporter.get_finished_spans()
307+
self.assertEqual(len(spans), 2)
308+
root_span = next(span for span in spans if span.name == "root")
309+
self.assertIsNotNone(root_span)
310+
self.assertIsNone(root_span.parent)
311+
square_span = next(span for span in spans if span.name == "square")
312+
self.assertIsNotNone(square_span)
313+
self.assertIs(square_span.parent, root_span.get_span_context())
314+
315+
def test_threading_run_with_custom_run(self):
316+
_tracer = self._tracer
317+
318+
class ThreadWithCustomRun(threading.Thread):
319+
def run(self):
320+
# don't call super().run() on purpose
321+
# Thread.run() cannot be called twice
322+
with _tracer.start_as_current_span("square"):
323+
pass
324+
325+
square_thread = ThreadWithCustomRun(
326+
target=self.print_square, args=(10,)
327+
)
328+
with self._tracer.start_as_current_span("run_1"):
329+
square_thread.run()
330+
with self._tracer.start_as_current_span("run_2"):
331+
square_thread.run()
332+
333+
spans = self.memory_exporter.get_finished_spans()
334+
self.assertEqual(len(spans), 4)
335+
run_1_span = next(span for span in spans if span.name == "run_1")
336+
run_2_span = next(span for span in spans if span.name == "run_2")
337+
square_spans = [span for span in spans if span.name == "square"]
338+
square_spans.sort(key=lambda x: x.start_time or 0)
339+
run_1_child_span = square_spans[0]
340+
run_2_child_span = square_spans[1]
341+
self.assertIs(run_1_child_span.parent, run_1_span.get_span_context())
342+
self.assertIs(run_2_child_span.parent, run_2_span.get_span_context())
343+
self.assertIsNone(run_1_span.parent)
344+
self.assertIsNone(run_2_span.parent)

0 commit comments

Comments
 (0)