Skip to content

Commit 2f97cda

Browse files
authored
fix(celery): Do not send extra check-in (#4395)
When using the RedBeatScheduler, we're sending an extra in-progress check-in at scheduler start. Since this is never followed by an ok or error check-in, the check-in is marked as timed out in Sentry. We're patching the scheduler's `maybe_due`, which (as the name implies) [might not end up executing the task](https://github.com/sibson/redbeat/blob/bdefad23b47f75e2e85d45f46f9f16dd00a93d40/redbeat/schedulers.py#L506-L508). This is indeed what seems to be happening -- `maybe_due` is run when the scheduler starts, but without scheduling the task. We don't check whether `maybe_due` actually ended up scheduling anything and always fire an in-progress check-in. Patching the [scheduler's `apply_async`](https://github.com/sibson/redbeat/blob/bdefad23b47f75e2e85d45f46f9f16dd00a93d40/redbeat/schedulers.py#L511) instead. Closes #4392
1 parent aa0eaab commit 2f97cda

File tree

3 files changed

+14
-12
lines changed

3 files changed

+14
-12
lines changed

sentry_sdk/integrations/celery/__init__.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99
from sentry_sdk.integrations import _check_minimum_version, Integration, DidNotEnable
1010
from sentry_sdk.integrations.celery.beat import (
1111
_patch_beat_apply_entry,
12-
_patch_redbeat_maybe_due,
12+
_patch_redbeat_apply_async,
1313
_setup_celery_beat_signals,
1414
)
1515
from sentry_sdk.integrations.celery.utils import _now_seconds_since_epoch
@@ -73,7 +73,7 @@ def __init__(
7373
self.exclude_beat_tasks = exclude_beat_tasks
7474

7575
_patch_beat_apply_entry()
76-
_patch_redbeat_maybe_due()
76+
_patch_redbeat_apply_async()
7777
_setup_celery_beat_signals(monitor_beat_tasks)
7878

7979
@staticmethod

sentry_sdk/integrations/celery/beat.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -202,12 +202,12 @@ def _patch_beat_apply_entry():
202202
Scheduler.apply_entry = _wrap_beat_scheduler(Scheduler.apply_entry)
203203

204204

205-
def _patch_redbeat_maybe_due():
205+
def _patch_redbeat_apply_async():
206206
# type: () -> None
207207
if RedBeatScheduler is None:
208208
return
209209

210-
RedBeatScheduler.maybe_due = _wrap_beat_scheduler(RedBeatScheduler.maybe_due)
210+
RedBeatScheduler.apply_async = _wrap_beat_scheduler(RedBeatScheduler.apply_async)
211211

212212

213213
def _setup_celery_beat_signals(monitor_beat_tasks):

tests/integrations/celery/test_celery_beat_crons.py

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@
1010
_get_headers,
1111
_get_monitor_config,
1212
_patch_beat_apply_entry,
13-
_patch_redbeat_maybe_due,
13+
_patch_redbeat_apply_async,
1414
crons_task_failure,
1515
crons_task_retry,
1616
crons_task_success,
@@ -454,10 +454,10 @@ def test_exclude_redbeat_tasks_option(
454454
"""
455455
Test excluding Celery RedBeat tasks from automatic instrumentation.
456456
"""
457-
fake_maybe_due = MagicMock()
457+
fake_apply_async = MagicMock()
458458

459459
fake_redbeat_scheduler = MagicMock()
460-
fake_redbeat_scheduler.maybe_due = fake_maybe_due
460+
fake_redbeat_scheduler.apply_async = fake_apply_async
461461

462462
fake_integration = MagicMock()
463463
fake_integration.exclude_beat_tasks = exclude_beat_tasks
@@ -481,17 +481,19 @@ def test_exclude_redbeat_tasks_option(
481481
"sentry_sdk.integrations.celery.beat._get_monitor_config",
482482
fake_get_monitor_config,
483483
) as _get_monitor_config:
484-
# Mimic CeleryIntegration patching of RedBeatScheduler.maybe_due()
485-
_patch_redbeat_maybe_due()
484+
# Mimic CeleryIntegration patching of RedBeatScheduler.apply_async()
485+
_patch_redbeat_apply_async()
486486
# Mimic Celery RedBeat calling a task from the RedBeat schedule
487-
RedBeatScheduler.maybe_due(fake_redbeat_scheduler, fake_schedule_entry)
487+
RedBeatScheduler.apply_async(
488+
fake_redbeat_scheduler, fake_schedule_entry
489+
)
488490

489491
if task_in_excluded_beat_tasks:
490492
# Only the original RedBeatScheduler.maybe_due() is called, _get_monitor_config is NOT called.
491-
assert fake_maybe_due.call_count == 1
493+
assert fake_apply_async.call_count == 1
492494
_get_monitor_config.assert_not_called()
493495

494496
else:
495497
# The original RedBeatScheduler.maybe_due() is called, AND _get_monitor_config is called.
496-
assert fake_maybe_due.call_count == 1
498+
assert fake_apply_async.call_count == 1
497499
assert _get_monitor_config.call_count == 1

0 commit comments

Comments
 (0)