Skip to content

Commit 4745454

Browse files
authored
feat(node): Instrumentation for node-schedule library (#10086)
This PR adds auto instrumented check-ins for the `node-schedule` library. It's not shown in the readme, but `scheduleJob` can be passed a job name as the first parameter: https://github.com/node-schedule/node-schedule/blob/c5a4d9a0dbcd5bda4996e089817e5669b5acd95f/lib/schedule.js#L28 ```ts import * as Sentry from '@sentry/node'; import * as schedule from 'node-schedule'; const scheduleWithCheckIn = Sentry.cron.instrumentNodeSchedule(schedule); const job = scheduleWithCheckIn.scheduleJob('my-cron-job', '* * * * *', () => { console.log('You will see this message every minute'); }); ``` This PR also adds a check to the `cron` instrumentation that ensures that you can't create multiple schedules with the same monitor slug.
1 parent 0cbdf67 commit 4745454

File tree

4 files changed

+161
-0
lines changed

4 files changed

+161
-0
lines changed

packages/node/src/cron/cron.ts

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,8 @@ const ERROR_TEXT = 'Automatic instrumentation of CronJob only supports crontab s
5656
* ```
5757
*/
5858
export function instrumentCron<T>(lib: T & CronJobConstructor, monitorSlug: string): T {
59+
let jobScheduled = false;
60+
5961
return new Proxy(lib, {
6062
construct(target, args: ConstructorParameters<CronJobConstructor>) {
6163
const [cronTime, onTick, onComplete, start, timeZone, ...rest] = args;
@@ -64,6 +66,12 @@ export function instrumentCron<T>(lib: T & CronJobConstructor, monitorSlug: stri
6466
throw new Error(ERROR_TEXT);
6567
}
6668

69+
if (jobScheduled) {
70+
throw new Error(`A job named '${monitorSlug}' has already been scheduled`);
71+
}
72+
73+
jobScheduled = true;
74+
6775
const cronString = replaceCronNames(cronTime);
6876

6977
function monitoredTick(context: unknown, onComplete?: unknown): void | Promise<void> {
@@ -90,6 +98,12 @@ export function instrumentCron<T>(lib: T & CronJobConstructor, monitorSlug: stri
9098
throw new Error(ERROR_TEXT);
9199
}
92100

101+
if (jobScheduled) {
102+
throw new Error(`A job named '${monitorSlug}' has already been scheduled`);
103+
}
104+
105+
jobScheduled = true;
106+
93107
const cronString = replaceCronNames(cronTime);
94108

95109
param.onTick = (context: unknown, onComplete?: unknown) => {
Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
import { withMonitor } from '@sentry/core';
2+
import { replaceCronNames } from './common';
3+
4+
export interface NodeSchedule {
5+
scheduleJob(
6+
nameOrExpression: string | Date | object,
7+
expressionOrCallback: string | Date | object | (() => void),
8+
callback?: () => void,
9+
): unknown;
10+
}
11+
12+
/**
13+
* Instruments the `node-schedule` library to send a check-in event to Sentry for each job execution.
14+
*
15+
* ```ts
16+
* import * as Sentry from '@sentry/node';
17+
* import * as schedule from 'node-schedule';
18+
*
19+
* const scheduleWithCheckIn = Sentry.cron.instrumentNodeSchedule(schedule);
20+
*
21+
* const job = scheduleWithCheckIn.scheduleJob('my-cron-job', '* * * * *', () => {
22+
* console.log('You will see this message every minute');
23+
* });
24+
* ```
25+
*/
26+
export function instrumentNodeSchedule<T>(lib: T & NodeSchedule): T {
27+
return new Proxy(lib, {
28+
get(target, prop: keyof NodeSchedule) {
29+
if (prop === 'scheduleJob') {
30+
// eslint-disable-next-line @typescript-eslint/unbound-method
31+
return new Proxy(target.scheduleJob, {
32+
apply(target, thisArg, argArray: Parameters<NodeSchedule['scheduleJob']>) {
33+
const [nameOrExpression, expressionOrCallback] = argArray;
34+
35+
if (typeof nameOrExpression !== 'string' || typeof expressionOrCallback !== 'string') {
36+
throw new Error(
37+
"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",
38+
);
39+
}
40+
41+
const monitorSlug = nameOrExpression;
42+
const expression = expressionOrCallback;
43+
44+
return withMonitor(
45+
monitorSlug,
46+
() => {
47+
return target.apply(thisArg, argArray);
48+
},
49+
{
50+
schedule: { type: 'crontab', value: replaceCronNames(expression) },
51+
},
52+
);
53+
},
54+
});
55+
}
56+
57+
return target[prop];
58+
},
59+
});
60+
}

packages/node/src/index.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -124,9 +124,11 @@ export { hapiErrorPlugin } from './integrations/hapi';
124124

125125
import { instrumentCron } from './cron/cron';
126126
import { instrumentNodeCron } from './cron/node-cron';
127+
import { instrumentNodeSchedule } from './cron/node-schedule';
127128

128129
/** Methods to instrument cron libraries for Sentry check-ins */
129130
export const cron = {
130131
instrumentCron,
131132
instrumentNodeCron,
133+
instrumentNodeSchedule,
132134
};

packages/node/test/cron.test.ts

Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,26 @@ describe('cron check-ins', () => {
7878
},
7979
});
8080
});
81+
82+
test('throws with multiple jobs same name', () => {
83+
const CronJobWithCheckIn = cron.instrumentCron(CronJobMock, 'my-cron-job');
84+
85+
CronJobWithCheckIn.from({
86+
cronTime: '* * * Jan,Sep Sun',
87+
onTick: () => {
88+
//
89+
},
90+
});
91+
92+
expect(() => {
93+
CronJobWithCheckIn.from({
94+
cronTime: '* * * Jan,Sep Sun',
95+
onTick: () => {
96+
//
97+
},
98+
});
99+
}).toThrowError("A job named 'my-cron-job' has already been scheduled");
100+
});
81101
});
82102

83103
describe('node-cron', () => {
@@ -125,4 +145,69 @@ describe('cron check-ins', () => {
125145
}).toThrowError('Missing "name" for scheduled job. A name is required for Sentry check-in monitoring.');
126146
});
127147
});
148+
149+
describe('node-schedule', () => {
150+
test('calls withMonitor', done => {
151+
expect.assertions(5);
152+
153+
class NodeScheduleMock {
154+
scheduleJob(
155+
nameOrExpression: string | Date | object,
156+
expressionOrCallback: string | Date | object | (() => void),
157+
callback: () => void,
158+
): unknown {
159+
expect(nameOrExpression).toBe('my-cron-job');
160+
expect(expressionOrCallback).toBe('* * * Jan,Sep Sun');
161+
expect(callback).toBeInstanceOf(Function);
162+
return callback();
163+
}
164+
}
165+
166+
const scheduleWithCheckIn = cron.instrumentNodeSchedule(new NodeScheduleMock());
167+
168+
scheduleWithCheckIn.scheduleJob('my-cron-job', '* * * Jan,Sep Sun', () => {
169+
expect(withMonitorSpy).toHaveBeenCalledTimes(1);
170+
expect(withMonitorSpy).toHaveBeenLastCalledWith('my-cron-job', expect.anything(), {
171+
schedule: { type: 'crontab', value: '* * * 1,9 0' },
172+
});
173+
done();
174+
});
175+
});
176+
177+
test('throws without crontab string', () => {
178+
class NodeScheduleMock {
179+
scheduleJob(_: string, __: string | Date, ___: () => void): unknown {
180+
return undefined;
181+
}
182+
}
183+
184+
const scheduleWithCheckIn = cron.instrumentNodeSchedule(new NodeScheduleMock());
185+
186+
expect(() => {
187+
scheduleWithCheckIn.scheduleJob('my-cron-job', new Date(), () => {
188+
//
189+
});
190+
}).toThrowError(
191+
"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",
192+
);
193+
});
194+
195+
test('throws without job name', () => {
196+
class NodeScheduleMock {
197+
scheduleJob(_: string, __: () => void): unknown {
198+
return undefined;
199+
}
200+
}
201+
202+
const scheduleWithCheckIn = cron.instrumentNodeSchedule(new NodeScheduleMock());
203+
204+
expect(() => {
205+
scheduleWithCheckIn.scheduleJob('* * * * *', () => {
206+
//
207+
});
208+
}).toThrowError(
209+
"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",
210+
);
211+
});
212+
});
128213
});

0 commit comments

Comments
 (0)