Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -183,10 +183,17 @@ def _trace_before_publish(self, *args, **kwargs):
task = utils.retrieve_task_from_sender(kwargs)
task_id = utils.retrieve_task_id_from_message(kwargs)

if task is None or task_id is None:
if task_id is None:
return

operation_name = f"{_TASK_APPLY_ASYNC}/{task.name}"
if task is None:
# task is an anonymous task send using send_task or using canvas workflow
# Signatures() to send to a task not in the current processes dependency
# tree
task_name = kwargs.get("sender", "unknown")
else:
task_name = task.name
operation_name = f"{_TASK_APPLY_ASYNC}/{task_name}"
span = self._tracer.start_span(
operation_name, kind=trace.SpanKind.PRODUCER
)
Expand All @@ -195,7 +202,7 @@ def _trace_before_publish(self, *args, **kwargs):
if span.is_recording():
span.set_attribute(_TASK_TAG_KEY, _TASK_APPLY_ASYNC)
span.set_attribute(SpanAttributes.MESSAGING_MESSAGE_ID, task_id)
span.set_attribute(_TASK_NAME_KEY, task.name)
span.set_attribute(_TASK_NAME_KEY, task_name)
utils.set_attributes_from_context(span, kwargs)

activation = trace.use_span(span, end_on_exit=True)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,11 @@ def attach_span(task, task_id, span, is_publish=False):
NOTE: We cannot test for this well yet, because we do not run a celery worker,
and cannot run `task.apply_async()`
"""
if task is None:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll copy the comment I wrote on the original PR in your fork for the wide discussion:

I think the effect of this return statement is that the span (along with activation) will not be recorded into the CTX_KEY, which means that a later invocation of the _trace_after_publish signal will not be able to retrieve_span thus will not end the span.
Am I missing something?

Didn't actually try to run it, so I might just not fully understand the details of what causes the "publish" span to be ended.

# task objects can be optional, if None is passed in, there is nothing to attach
# the context state to...
return

span_dict = getattr(task, CTX_KEY, None)
if span_dict is None:
span_dict = {}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -77,3 +77,46 @@ def test_task(self):
self.assertNotEqual(consumer.parent, producer.context)
self.assertEqual(consumer.parent.span_id, producer.context.span_id)
self.assertEqual(consumer.context.trace_id, producer.context.trace_id)


class TestCelerySignatureTask(TestBase):
def setUp(self):
super().setUp()

def start_app(*args, **kwargs):
# Add an additional task that will not be registered with parent thread
@app.task
def hidden_task(x):
return x * 2

self._worker = app.Worker(app=app, pool="solo", concurrency=1)
return self._worker.start(*args, **kwargs)

self._thread = threading.Thread(target=start_app)
self._worker = app.Worker(app=app, pool="solo", concurrency=1)
self._thread.daemon = True
self._thread.start()

def tearDown(self):
super().tearDown()
self._worker.stop()
self._thread.join()

def test_hidden_task(self):
# no-op since already instrumented
CeleryInstrumentor().instrument()
import ipdb; ipdb.set_trace()
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
import ipdb; ipdb.set_trace()
import ipdb
ipdb.set_trace()

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pytest-ipdb is unfortunately not supported anymore.
You can remove these lines.

The solution is to add "--pdb --pdbcls=IPython.terminal.debugger:Pdb" with pytest command

From the help command:

pytest -h
--pdb start the interactive Python debugger on errors.
--pdbcls=modulename:classname
start a custom interactive Python debugger on errors.
For example:
--pdbcls=IPython.terminal.debugger:TerminalPdb


res = app.signature("app.hidden_task", (2,)).apply_async()
while not res.ready():
time.sleep(0.05)

spans = self.sorted_spans(self.memory_exporter.get_finished_spans())
self.assertEqual(len(spans), 1)

producer = spans

self.assertEqual(
producer.name, "apply_async/app.hidden_task"
)
Comment on lines +119 to +121
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
self.assertEqual(
producer.name, "apply_async/app.hidden_task"
)
self.assertEqual(producer.name, "apply_async/app.hidden_task")

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are we still working on this issue ?

self.assertEqual(producer.kind, SpanKind.PRODUCER)
Original file line number Diff line number Diff line change
Expand Up @@ -181,10 +181,18 @@ def fn_task():
task_id = "7c6731af-9533-40c3-83a9-25b58f0d837f"
span = trace._Span("name", mock.Mock(spec=trace_api.SpanContext))
utils.attach_span(fn_task, task_id, span)

# delete the Span
utils.detach_span(fn_task, task_id)
self.assertEqual(utils.retrieve_span(fn_task, task_id), (None, None))

def test_optional_task_span_attach(self):
task_id = "7c6731af-9533-40c3-83a9-25b58f0d837f"
span = trace._Span("name", mock.Mock(spec=trace_api.SpanContext))

# assert this is is a no-aop
self.assertIsNone(utils.attach_span(None, task_id, span))

def test_span_delete_empty(self):
# ensure detach_span doesn't raise an exception if span is not present
@self.app.task
Expand Down