Skip to content

Commit 51a1166

Browse files
snopokeclaude
andcommitted
Don't nest chain links and callbacks
Celery dispatches the next chain link and any `link` callbacks from inside `trace_task` after the body returns but before `task_postrun`, so the task they follow still looks current and they were being recorded as its children. They're successors, not subtasks. Nothing on the wire distinguishes them from a publish the body made itself, so this matches against the running task's own `request.chain`/`request.callbacks` rather than inspecting the stack. Retries still nest — a retry republishes the same task, which isn't among its own successors. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent d1c59dd commit 51a1166

4 files changed

Lines changed: 105 additions & 2 deletions

File tree

integration_tests/tasks.py

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,16 @@ def spawns_child(self, x, y):
3939
return spawns_grandchild.delay(x, y).id
4040

4141

42+
@shared_task(bind=True, base=taskbadger.celery.Task)
43+
def chain_head(self):
44+
return self.taskbadger_task_id
45+
46+
47+
@shared_task(bind=True, base=taskbadger.celery.Task)
48+
def chain_tail(self, head_tb_id):
49+
return {"head_tb_id": head_tb_id, "own_tb_id": self.taskbadger_task_id}
50+
51+
4252
@shared_task(bind=True, base=taskbadger.celery.Task, taskbadger_heartbeat_interval=HEARTBEAT_INTERVAL)
4353
def slow_add(self, x, y):
4454
"""Runs long enough to go stale without a heartbeat, and never updates itself."""

integration_tests/test_celery.py

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,11 +3,12 @@
33
import time
44

55
import pytest
6+
from celery import chain
67

78
import taskbadger
89
from taskbadger import StatusEnum
910

10-
from .tasks import HEARTBEAT_INTERVAL, add, add_auto_track, slow_add, spawns_child
11+
from .tasks import HEARTBEAT_INTERVAL, add, add_auto_track, chain_head, chain_tail, slow_add, spawns_child
1112

1213

1314
@pytest.fixture(autouse=True)
@@ -69,6 +70,18 @@ def test_celery_grandchild_is_flattened_onto_the_root(celery_session_app, celery
6970
assert grandchild.parent != child["own_tb_id"]
7071

7172

73+
def test_celery_chain_links_are_not_nested(celery_session_app, celery_session_worker):
74+
"""Celery dispatches the next chain link from inside the previous task's run,
75+
so it would otherwise be nested under it. Links are successors, not subtasks.
76+
"""
77+
ids = chain(chain_head.s(), chain_tail.s()).apply_async().get(timeout=20, propagate=True)
78+
79+
assert ids["head_tb_id"], "the first link should be tracked"
80+
assert ids["own_tb_id"], "the second link should be tracked"
81+
assert not taskbadger.get_task(ids["own_tb_id"]).parent
82+
assert taskbadger.list_tasks(parent=ids["head_tb_id"]).results == []
83+
84+
7285
def test_celery_heartbeat(celery_session_app, celery_session_worker):
7386
"""The worker pings the task while it runs, so it doesn't go stale."""
7487
a, b = random.randint(1, 1000), random.randint(1, 1000)

taskbadger/celery.py

Lines changed: 36 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -141,6 +141,41 @@ def taskbadger_task(self):
141141
return task
142142

143143

144+
def _signature_names(signature):
145+
"""The task names ``signature`` will publish, expanding groups.
146+
147+
Signatures reach us either as ``Signature`` objects or as the dicts they
148+
serialize to, depending on where in Celery they came from.
149+
"""
150+
if isinstance(signature, dict):
151+
name = signature.get("task")
152+
nested = (signature.get("kwargs") or {}).get("tasks")
153+
else:
154+
name = getattr(signature, "task", None)
155+
nested = getattr(signature, "tasks", None)
156+
if nested:
157+
for inner in nested:
158+
yield from _signature_names(inner)
159+
elif name:
160+
yield name
161+
162+
163+
def _is_workflow_successor(task_name):
164+
"""Whether the task being published is the running task's next chain link or
165+
one of its callbacks, rather than something its body chose to enqueue.
166+
167+
Celery dispatches both from inside ``trace_task`` after the task body
168+
returns but before ``task_postrun``, so the running task still looks
169+
current. They're the task's successors rather than its subtasks, so they
170+
shouldn't be nested under it.
171+
"""
172+
request = getattr(celery.current_task, "request", None)
173+
if request is None:
174+
return False
175+
successors = list(request.chain or []) + list(request.callbacks or [])
176+
return any(name == task_name for sig in successors for name in _signature_names(sig))
177+
178+
144179
@before_task_publish.connect
145180
def task_publish_handler(sender=None, headers=None, body=None, **kwargs):
146181
routing_key = kwargs.get("routing_key")
@@ -174,7 +209,7 @@ def task_publish_handler(sender=None, headers=None, body=None, **kwargs):
174209
# `before_task_publish` fires in the process doing the publishing, so if that
175210
# is itself a tracked task this nests the new task under it.
176211
enclosing_task = parent_id()
177-
if enclosing_task:
212+
if enclosing_task and not _is_workflow_successor(sender):
178213
kwargs.setdefault("parent", enclosing_task)
179214
name = kwargs.pop("name", headers["task"])
180215

tests/test_parents.py

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -263,6 +263,51 @@ def test_celery_publish_has_no_parent_outside_a_task():
263263
assert "parent" not in create.call_args.kwargs
264264

265265

266+
def _running_task(chain=None, callbacks=None):
267+
"""Pretend a task with these successors is mid-run."""
268+
request = mock.Mock(chain=chain, callbacks=callbacks)
269+
return mock.patch("celery.current_task", mock.Mock(request=request))
270+
271+
272+
@pytest.mark.usefixtures("_bind_settings")
273+
@pytest.mark.parametrize(
274+
"successors",
275+
[
276+
{"chain": [{"task": "child.task"}]},
277+
{"callbacks": [{"task": "child.task"}]},
278+
# a group of callbacks arrives as one signature wrapping the real tasks
279+
{"callbacks": [{"task": "celery.group", "kwargs": {"tasks": [{"task": "child.task"}]}}]},
280+
],
281+
ids=["chain", "callback", "callback_group"],
282+
)
283+
def test_celery_workflow_successors_are_not_nested(successors):
284+
"""Chain links and callbacks are dispatched while the task that precedes
285+
them still looks current, but they follow it rather than belong to it."""
286+
with mock.patch("taskbadger.celery.create_task_safe") as create, _running_task(**successors):
287+
create.return_value = task_for_test()
288+
token = enter_task("root_id")
289+
try:
290+
_publish()
291+
finally:
292+
exit_task(token)
293+
294+
assert "parent" not in create.call_args.kwargs
295+
296+
297+
@pytest.mark.usefixtures("_bind_settings")
298+
def test_celery_publish_from_a_task_body_still_nests_while_a_chain_is_pending():
299+
"""Only the successor itself is exempt — a task the body enqueues is not."""
300+
with mock.patch("taskbadger.celery.create_task_safe") as create, _running_task(chain=[{"task": "next.task"}]):
301+
create.return_value = task_for_test()
302+
token = enter_task("root_id")
303+
try:
304+
_publish()
305+
finally:
306+
exit_task(token)
307+
308+
assert create.call_args.kwargs["parent"] == "root_id"
309+
310+
266311
@pytest.mark.usefixtures("_bind_settings")
267312
def test_celery_publish_explicit_parent_wins():
268313
with mock.patch("taskbadger.celery.create_task_safe") as create:

0 commit comments

Comments
 (0)