Repository navigation
Conversation
When `@SentryCron` is used without a monitor config, read the schedule and time zone from the `@Cron()` decorator of `@nestjs/schedule` on the same method. Check-ins then carry a monitor config, so Sentry creates the monitor on the first run instead of dropping the check-ins. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Only read the `@Cron()` schedule when `@SentryCron` gets
`{ fromCronDecorator: true }`, so existing usage keeps sending check-ins
without a monitor config. Other monitor settings passed alongside the flag
are kept.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
`@SentryCron` now sends the schedule of the method's `@Cron()` decorator unless it gets a monitor config with a schedule. Other monitor settings can still be passed and are merged in, and `fromCronDecorator: false` turns this off. When `@Cron()` has no `timeZone`, the job runs in the server's local time zone, so that zone is sent instead of leaving Sentry to assume UTC. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
`@Cron()` presets are now sent: the ones Sentry accepts as is, and `@midnight`, `@minutely`, `@weekdays` and `@weekends` as crontabs. When no schedule can be derived but monitor settings were passed, a debug warning says no monitor config is sent. The settings type now rejects `schedule` and `timezone`, and a full monitor config rejects `fromCronDecorator`, so mixed objects no longer drop fields silently. Adds an E2E case for a disabled `@Cron()` job triggered through an endpoint. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Skip numeric months (cron 2.x counts from 0), day-of-month/day-of-week combinations Sentry would AND, and time zones Sentry rejects. Clarify the warning when fromCronDecorator is false. Co-Authored-By: Claude <noreply@anthropic.com>
Allow month steps like */3 (CronExpression.EVERY_QUARTER), still skipping month numbers. Drop the @midnight and @annually presets, which cron rejects. Skip full day ranges like 1-31 when the other day field is set, and send the canonical time zone name. Co-Authored-By: Claude <noreply@anthropic.com>
5d88d04 to
8cd5e98
Compare
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 8cd5e98. Configure here.
Co-Authored-By: Claude <noreply@anthropic.com>
|
👋 @chargome, @nicohrubec — Please review this PR when you get a chance! |
2 similar comments
|
👋 @chargome, @nicohrubec — Please review this PR when you get a chance! |
|
👋 @chargome, @nicohrubec — Please review this PR when you get a chance! |
Co-Authored-By: Claude <noreply@anthropic.com>
|
👋 @chargome, @nicohrubec — Please review this PR when you get a chance! |
1 similar comment
|
👋 @chargome, @nicohrubec — Please review this PR when you get a chance! |
size-limit report 📦Compared: 3730ab6 (PR test merge using develop at b722d3c) Against: b722d3c (develop) · base run #64034 ✅ Baseline matches: this diff contains only this PR's changes.
|
isaacs
left a comment
There was a problem hiding this comment.
A few things below I'd like to confirm and/or fix before landing, but this looks good. Does what it says on the tin :)
| return monitorConfig; | ||
| } | ||
|
|
||
| const { fromCronDecorator = true, ...monitorSettings } = monitorConfig || {}; |
There was a problem hiding this comment.
When this lands, every current @SentryCron user who passes no schedule starts to upsert monitors on the next deploy. That has three effects:
- Monitors that did not exist get created, and they are billed.
- Monitors that users created by hand get their schedule and time zone overwritten on each check-in. If a user picked a different time zone in the UI on purpose (for example, the one they think in), the SDK now silently replaces it.
- If the org is over its monitor quota, creation can fail, and the check-ins are dropped as before. That is not worse, but users get no signal.
The PR description says this, but users of the SDK are more likely to notice a changelog entry. This repo writes the changelog at release time, so this is mainly a request to make sure the release notes put it under "Important Changes".
Can you confirm that default-on is the intended product choice?
If so, then this is what this is doing, so great! Let's just add a clear note for the release changelog that names the billing effect and the fromCronDecorator: false opt-out.
There was a problem hiding this comment.
Yeah default-on is intended. Generally, we expect that people want to just use their cron's built in schedule, and that it's most likely a misconfiguration if they didn't do so.
| } | ||
|
|
||
| // Without a `timeZone`, `cron` uses the server's time zone, but Sentry would assume UTC. | ||
| const timezone = toSentryTimeZone(typeof timeZone === 'string' && timeZone ? timeZone : getLocalTimeZone()); |
There was a problem hiding this comment.
Without timeZone on @Cron(), this'll send the host's local time zone. That matches what cron does on that host, so each single check-in is correct.
But a monitor config is shared by all environments that use the same slug. If one host runs in UTC (eg, a container) and another in America/New_York (a developer laptop, a staging box, etc), each check-in overwrites the monitor's time zone. The monitor then expects runs at the wrong hour, and Sentry can report missed check-ins for runs that happened on time.
Before this PR, the user had to pick one time zone by hand, so that failure mode did not exist.
Suggestion: probably just need to document that jobs without an explicit timeZone take the host's time zone (if it's not already in the linked docs PR), and that hosts with different zones must set timeZone on @Cron(). I can't think of a correct automatic fix, because the job really does run in the host's zone.
There was a problem hiding this comment.
Agreed, updated the docs pr getsentry/sentry-docs#19782
…s on @nestjs/schedule 4+ A day field that covers every day (like sun-sat or 0,1,2,3,4,5,6) is ignored by cron but ORed by Sentry, so those schedules are now found by counting the expanded values. Numeric months are sent when the @Cron() instrumentation saw @nestjs/schedule 4 or later, where cron counts months from 1.
Optional E2E testsSome optional checks were not successful. They do not block the merge. 2 not successful, 65 successful. See the run.
Successful checks
|

Part of a series adding automatic Crons monitoring for the most-used job schedulers in each SDK. Check-ins for a monitor that doesn't exist yet are dropped, so today each monitor has to be created by hand with its schedule copied from the code. Sending the job's schedule as the monitor config lets Sentry create the monitor and keep it in sync.
@SentryCronnow sends the schedule and time zone of the method's@Cron()decorator with its check-ins, so Sentry creates the monitor on the first run. It's on by default; passfromCronDecorator: falseto send no config. A config with its ownscheduleis used as is. Monitors created this way are billed, and their schedule and time zone are overwritten from@Cron()on each check-in.Without a
timeZone, the server's local time zone is sent. Schedules Sentry would read differently from@nestjs/schedule(sub-minute, numeric months, some day-field combinations, fixed UTC offsets) check in without a config.Docs: getsentry/sentry-docs#19782