diff --git a/dev-packages/node-integration-tests/suites/public-api/withMonitor/scenario.ts b/dev-packages/node-integration-tests/suites/public-api/withMonitor/scenario.ts new file mode 100644 index 000000000000..42a1da41a195 --- /dev/null +++ b/dev-packages/node-integration-tests/suites/public-api/withMonitor/scenario.ts @@ -0,0 +1,43 @@ +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, +}); + +// eslint-disable-next-line @typescript-eslint/no-floating-promises +Sentry.withMonitor( + 'cron-job-1', + async () => { + await new Promise(resolve => { + setTimeout(() => { + resolve(); + }, 100); + }); + }, + { + schedule: { type: 'crontab', value: '* * * * *' }, + }, +); + +// eslint-disable-next-line @typescript-eslint/no-floating-promises +Sentry.withMonitor( + 'cron-job-2', + async () => { + await new Promise(resolve => { + setTimeout(() => { + resolve(); + }, 100); + }); + }, + { + schedule: { type: 'crontab', value: '* * * * *' }, + isolateTrace: true, + }, +); + +setTimeout(() => { + process.exit(); +}, 500); diff --git a/dev-packages/node-integration-tests/suites/public-api/withMonitor/test.ts b/dev-packages/node-integration-tests/suites/public-api/withMonitor/test.ts new file mode 100644 index 000000000000..e5483682ddfe --- /dev/null +++ b/dev-packages/node-integration-tests/suites/public-api/withMonitor/test.ts @@ -0,0 +1,54 @@ +import type { SerializedCheckIn } from '@sentry/core'; +import { afterAll, describe, expect, test } from 'vitest'; +import { cleanupChildProcesses, createRunner } from '../../../utils/runner'; + +describe('withMonitor isolateTrace', () => { + afterAll(() => { + cleanupChildProcesses(); + }); + + test('creates distinct traces when isolateTrace is enabled', async () => { + const checkIns: SerializedCheckIn[] = []; + + await createRunner(__dirname, 'scenario.ts') + .expect({ + check_in: checkIn => { + checkIns.push(checkIn); + }, + }) + .expect({ + check_in: checkIn => { + checkIns.push(checkIn); + }, + }) + .expect({ + check_in: checkIn => { + checkIns.push(checkIn); + }, + }) + .expect({ + check_in: checkIn => { + checkIns.push(checkIn); + }, + }) + .start() + .completed(); + + const checkIn1InProgress = checkIns.find(c => c.monitor_slug === 'cron-job-1' && c.status === 'in_progress'); + const checkIn1Ok = checkIns.find(c => c.monitor_slug === 'cron-job-1' && c.status === 'ok'); + + const checkIn2InProgress = checkIns.find(c => c.monitor_slug === 'cron-job-2' && c.status === 'in_progress'); + const checkIn2Ok = checkIns.find(c => c.monitor_slug === 'cron-job-2' && c.status === 'ok'); + + expect(checkIn1InProgress?.contexts?.trace?.trace_id).toMatch(/[a-f\d]{32}/); + expect(checkIn1Ok?.contexts?.trace?.trace_id).toMatch(/[a-f\d]{32}/); + expect(checkIn2InProgress?.contexts?.trace?.trace_id).toMatch(/[a-f\d]{32}/); + expect(checkIn2Ok?.contexts?.trace?.trace_id).toMatch(/[a-f\d]{32}/); + + expect(checkIn1InProgress!.contexts?.trace?.trace_id).not.toBe(checkIn2InProgress!.contexts?.trace?.trace_id); + expect(checkIn1Ok!.contexts?.trace?.span_id).not.toBe(checkIn2Ok!.contexts?.trace?.span_id); + + expect(checkIn1Ok!.contexts?.trace?.trace_id).toBe(checkIn1InProgress!.contexts?.trace?.trace_id); + expect(checkIn2Ok!.contexts?.trace?.trace_id).toBe(checkIn2InProgress!.contexts?.trace?.trace_id); + }); +}); diff --git a/packages/core/src/exports.ts b/packages/core/src/exports.ts index e3f658c88f2a..a59e521febc7 100644 --- a/packages/core/src/exports.ts +++ b/packages/core/src/exports.ts @@ -2,6 +2,7 @@ import { getClient, getCurrentScope, getIsolationScope, withIsolationScope } fro import { DEBUG_BUILD } from './debug-build'; import type { CaptureContext } from './scope'; import { closeSession, makeSession, updateSession } from './session'; +import { startNewTrace } from './tracing/trace'; import type { CheckIn, FinishedCheckIn, MonitorConfig } from './types-hoist/checkin'; import type { Event, EventHint } from './types-hoist/event'; import type { EventProcessor } from './types-hoist/eventprocessor'; @@ -159,14 +160,14 @@ export function withMonitor( callback: () => T, upsertMonitorConfig?: MonitorConfig, ): T { - const checkInId = captureCheckIn({ monitorSlug, status: 'in_progress' }, upsertMonitorConfig); - const now = timestampInSeconds(); + function runCallback(): T { + const checkInId = captureCheckIn({ monitorSlug, status: 'in_progress' }, upsertMonitorConfig); + const now = timestampInSeconds(); - function finishCheckIn(status: FinishedCheckIn['status']): void { - captureCheckIn({ monitorSlug, status, checkInId, duration: timestampInSeconds() - now }); - } - - return withIsolationScope(() => { + function finishCheckIn(status: FinishedCheckIn['status']): void { + captureCheckIn({ monitorSlug, status, checkInId, duration: timestampInSeconds() - now }); + } + // Default behavior without isolateTrace let maybePromiseResult: T; try { maybePromiseResult = callback(); @@ -190,7 +191,9 @@ export function withMonitor( finishCheckIn('ok'); return maybePromiseResult; - }); + } + + return withIsolationScope(() => (upsertMonitorConfig?.isolateTrace ? startNewTrace(runCallback) : runCallback())); } /** diff --git a/packages/core/src/types-hoist/checkin.ts b/packages/core/src/types-hoist/checkin.ts index 9d200811183a..6d25998099ad 100644 --- a/packages/core/src/types-hoist/checkin.ts +++ b/packages/core/src/types-hoist/checkin.ts @@ -105,4 +105,9 @@ export interface MonitorConfig { failureIssueThreshold?: SerializedMonitorConfig['failure_issue_threshold']; /** How many consecutive OK check-ins it takes to resolve an issue. */ recoveryThreshold?: SerializedMonitorConfig['recovery_threshold']; + /** + * If set to true, creates a new trace for the monitor callback instead of continuing the current trace. + * This allows distinguishing between different cron job executions. + */ + isolateTrace?: boolean; } diff --git a/packages/core/test/lib/client.test.ts b/packages/core/test/lib/client.test.ts index a59a8bbb8780..0945af5f019f 100644 --- a/packages/core/test/lib/client.test.ts +++ b/packages/core/test/lib/client.test.ts @@ -15,6 +15,7 @@ import { import * as integrationModule from '../../src/integration'; import { _INTERNAL_captureLog } from '../../src/logs/internal'; import { _INTERNAL_captureMetric } from '../../src/metrics/internal'; +import * as traceModule from '../../src/tracing/trace'; import { DEFAULT_TRANSPORT_BUFFER_SIZE } from '../../src/transports/base'; import type { Envelope } from '../../src/types-hoist/envelope'; import type { ErrorEvent, Event, TransactionEvent } from '../../src/types-hoist/event'; @@ -2732,6 +2733,82 @@ describe('Client', () => { const promise = await withMonitor('test-monitor', callback); await expect(promise).rejects.toThrowError(error); }); + + describe('isolateTrace', () => { + const startNewTraceSpy = vi.spyOn(traceModule, 'startNewTrace').mockImplementation(cb => cb()); + + beforeEach(() => { + startNewTraceSpy.mockClear(); + }); + + it('starts a new trace when isolateTrace is true (sync)', () => { + const result = 'foo'; + const callback = vi.fn().mockReturnValue(result); + + const returnedResult = withMonitor('test-monitor', callback, { + schedule: { type: 'crontab', value: '* * * * *' }, + isolateTrace: true, + }); + + expect(returnedResult).toBe(result); + expect(callback).toHaveBeenCalledTimes(1); + expect(startNewTraceSpy).toHaveBeenCalledTimes(1); + }); + + it('starts a new trace when isolateTrace is true (async)', async () => { + const result = 'foo'; + const callback = vi.fn().mockResolvedValue(result); + + const promise = withMonitor('test-monitor', callback, { + schedule: { type: 'crontab', value: '* * * * *' }, + isolateTrace: true, + }); + await expect(promise).resolves.toEqual(result); + expect(callback).toHaveBeenCalledTimes(1); + expect(startNewTraceSpy).toHaveBeenCalledTimes(1); + }); + + it("doesn't start a new trace when isolateTrace is false (sync)", () => { + const result = 'foo'; + const callback = vi.fn().mockReturnValue(result); + + const returnedResult = withMonitor('test-monitor', callback, { + schedule: { type: 'crontab', value: '* * * * *' }, + isolateTrace: false, + }); + + expect(returnedResult).toBe(result); + expect(callback).toHaveBeenCalledTimes(1); + expect(startNewTraceSpy).not.toHaveBeenCalled(); + }); + + it("doesn't start a new trace when isolateTrace is false (async)", async () => { + const result = 'foo'; + const callback = vi.fn().mockResolvedValue(result); + + const promise = withMonitor('test-monitor', callback, { + schedule: { type: 'crontab', value: '* * * * *' }, + isolateTrace: false, + }); + + await expect(promise).resolves.toEqual(result); + expect(callback).toHaveBeenCalledTimes(1); + expect(startNewTraceSpy).not.toHaveBeenCalled(); + }); + + it("doesn't start a new trace by default", () => { + const result = 'foo'; + const callback = vi.fn().mockReturnValue(result); + + const returnedResult = withMonitor('test-monitor', callback, { + schedule: { type: 'crontab', value: '* * * * *' }, + }); + + expect(returnedResult).toBe(result); + expect(callback).toHaveBeenCalledTimes(1); + expect(startNewTraceSpy).not.toHaveBeenCalled(); + }); + }); }); describe('enableLogs', () => {