Repository navigation
Conversation
Add an opt-in APSchedulerIntegration(monitor_jobs=True) for APScheduler 3.x. When a job with a cron or interval trigger is submitted to its executor, send an in_progress check-in with a monitor config derived from the trigger, so the monitor is created on first run. A scheduler listener sends the ok/error check-in when the run finishes or is missed. The integration is not auto-enabled. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Contributor
Coverage Results 📊✅ 137666 passed | ⏭️ 7503 skipped | Total: 145169 | Pass Rate: 94.83% | Execution Time: 449m 22s 📊 Comparison with Base Branch
All tests are passing successfully. ✅ Patch coverage is 96.02% (193 of 201 changed executable lines covered; target 80%). Changed files with executable lines (1)
Coverage diff@@ Coverage Diff @@
## master #7840 +/-##
==========================================
+ Coverage 90.33% 90.34% +0.01%
==========================================
Files 203 204 +1
Tracked lines 26922 27134 +212
Branches 10056 10130 +74
==========================================
+ Hits 24319 24514 +195
- Misses 2603 2620 +17
- Partials 1516 1523 +7Generated by Coverage Action |
![sentry-warden[bot]](https://avatars.githubusercontent.com/in/2755838?s=60&v=4){kind=small}
Regenerating locally dropped cached releases of other packages. Restore them and keep only the APScheduler entries. Co-Authored-By: Claude <noreply@anthropic.com>
Move the APScheduler interval-to-schedule conversion into a private helper in sentry_sdk.crons so other integrations can reuse it. Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude <noreply@anthropic.com>
- Handle the Optional[str] check-in id that capture_checkin now returns. - Send the monitor config only when the trigger's timezone is an IANA name. Sentry rejects names like "UTC+02:00", and without a timezone it would read the crontab as UTC. - Open the check-ins only once the executor accepts the job, so runs skipped because of max_instances send none, instead of in_progress followed by error. - Warn once per job when it has no explicit id and its name is used as the monitor slug. - Say that only APScheduler 3.x is supported when the import fails. Co-Authored-By: Claude <noreply@anthropic.com>
# Conflicts: # scripts/populate_tox/package_dependencies.jsonl # scripts/populate_tox/releases.jsonl
Sentry monitor schedules don't end, so a finite cron or interval trigger would raise missed check-ins after its last run.
Move arq's timezone name logic into sentry_sdk.crons.utils and use it for APScheduler too, so fixed-offset timezones with whole-hour offsets are monitored with Etc/GMT names. tzlocal's "local" zone is rejected.
Jitter delays runs by up to that many seconds, so add it on top of Sentry's default 1 minute check-in margin.
# Conflicts: # scripts/populate_tox/releases.jsonl
A run that dies inside the executor, like a crashed process pool worker, goes through BaseExecutor._run_job_error, which dispatches no event. Close that job's oldest pending check-in as an error there.
A job function wrapped in @sentry_sdk.monitor sends its own check-ins, so monitoring it again reported every run twice. The decorator marks its wrapper with _sentry_monitored, and the integration skips marked jobs.
With coalesce=False, a submit can carry several overdue run times, which sent a burst of in_progress/ok pairs. Open one check-in per submit and close it on the last run time's event.
With TZ set to a file path, tzlocal names the zone after the file's basename, like "localtime", which Sentry rejects as an invalid monitor config. Return a zone's name only when zoneinfo or pytz knows it.
APScheduler 4 fails the 3.x imports, which already raises DidNotEnable.
On Ubuntu, available_timezones() lists /usr/share/zoneinfo/localtime, so a zone named after it passed as an IANA name.
A cron coroutine wrapped in @sentry_sdk.monitor sends its own check-ins, so monitoring it again reported every run twice. The arq wrapper keeps the coroutine's attributes, so the _sentry_monitored marker stays visible.
Sentry's missed-run detection reports the slot, so a run APScheduler skips for being past its misfire grace time gets no check-in, and its pending state is dropped.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit fcdeaca. Configure here.
A job that waits in a busy pool past its misfire grace time has already sent in_progress when APScheduler reports it missed. Close that check-in as error instead of leaving it to time out.
# Conflicts: # scripts/populate_tox/releases.jsonl
With coalesce=False a backlog runs in one submit and sends one check-in, closed by the last run's event. An earlier failed run now makes it an error.
![sentry-junior[bot]](https://avatars.githubusercontent.com/in/2960336?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
The cron translation holds up. I checked the weekday mapping, */2 → 1-7/2, and the bail-outs for last, 1st mon, seconds, and day + day_of_week against cronsim, which Sentry uses to parse schedules. But this PR is doing too much and has picked up a lot of edge-case handling. Two asks overall:
- Please split it. The arq changes (
@wraps, skipping jobs that already use@monitor, stricter timezone names), the sharedcrons/utilshelper, and the decorator marker can go in their own PR, with APScheduler stacked on top. - Please trim the listener down to the common case. Details are inline.
Comment on lines
+144
to
+152
| def finish_missed(self, job_id: str, run_time: "Any") -> None: | ||
| with self.lock: | ||
| if self.earlier.pop((job_id, run_time), None) is not None: | ||
| return | ||
| pending = self._pop((job_id, run_time)) | ||
| # Send nothing, so Sentry reports the slot as missed, unless a run that | ||
| # waited in a busy pool already sent `in_progress`. | ||
| if pending is not None and pending.opened: | ||
| self._close(pending, MonitorStatus.ERROR) |
Contributor
There was a problem hiding this comment.
A run that is missed after in_progress was sent is rare. Sentry already times out an open in_progress, so I'd drop this path.
Member
Author
There was a problem hiding this comment.
I'd like to keep this one. Without it the user gets a timeout half an hour later, which looks like the job hung when it never actually ran. It's only a few lines once the rest of the listener state is gone.
Collaborator
|
Hey @wedamija, we had Junior do a first pass on the PR, please have a look. We'll have a human review afterwards, probably next week 🙏🏻 |
# Conflicts: # sentry_sdk/crons/utils.py # sentry_sdk/integrations/arq.py
Adding APSchedulerIntegration turns monitoring on, without a separate flag. Pending check-ins are tracked by the last run time of each submit, and the patch for crashed process pool workers is gone, since Sentry times out an open in_progress check-in. Tests assert on captured check-ins only.
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Stacked on #8198.
Adds an APScheduler integration (3.3+) that sends cron check-ins for scheduled jobs. It isn't auto-enabled; adding it turns monitoring on:
Each run sends check-ins with a monitor config taken from the job's trigger, so monitors are created on the first run and stay in sync with the schedule. Runs that never start report as missed.
Jobs we can't describe exactly (some triggers, non-IANA time zones, one-off
DateTriggerjobs) aren't monitored, since a wrong schedule causes false missed alerts. Jobs that already use@sentry_sdk.monitorare skipped, so they don't report twice.exclude_jobsskips jobs by regex.Motivation
Today APScheduler users have to create every monitor by hand and add check-in calls to each job. The schedule is already on the trigger, so the SDK can do both.
Docs: getsentry/sentry-docs#19786