Skip to content

Commit 775ad19

Browse files
naaa760Lms24
andauthored
feat(core): Add isolateTrace option to Sentry.withMonitor() (#18079)
Currently `withMonitor()` reuses the same trace for all cron executions, making it impossible to distinguish between different runs in Sentry. This patch adds an optional `isolateTrace: boolean` to `MonitorConfig` that creates a separate trace for each monitor execution when enabled. --------- Co-authored-by: Lukas Stracke <[email protected]>
1 parent 81f9424 commit 775ad19

File tree

5 files changed

+190
-8
lines changed

5 files changed

+190
-8
lines changed
Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
import * as Sentry from '@sentry/node';
2+
import { loggingTransport } from '@sentry-internal/node-integration-tests';
3+
4+
Sentry.init({
5+
dsn: 'https://[email protected]/1337',
6+
release: '1.0',
7+
transport: loggingTransport,
8+
});
9+
10+
// eslint-disable-next-line @typescript-eslint/no-floating-promises
11+
Sentry.withMonitor(
12+
'cron-job-1',
13+
async () => {
14+
await new Promise<void>(resolve => {
15+
setTimeout(() => {
16+
resolve();
17+
}, 100);
18+
});
19+
},
20+
{
21+
schedule: { type: 'crontab', value: '* * * * *' },
22+
},
23+
);
24+
25+
// eslint-disable-next-line @typescript-eslint/no-floating-promises
26+
Sentry.withMonitor(
27+
'cron-job-2',
28+
async () => {
29+
await new Promise<void>(resolve => {
30+
setTimeout(() => {
31+
resolve();
32+
}, 100);
33+
});
34+
},
35+
{
36+
schedule: { type: 'crontab', value: '* * * * *' },
37+
isolateTrace: true,
38+
},
39+
);
40+
41+
setTimeout(() => {
42+
process.exit();
43+
}, 500);
Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
import type { SerializedCheckIn } from '@sentry/core';
2+
import { afterAll, describe, expect, test } from 'vitest';
3+
import { cleanupChildProcesses, createRunner } from '../../../utils/runner';
4+
5+
describe('withMonitor isolateTrace', () => {
6+
afterAll(() => {
7+
cleanupChildProcesses();
8+
});
9+
10+
test('creates distinct traces when isolateTrace is enabled', async () => {
11+
const checkIns: SerializedCheckIn[] = [];
12+
13+
await createRunner(__dirname, 'scenario.ts')
14+
.expect({
15+
check_in: checkIn => {
16+
checkIns.push(checkIn);
17+
},
18+
})
19+
.expect({
20+
check_in: checkIn => {
21+
checkIns.push(checkIn);
22+
},
23+
})
24+
.expect({
25+
check_in: checkIn => {
26+
checkIns.push(checkIn);
27+
},
28+
})
29+
.expect({
30+
check_in: checkIn => {
31+
checkIns.push(checkIn);
32+
},
33+
})
34+
.start()
35+
.completed();
36+
37+
const checkIn1InProgress = checkIns.find(c => c.monitor_slug === 'cron-job-1' && c.status === 'in_progress');
38+
const checkIn1Ok = checkIns.find(c => c.monitor_slug === 'cron-job-1' && c.status === 'ok');
39+
40+
const checkIn2InProgress = checkIns.find(c => c.monitor_slug === 'cron-job-2' && c.status === 'in_progress');
41+
const checkIn2Ok = checkIns.find(c => c.monitor_slug === 'cron-job-2' && c.status === 'ok');
42+
43+
expect(checkIn1InProgress?.contexts?.trace?.trace_id).toMatch(/[a-f\d]{32}/);
44+
expect(checkIn1Ok?.contexts?.trace?.trace_id).toMatch(/[a-f\d]{32}/);
45+
expect(checkIn2InProgress?.contexts?.trace?.trace_id).toMatch(/[a-f\d]{32}/);
46+
expect(checkIn2Ok?.contexts?.trace?.trace_id).toMatch(/[a-f\d]{32}/);
47+
48+
expect(checkIn1InProgress!.contexts?.trace?.trace_id).not.toBe(checkIn2InProgress!.contexts?.trace?.trace_id);
49+
expect(checkIn1Ok!.contexts?.trace?.span_id).not.toBe(checkIn2Ok!.contexts?.trace?.span_id);
50+
51+
expect(checkIn1Ok!.contexts?.trace?.trace_id).toBe(checkIn1InProgress!.contexts?.trace?.trace_id);
52+
expect(checkIn2Ok!.contexts?.trace?.trace_id).toBe(checkIn2InProgress!.contexts?.trace?.trace_id);
53+
});
54+
});

packages/core/src/exports.ts

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import { getClient, getCurrentScope, getIsolationScope, withIsolationScope } fro
22
import { DEBUG_BUILD } from './debug-build';
33
import type { CaptureContext } from './scope';
44
import { closeSession, makeSession, updateSession } from './session';
5+
import { startNewTrace } from './tracing/trace';
56
import type { CheckIn, FinishedCheckIn, MonitorConfig } from './types-hoist/checkin';
67
import type { Event, EventHint } from './types-hoist/event';
78
import type { EventProcessor } from './types-hoist/eventprocessor';
@@ -159,14 +160,14 @@ export function withMonitor<T>(
159160
callback: () => T,
160161
upsertMonitorConfig?: MonitorConfig,
161162
): T {
162-
const checkInId = captureCheckIn({ monitorSlug, status: 'in_progress' }, upsertMonitorConfig);
163-
const now = timestampInSeconds();
163+
function runCallback(): T {
164+
const checkInId = captureCheckIn({ monitorSlug, status: 'in_progress' }, upsertMonitorConfig);
165+
const now = timestampInSeconds();
164166

165-
function finishCheckIn(status: FinishedCheckIn['status']): void {
166-
captureCheckIn({ monitorSlug, status, checkInId, duration: timestampInSeconds() - now });
167-
}
168-
169-
return withIsolationScope(() => {
167+
function finishCheckIn(status: FinishedCheckIn['status']): void {
168+
captureCheckIn({ monitorSlug, status, checkInId, duration: timestampInSeconds() - now });
169+
}
170+
// Default behavior without isolateTrace
170171
let maybePromiseResult: T;
171172
try {
172173
maybePromiseResult = callback();
@@ -190,7 +191,9 @@ export function withMonitor<T>(
190191
finishCheckIn('ok');
191192

192193
return maybePromiseResult;
193-
});
194+
}
195+
196+
return withIsolationScope(() => (upsertMonitorConfig?.isolateTrace ? startNewTrace(runCallback) : runCallback()));
194197
}
195198

196199
/**

packages/core/src/types-hoist/checkin.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -105,4 +105,9 @@ export interface MonitorConfig {
105105
failureIssueThreshold?: SerializedMonitorConfig['failure_issue_threshold'];
106106
/** How many consecutive OK check-ins it takes to resolve an issue. */
107107
recoveryThreshold?: SerializedMonitorConfig['recovery_threshold'];
108+
/**
109+
* If set to true, creates a new trace for the monitor callback instead of continuing the current trace.
110+
* This allows distinguishing between different cron job executions.
111+
*/
112+
isolateTrace?: boolean;
108113
}

packages/core/test/lib/client.test.ts

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import {
1515
import * as integrationModule from '../../src/integration';
1616
import { _INTERNAL_captureLog } from '../../src/logs/internal';
1717
import { _INTERNAL_captureMetric } from '../../src/metrics/internal';
18+
import * as traceModule from '../../src/tracing/trace';
1819
import { DEFAULT_TRANSPORT_BUFFER_SIZE } from '../../src/transports/base';
1920
import type { Envelope } from '../../src/types-hoist/envelope';
2021
import type { ErrorEvent, Event, TransactionEvent } from '../../src/types-hoist/event';
@@ -2732,6 +2733,82 @@ describe('Client', () => {
27322733
const promise = await withMonitor('test-monitor', callback);
27332734
await expect(promise).rejects.toThrowError(error);
27342735
});
2736+
2737+
describe('isolateTrace', () => {
2738+
const startNewTraceSpy = vi.spyOn(traceModule, 'startNewTrace').mockImplementation(cb => cb());
2739+
2740+
beforeEach(() => {
2741+
startNewTraceSpy.mockClear();
2742+
});
2743+
2744+
it('starts a new trace when isolateTrace is true (sync)', () => {
2745+
const result = 'foo';
2746+
const callback = vi.fn().mockReturnValue(result);
2747+
2748+
const returnedResult = withMonitor('test-monitor', callback, {
2749+
schedule: { type: 'crontab', value: '* * * * *' },
2750+
isolateTrace: true,
2751+
});
2752+
2753+
expect(returnedResult).toBe(result);
2754+
expect(callback).toHaveBeenCalledTimes(1);
2755+
expect(startNewTraceSpy).toHaveBeenCalledTimes(1);
2756+
});
2757+
2758+
it('starts a new trace when isolateTrace is true (async)', async () => {
2759+
const result = 'foo';
2760+
const callback = vi.fn().mockResolvedValue(result);
2761+
2762+
const promise = withMonitor('test-monitor', callback, {
2763+
schedule: { type: 'crontab', value: '* * * * *' },
2764+
isolateTrace: true,
2765+
});
2766+
await expect(promise).resolves.toEqual(result);
2767+
expect(callback).toHaveBeenCalledTimes(1);
2768+
expect(startNewTraceSpy).toHaveBeenCalledTimes(1);
2769+
});
2770+
2771+
it("doesn't start a new trace when isolateTrace is false (sync)", () => {
2772+
const result = 'foo';
2773+
const callback = vi.fn().mockReturnValue(result);
2774+
2775+
const returnedResult = withMonitor('test-monitor', callback, {
2776+
schedule: { type: 'crontab', value: '* * * * *' },
2777+
isolateTrace: false,
2778+
});
2779+
2780+
expect(returnedResult).toBe(result);
2781+
expect(callback).toHaveBeenCalledTimes(1);
2782+
expect(startNewTraceSpy).not.toHaveBeenCalled();
2783+
});
2784+
2785+
it("doesn't start a new trace when isolateTrace is false (async)", async () => {
2786+
const result = 'foo';
2787+
const callback = vi.fn().mockResolvedValue(result);
2788+
2789+
const promise = withMonitor('test-monitor', callback, {
2790+
schedule: { type: 'crontab', value: '* * * * *' },
2791+
isolateTrace: false,
2792+
});
2793+
2794+
await expect(promise).resolves.toEqual(result);
2795+
expect(callback).toHaveBeenCalledTimes(1);
2796+
expect(startNewTraceSpy).not.toHaveBeenCalled();
2797+
});
2798+
2799+
it("doesn't start a new trace by default", () => {
2800+
const result = 'foo';
2801+
const callback = vi.fn().mockReturnValue(result);
2802+
2803+
const returnedResult = withMonitor('test-monitor', callback, {
2804+
schedule: { type: 'crontab', value: '* * * * *' },
2805+
});
2806+
2807+
expect(returnedResult).toBe(result);
2808+
expect(callback).toHaveBeenCalledTimes(1);
2809+
expect(startNewTraceSpy).not.toHaveBeenCalled();
2810+
});
2811+
});
27352812
});
27362813

27372814
describe('enableLogs', () => {

0 commit comments

Comments
 (0)