Auto-upsert Sentry cron monitors for the tasks#42
Conversation
for more information, see https://pre-commit.ci
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #42 +/- ##
==========================================
+ Coverage 98.59% 98.85% +0.25%
==========================================
Files 5 6 +1
Lines 142 174 +32
==========================================
+ Hits 140 172 +32
Misses 2 2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
Adds support for automatically upserting Sentry cron monitor configuration when sentry-sdk is available, reducing manual Sentry monitor setup for scheduled tasks.
Changes:
- Build and attach
monitor_config(schedule + timezone) when wrapping tasks withsentry_sdk.crons.monitor. - Add
_monitor_config()helper to translate crontab strings andIntervalTriggerinto Sentry cron config payloads. - Extend tests to validate
monitor_configis passed for crontab and interval schedules.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
crontask/__init__.py |
Generates Sentry monitor configuration and passes it to monitor() when available. |
tests/test_tasks.py |
Adds tests/fixture to validate Sentry monitor_config integration for cron + interval schedules. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
for more information, see https://pre-commit.ci
codingjoe
left a comment
There was a problem hiding this comment.
Sorry, I previously missed something.
96643ae to
cdc34f6
Compare
amureki
left a comment
There was a problem hiding this comment.
Awesome, thanks for wrapping it up! LGTM
|
@amureki I thought about this while sleeping. I think we need to disable the sentry checking, if it's based on an unsupported trigger. So not only no upsert, but no checking. You shouldn't have a monitor on those anyway. Especially if they are sub-minute, this would result in crazy telemetry overhead. Thoughts? |
That probably would make sense, yes. But would you still want to have it in the same PR or separately? |
|
Na, let's get it right now. @amureki what do you think? |
Co-authored-by: Rust Saiargaliev <hey@amureki.me>
Hey 👋
This PR mirrors voiio/dramatiq-crontab#186
Sentry introduced an ability to automatically create cron monitors in case the monitor config is provided along with the payload:
https://docs.sentry.io/platforms/python/crons/#configuring-cron-monitors
In case we are positive that Sentry is used within the project, instead of simply sending the signal, we can upsert the config right away to reduce the amount of manual work.