From 1aa125d38fd7d161d0d969ac5858ca0f8b1a36e5 Mon Sep 17 00:00:00 2001 From: Dan Fuller Date: Fri, 16 Oct 2026 17:50:13 -0700 Subject: [PATCH 01/10] feat(node): Add nodeScheduleIntegration to monitor node-schedule jobs Sends check-ins for every named job created with scheduleJob through the orchestrion channel injection. Cron strings (with their tz) are sent as the monitor config when Sentry reads them the same way; recurrence rules check in without a config. Unnamed and one-off jobs are skipped. Co-Authored-By: Claude --- .../node-schedule-integration/instrument.mjs | 9 +++ .../node-schedule-integration/scenario.mjs | 26 ++++++ .../cron/node-schedule-integration/test.ts | 44 +++++++++++ packages/astro/src/index.server.ts | 1 + packages/aws-serverless/src/index.ts | 1 + packages/bun/src/index.ts | 1 + packages/elysia/src/index.ts | 1 + packages/google-cloud-serverless/src/index.ts | 1 + packages/node/src/cron/node-schedule.ts | 3 + packages/node/src/index.ts | 2 + packages/remix/src/server/index.ts | 1 + packages/server-utils/src/index.ts | 2 + .../src/integrations/node-schedule.ts | 79 +++++++++++++++++++ .../server-utils/src/orchestrion/channels.ts | 2 + .../src/orchestrion/config/index.ts | 2 + .../src/orchestrion/config/node-schedule.ts | 16 ++++ packages/solidstart/src/server/index.ts | 1 + packages/sveltekit/src/server/index.ts | 1 + 18 files changed, 193 insertions(+) create mode 100644 dev-packages/node-integration-tests/suites/cron/node-schedule-integration/instrument.mjs create mode 100644 dev-packages/node-integration-tests/suites/cron/node-schedule-integration/scenario.mjs create mode 100644 dev-packages/node-integration-tests/suites/cron/node-schedule-integration/test.ts create mode 100644 packages/server-utils/src/integrations/node-schedule.ts create mode 100644 packages/server-utils/src/orchestrion/config/node-schedule.ts diff --git a/dev-packages/node-integration-tests/suites/cron/node-schedule-integration/instrument.mjs b/dev-packages/node-integration-tests/suites/cron/node-schedule-integration/instrument.mjs new file mode 100644 index 000000000000..a3011f204b5e --- /dev/null +++ b/dev-packages/node-integration-tests/suites/cron/node-schedule-integration/instrument.mjs @@ -0,0 +1,9 @@ +import * as Sentry from '@sentry/node'; +import { loggingTransport } from '@sentry-internal/node-integration-tests'; + +Sentry.init({ + dsn: 'https://public@dsn.ingest.sentry.io/1337', + release: '1.0', + transport: loggingTransport, + integrations: [Sentry.nodeScheduleIntegration({ excludeJobs: ['excluded-job'] })], +}); diff --git a/dev-packages/node-integration-tests/suites/cron/node-schedule-integration/scenario.mjs b/dev-packages/node-integration-tests/suites/cron/node-schedule-integration/scenario.mjs new file mode 100644 index 000000000000..3b05251a223d --- /dev/null +++ b/dev-packages/node-integration-tests/suites/cron/node-schedule-integration/scenario.mjs @@ -0,0 +1,26 @@ +import schedule from 'node-schedule'; + +const excluded = schedule.scheduleJob('excluded-job', '0 0 * * *', () => {}); +const unnamed = schedule.scheduleJob('0 0 * * *', () => {}); +const recurrenceRule = schedule.scheduleJob('rule-job', { hour: 3, minute: 0 }, () => {}); + +let runs = 0; +const job = schedule.scheduleJob('my-schedule-job', { rule: '30 9 * * 1-5', tz: 'Europe/Vienna' }, () => { + runs++; + if (runs > 1) { + throw new Error('Error in scheduled job'); + } +}); + +// Run the jobs without waiting for their schedule. +excluded.invoke(); +unnamed.invoke(); +recurrenceRule.invoke(); +job.invoke(); +try { + job.invoke(); +} catch { + // node-schedule catches errors when it runs the job itself +} + +setTimeout(() => process.exit(), 10_000); diff --git a/dev-packages/node-integration-tests/suites/cron/node-schedule-integration/test.ts b/dev-packages/node-integration-tests/suites/cron/node-schedule-integration/test.ts new file mode 100644 index 000000000000..2266b7e11216 --- /dev/null +++ b/dev-packages/node-integration-tests/suites/cron/node-schedule-integration/test.ts @@ -0,0 +1,44 @@ +import { afterAll, describe, expect } from 'vitest'; +import { cleanupChildProcesses, createEsmAndCjsTests } from '../../../utils/runner'; + +const MONITOR_CONFIG = { + schedule: { type: 'crontab', value: '30 9 * * 1-5' }, + timezone: 'Europe/Vienna', +}; + +describe('nodeScheduleIntegration', () => { + afterAll(() => { + cleanupChildProcesses(); + }); + + createEsmAndCjsTests(__dirname, 'scenario.mjs', 'instrument.mjs', (createRunner, test) => { + test('sends check-ins for named jobs', async () => { + await createRunner() + .expect({ check_in: { monitor_slug: 'rule-job', status: 'in_progress' } }) + .expect({ check_in: { monitor_slug: 'rule-job', status: 'ok' } }) + .expect({ + check_in: { monitor_slug: 'my-schedule-job', status: 'in_progress', monitor_config: MONITOR_CONFIG }, + }) + .expect({ check_in: { monitor_slug: 'my-schedule-job', status: 'ok', duration: expect.any(Number) } }) + .expect({ + check_in: { monitor_slug: 'my-schedule-job', status: 'in_progress', monitor_config: MONITOR_CONFIG }, + }) + .expect({ check_in: { monitor_slug: 'my-schedule-job', status: 'error', duration: expect.any(Number) } }) + .expect({ + event: { + exception: { + values: [ + { + type: 'Error', + value: 'Error in scheduled job', + mechanism: { type: 'auto.function.node-schedule', handled: false }, + }, + ], + }, + }, + }) + .start() + .completed(); + }); + }); +}); diff --git a/packages/astro/src/index.server.ts b/packages/astro/src/index.server.ts index 5bf4270cc686..69d9dee08287 100644 --- a/packages/astro/src/index.server.ts +++ b/packages/astro/src/index.server.ts @@ -48,6 +48,7 @@ export { functionToStringIntegration, genericPoolIntegration, nodeCronIntegration, + nodeScheduleIntegration, getActiveSpan, getAutoPerformanceIntegrations, getClient, diff --git a/packages/aws-serverless/src/index.ts b/packages/aws-serverless/src/index.ts index b1d9f3118ffd..8c45d2f267d9 100644 --- a/packages/aws-serverless/src/index.ts +++ b/packages/aws-serverless/src/index.ts @@ -117,6 +117,7 @@ export { fsIntegration, genericPoolIntegration, nodeCronIntegration, + nodeScheduleIntegration, graphqlIntegration, knexIntegration, kafkaIntegration, diff --git a/packages/bun/src/index.ts b/packages/bun/src/index.ts index 3fe12fa763f0..4c4e7c48b010 100644 --- a/packages/bun/src/index.ts +++ b/packages/bun/src/index.ts @@ -139,6 +139,7 @@ export { setupKoaErrorHandler, genericPoolIntegration, nodeCronIntegration, + nodeScheduleIntegration, graphqlIntegration, knexIntegration, kafkaIntegration, diff --git a/packages/elysia/src/index.ts b/packages/elysia/src/index.ts index 7ce2acd76db9..a796fc84ecc7 100644 --- a/packages/elysia/src/index.ts +++ b/packages/elysia/src/index.ts @@ -118,6 +118,7 @@ export { setupKoaErrorHandler, genericPoolIntegration, nodeCronIntegration, + nodeScheduleIntegration, graphqlIntegration, knexIntegration, kafkaIntegration, diff --git a/packages/google-cloud-serverless/src/index.ts b/packages/google-cloud-serverless/src/index.ts index d6479c4bdc7c..317424507421 100644 --- a/packages/google-cloud-serverless/src/index.ts +++ b/packages/google-cloud-serverless/src/index.ts @@ -117,6 +117,7 @@ export { firebaseIntegration, genericPoolIntegration, nodeCronIntegration, + nodeScheduleIntegration, graphqlIntegration, knexIntegration, kafkaIntegration, diff --git a/packages/node/src/cron/node-schedule.ts b/packages/node/src/cron/node-schedule.ts index f174ef7c904b..df7c775fc27b 100644 --- a/packages/node/src/cron/node-schedule.ts +++ b/packages/node/src/cron/node-schedule.ts @@ -58,6 +58,9 @@ export function instrumentNodeSchedule(lib: T & NodeSchedule): T { ); } + // Tells `nodeScheduleIntegration` that this job is already monitored. + (monitoredCallback as unknown as Record)[Symbol.for('sentry.cron.monitored')] = true; + return target.apply(thisArg, [monitorSlug, expression, monitoredCallback]); }, }); diff --git a/packages/node/src/index.ts b/packages/node/src/index.ts index a03c2953ceb5..5e4c60e9706e 100644 --- a/packages/node/src/index.ts +++ b/packages/node/src/index.ts @@ -13,6 +13,8 @@ export { genericPoolIntegration, nodeCronIntegration, type NodeCronIntegrationOptions, + nodeScheduleIntegration, + type NodeScheduleIntegrationOptions, googleGenAIIntegration, graphqlIntegration, groqIntegration, diff --git a/packages/remix/src/server/index.ts b/packages/remix/src/server/index.ts index ae0ea866bfc3..88ef6a841518 100644 --- a/packages/remix/src/server/index.ts +++ b/packages/remix/src/server/index.ts @@ -38,6 +38,7 @@ export { functionToStringIntegration, genericPoolIntegration, nodeCronIntegration, + nodeScheduleIntegration, getActiveSpan, getAutoPerformanceIntegrations, getClient, diff --git a/packages/server-utils/src/index.ts b/packages/server-utils/src/index.ts index acb0ffb47c75..005e8a4a8b79 100644 --- a/packages/server-utils/src/index.ts +++ b/packages/server-utils/src/index.ts @@ -40,6 +40,8 @@ export { dataloaderIntegration } from './integrations/dataloader'; export { genericPoolIntegration } from './integrations/generic-pool'; export { nodeCronIntegration } from './integrations/node-cron'; export type { NodeCronIntegrationOptions } from './integrations/node-cron'; +export { nodeScheduleIntegration } from './integrations/node-schedule'; +export type { NodeScheduleIntegrationOptions } from './integrations/node-schedule'; export { googleGenAIIntegration } from './integrations/google-genai'; export { graphqlIntegration } from './integrations/graphql'; export { hapiIntegration } from './integrations/hapi'; diff --git a/packages/server-utils/src/integrations/node-schedule.ts b/packages/server-utils/src/integrations/node-schedule.ts new file mode 100644 index 000000000000..7fea20bb3b7f --- /dev/null +++ b/packages/server-utils/src/integrations/node-schedule.ts @@ -0,0 +1,79 @@ +import * as diagnosticsChannel from '../utils/diagnosticsChannel'; +import type { IntegrationFn, MonitorConfig } from '@sentry/core'; +import { debug, defineIntegration, stringMatchesSomePattern } from '@sentry/core'; +import { DEBUG_BUILD } from '../debug-build'; +import { CHANNELS } from '../orchestrion/channels'; +import { nodeScheduleModuleNames } from '../orchestrion/config/node-schedule'; +import { invokeOrchestrionInstrumentation } from '../orchestrion/instrumentation'; +import type { AnyFn } from '../utils/cronMonitoring'; +import { cronExpressionToMonitorConfig, wrapScheduledJob } from '../utils/cronMonitoring'; + +const INTEGRATION_NAME = 'NodeSchedule' as const; + +// Set by `Sentry.cron.instrumentNodeSchedule`, which already sends check-ins. +const MONITORED = Symbol.for('sentry.cron.monitored'); + +const NOOP = (): void => {}; + +export interface NodeScheduleIntegrationOptions { + /** Job names not to monitor. Strings must match the name exactly. */ + excludeJobs?: Array; +} + +let options: NodeScheduleIntegrationOptions = {}; + +const _nodeScheduleIntegration = ((integrationOptions: NodeScheduleIntegrationOptions = {}) => { + return { + name: INTEGRATION_NAME, + setup(client) { + options = integrationOptions; + invokeOrchestrionInstrumentation(client, nodeScheduleModuleNames, subscribe, [], { + requiresTracingChannelBinding: false, + }); + }, + }; +}) satisfies IntegrationFn; + +/** + * Sends Crons check-ins for every named `node-schedule` job created with `scheduleJob`. + * The monitor slug is the job name; jobs without a name, and one-off jobs, are not monitored. + */ +export const nodeScheduleIntegration = defineIntegration(_nodeScheduleIntegration); + +function subscribe(): void { + diagnosticsChannel.tracingChannel<{ arguments: unknown[] }>(CHANNELS.NODE_SCHEDULE_SCHEDULE_JOB).subscribe({ + start(data) { + try { + // `scheduleJob([name], spec, method, [callback])`, like node-schedule reads it. + const args = data.arguments; + const name = args.length >= 3 && typeof args[0] === 'string' ? args[0] : undefined; + const [spec, method] = name ? [args[1], args[2]] : [args[0], args[1]]; + if ( + !name || + typeof method !== 'function' || + (method as { [MONITORED]?: boolean })[MONITORED] || + spec instanceof Date || + stringMatchesSomePattern(name, options.excludeJobs, true) + ) { + return; + } + args[2] = wrapScheduledJob(method as AnyFn, name, getMonitorConfig(spec), 'auto.function.node-schedule'); + } catch (e) { + DEBUG_BUILD && debug.error('[NodeSchedule] Failed to set up the cron monitor:', e); + } + }, + end: NOOP, + asyncStart: NOOP, + asyncEnd: NOOP, + error: NOOP, + }); +} + +// Only cron strings are sent; recurrence rules check in without a config. +function getMonitorConfig(spec: unknown): MonitorConfig | undefined { + if (typeof spec === 'string') { + return cronExpressionToMonitorConfig(spec, undefined, 'or'); + } + const { rule, tz } = (spec || {}) as { rule?: unknown; tz?: unknown }; + return typeof rule === 'string' ? cronExpressionToMonitorConfig(rule, tz, 'or') : undefined; +} diff --git a/packages/server-utils/src/orchestrion/channels.ts b/packages/server-utils/src/orchestrion/channels.ts index 81f9b34d2e27..67a88bbb3825 100644 --- a/packages/server-utils/src/orchestrion/channels.ts +++ b/packages/server-utils/src/orchestrion/channels.ts @@ -27,6 +27,7 @@ import { mysqlChannels } from './config/mysql'; import { neonChannels } from './config/neon'; import { nestjsChannels } from './config/nestjs'; import { nodeCronChannels } from './config/node-cron'; +import { nodeScheduleChannels } from './config/node-schedule'; import { openaiChannels } from './config/openai'; import { pgChannels } from './config/pg'; import { piDurableChannels } from './config/pi-durable'; @@ -85,6 +86,7 @@ export const CHANNELS = { ...neonChannels, ...nestjsChannels, ...nodeCronChannels, + ...nodeScheduleChannels, ...openaiChannels, ...pgChannels, ...piDurableChannels, diff --git a/packages/server-utils/src/orchestrion/config/index.ts b/packages/server-utils/src/orchestrion/config/index.ts index 56b7bf008c56..bf2c4b50b178 100644 --- a/packages/server-utils/src/orchestrion/config/index.ts +++ b/packages/server-utils/src/orchestrion/config/index.ts @@ -34,6 +34,7 @@ import { neonConfig } from './neon'; import { nestjsConfig } from './nestjs'; import { nitroConfig } from './nitro'; import { nodeCronConfig } from './node-cron'; +import { nodeScheduleConfig } from './node-schedule'; import { openaiConfig } from './openai'; import { pgConfig } from './pg'; import { postgresJsConfig } from './postgres'; @@ -93,6 +94,7 @@ export const SENTRY_INSTRUMENTATIONS: InstrumentationConfig[] = [ ...nestjsConfig, ...nitroConfig, ...nodeCronConfig, + ...nodeScheduleConfig, ...openaiConfig, ...pgConfig, ...piDurableConfig, diff --git a/packages/server-utils/src/orchestrion/config/node-schedule.ts b/packages/server-utils/src/orchestrion/config/node-schedule.ts new file mode 100644 index 000000000000..3dd231617e64 --- /dev/null +++ b/packages/server-utils/src/orchestrion/config/node-schedule.ts @@ -0,0 +1,16 @@ +import type { InstrumentationConfig } from '../apmTypes'; +import { getModuleNames } from './module-names'; + +export const nodeScheduleConfig = [ + { + channelName: 'scheduleJob', + module: { name: 'node-schedule', versionRange: '>=1.3.0 <3', filePath: 'lib/schedule.js' }, + functionQuery: { functionName: 'scheduleJob', kind: 'Sync' }, + }, +] satisfies InstrumentationConfig[]; + +export const nodeScheduleModuleNames = getModuleNames(nodeScheduleConfig); + +export const nodeScheduleChannels = { + NODE_SCHEDULE_SCHEDULE_JOB: 'orchestrion:node-schedule:scheduleJob', +} as const; diff --git a/packages/solidstart/src/server/index.ts b/packages/solidstart/src/server/index.ts index 90710a604eb5..2237235f5c1b 100644 --- a/packages/solidstart/src/server/index.ts +++ b/packages/solidstart/src/server/index.ts @@ -41,6 +41,7 @@ export { functionToStringIntegration, genericPoolIntegration, nodeCronIntegration, + nodeScheduleIntegration, getActiveSpan, getAutoPerformanceIntegrations, getClient, diff --git a/packages/sveltekit/src/server/index.ts b/packages/sveltekit/src/server/index.ts index 75f610ba0789..e5b73bcd2acc 100644 --- a/packages/sveltekit/src/server/index.ts +++ b/packages/sveltekit/src/server/index.ts @@ -39,6 +39,7 @@ export { functionToStringIntegration, genericPoolIntegration, nodeCronIntegration, + nodeScheduleIntegration, getActiveSpan, getAutoPerformanceIntegrations, getClient, From 7f4f833ab0a68a3c7422d98e6a506f628cf6757e Mon Sep 17 00:00:00 2001 From: Dan Fuller Date: Fri, 16 Oct 2026 17:54:10 -0700 Subject: [PATCH 02/10] test(node): Assert node-schedule check-ins and errors separately Co-Authored-By: Claude --- .../suites/cron/node-schedule-integration/test.ts | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/dev-packages/node-integration-tests/suites/cron/node-schedule-integration/test.ts b/dev-packages/node-integration-tests/suites/cron/node-schedule-integration/test.ts index 2266b7e11216..c74df35d4f21 100644 --- a/dev-packages/node-integration-tests/suites/cron/node-schedule-integration/test.ts +++ b/dev-packages/node-integration-tests/suites/cron/node-schedule-integration/test.ts @@ -14,6 +14,7 @@ describe('nodeScheduleIntegration', () => { createEsmAndCjsTests(__dirname, 'scenario.mjs', 'instrument.mjs', (createRunner, test) => { test('sends check-ins for named jobs', async () => { await createRunner() + .ignore('event') .expect({ check_in: { monitor_slug: 'rule-job', status: 'in_progress' } }) .expect({ check_in: { monitor_slug: 'rule-job', status: 'ok' } }) .expect({ @@ -24,6 +25,13 @@ describe('nodeScheduleIntegration', () => { check_in: { monitor_slug: 'my-schedule-job', status: 'in_progress', monitor_config: MONITOR_CONFIG }, }) .expect({ check_in: { monitor_slug: 'my-schedule-job', status: 'error', duration: expect.any(Number) } }) + .start() + .completed(); + }); + + test('captures job errors', async () => { + await createRunner() + .unordered() .expect({ event: { exception: { From 231f7bca351773a58ee4cff458b68de3ed2e7d3c Mon Sep 17 00:00:00 2001 From: Dan Fuller Date: Fri, 16 Oct 2026 18:11:28 -0700 Subject: [PATCH 03/10] test(node): Skip node-schedule integration suite on bun run Co-Authored-By: Claude --- dev-packages/bun-integration-tests/node-suites/excludes.ts | 1 + .../test/orchestrion/moduleInjectedTransform.test.ts | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/dev-packages/bun-integration-tests/node-suites/excludes.ts b/dev-packages/bun-integration-tests/node-suites/excludes.ts index 5d88d630c7c0..e38eeeb56384 100644 --- a/dev-packages/bun-integration-tests/node-suites/excludes.ts +++ b/dev-packages/bun-integration-tests/node-suites/excludes.ts @@ -63,6 +63,7 @@ const NO_FETCH_INSTRUMENTATION = [ // See https://github.com/getsentry/sentry-javascript/issues/23882 export const NO_AUTO_INSTRUMENTATION = [ 'suites/cron/node-cron/integration/test.ts', + 'suites/cron/node-schedule-integration/test.ts', 'suites/express/**', 'suites/fs-instrumentation/test.ts', 'suites/hono/test.ts', diff --git a/packages/server-utils/test/orchestrion/moduleInjectedTransform.test.ts b/packages/server-utils/test/orchestrion/moduleInjectedTransform.test.ts index 1697ea30d7c0..ed55b1bec34e 100644 --- a/packages/server-utils/test/orchestrion/moduleInjectedTransform.test.ts +++ b/packages/server-utils/test/orchestrion/moduleInjectedTransform.test.ts @@ -44,7 +44,7 @@ describe('channel integration definitions', () => { // `@nestjs/*` and `@remix-run/*` are covered by listeners in their own SDK packages. // Crons integrations are opt-in, so bundler-only SDKs must not install them for every app. - const optIn = new Set(['node-cron']); + const optIn = new Set(['node-cron', 'node-schedule']); const uncovered = [...configured].filter( name => !defined.has(name) && !optIn.has(name) && !name.startsWith('@nestjs/') && !name.startsWith('@remix-run/'), ); From 4f360eb79b1ff6bf0a6220cfe09941a6be744e50 Mon Sep 17 00:00:00 2001 From: Dan Fuller Date: Tue, 20 Oct 2026 10:47:12 -0700 Subject: [PATCH 04/10] fix(node): Send no monitor config for node-schedule rules with a start or end A rule limited by `start` or `end` doesn't run outside that window, so sending its schedule would report missed check-ins. Also uses the shared job-creation helpers. Co-Authored-By: Claude --- .../node-schedule-integration/scenario.mjs | 2 + .../cron/node-schedule-integration/test.ts | 10 +++- .../src/integrations/node-schedule.ts | 59 +++++++------------ 3 files changed, 32 insertions(+), 39 deletions(-) diff --git a/dev-packages/node-integration-tests/suites/cron/node-schedule-integration/scenario.mjs b/dev-packages/node-integration-tests/suites/cron/node-schedule-integration/scenario.mjs index 3b05251a223d..de750a422252 100644 --- a/dev-packages/node-integration-tests/suites/cron/node-schedule-integration/scenario.mjs +++ b/dev-packages/node-integration-tests/suites/cron/node-schedule-integration/scenario.mjs @@ -3,6 +3,7 @@ import schedule from 'node-schedule'; const excluded = schedule.scheduleJob('excluded-job', '0 0 * * *', () => {}); const unnamed = schedule.scheduleJob('0 0 * * *', () => {}); const recurrenceRule = schedule.scheduleJob('rule-job', { hour: 3, minute: 0 }, () => {}); +const ending = schedule.scheduleJob('ending-job', { rule: '0 0 * * *', end: Date.now() + 86_400_000 }, () => {}); let runs = 0; const job = schedule.scheduleJob('my-schedule-job', { rule: '30 9 * * 1-5', tz: 'Europe/Vienna' }, () => { @@ -16,6 +17,7 @@ const job = schedule.scheduleJob('my-schedule-job', { rule: '30 9 * * 1-5', tz: excluded.invoke(); unnamed.invoke(); recurrenceRule.invoke(); +ending.invoke(); job.invoke(); try { job.invoke(); diff --git a/dev-packages/node-integration-tests/suites/cron/node-schedule-integration/test.ts b/dev-packages/node-integration-tests/suites/cron/node-schedule-integration/test.ts index c74df35d4f21..61e9d261718c 100644 --- a/dev-packages/node-integration-tests/suites/cron/node-schedule-integration/test.ts +++ b/dev-packages/node-integration-tests/suites/cron/node-schedule-integration/test.ts @@ -1,3 +1,4 @@ +import type { SerializedCheckIn } from '@sentry/core'; import { afterAll, describe, expect } from 'vitest'; import { cleanupChildProcesses, createEsmAndCjsTests } from '../../../utils/runner'; @@ -6,6 +7,11 @@ const MONITOR_CONFIG = { timezone: 'Europe/Vienna', }; +const expectNoConfig = (slug: string) => (checkIn: SerializedCheckIn) => { + expect(checkIn).toMatchObject({ monitor_slug: slug, status: 'in_progress' }); + expect(checkIn.monitor_config).toBeUndefined(); +}; + describe('nodeScheduleIntegration', () => { afterAll(() => { cleanupChildProcesses(); @@ -15,8 +21,10 @@ describe('nodeScheduleIntegration', () => { test('sends check-ins for named jobs', async () => { await createRunner() .ignore('event') - .expect({ check_in: { monitor_slug: 'rule-job', status: 'in_progress' } }) + .expect({ check_in: expectNoConfig('rule-job') }) .expect({ check_in: { monitor_slug: 'rule-job', status: 'ok' } }) + .expect({ check_in: expectNoConfig('ending-job') }) + .expect({ check_in: { monitor_slug: 'ending-job', status: 'ok' } }) .expect({ check_in: { monitor_slug: 'my-schedule-job', status: 'in_progress', monitor_config: MONITOR_CONFIG }, }) diff --git a/packages/server-utils/src/integrations/node-schedule.ts b/packages/server-utils/src/integrations/node-schedule.ts index 7fea20bb3b7f..6e139a84849d 100644 --- a/packages/server-utils/src/integrations/node-schedule.ts +++ b/packages/server-utils/src/integrations/node-schedule.ts @@ -1,20 +1,18 @@ -import * as diagnosticsChannel from '../utils/diagnosticsChannel'; import type { IntegrationFn, MonitorConfig } from '@sentry/core'; -import { debug, defineIntegration, stringMatchesSomePattern } from '@sentry/core'; -import { DEBUG_BUILD } from '../debug-build'; +import { defineIntegration } from '@sentry/core'; import { CHANNELS } from '../orchestrion/channels'; import { nodeScheduleModuleNames } from '../orchestrion/config/node-schedule'; import { invokeOrchestrionInstrumentation } from '../orchestrion/instrumentation'; import type { AnyFn } from '../utils/cronMonitoring'; -import { cronExpressionToMonitorConfig, wrapScheduledJob } from '../utils/cronMonitoring'; +import { + cronExpressionToMonitorConfig, + shouldMonitorJob, + subscribeToJobCreation, + wrapScheduledJob, +} from '../utils/cronMonitoring'; const INTEGRATION_NAME = 'NodeSchedule' as const; -// Set by `Sentry.cron.instrumentNodeSchedule`, which already sends check-ins. -const MONITORED = Symbol.for('sentry.cron.monitored'); - -const NOOP = (): void => {}; - export interface NodeScheduleIntegrationOptions { /** Job names not to monitor. Strings must match the name exactly. */ excludeJobs?: Array; @@ -41,39 +39,24 @@ const _nodeScheduleIntegration = ((integrationOptions: NodeScheduleIntegrationOp export const nodeScheduleIntegration = defineIntegration(_nodeScheduleIntegration); function subscribe(): void { - diagnosticsChannel.tracingChannel<{ arguments: unknown[] }>(CHANNELS.NODE_SCHEDULE_SCHEDULE_JOB).subscribe({ - start(data) { - try { - // `scheduleJob([name], spec, method, [callback])`, like node-schedule reads it. - const args = data.arguments; - const name = args.length >= 3 && typeof args[0] === 'string' ? args[0] : undefined; - const [spec, method] = name ? [args[1], args[2]] : [args[0], args[1]]; - if ( - !name || - typeof method !== 'function' || - (method as { [MONITORED]?: boolean })[MONITORED] || - spec instanceof Date || - stringMatchesSomePattern(name, options.excludeJobs, true) - ) { - return; - } - args[2] = wrapScheduledJob(method as AnyFn, name, getMonitorConfig(spec), 'auto.function.node-schedule'); - } catch (e) { - DEBUG_BUILD && debug.error('[NodeSchedule] Failed to set up the cron monitor:', e); - } - }, - end: NOOP, - asyncStart: NOOP, - asyncEnd: NOOP, - error: NOOP, - }); + subscribeToJobCreation(CHANNELS.NODE_SCHEDULE_SCHEDULE_JOB, INTEGRATION_NAME, wrapJob); +} + +// `scheduleJob([name], spec, method, [callback])`, like node-schedule reads it. +function wrapJob(args: unknown[]): void { + const name = args.length >= 3 && typeof args[0] === 'string' ? args[0] : undefined; + const [spec, method] = name ? [args[1], args[2]] : [args[0], args[1]]; + if (spec instanceof Date || !shouldMonitorJob(name, method, options.excludeJobs)) { + return; + } + args[2] = wrapScheduledJob(method as AnyFn, name, getMonitorConfig(spec), 'auto.function.node-schedule'); } -// Only cron strings are sent; recurrence rules check in without a config. +// Only cron strings are sent; recurrence rules and rules limited by `start` or `end` check in without a config. function getMonitorConfig(spec: unknown): MonitorConfig | undefined { if (typeof spec === 'string') { return cronExpressionToMonitorConfig(spec, undefined, 'or'); } - const { rule, tz } = (spec || {}) as { rule?: unknown; tz?: unknown }; - return typeof rule === 'string' ? cronExpressionToMonitorConfig(rule, tz, 'or') : undefined; + const { rule, tz, start, end } = (spec || {}) as { rule?: unknown; tz?: unknown; start?: unknown; end?: unknown }; + return typeof rule === 'string' && !start && !end ? cronExpressionToMonitorConfig(rule, tz, 'or') : undefined; } From 8184b788f29f825d7a5bb76461c2859e158a6c27 Mon Sep 17 00:00:00 2001 From: Dan Fuller Date: Wed, 21 Oct 2026 08:59:54 -0700 Subject: [PATCH 05/10] ref(node): Mark monitored node-schedule callbacks without a double cast Co-Authored-By: Claude --- packages/node/src/cron/node-schedule.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/node/src/cron/node-schedule.ts b/packages/node/src/cron/node-schedule.ts index df7c775fc27b..548f60ac48f8 100644 --- a/packages/node/src/cron/node-schedule.ts +++ b/packages/node/src/cron/node-schedule.ts @@ -1,4 +1,4 @@ -import { withMonitor } from '@sentry/core'; +import { addNonEnumerableProperty, withMonitor } from '@sentry/core'; import { replaceCronNames } from './common'; export interface NodeSchedule { @@ -59,7 +59,7 @@ export function instrumentNodeSchedule(lib: T & NodeSchedule): T { } // Tells `nodeScheduleIntegration` that this job is already monitored. - (monitoredCallback as unknown as Record)[Symbol.for('sentry.cron.monitored')] = true; + addNonEnumerableProperty(monitoredCallback, Symbol.for('sentry.cron.monitored'), true); return target.apply(thisArg, [monitorSlug, expression, monitoredCallback]); }, From 6f712c71c59c3688381b96286056fc7480181b0b Mon Sep 17 00:00:00 2001 From: Dan Fuller Date: Wed, 21 Oct 2026 17:29:23 -0700 Subject: [PATCH 06/10] fix(node): Don't monitor one-off node-schedule jobs from timestamps or date strings Co-Authored-By: Claude --- .../node-schedule-integration/scenario.mjs | 2 + .../src/integrations/node-schedule.ts | 23 ++++++++- .../test/integrations/node-schedule.test.ts | 51 +++++++++++++++++++ 3 files changed, 74 insertions(+), 2 deletions(-) create mode 100644 packages/server-utils/test/integrations/node-schedule.test.ts diff --git a/dev-packages/node-integration-tests/suites/cron/node-schedule-integration/scenario.mjs b/dev-packages/node-integration-tests/suites/cron/node-schedule-integration/scenario.mjs index de750a422252..70393dfa602f 100644 --- a/dev-packages/node-integration-tests/suites/cron/node-schedule-integration/scenario.mjs +++ b/dev-packages/node-integration-tests/suites/cron/node-schedule-integration/scenario.mjs @@ -2,6 +2,7 @@ import schedule from 'node-schedule'; const excluded = schedule.scheduleJob('excluded-job', '0 0 * * *', () => {}); const unnamed = schedule.scheduleJob('0 0 * * *', () => {}); +const oneOff = schedule.scheduleJob('one-off-job', Date.now() + 86_400_000, () => {}); const recurrenceRule = schedule.scheduleJob('rule-job', { hour: 3, minute: 0 }, () => {}); const ending = schedule.scheduleJob('ending-job', { rule: '0 0 * * *', end: Date.now() + 86_400_000 }, () => {}); @@ -16,6 +17,7 @@ const job = schedule.scheduleJob('my-schedule-job', { rule: '30 9 * * 1-5', tz: // Run the jobs without waiting for their schedule. excluded.invoke(); unnamed.invoke(); +oneOff.invoke(); recurrenceRule.invoke(); ending.invoke(); job.invoke(); diff --git a/packages/server-utils/src/integrations/node-schedule.ts b/packages/server-utils/src/integrations/node-schedule.ts index 6e139a84849d..592bcf4b851f 100644 --- a/packages/server-utils/src/integrations/node-schedule.ts +++ b/packages/server-utils/src/integrations/node-schedule.ts @@ -46,14 +46,33 @@ function subscribe(): void { function wrapJob(args: unknown[]): void { const name = args.length >= 3 && typeof args[0] === 'string' ? args[0] : undefined; const [spec, method] = name ? [args[1], args[2]] : [args[0], args[1]]; - if (spec instanceof Date || !shouldMonitorJob(name, method, options.excludeJobs)) { + if (isOneOffSpec(spec) || !shouldMonitorJob(name, method, options.excludeJobs)) { return; } args[2] = wrapScheduledJob(method as AnyFn, name, getMonitorConfig(spec), 'auto.function.node-schedule'); } +/** Whether node-schedule runs a job with this spec once: a `Date`, a timestamp, or a date string. */ +export function isOneOffSpec(spec: unknown): boolean { + const { rule } = (spec || {}) as { rule?: unknown }; + const when = typeof spec === 'object' && rule ? rule : spec; + if (typeof when === 'string') { + // `Date.parse` also accepts some crons, like `0 9 * * *`. + return !isCronShaped(when) && !Number.isNaN(Date.parse(when)); + } + return when instanceof Date || typeof when === 'number'; +} + +function isCronShaped(value: string): boolean { + const fields = value.trim().split(/\s+/); + return ( + /^@[a-z]+$/i.test(value.trim()) || + ((fields.length === 5 || fields.length === 6) && fields.every(field => /^[\d*,/?#a-z-]+$/i.test(field))) + ); +} + // Only cron strings are sent; recurrence rules and rules limited by `start` or `end` check in without a config. -function getMonitorConfig(spec: unknown): MonitorConfig | undefined { +export function getMonitorConfig(spec: unknown): MonitorConfig | undefined { if (typeof spec === 'string') { return cronExpressionToMonitorConfig(spec, undefined, 'or'); } diff --git a/packages/server-utils/test/integrations/node-schedule.test.ts b/packages/server-utils/test/integrations/node-schedule.test.ts new file mode 100644 index 000000000000..cc2fdb957a50 --- /dev/null +++ b/packages/server-utils/test/integrations/node-schedule.test.ts @@ -0,0 +1,51 @@ +import { describe, expect, it } from 'vitest'; +import { getMonitorConfig, isOneOffSpec } from '../../src/integrations/node-schedule'; + +describe('isOneOffSpec', () => { + it.each([ + ['a Date', new Date()], + ['a timestamp', 1_793_527_200_000], + ['a date string', '2026-11-01T10:00:00Z'], + ['a long date string', 'Sun Nov 01 2026 10:00:00 GMT+0000'], + ['a Date rule', { rule: new Date(), tz: 'UTC' }], + ])('is true for %s', (_, spec) => { + expect(isOneOffSpec(spec)).toBe(true); + }); + + it.each([ + ['a cron string', '0 9 * * *'], + ['a cron with names', '0 9 * jan mon-fri'], + ['a cron with seconds', '*/10 * * * * *'], + ['a nickname', '@daily'], + ['a cron rule', { rule: '0 9 * * *', tz: 'UTC' }], + ['a recurrence rule', { hour: 3, minute: 0 }], + ])('is false for %s', (_, spec) => { + expect(isOneOffSpec(spec)).toBe(false); + }); +}); + +describe('getMonitorConfig', () => { + it('sends a cron rule with its time zone', () => { + expect(getMonitorConfig({ rule: '30 9 * * 1-5', tz: 'Europe/Vienna' })).toEqual({ + schedule: { type: 'crontab', value: '30 9 * * 1-5' }, + timezone: 'Europe/Vienna', + }); + }); + + it('drops a fixed second', () => { + expect(getMonitorConfig({ rule: '0 30 9 * * *', tz: 'UTC' })?.schedule).toEqual({ + type: 'crontab', + value: '30 9 * * *', + }); + }); + + it.each([ + ['a sub-minute cron', '*/10 * * * * *'], + ['a rule with a start', { rule: '0 9 * * *', start: Date.now() + 1000 }], + ['a rule with an end', { rule: '0 9 * * *', end: Date.now() + 1000 }], + ['a recurrence rule', { hour: 3, minute: 0 }], + ['a UTC offset', { rule: '0 9 * * *', tz: '+02:00' }], + ])('is undefined for %s', (_, spec) => { + expect(getMonitorConfig(spec)).toBeUndefined(); + }); +}); From afaaa48f182a4199141abee9a850e405f43d6551 Mon Sep 17 00:00:00 2001 From: Dan Fuller Date: Fri, 23 Oct 2026 09:54:54 -0700 Subject: [PATCH 07/10] ref(node): Pass nodeScheduleIntegration options to the subscriber --- .../server-utils/src/integrations/node-schedule.ts | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/packages/server-utils/src/integrations/node-schedule.ts b/packages/server-utils/src/integrations/node-schedule.ts index 592bcf4b851f..4c3011808d6c 100644 --- a/packages/server-utils/src/integrations/node-schedule.ts +++ b/packages/server-utils/src/integrations/node-schedule.ts @@ -18,14 +18,11 @@ export interface NodeScheduleIntegrationOptions { excludeJobs?: Array; } -let options: NodeScheduleIntegrationOptions = {}; - -const _nodeScheduleIntegration = ((integrationOptions: NodeScheduleIntegrationOptions = {}) => { +const _nodeScheduleIntegration = ((options: NodeScheduleIntegrationOptions = {}) => { return { name: INTEGRATION_NAME, setup(client) { - options = integrationOptions; - invokeOrchestrionInstrumentation(client, nodeScheduleModuleNames, subscribe, [], { + invokeOrchestrionInstrumentation(client, nodeScheduleModuleNames, subscribe, [options], { requiresTracingChannelBinding: false, }); }, @@ -38,12 +35,12 @@ const _nodeScheduleIntegration = ((integrationOptions: NodeScheduleIntegrationOp */ export const nodeScheduleIntegration = defineIntegration(_nodeScheduleIntegration); -function subscribe(): void { - subscribeToJobCreation(CHANNELS.NODE_SCHEDULE_SCHEDULE_JOB, INTEGRATION_NAME, wrapJob); +function subscribe(options: NodeScheduleIntegrationOptions): void { + subscribeToJobCreation(CHANNELS.NODE_SCHEDULE_SCHEDULE_JOB, INTEGRATION_NAME, args => wrapJob(args, options)); } // `scheduleJob([name], spec, method, [callback])`, like node-schedule reads it. -function wrapJob(args: unknown[]): void { +function wrapJob(args: unknown[], options: NodeScheduleIntegrationOptions): void { const name = args.length >= 3 && typeof args[0] === 'string' ? args[0] : undefined; const [spec, method] = name ? [args[1], args[2]] : [args[0], args[1]]; if (isOneOffSpec(spec) || !shouldMonitorJob(name, method, options.excludeJobs)) { From 82b2bffea5bd2813c02446aa57b26b17522cc183 Mon Sep 17 00:00:00 2001 From: Dan Fuller Date: Wed, 28 Oct 2026 09:51:59 -0700 Subject: [PATCH 08/10] ref(node): Export nodeScheduleIntegration from Deno --- packages/deno/src/index.ts | 1 + packages/server-utils/src/utils/cronMonitoring.ts | 4 ++-- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/packages/deno/src/index.ts b/packages/deno/src/index.ts index b22aac2bdf6c..8d922f286434 100644 --- a/packages/deno/src/index.ts +++ b/packages/deno/src/index.ts @@ -184,6 +184,7 @@ export { postgresJsIntegration, neonIntegration, nodeCronIntegration, + nodeScheduleIntegration, tediousIntegration, eveConversationHook, eveIntegration, diff --git a/packages/server-utils/src/utils/cronMonitoring.ts b/packages/server-utils/src/utils/cronMonitoring.ts index 09c532449814..c6471668f4dd 100644 --- a/packages/server-utils/src/utils/cronMonitoring.ts +++ b/packages/server-utils/src/utils/cronMonitoring.ts @@ -5,7 +5,7 @@ import * as diagnosticsChannel from './diagnosticsChannel'; export type AnyFn = (this: unknown, ...args: unknown[]) => unknown; -// Set by `Sentry.cron.instrumentNodeCron`, which already sends check-ins. +// Set by `Sentry.cron.instrumentNodeCron` and `Sentry.cron.instrumentNodeSchedule`, which already send check-ins. const MONITORED = Symbol.for('sentry.cron.monitored'); /** @@ -26,7 +26,7 @@ export function subscribeToJobCreation( }); } -/** Whether a job is named, not excluded, and not already monitored by `Sentry.cron.instrumentNodeCron`. */ +/** Whether a job is named, not excluded, and not already monitored by `Sentry.cron.instrumentNodeCron` or `instrumentNodeSchedule`. */ export function shouldMonitorJob( name: unknown, job: unknown, From 0af113c30c6506d7715550b47387205a838ff7d2 Mon Sep 17 00:00:00 2001 From: Dan Fuller Date: Wed, 28 Oct 2026 10:34:24 -0700 Subject: [PATCH 09/10] fix(node): Send the new schedule after a node-schedule job is rescheduled nodeScheduleIntegration kept sending the config from scheduleJob after rescheduleJob or job.reschedule, so Sentry reverted the monitor and reported false misses. Update the job's config when a reschedule succeeds. --- .../node-schedule-integration/scenario.mjs | 4 +- .../cron/node-schedule-integration/test.ts | 8 +++ .../src/integrations/node-schedule.ts | 13 ++++- .../src/orchestrion/config/node-schedule.ts | 16 ++++++ .../server-utils/src/utils/cronMonitoring.ts | 40 ++++++++++++++- .../test/integrations/node-schedule.test.ts | 51 ++++++++++++++++++- 6 files changed, 126 insertions(+), 6 deletions(-) diff --git a/dev-packages/node-integration-tests/suites/cron/node-schedule-integration/scenario.mjs b/dev-packages/node-integration-tests/suites/cron/node-schedule-integration/scenario.mjs index 70393dfa602f..a8cdd91ed74a 100644 --- a/dev-packages/node-integration-tests/suites/cron/node-schedule-integration/scenario.mjs +++ b/dev-packages/node-integration-tests/suites/cron/node-schedule-integration/scenario.mjs @@ -9,7 +9,7 @@ const ending = schedule.scheduleJob('ending-job', { rule: '0 0 * * *', end: Date let runs = 0; const job = schedule.scheduleJob('my-schedule-job', { rule: '30 9 * * 1-5', tz: 'Europe/Vienna' }, () => { runs++; - if (runs > 1) { + if (runs === 2) { throw new Error('Error in scheduled job'); } }); @@ -26,5 +26,7 @@ try { } catch { // node-schedule catches errors when it runs the job itself } +schedule.rescheduleJob(job, { rule: '0 8 * * *', tz: 'UTC' }); +job.invoke(); setTimeout(() => process.exit(), 10_000); diff --git a/dev-packages/node-integration-tests/suites/cron/node-schedule-integration/test.ts b/dev-packages/node-integration-tests/suites/cron/node-schedule-integration/test.ts index 61e9d261718c..dbe3c9ac5fd7 100644 --- a/dev-packages/node-integration-tests/suites/cron/node-schedule-integration/test.ts +++ b/dev-packages/node-integration-tests/suites/cron/node-schedule-integration/test.ts @@ -33,6 +33,14 @@ describe('nodeScheduleIntegration', () => { check_in: { monitor_slug: 'my-schedule-job', status: 'in_progress', monitor_config: MONITOR_CONFIG }, }) .expect({ check_in: { monitor_slug: 'my-schedule-job', status: 'error', duration: expect.any(Number) } }) + .expect({ + check_in: { + monitor_slug: 'my-schedule-job', + status: 'in_progress', + monitor_config: { schedule: { type: 'crontab', value: '0 8 * * *' }, timezone: 'UTC' }, + }, + }) + .expect({ check_in: { monitor_slug: 'my-schedule-job', status: 'ok', duration: expect.any(Number) } }) .start() .completed(); }); diff --git a/packages/server-utils/src/integrations/node-schedule.ts b/packages/server-utils/src/integrations/node-schedule.ts index 4c3011808d6c..1c532c2fb0ba 100644 --- a/packages/server-utils/src/integrations/node-schedule.ts +++ b/packages/server-utils/src/integrations/node-schedule.ts @@ -3,11 +3,13 @@ import { defineIntegration } from '@sentry/core'; import { CHANNELS } from '../orchestrion/channels'; import { nodeScheduleModuleNames } from '../orchestrion/config/node-schedule'; import { invokeOrchestrionInstrumentation } from '../orchestrion/instrumentation'; -import type { AnyFn } from '../utils/cronMonitoring'; +import type { AnyFn, ScheduleChange } from '../utils/cronMonitoring'; import { cronExpressionToMonitorConfig, shouldMonitorJob, subscribeToJobCreation, + subscribeToScheduleChange, + updateMonitorConfig, wrapScheduledJob, } from '../utils/cronMonitoring'; @@ -37,6 +39,15 @@ export const nodeScheduleIntegration = defineIntegration(_nodeScheduleIntegratio function subscribe(options: NodeScheduleIntegrationOptions): void { subscribeToJobCreation(CHANNELS.NODE_SCHEDULE_SCHEDULE_JOB, INTEGRATION_NAME, args => wrapJob(args, options)); + subscribeToScheduleChange(CHANNELS.NODE_SCHEDULE_RESCHEDULE, INTEGRATION_NAME, onReschedule); +} + +/** Sends the new spec's config after `job.reschedule(spec)`, which `rescheduleJob` calls. */ +export function onReschedule({ arguments: [spec], self, result }: ScheduleChange): void { + // node-schedule keeps the old schedule when this returns false. + if (result === true) { + updateMonitorConfig((self as { job?: unknown } | undefined)?.job, getMonitorConfig(spec)); + } } // `scheduleJob([name], spec, method, [callback])`, like node-schedule reads it. diff --git a/packages/server-utils/src/orchestrion/config/node-schedule.ts b/packages/server-utils/src/orchestrion/config/node-schedule.ts index 3dd231617e64..8ed2398df597 100644 --- a/packages/server-utils/src/orchestrion/config/node-schedule.ts +++ b/packages/server-utils/src/orchestrion/config/node-schedule.ts @@ -1,16 +1,32 @@ import type { InstrumentationConfig } from '../apmTypes'; import { getModuleNames } from './module-names'; +// `Job` sets `reschedule` on each instance in its constructor. It moved from `lib/schedule.js` to `lib/Job.js` in 2.0.0. +const reschedule = { + channelName: 'reschedule', + astQuery: "AssignmentExpression[left.property.name='reschedule'] > FunctionExpression", + functionQuery: { kind: 'Sync' }, +} as const; + export const nodeScheduleConfig = [ { channelName: 'scheduleJob', module: { name: 'node-schedule', versionRange: '>=1.3.0 <3', filePath: 'lib/schedule.js' }, functionQuery: { functionName: 'scheduleJob', kind: 'Sync' }, }, + { + ...reschedule, + module: { name: 'node-schedule', versionRange: '>=1.3.0 <2', filePath: 'lib/schedule.js' }, + }, + { + ...reschedule, + module: { name: 'node-schedule', versionRange: '>=2.0.0 <3', filePath: 'lib/Job.js' }, + }, ] satisfies InstrumentationConfig[]; export const nodeScheduleModuleNames = getModuleNames(nodeScheduleConfig); export const nodeScheduleChannels = { NODE_SCHEDULE_SCHEDULE_JOB: 'orchestrion:node-schedule:scheduleJob', + NODE_SCHEDULE_RESCHEDULE: 'orchestrion:node-schedule:reschedule', } as const; diff --git a/packages/server-utils/src/utils/cronMonitoring.ts b/packages/server-utils/src/utils/cronMonitoring.ts index c6471668f4dd..8debd6d6ff95 100644 --- a/packages/server-utils/src/utils/cronMonitoring.ts +++ b/packages/server-utils/src/utils/cronMonitoring.ts @@ -26,6 +26,31 @@ export function subscribeToJobCreation( }); } +/** A call on a schedule-change channel: its arguments, the job it was called on, and what it returned. */ +export interface ScheduleChange { + arguments: unknown[]; + self?: unknown; + result?: unknown; +} + +/** + * Calls `onChange` after each call on a scheduler's schedule-change channel, so it can update the job's monitor config. + * Errors are logged and the job keeps its config. + */ +export function subscribeToScheduleChange( + channelName: string, + integrationName: string, + onChange: (change: ScheduleChange) => void, +): void { + diagnosticsChannel.tracingChannel(channelName).end.subscribe(message => { + try { + onChange(message as ScheduleChange); + } catch (e) { + DEBUG_BUILD && debug.error(`[${integrationName}] Failed to update the cron monitor:`, e); + } + }); +} + /** Whether a job is named, not excluded, and not already monitored by `Sentry.cron.instrumentNodeCron` or `instrumentNodeSchedule`. */ export function shouldMonitorJob( name: unknown, @@ -134,6 +159,8 @@ function toSentryTimezone(timezone: string | undefined): string | undefined { } } +const monitorConfigs = new WeakMap(); + /** * Wraps a scheduled job with check-ins. Errors are captured, because schedulers catch them, and rethrown. */ @@ -148,7 +175,7 @@ export function wrapScheduledJob( throw error; }; - return function (this: unknown, ...args: unknown[]): unknown { + const wrapped = function (this: unknown, ...args: unknown[]): unknown { return withMonitor( monitorSlug, () => { @@ -159,7 +186,16 @@ export function wrapScheduledJob( return captureAndRethrow(e); } }, - monitorConfig, + monitorConfigs.get(wrapped), ); }; + monitorConfigs.set(wrapped, monitorConfig); + return wrapped; +} + +/** Sets the monitor config a job from `wrapScheduledJob` sends on its next runs. Other functions are ignored. */ +export function updateMonitorConfig(job: unknown, monitorConfig: MonitorConfig | undefined): void { + if (typeof job === 'function' && monitorConfigs.has(job as AnyFn)) { + monitorConfigs.set(job as AnyFn, monitorConfig); + } } diff --git a/packages/server-utils/test/integrations/node-schedule.test.ts b/packages/server-utils/test/integrations/node-schedule.test.ts index cc2fdb957a50..c5a6d5339594 100644 --- a/packages/server-utils/test/integrations/node-schedule.test.ts +++ b/packages/server-utils/test/integrations/node-schedule.test.ts @@ -1,5 +1,9 @@ -import { describe, expect, it } from 'vitest'; -import { getMonitorConfig, isOneOffSpec } from '../../src/integrations/node-schedule'; +import type { MonitorConfig } from '@sentry/core'; +import { setCurrentClient } from '@sentry/core'; +import { beforeEach, describe, expect, it } from 'vitest'; +import { getMonitorConfig, isOneOffSpec, onReschedule } from '../../src/integrations/node-schedule'; +import { wrapScheduledJob } from '../../src/utils/cronMonitoring'; +import { getDefaultTestClientOptions, TestClient } from '../mocks/client'; describe('isOneOffSpec', () => { it.each([ @@ -49,3 +53,46 @@ describe('getMonitorConfig', () => { expect(getMonitorConfig(spec)).toBeUndefined(); }); }); + +describe('onReschedule', () => { + let sentConfigs: Array; + + beforeEach(() => { + sentConfigs = []; + const client = new TestClient(getDefaultTestClientOptions()); + client.captureCheckIn = (_checkIn, monitorConfig) => { + sentConfigs.push(monitorConfig); + return 'check-in-id'; + }; + setCurrentClient(client); + }); + + const createJob = () => ({ + job: wrapScheduledJob(() => {}, 'my-job', getMonitorConfig('0 9 * * *'), 'auto.function.node-schedule'), + }); + + it('sends the new schedule after a reschedule', () => { + const job = createJob(); + onReschedule({ arguments: [{ rule: '30 10 * * 1-5', tz: 'UTC' }], self: job, result: true }); + job.job(); + + expect(sentConfigs[0]).toEqual({ schedule: { type: 'crontab', value: '30 10 * * 1-5' }, timezone: 'UTC' }); + }); + + it('sends no config after a reschedule to a spec it cannot convert', () => { + const job = createJob(); + onReschedule({ arguments: [{ hour: 3, minute: 0 }], self: job, result: true }); + job.job(); + + expect(sentConfigs).toHaveLength(2); + expect(sentConfigs[0]).toBeUndefined(); + }); + + it('keeps the old schedule when the reschedule fails', () => { + const job = createJob(); + onReschedule({ arguments: ['not a spec'], self: job, result: false }); + job.job(); + + expect(sentConfigs[0]?.schedule).toEqual({ type: 'crontab', value: '0 9 * * *' }); + }); +}); From 754fd55f775849173130fa066256f8df220a7582 Mon Sep 17 00:00:00 2001 From: Dan Fuller Date: Wed, 28 Oct 2026 17:27:01 -0700 Subject: [PATCH 10/10] feat(node): Monitor node-schedule jobs started with new Job().schedule() Hook Job#schedule instead of scheduleJob and reschedule, since both call it. Also test that instrumentNodeSchedule marks its callback as monitored. --- .../node-schedule-integration/scenario.mjs | 3 + .../cron/node-schedule-integration/test.ts | 8 ++ packages/node/test/cron.test.ts | 9 +++ .../src/integrations/node-schedule.ts | 51 ++++++++---- .../src/orchestrion/config/node-schedule.ts | 25 +++--- .../server-utils/src/utils/cronMonitoring.ts | 20 +++-- .../test/integrations/node-schedule.test.ts | 78 ++++++++++++++++--- 7 files changed, 146 insertions(+), 48 deletions(-) diff --git a/dev-packages/node-integration-tests/suites/cron/node-schedule-integration/scenario.mjs b/dev-packages/node-integration-tests/suites/cron/node-schedule-integration/scenario.mjs index a8cdd91ed74a..7cb179a7483a 100644 --- a/dev-packages/node-integration-tests/suites/cron/node-schedule-integration/scenario.mjs +++ b/dev-packages/node-integration-tests/suites/cron/node-schedule-integration/scenario.mjs @@ -5,6 +5,8 @@ const unnamed = schedule.scheduleJob('0 0 * * *', () => {}); const oneOff = schedule.scheduleJob('one-off-job', Date.now() + 86_400_000, () => {}); const recurrenceRule = schedule.scheduleJob('rule-job', { hour: 3, minute: 0 }, () => {}); const ending = schedule.scheduleJob('ending-job', { rule: '0 0 * * *', end: Date.now() + 86_400_000 }, () => {}); +const newJob = new schedule.Job('new-job', () => {}); +newJob.schedule({ rule: '0 0 * * *', tz: 'UTC' }); let runs = 0; const job = schedule.scheduleJob('my-schedule-job', { rule: '30 9 * * 1-5', tz: 'Europe/Vienna' }, () => { @@ -20,6 +22,7 @@ unnamed.invoke(); oneOff.invoke(); recurrenceRule.invoke(); ending.invoke(); +newJob.invoke(); job.invoke(); try { job.invoke(); diff --git a/dev-packages/node-integration-tests/suites/cron/node-schedule-integration/test.ts b/dev-packages/node-integration-tests/suites/cron/node-schedule-integration/test.ts index dbe3c9ac5fd7..de5b683e9b35 100644 --- a/dev-packages/node-integration-tests/suites/cron/node-schedule-integration/test.ts +++ b/dev-packages/node-integration-tests/suites/cron/node-schedule-integration/test.ts @@ -25,6 +25,14 @@ describe('nodeScheduleIntegration', () => { .expect({ check_in: { monitor_slug: 'rule-job', status: 'ok' } }) .expect({ check_in: expectNoConfig('ending-job') }) .expect({ check_in: { monitor_slug: 'ending-job', status: 'ok' } }) + .expect({ + check_in: { + monitor_slug: 'new-job', + status: 'in_progress', + monitor_config: { schedule: { type: 'crontab', value: '0 0 * * *' }, timezone: 'UTC' }, + }, + }) + .expect({ check_in: { monitor_slug: 'new-job', status: 'ok' } }) .expect({ check_in: { monitor_slug: 'my-schedule-job', status: 'in_progress', monitor_config: MONITOR_CONFIG }, }) diff --git a/packages/node/test/cron.test.ts b/packages/node/test/cron.test.ts index bf1e83d315e5..f5178dc98e88 100644 --- a/packages/node/test/cron.test.ts +++ b/packages/node/test/cron.test.ts @@ -220,5 +220,14 @@ describe('cron check-ins', () => { "Automatic instrumentation of 'node-schedule' requires the first parameter of 'scheduleJob' to be a job name string and the second parameter to be a crontab string", ); }); + + test('marks the callback as monitored, so nodeScheduleIntegration skips it', () => { + const scheduleWithCheckIn = cron.instrumentNodeSchedule({ + scheduleJob: (_name: unknown, _expression: unknown, callback: unknown) => callback, + }); + + const callback = scheduleWithCheckIn.scheduleJob('my-cron-job', '* * * * *', () => {}) as Record; + expect(callback[Symbol.for('sentry.cron.monitored')]).toBe(true); + }); }); }); diff --git a/packages/server-utils/src/integrations/node-schedule.ts b/packages/server-utils/src/integrations/node-schedule.ts index 1c532c2fb0ba..1d86113a0706 100644 --- a/packages/server-utils/src/integrations/node-schedule.ts +++ b/packages/server-utils/src/integrations/node-schedule.ts @@ -7,7 +7,6 @@ import type { AnyFn, ScheduleChange } from '../utils/cronMonitoring'; import { cronExpressionToMonitorConfig, shouldMonitorJob, - subscribeToJobCreation, subscribeToScheduleChange, updateMonitorConfig, wrapScheduledJob, @@ -32,32 +31,52 @@ const _nodeScheduleIntegration = ((options: NodeScheduleIntegrationOptions = {}) }) satisfies IntegrationFn; /** - * Sends Crons check-ins for every named `node-schedule` job created with `scheduleJob`. + * Sends Crons check-ins for every named `node-schedule` job, whether it's created with `scheduleJob` or `new Job()`. * The monitor slug is the job name; jobs without a name, and one-off jobs, are not monitored. */ export const nodeScheduleIntegration = defineIntegration(_nodeScheduleIntegration); function subscribe(options: NodeScheduleIntegrationOptions): void { - subscribeToJobCreation(CHANNELS.NODE_SCHEDULE_SCHEDULE_JOB, INTEGRATION_NAME, args => wrapJob(args, options)); - subscribeToScheduleChange(CHANNELS.NODE_SCHEDULE_RESCHEDULE, INTEGRATION_NAME, onReschedule); + subscribeToScheduleChange(CHANNELS.NODE_SCHEDULE_SCHEDULE, INTEGRATION_NAME, onSchedule, change => + beforeSchedule(change, options), + ); } -/** Sends the new spec's config after `job.reschedule(spec)`, which `rescheduleJob` calls. */ -export function onReschedule({ arguments: [spec], self, result }: ScheduleChange): void { - // node-schedule keeps the old schedule when this returns false. - if (result === true) { - updateMonitorConfig((self as { job?: unknown } | undefined)?.job, getMonitorConfig(spec)); - } +interface Job { + name?: unknown; + job?: unknown; + nextInvocation?: () => unknown; } -// `scheduleJob([name], spec, method, [callback])`, like node-schedule reads it. -function wrapJob(args: unknown[], options: NodeScheduleIntegrationOptions): void { - const name = args.length >= 3 && typeof args[0] === 'string' ? args[0] : undefined; - const [spec, method] = name ? [args[1], args[2]] : [args[0], args[1]]; - if (isOneOffSpec(spec) || !shouldMonitorJob(name, method, options.excludeJobs)) { +// The name node-schedule gives jobs created without one. +const ANONYMOUS_NAME = /^]/; + +// `job.schedule(spec)` calls that add to runs the job already has pending, rather than replace them. +const addsToPendingRuns = new WeakSet(); + +/** Wraps a named job's method before `job.schedule(spec)` with a recurring spec, unless it's already wrapped. */ +export function beforeSchedule(change: ScheduleChange, options: NodeScheduleIntegrationOptions): void { + const job = change.self as Job | undefined; + if (!job) { + return; + } + if (typeof job.nextInvocation === 'function' && job.nextInvocation() != null) { + addsToPendingRuns.add(change); + } + const name = typeof job.name === 'string' && !ANONYMOUS_NAME.test(job.name) ? job.name : undefined; + if (isOneOffSpec(change.arguments[0]) || !shouldMonitorJob(name, job.job, options.excludeJobs)) { return; } - args[2] = wrapScheduledJob(method as AnyFn, name, getMonitorConfig(spec), 'auto.function.node-schedule'); + job.job = wrapScheduledJob(job.job as AnyFn, name, undefined, 'auto.function.node-schedule'); +} + +/** Sends the spec's config after `job.schedule(spec)`, or none when the job also keeps runs from an earlier spec. */ +export function onSchedule(change: ScheduleChange): void { + // `schedule` returns false when it scheduled nothing, and `reschedule` then keeps the old schedule. + if (change.result === true) { + const job = change.self as Job | undefined; + updateMonitorConfig(job?.job, addsToPendingRuns.has(change) ? undefined : getMonitorConfig(change.arguments[0])); + } } /** Whether node-schedule runs a job with this spec once: a `Date`, a timestamp, or a date string. */ diff --git a/packages/server-utils/src/orchestrion/config/node-schedule.ts b/packages/server-utils/src/orchestrion/config/node-schedule.ts index 8ed2398df597..acb3733a1b69 100644 --- a/packages/server-utils/src/orchestrion/config/node-schedule.ts +++ b/packages/server-utils/src/orchestrion/config/node-schedule.ts @@ -1,25 +1,23 @@ import type { InstrumentationConfig } from '../apmTypes'; import { getModuleNames } from './module-names'; -// `Job` sets `reschedule` on each instance in its constructor. It moved from `lib/schedule.js` to `lib/Job.js` in 2.0.0. -const reschedule = { - channelName: 'reschedule', - astQuery: "AssignmentExpression[left.property.name='reschedule'] > FunctionExpression", - functionQuery: { kind: 'Sync' }, +// `scheduleJob`, `rescheduleJob` and `job.reschedule` all call `job.schedule(spec)`. `Job` moved from +// `lib/schedule.js` to `lib/Job.js` in 2.0.0. +const schedule = { + channelName: 'schedule', + astQuery: + "AssignmentExpression[left.object.property.name='prototype'][left.property.name='schedule'] > FunctionExpression", + // `methodName` gives subscribers `self` before the call, not just after. `astQuery` still picks the function. + functionQuery: { methodName: 'schedule', kind: 'Sync' }, } as const; export const nodeScheduleConfig = [ { - channelName: 'scheduleJob', - module: { name: 'node-schedule', versionRange: '>=1.3.0 <3', filePath: 'lib/schedule.js' }, - functionQuery: { functionName: 'scheduleJob', kind: 'Sync' }, - }, - { - ...reschedule, + ...schedule, module: { name: 'node-schedule', versionRange: '>=1.3.0 <2', filePath: 'lib/schedule.js' }, }, { - ...reschedule, + ...schedule, module: { name: 'node-schedule', versionRange: '>=2.0.0 <3', filePath: 'lib/Job.js' }, }, ] satisfies InstrumentationConfig[]; @@ -27,6 +25,5 @@ export const nodeScheduleConfig = [ export const nodeScheduleModuleNames = getModuleNames(nodeScheduleConfig); export const nodeScheduleChannels = { - NODE_SCHEDULE_SCHEDULE_JOB: 'orchestrion:node-schedule:scheduleJob', - NODE_SCHEDULE_RESCHEDULE: 'orchestrion:node-schedule:reschedule', + NODE_SCHEDULE_SCHEDULE: 'orchestrion:node-schedule:schedule', } as const; diff --git a/packages/server-utils/src/utils/cronMonitoring.ts b/packages/server-utils/src/utils/cronMonitoring.ts index fc337255ed77..aabbbf0d9d79 100644 --- a/packages/server-utils/src/utils/cronMonitoring.ts +++ b/packages/server-utils/src/utils/cronMonitoring.ts @@ -1,5 +1,6 @@ import type { MonitorConfig } from '@sentry/core'; import { + addNonEnumerableProperty, captureException, debug, isThenable, @@ -12,7 +13,7 @@ import * as diagnosticsChannel from './diagnosticsChannel'; export type AnyFn = (this: unknown, ...args: unknown[]) => unknown; -// Set by `Sentry.cron.instrumentNodeCron` and `Sentry.cron.instrumentNodeSchedule`, which already send check-ins. +// Set by `Sentry.cron.instrumentNodeCron`, `Sentry.cron.instrumentNodeSchedule` and `wrapScheduledJob`, which already send check-ins. const MONITORED = Symbol.for('sentry.cron.monitored'); /** @@ -42,23 +43,29 @@ export interface ScheduleChange { /** * Calls `onChange` after each call on a scheduler's schedule-change channel, so it can update the job's monitor config. - * Errors are logged and the job keeps its config. + * `onStart` gets the same object before the call. Errors are logged and the job keeps its config. */ export function subscribeToScheduleChange( channelName: string, integrationName: string, onChange: (change: ScheduleChange) => void, + onStart?: (change: ScheduleChange) => void, ): void { - diagnosticsChannel.tracingChannel(channelName).end.subscribe(message => { + const channel = diagnosticsChannel.tracingChannel(channelName); + const handle = (handler: (change: ScheduleChange) => void) => (message: unknown) => { try { - onChange(message as ScheduleChange); + handler(message as ScheduleChange); } catch (e) { DEBUG_BUILD && debug.error(`[${integrationName}] Failed to update the cron monitor:`, e); } - }); + }; + if (onStart) { + channel.start.subscribe(handle(onStart)); + } + channel.end.subscribe(handle(onChange)); } -/** Whether a job is named, not excluded, and not already monitored by `Sentry.cron.instrumentNodeCron` or `instrumentNodeSchedule`. */ +/** Whether a job is named, not excluded, and not already monitored by `Sentry.cron.instrumentNode*` or `wrapScheduledJob`. */ export function shouldMonitorJob( name: unknown, job: unknown, @@ -219,6 +226,7 @@ export function wrapScheduledJob( ); }; monitorConfigs.set(wrapped, monitorConfig); + addNonEnumerableProperty(wrapped, MONITORED, true); return wrapped; } diff --git a/packages/server-utils/test/integrations/node-schedule.test.ts b/packages/server-utils/test/integrations/node-schedule.test.ts index c5a6d5339594..52e1fb586b9c 100644 --- a/packages/server-utils/test/integrations/node-schedule.test.ts +++ b/packages/server-utils/test/integrations/node-schedule.test.ts @@ -1,8 +1,7 @@ import type { MonitorConfig } from '@sentry/core'; import { setCurrentClient } from '@sentry/core'; import { beforeEach, describe, expect, it } from 'vitest'; -import { getMonitorConfig, isOneOffSpec, onReschedule } from '../../src/integrations/node-schedule'; -import { wrapScheduledJob } from '../../src/utils/cronMonitoring'; +import { beforeSchedule, getMonitorConfig, isOneOffSpec, onSchedule } from '../../src/integrations/node-schedule'; import { getDefaultTestClientOptions, TestClient } from '../mocks/client'; describe('isOneOffSpec', () => { @@ -54,7 +53,7 @@ describe('getMonitorConfig', () => { }); }); -describe('onReschedule', () => { +describe('job.schedule', () => { let sentConfigs: Array; beforeEach(() => { @@ -67,22 +66,66 @@ describe('onReschedule', () => { setCurrentClient(client); }); - const createJob = () => ({ - job: wrapScheduledJob(() => {}, 'my-job', getMonitorConfig('0 9 * * *'), 'auto.function.node-schedule'), + const createJob = (name = 'my-job') => { + let pending = false; + return { + name, + job: (() => {}) as unknown, + nextInvocation: () => (pending ? new Date() : null), + schedule(spec: unknown) { + const change = { arguments: [spec], self: this, result: undefined as unknown }; + beforeSchedule(change, { excludeJobs: ['excluded-job'] }); + change.result = spec !== 'not a spec'; + pending ||= change.result === true; + onSchedule(change); + return change.result; + }, + reschedule(spec: unknown) { + pending = false; + return this.schedule(spec); + }, + run() { + (this.job as () => void)(); + }, + }; + }; + + it('sends the schedule of a named job', () => { + const job = createJob(); + job.schedule({ rule: '30 9 * * 1-5', tz: 'UTC' }); + job.run(); + + expect(sentConfigs[0]).toEqual({ schedule: { type: 'crontab', value: '30 9 * * 1-5' }, timezone: 'UTC' }); + }); + + it.each([ + ['an unnamed job', createJob(''), '0 9 * * *'], + ['an excluded job', createJob('excluded-job'), '0 9 * * *'], + ['a one-off job', createJob(), new Date(Date.now() + 86_400_000)], + ])('does not monitor %s', (_, job, spec) => { + job.schedule(spec); + job.run(); + + expect(sentConfigs).toEqual([]); }); - it('sends the new schedule after a reschedule', () => { + it('wraps the job once and sends the new schedule after a reschedule', () => { const job = createJob(); - onReschedule({ arguments: [{ rule: '30 10 * * 1-5', tz: 'UTC' }], self: job, result: true }); - job.job(); + job.schedule('0 9 * * *'); + const wrapped = job.job; + job.reschedule({ rule: '30 10 * * 1-5', tz: 'UTC' }); + job.run(); + expect(job.job).toBe(wrapped); + expect(sentConfigs).toHaveLength(2); expect(sentConfigs[0]).toEqual({ schedule: { type: 'crontab', value: '30 10 * * 1-5' }, timezone: 'UTC' }); }); it('sends no config after a reschedule to a spec it cannot convert', () => { const job = createJob(); - onReschedule({ arguments: [{ hour: 3, minute: 0 }], self: job, result: true }); - job.job(); + job.schedule('0 9 * * *'); + job.reschedule({ hour: 3, minute: 0 }); + job.run(); expect(sentConfigs).toHaveLength(2); expect(sentConfigs[0]).toBeUndefined(); @@ -90,9 +133,20 @@ describe('onReschedule', () => { it('keeps the old schedule when the reschedule fails', () => { const job = createJob(); - onReschedule({ arguments: ['not a spec'], self: job, result: false }); - job.job(); + job.schedule('0 9 * * *'); + job.reschedule('not a spec'); + job.run(); expect(sentConfigs[0]?.schedule).toEqual({ type: 'crontab', value: '0 9 * * *' }); }); + + it('sends no config when a second schedule adds to the first', () => { + const job = createJob(); + job.schedule('0 9 * * *'); + job.schedule('0 18 * * *'); + job.run(); + + expect(sentConfigs).toHaveLength(2); + expect(sentConfigs[0]).toBeUndefined(); + }); });