Repository navigation
feat(nestjs): Derive SentryCron monitor config from @Cron #25013
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
Changes from all commits
5833c8f
20a2edd
2bd91a3
3c1da94
6413911
8cd5e98
258a3fc
b253af4
47086d7
0576af0
a9a89d7
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,34 +1,268 @@ | ||
| import type { MonitorConfig } from '@sentry/core'; | ||
| import { CODE_FUNCTION_NAME, SENTRY_OP, SENTRY_ORIGIN } from '@sentry/conventions/attributes'; | ||
| import { captureException } from '@sentry/core'; | ||
| import { captureException, debug } from '@sentry/core'; | ||
| import * as Sentry from '@sentry/node'; | ||
| import { startSpan } from '@sentry/node'; | ||
| import { DEBUG_BUILD } from './debug-build'; | ||
| import { isExpectedError } from './helpers'; | ||
| import { copyReflectMetadata } from './integrations/helpers'; | ||
| import type { ReflectWithMetadata } from './integrations/helpers'; | ||
| import { copyReflectMetadata, getNestScheduleVersion } from './integrations/helpers'; | ||
|
|
||
| /** | ||
| * Monitor settings for `@SentryCron` whose schedule and time zone come from the `@Cron()` decorator | ||
| * of `@nestjs/schedule` on the same method. Set `fromCronDecorator: false` to not send them. | ||
| */ | ||
| export type SentryCronMonitorSettings = Omit<MonitorConfig, 'schedule' | 'timezone'> & { | ||
| schedule?: never; | ||
| timezone?: never; | ||
| fromCronDecorator?: boolean; | ||
| }; | ||
|
|
||
| /** | ||
| * A decorator wrapping the native nest Cron decorator, sending check-ins to Sentry. | ||
| * | ||
| * Unless a monitor config with a `schedule` is passed, the schedule and time zone of the method's | ||
| * `@Cron()` decorator are sent with each check-in, so Sentry can create the monitor on the first run. | ||
| */ | ||
| export const SentryCron = (monitorSlug: string, monitorConfig?: MonitorConfig): MethodDecorator => { | ||
| export const SentryCron = ( | ||
| monitorSlug: string, | ||
| monitorConfig?: (MonitorConfig & { fromCronDecorator?: never }) | SentryCronMonitorSettings, | ||
| ): MethodDecorator => { | ||
| return (target: unknown, propertyKey, descriptor: PropertyDescriptor) => { | ||
| const originalMethod = descriptor.value as (...args: unknown[]) => Promise<unknown>; | ||
|
|
||
| descriptor.value = function (...args: unknown[]) { | ||
| return Sentry.withMonitor( | ||
| monitorSlug, | ||
| () => { | ||
| return originalMethod.apply(this, args); | ||
| }, | ||
| monitorConfig, | ||
| ); | ||
| let resolvedMonitorConfig: MonitorConfig | undefined; | ||
| let isolateTraceWithoutConfig = false; | ||
| let resolved = false; | ||
|
|
||
| const wrappedMethod = function (this: unknown, ...args: unknown[]): unknown { | ||
| if (!resolved) { | ||
| resolved = true; | ||
| // A `@Cron()` applied after `@SentryCron()` puts its metadata on this wrapper, so read it at call time. | ||
| resolvedMonitorConfig = resolveMonitorConfig(monitorSlug, monitorConfig, [ | ||
| wrappedMethod, | ||
| (target as Record<PropertyKey, unknown> | undefined)?.[propertyKey], | ||
| ]); | ||
| isolateTraceWithoutConfig = !resolvedMonitorConfig && !!monitorConfig?.isolateTrace; | ||
| } | ||
|
|
||
| const runWithMonitor = (): unknown => | ||
| Sentry.withMonitor( | ||
| monitorSlug, | ||
| () => { | ||
| return originalMethod.apply(this, args); | ||
| }, | ||
| resolvedMonitorConfig, | ||
| ); | ||
|
|
||
| // `withMonitor` reads `isolateTrace` from the monitor config, which is not sent without a schedule. | ||
| return isolateTraceWithoutConfig ? Sentry.startNewTrace(runWithMonitor) : runWithMonitor(); | ||
| }; | ||
|
|
||
| descriptor.value = wrappedMethod; | ||
|
|
||
| copyFunctionNameAndMetadata({ originalMethod, descriptor }); | ||
|
|
||
| return descriptor; | ||
| }; | ||
| }; | ||
|
|
||
| function resolveMonitorConfig( | ||
| monitorSlug: string, | ||
| monitorConfig: MonitorConfig | SentryCronMonitorSettings | undefined, | ||
| candidates: unknown[], | ||
| ): MonitorConfig | undefined { | ||
| if (monitorConfig?.schedule) { | ||
| return monitorConfig; | ||
| } | ||
|
|
||
| const { fromCronDecorator = true, ...monitorSettings } = monitorConfig || {}; | ||
| const cronConfig = fromCronDecorator ? getMonitorConfigFromNestCron(candidates) : undefined; | ||
|
|
||
| if (!cronConfig) { | ||
| const { isolateTrace: _isolateTrace, ...unsentSettings } = monitorSettings; | ||
| if (DEBUG_BUILD && Object.keys(unsentSettings).length) { | ||
| const reason = fromCronDecorator | ||
| ? 'no schedule could be taken from @Cron()' | ||
| : 'fromCronDecorator is false and no schedule was passed'; | ||
| debug.warn( | ||
| `[SentryCron] The monitor settings for "${monitorSlug}" are not sent, because ${reason}. A monitor config needs a schedule.`, | ||
| ); | ||
| } | ||
| return undefined; | ||
|
wedamija marked this conversation as resolved.
|
||
| } | ||
|
|
||
| return { ...monitorSettings, ...cronConfig }; | ||
| } | ||
|
|
||
| const SCHEDULE_CRON_OPTIONS = 'SCHEDULE_CRON_OPTIONS'; | ||
|
wedamija marked this conversation as resolved.
|
||
|
|
||
| // `cron` presets (case-insensitive). Sentry accepts only the first five, so the others are sent as crontabs. | ||
| const CRON_PRESETS: Record<string, string | undefined> = { | ||
| '@yearly': '@yearly', | ||
| '@monthly': '@monthly', | ||
| '@weekly': '@weekly', | ||
| '@daily': '@daily', | ||
| '@hourly': '@hourly', | ||
| '@minutely': '* * * * *', | ||
| '@weekdays': '0 0 * * 1-5', | ||
| '@weekends': '0 0 * * 0,6', | ||
| }; | ||
|
|
||
| interface NestCronOptions { | ||
| cronTime?: unknown; | ||
| timeZone?: unknown; | ||
| utcOffset?: unknown; | ||
| } | ||
|
|
||
| function getMonitorConfigFromNestCron(candidates: unknown[]): MonitorConfig | undefined { | ||
| const R = Reflect as ReflectWithMetadata; | ||
| if (typeof R.getMetadata !== 'function') { | ||
| return undefined; | ||
| } | ||
|
|
||
| for (const candidate of candidates) { | ||
| if (typeof candidate !== 'function') { | ||
| continue; | ||
| } | ||
| const cronOptions = R.getMetadata(SCHEDULE_CRON_OPTIONS, candidate); | ||
| if (cronOptions && typeof cronOptions === 'object') { | ||
| return nestCronOptionsToMonitorConfig(cronOptions, getNestScheduleVersion(cronOptions)); | ||
| } | ||
| } | ||
|
|
||
| return undefined; | ||
| } | ||
|
|
||
| /** | ||
| * Converts the options `@Cron()` stores as metadata into a Sentry monitor config. | ||
| * Returns `undefined` for schedules Sentry can't represent. | ||
| */ | ||
| function nestCronOptionsToMonitorConfig( | ||
| cronOptions: NestCronOptions, | ||
| scheduleVersion: string | undefined, | ||
| ): MonitorConfig | undefined { | ||
| const { cronTime, timeZone, utcOffset } = cronOptions; | ||
|
|
||
| // A fixed UTC offset has no IANA time zone equivalent. | ||
| if (typeof cronTime !== 'string' || utcOffset != null) { | ||
| return undefined; | ||
| } | ||
|
|
||
| const fields = cronTime.trim().split(/\s+/); | ||
| let crontab: string | undefined; | ||
| if (fields.length === 1) { | ||
| crontab = CRON_PRESETS[(fields[0] as string).toLowerCase()]; | ||
| } else if (fields.length === 5) { | ||
| crontab = isSupportedCrontab(fields, scheduleVersion) ? fields.join(' ') : undefined; | ||
| } else if (fields.length === 6 && /^\d+$/.test(fields[0] as string)) { | ||
| // Sentry schedules have minute granularity, so only a fixed second can be dropped. | ||
| crontab = isSupportedCrontab(fields.slice(1), scheduleVersion) ? fields.slice(1).join(' ') : undefined; | ||
| } | ||
|
|
||
| if (!crontab) { | ||
| return undefined; | ||
| } | ||
|
|
||
| // Without a `timeZone`, `cron` uses the server's time zone, but Sentry would assume UTC. | ||
| const timezone = toSentryTimeZone(typeof timeZone === 'string' && timeZone ? timeZone : getLocalTimeZone()); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Without timeZone on 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
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Agreed, updated the docs pr getsentry/sentry-docs#19782 |
||
| if (!timezone) { | ||
| return undefined; | ||
| } | ||
|
|
||
| return { | ||
| schedule: { type: 'crontab', value: crontab }, | ||
| timezone, | ||
| }; | ||
| } | ||
|
|
||
| /** | ||
| * Whether Sentry reads the 5 crontab fields the same way `cron` (used by `@nestjs/schedule`) runs them. | ||
| */ | ||
| function isSupportedCrontab( | ||
| [, , dayOfMonth = '', month = '', dayOfWeek = '']: string[], | ||
| scheduleVersion: string | undefined, | ||
| ): boolean { | ||
| // `cron` 2.x (`@nestjs/schedule` 2 and 3) counts months from 0. Steps don't depend on that. | ||
| const countsMonthsFromOne = parseInt(scheduleVersion ?? '', 10) >= 4; | ||
| if (!countsMonthsFromOne && /\d/.test(month.replace(/\/\d+/g, ''))) { | ||
| return false; | ||
| } | ||
|
|
||
| const daysOfMonth = countCronValues(dayOfMonth, 1, 31); | ||
| const daysOfWeek = countCronValues(dayOfWeek, 0, 7); | ||
| if (daysOfMonth === undefined || daysOfWeek === undefined) { | ||
| return false; | ||
| } | ||
|
|
||
| // `cron` ignores a day field that covers every day (like `1-31` or `sun-sat`) and runs on either day field | ||
| // otherwise. Sentry reads full ranges as they are and needs both day fields when one starts with `*` (like `*/2`). | ||
| const allDaysOfMonth = daysOfMonth === 31; | ||
| const allDaysOfWeek = daysOfWeek === 7; | ||
| const cronRunsOnEither = !allDaysOfMonth && !allDaysOfWeek; | ||
| const sentryRunsOnEither = !dayOfMonth.startsWith('*') && !dayOfWeek.startsWith('*'); | ||
| return (allDaysOfMonth && allDaysOfWeek) || cronRunsOnEither === sentryRunsOnEither; | ||
| } | ||
|
|
||
| const DAY_NAMES: Record<string, number | undefined> = { sun: 0, mon: 1, tue: 2, wed: 3, thu: 4, fri: 5, sat: 6 }; | ||
|
|
||
| /** | ||
| * How many days a day field matches, expanded the way `cron` does it, or `undefined` if `cron` can't parse it. | ||
| * Day of week `7` is Sunday, like `0`. | ||
| */ | ||
| function countCronValues(field: string, low: number, high: number): number | undefined { | ||
| let unknownName = false; | ||
| const numericField = field.replace(/[a-z]{1,3}/gi, name => { | ||
| const value = DAY_NAMES[name.toLowerCase()]; | ||
| unknownName ||= value === undefined; | ||
| return String(value); | ||
| }); | ||
| if (unknownName) { | ||
| return undefined; | ||
| } | ||
|
|
||
| const values = new Set<number>(); | ||
| for (const part of numericField.split(',')) { | ||
| const match = part.match(/^(?:(\*)|(\d+)(?:-(\d+))?)(?:\/(\d+))?$/); | ||
| if (!match) { | ||
| return undefined; | ||
| } | ||
| const [, wildcard, from, to, step] = match; | ||
| const lower = wildcard ? low : Number(from); | ||
| const upper = wildcard || (step && !to) ? high : Number(to ?? from); | ||
| const increment = Number(step ?? 1); | ||
| if (!increment || lower < low || upper > high || lower > upper) { | ||
| return undefined; | ||
| } | ||
| for (let value = lower; value <= upper; value += increment) { | ||
| values.add(high === 7 && value === 7 ? 0 : value); | ||
| } | ||
| } | ||
| return values.size; | ||
| } | ||
|
|
||
| /** | ||
| * The canonical IANA name of the time zone, or `undefined` if Sentry won't accept it (like `UTC+3`). | ||
| */ | ||
| function toSentryTimeZone(timezone: string | undefined): string | undefined { | ||
| if (!timezone || timezone === 'Etc/Unknown' || /^(?:utc|gmt)?[+-]/i.test(timezone)) { | ||
| return undefined; | ||
| } | ||
| try { | ||
| return new Intl.DateTimeFormat('en-US', { timeZone: timezone }).resolvedOptions().timeZone || undefined; | ||
| } catch { | ||
| return undefined; | ||
| } | ||
| } | ||
|
|
||
| function getLocalTimeZone(): string | undefined { | ||
| try { | ||
| return Intl.DateTimeFormat().resolvedOptions().timeZone || undefined; | ||
| } catch { | ||
| return undefined; | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * A decorator usable to wrap arbitrary functions with spans. | ||
| */ | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When this lands, every current @SentryCron user who passes no schedule starts to upsert monitors on the next deploy. That has three effects:
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: falseopt-out.Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.