From 2b4f7c1178698a7606c7b491ec21170b547d4597 Mon Sep 17 00:00:00 2001 From: naaa760 Date: Mon, 3 Nov 2025 14:58:18 +0530 Subject: [PATCH 01/11] feat(core): Add isolateTrace option to MonitorConfig Add optional isolateTrace boolean property to MonitorConfig interface to allow creating separate traces for each withMonitor execution. --- packages/core/src/types-hoist/checkin.ts | 5 +++++ 1 file changed, 5 insertions(+) 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; } From 382fdc61129626dc4de8ba5935ef9a777224cc54 Mon Sep 17 00:00:00 2001 From: naaa760 Date: Mon, 3 Nov 2025 14:58:37 +0530 Subject: [PATCH 02/11] feat(core): Implement isolateTrace in withMonitor function Modify withMonitor to create new traces when isolateTrace option is enabled. When isolateTrace: true, starts a new span with monitor-specific naming to allow distinguishing between different cron job executions. --- packages/core/src/exports.ts | 38 ++++++++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/packages/core/src/exports.ts b/packages/core/src/exports.ts index a5dc716d8124..8de33aec7e22 100644 --- a/packages/core/src/exports.ts +++ b/packages/core/src/exports.ts @@ -15,6 +15,7 @@ import { isThenable } from './utils/is'; import { uuid4 } from './utils/misc'; import type { ExclusiveEventHintOrCaptureContext } from './utils/prepareEvent'; import { parseEventHintOrCaptureContext } from './utils/prepareEvent'; +import { startSpan } from './tracing/trace'; import { timestampInSeconds } from './utils/time'; import { GLOBAL_OBJ } from './utils/worldwide'; @@ -167,6 +168,43 @@ export function withMonitor( } return withIsolationScope(() => { + // If isolateTrace is enabled, start a new trace for this monitor execution + if (upsertMonitorConfig?.isolateTrace) { + return startSpan( + { + name: `monitor.${monitorSlug}`, + op: 'monitor', + forceTransaction: true, + }, + () => { + let maybePromiseResult: T; + try { + maybePromiseResult = callback(); + } catch (e) { + finishCheckIn('error'); + throw e; + } + + if (isThenable(maybePromiseResult)) { + return maybePromiseResult.then( + r => { + finishCheckIn('ok'); + return r; + }, + e => { + finishCheckIn('error'); + throw e; + }, + ) as T; + } + finishCheckIn('ok'); + + return maybePromiseResult; + }, + ); + } + + // Default behavior without isolateTrace let maybePromiseResult: T; try { maybePromiseResult = callback(); From 4dfa0af7526969e1c6db631dafccd17926ce4bc6 Mon Sep 17 00:00:00 2001 From: naaa760 Date: Mon, 3 Nov 2025 14:58:47 +0530 Subject: [PATCH 03/11] test(core): Add tests for isolateTrace functionality Add unit tests for the new isolateTrace option in withMonitor: - Test isolateTrace: true option acceptance - Test isolateTrace: false maintains default behavior - Test isolateTrace works with async operations --- packages/core/test/lib/client.test.ts | 37 +++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/packages/core/test/lib/client.test.ts b/packages/core/test/lib/client.test.ts index ae324aa40f9f..8ac3d2487d5d 100644 --- a/packages/core/test/lib/client.test.ts +++ b/packages/core/test/lib/client.test.ts @@ -2600,6 +2600,43 @@ describe('Client', () => { const promise = await withMonitor('test-monitor', callback); await expect(promise).rejects.toThrowError(error); }); + + test('accepts isolateTrace option without error', () => { + 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); + }); + + test('works with isolateTrace set to false', () => { + 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); + }); + + test('handles isolateTrace with asynchronous operations', 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); + }); }); describe('log weight-based flushing', () => { From e06ae3044506cd46b88067d479151134672388c2 Mon Sep 17 00:00:00 2001 From: naaa760 Date: Fri, 7 Nov 2025 15:32:59 +0530 Subject: [PATCH 04/11] feat: Update isolateTrace implementation to use startNewTrace Update withMonitor to use startNewTrace() instead of startSpan() when isolateTrace is enabled, following PR feedback to only isolate traces without creating spans. --- packages/core/src/exports.ts | 59 ++++++++++++--------------- packages/core/test/lib/client.test.ts | 1 + 2 files changed, 27 insertions(+), 33 deletions(-) diff --git a/packages/core/src/exports.ts b/packages/core/src/exports.ts index 8de33aec7e22..5476aa69014b 100644 --- a/packages/core/src/exports.ts +++ b/packages/core/src/exports.ts @@ -15,7 +15,7 @@ import { isThenable } from './utils/is'; import { uuid4 } from './utils/misc'; import type { ExclusiveEventHintOrCaptureContext } from './utils/prepareEvent'; import { parseEventHintOrCaptureContext } from './utils/prepareEvent'; -import { startSpan } from './tracing/trace'; +import { startNewTrace } from './tracing/trace'; import { timestampInSeconds } from './utils/time'; import { GLOBAL_OBJ } from './utils/worldwide'; @@ -170,38 +170,31 @@ export function withMonitor( return withIsolationScope(() => { // If isolateTrace is enabled, start a new trace for this monitor execution if (upsertMonitorConfig?.isolateTrace) { - return startSpan( - { - name: `monitor.${monitorSlug}`, - op: 'monitor', - forceTransaction: true, - }, - () => { - let maybePromiseResult: T; - try { - maybePromiseResult = callback(); - } catch (e) { - finishCheckIn('error'); - throw e; - } - - if (isThenable(maybePromiseResult)) { - return maybePromiseResult.then( - r => { - finishCheckIn('ok'); - return r; - }, - e => { - finishCheckIn('error'); - throw e; - }, - ) as T; - } - finishCheckIn('ok'); - - return maybePromiseResult; - }, - ); + return startNewTrace(() => { + let maybePromiseResult: T; + try { + maybePromiseResult = callback(); + } catch (e) { + finishCheckIn('error'); + throw e; + } + + if (isThenable(maybePromiseResult)) { + return maybePromiseResult.then( + r => { + finishCheckIn('ok'); + return r; + }, + e => { + finishCheckIn('error'); + throw e; + }, + ) as T; + } + finishCheckIn('ok'); + + return maybePromiseResult; + }); } // Default behavior without isolateTrace diff --git a/packages/core/test/lib/client.test.ts b/packages/core/test/lib/client.test.ts index 8ac3d2487d5d..10f8d3a39c51 100644 --- a/packages/core/test/lib/client.test.ts +++ b/packages/core/test/lib/client.test.ts @@ -11,6 +11,7 @@ import { SyncPromise, withMonitor, } from '../../src'; +import { startNewTrace } from '../../src/tracing'; import * as integrationModule from '../../src/integration'; import { _INTERNAL_captureLog } from '../../src/logs/internal'; import { _INTERNAL_captureMetric } from '../../src/metrics/internal'; From a16ae77c8ce32227daac87841cd3781269483ed9 Mon Sep 17 00:00:00 2001 From: naaa760 Date: Fri, 7 Nov 2025 15:33:21 +0530 Subject: [PATCH 05/11] test: Add integration test for isolateTrace functionality Add node integration test to verify that withMonitor calls with isolateTrace: true generate different trace IDs, confirming trace isolation works as expected. --- .../suites/public-api/withMonitor/scenario.ts | 28 +++++++++++++++++++ .../suites/public-api/withMonitor/test.ts | 19 +++++++++++++ 2 files changed, 47 insertions(+) create mode 100644 dev-packages/node-integration-tests/suites/public-api/withMonitor/scenario.ts create mode 100644 dev-packages/node-integration-tests/suites/public-api/withMonitor/test.ts 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..9a24274f9ff9 --- /dev/null +++ b/dev-packages/node-integration-tests/suites/public-api/withMonitor/scenario.ts @@ -0,0 +1,28 @@ +import { withMonitor } from '@sentry/node'; + +export async function run(): Promise { + // First withMonitor call without isolateTrace (should share trace) + await withMonitor('cron-job-1', async () => { + // Simulate some work + await new Promise((resolve) => { + setTimeout(() => { + resolve(); + }, 100); + }); + }, { + schedule: { type: 'crontab', value: '* * * * *' } + }); + + // Second withMonitor call with isolateTrace (should have different trace) + await withMonitor('cron-job-2', async () => { + // Simulate some work + await new Promise((resolve) => { + setTimeout(() => { + resolve(); + }, 100); + }); + }, { + schedule: { type: 'crontab', value: '* * * * *' }, + isolateTrace: true + }); +} 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..43fa1b4e1ba8 --- /dev/null +++ b/dev-packages/node-integration-tests/suites/public-api/withMonitor/test.ts @@ -0,0 +1,19 @@ +import { expect } from 'vitest'; +import type { Event, TransactionEvent } from '@sentry/types'; + +export async function testResults(events: Event[]): Promise { + // Get all transaction events (which represent traces) + const transactionEvents = events.filter((event): event is TransactionEvent => event.type === 'transaction'); + + // Should have at least 2 transaction events (one for each withMonitor call) + expect(transactionEvents.length).toBeGreaterThanOrEqual(2); + + // Get trace IDs from the transactions + const traceIds = transactionEvents.map(event => event.contexts?.trace?.trace_id).filter(Boolean); + + // Should have at least 2 different trace IDs (verifying trace isolation) + const uniqueTraceIds = [...new Set(traceIds)]; + expect(uniqueTraceIds.length).toBeGreaterThanOrEqual(2); + + console.log('✅ Found traces with different trace IDs:', uniqueTraceIds); +} From 50d5386dd61f84053a18aa5ba453bfa06ea54b69 Mon Sep 17 00:00:00 2001 From: naaa760 Date: Wed, 26 Nov 2025 15:03:07 +0530 Subject: [PATCH 06/11] test: Fix withMonitor integration test to use proper test runner Rewrite integration test to use createRunner pattern like other tests. Test now properly captures check-ins and verifies distinct trace IDs when isolateTrace is enabled, following PR review feedback. --- .../suites/public-api/withMonitor/scenario.ts | 60 +++++++++++-------- .../suites/public-api/withMonitor/test.ts | 60 ++++++++++++++----- 2 files changed, 82 insertions(+), 38 deletions(-) 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 index 9a24274f9ff9..e03e50641237 100644 --- a/dev-packages/node-integration-tests/suites/public-api/withMonitor/scenario.ts +++ b/dev-packages/node-integration-tests/suites/public-api/withMonitor/scenario.ts @@ -1,28 +1,40 @@ -import { withMonitor } from '@sentry/node'; +import * as Sentry from '@sentry/node'; +import { loggingTransport } from '@sentry-internal/node-integration-tests'; -export async function run(): Promise { - // First withMonitor call without isolateTrace (should share trace) - await withMonitor('cron-job-1', async () => { - // Simulate some work - await new Promise((resolve) => { - setTimeout(() => { - resolve(); - }, 100); - }); - }, { - schedule: { type: 'crontab', value: '* * * * *' } +Sentry.init({ + dsn: 'https://public@dsn.ingest.sentry.io/1337', + release: '1.0', + transport: loggingTransport, +}); + +// First withMonitor call without isolateTrace +// eslint-disable-next-line @sentry-internal/sdk/no-floating-promises +Sentry.withMonitor('cron-job-1', async () => { + // Simulate some work + await new Promise((resolve) => { + setTimeout(() => { + resolve(); + }, 100); }); +}, { + schedule: { type: 'crontab', value: '* * * * *' }, +}); - // Second withMonitor call with isolateTrace (should have different trace) - await withMonitor('cron-job-2', async () => { - // Simulate some work - await new Promise((resolve) => { - setTimeout(() => { - resolve(); - }, 100); - }); - }, { - schedule: { type: 'crontab', value: '* * * * *' }, - isolateTrace: true +// Second withMonitor call with isolateTrace (should have different trace) +// eslint-disable-next-line @sentry-internal/sdk/no-floating-promises +Sentry.withMonitor('cron-job-2', async () => { + // Simulate some work + await new Promise((resolve) => { + setTimeout(() => { + resolve(); + }, 100); }); -} +}, { + schedule: { type: 'crontab', value: '* * * * *' }, + isolateTrace: true, +}); + +// Wait a bit for check-ins to complete before exiting +setTimeout(() => { + process.exit(); +}, 500); \ No newline at end of file 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 index 43fa1b4e1ba8..094f67d1c3ef 100644 --- a/dev-packages/node-integration-tests/suites/public-api/withMonitor/test.ts +++ b/dev-packages/node-integration-tests/suites/public-api/withMonitor/test.ts @@ -1,19 +1,51 @@ -import { expect } from 'vitest'; -import type { Event, TransactionEvent } from '@sentry/types'; +import type { SerializedCheckIn } from '@sentry/core'; +import { afterAll, describe, expect, test } from 'vitest'; +import { cleanupChildProcesses, createRunner } from '../../../utils/runner'; -export async function testResults(events: Event[]): Promise { - // Get all transaction events (which represent traces) - const transactionEvents = events.filter((event): event is TransactionEvent => event.type === 'transaction'); +describe('withMonitor isolateTrace', () => { + afterAll(() => { + cleanupChildProcesses(); + }); - // Should have at least 2 transaction events (one for each withMonitor call) - expect(transactionEvents.length).toBeGreaterThanOrEqual(2); + test('creates distinct traces when isolateTrace is enabled', async () => { + const checkIns: SerializedCheckIn[] = []; - // Get trace IDs from the transactions - const traceIds = transactionEvents.map(event => event.contexts?.trace?.trace_id).filter(Boolean); + 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(); - // Should have at least 2 different trace IDs (verifying trace isolation) - const uniqueTraceIds = [...new Set(traceIds)]; - expect(uniqueTraceIds.length).toBeGreaterThanOrEqual(2); + // Find the two 'ok' check-ins for comparison + const checkIn1Ok = checkIns.find(c => c.monitor_slug === 'cron-job-1' && c.status === 'ok'); + const checkIn2Ok = checkIns.find(c => c.monitor_slug === 'cron-job-2' && c.status === 'ok'); - console.log('✅ Found traces with different trace IDs:', uniqueTraceIds); -} + expect(checkIn1Ok).toBeDefined(); + expect(checkIn2Ok).toBeDefined(); + + // Verify both check-ins have trace contexts + expect(checkIn1Ok!.contexts?.trace?.trace_id).toBeDefined(); + expect(checkIn2Ok!.contexts?.trace?.trace_id).toBeDefined(); + + // The key assertion: trace IDs should be different when isolateTrace is enabled + expect(checkIn1Ok!.contexts!.trace!.trace_id).not.toBe(checkIn2Ok!.contexts!.trace!.trace_id); + }); +}); \ No newline at end of file From 4215d4919f122d22b774de864de91df23cf3e4ae Mon Sep 17 00:00:00 2001 From: naaa760 Date: Wed, 26 Nov 2025 15:03:21 +0530 Subject: [PATCH 07/11] fix: Add newline at end of scenario file --- .../suites/public-api/withMonitor/scenario.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 index e03e50641237..9e1eb8a5819a 100644 --- a/dev-packages/node-integration-tests/suites/public-api/withMonitor/scenario.ts +++ b/dev-packages/node-integration-tests/suites/public-api/withMonitor/scenario.ts @@ -37,4 +37,4 @@ Sentry.withMonitor('cron-job-2', async () => { // Wait a bit for check-ins to complete before exiting setTimeout(() => { process.exit(); -}, 500); \ No newline at end of file +}, 500); From 7d10a5395d6a753150d08732de1835a37afcfc81 Mon Sep 17 00:00:00 2001 From: naaa760 Date: Wed, 26 Nov 2025 15:08:04 +0530 Subject: [PATCH 08/11] test: Remove comments from withMonitor scenario file --- .../suites/public-api/withMonitor/scenario.ts | 5 ----- 1 file changed, 5 deletions(-) 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 index 9e1eb8a5819a..b3777011e74b 100644 --- a/dev-packages/node-integration-tests/suites/public-api/withMonitor/scenario.ts +++ b/dev-packages/node-integration-tests/suites/public-api/withMonitor/scenario.ts @@ -7,10 +7,8 @@ Sentry.init({ transport: loggingTransport, }); -// First withMonitor call without isolateTrace // eslint-disable-next-line @sentry-internal/sdk/no-floating-promises Sentry.withMonitor('cron-job-1', async () => { - // Simulate some work await new Promise((resolve) => { setTimeout(() => { resolve(); @@ -20,10 +18,8 @@ Sentry.withMonitor('cron-job-1', async () => { schedule: { type: 'crontab', value: '* * * * *' }, }); -// Second withMonitor call with isolateTrace (should have different trace) // eslint-disable-next-line @sentry-internal/sdk/no-floating-promises Sentry.withMonitor('cron-job-2', async () => { - // Simulate some work await new Promise((resolve) => { setTimeout(() => { resolve(); @@ -34,7 +30,6 @@ Sentry.withMonitor('cron-job-2', async () => { isolateTrace: true, }); -// Wait a bit for check-ins to complete before exiting setTimeout(() => { process.exit(); }, 500); From ad7c159336fe3ee0a194f00944a25803563e6839 Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Fri, 28 Nov 2025 13:32:24 +0100 Subject: [PATCH 09/11] ensure initial checkin and terminal checkin have same trace --- .../suites/public-api/withMonitor/scenario.ts | 50 +++++++++++-------- .../suites/public-api/withMonitor/test.ts | 21 ++++---- packages/core/src/exports.ts | 46 ++++------------- 3 files changed, 50 insertions(+), 67 deletions(-) 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 index b3777011e74b..42a1da41a195 100644 --- a/dev-packages/node-integration-tests/suites/public-api/withMonitor/scenario.ts +++ b/dev-packages/node-integration-tests/suites/public-api/withMonitor/scenario.ts @@ -7,28 +7,36 @@ Sentry.init({ transport: loggingTransport, }); -// eslint-disable-next-line @sentry-internal/sdk/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-1', + async () => { + await new Promise(resolve => { + setTimeout(() => { + resolve(); + }, 100); + }); + }, + { + schedule: { type: 'crontab', value: '* * * * *' }, + }, +); -// eslint-disable-next-line @sentry-internal/sdk/no-floating-promises -Sentry.withMonitor('cron-job-2', async () => { - await new Promise((resolve) => { - setTimeout(() => { - resolve(); - }, 100); - }); -}, { - schedule: { type: 'crontab', value: '* * * * *' }, - isolateTrace: true, -}); +// 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(); 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 index 094f67d1c3ef..fd66ac4feb8c 100644 --- a/dev-packages/node-integration-tests/suites/public-api/withMonitor/test.ts +++ b/dev-packages/node-integration-tests/suites/public-api/withMonitor/test.ts @@ -34,18 +34,21 @@ describe('withMonitor isolateTrace', () => { .start() .completed(); - // Find the two 'ok' check-ins for comparison + 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(checkIn1Ok).toBeDefined(); - expect(checkIn2Ok).toBeDefined(); + 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}/); - // Verify both check-ins have trace contexts - expect(checkIn1Ok!.contexts?.trace?.trace_id).toBeDefined(); - expect(checkIn2Ok!.contexts?.trace?.trace_id).toBeDefined(); + expect(checkIn1InProgress!.contexts?.trace?.trace_id).not.toBe(checkIn2InProgress!.contexts?.trace?.trace_id); + expect(checkIn1Ok!.contexts?.trace?.trace_id).toBe(checkIn1InProgress!.contexts?.trace?.trace_id); - // The key assertion: trace IDs should be different when isolateTrace is enabled - expect(checkIn1Ok!.contexts!.trace!.trace_id).not.toBe(checkIn2Ok!.contexts!.trace!.trace_id); + expect(checkIn2InProgress!.contexts?.trace?.trace_id).not.toBe(checkIn1InProgress!.contexts?.trace?.trace_id); + expect(checkIn2Ok!.contexts?.trace?.trace_id).toBe(checkIn2InProgress!.contexts?.trace?.trace_id); }); -}); \ No newline at end of file +}); diff --git a/packages/core/src/exports.ts b/packages/core/src/exports.ts index e388ed0ef00b..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'; @@ -15,7 +16,6 @@ import { isThenable } from './utils/is'; import { uuid4 } from './utils/misc'; import type { ExclusiveEventHintOrCaptureContext } from './utils/prepareEvent'; import { parseEventHintOrCaptureContext } from './utils/prepareEvent'; -import { startNewTrace } from './tracing/trace'; import { timestampInSeconds } from './utils/time'; import { GLOBAL_OBJ } from './utils/worldwide'; @@ -160,43 +160,13 @@ 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(() => { - // If isolateTrace is enabled, start a new trace for this monitor execution - if (upsertMonitorConfig?.isolateTrace) { - return startNewTrace(() => { - let maybePromiseResult: T; - try { - maybePromiseResult = callback(); - } catch (e) { - finishCheckIn('error'); - throw e; - } - - if (isThenable(maybePromiseResult)) { - return maybePromiseResult.then( - r => { - finishCheckIn('ok'); - return r; - }, - e => { - finishCheckIn('error'); - throw e; - }, - ) as T; - } - finishCheckIn('ok'); - - return maybePromiseResult; - }); + function finishCheckIn(status: FinishedCheckIn['status']): void { + captureCheckIn({ monitorSlug, status, checkInId, duration: timestampInSeconds() - now }); } - // Default behavior without isolateTrace let maybePromiseResult: T; try { @@ -221,7 +191,9 @@ export function withMonitor( finishCheckIn('ok'); return maybePromiseResult; - }); + } + + return withIsolationScope(() => (upsertMonitorConfig?.isolateTrace ? startNewTrace(runCallback) : runCallback())); } /** From b12304b6b0d15ddfdd0d7a3c5a5fb686fb08179e Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Fri, 28 Nov 2025 13:36:38 +0100 Subject: [PATCH 10/11] formatting and linting --- packages/core/test/lib/client.test.ts | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/packages/core/test/lib/client.test.ts b/packages/core/test/lib/client.test.ts index b2f2f8e4f712..68807c4d0633 100644 --- a/packages/core/test/lib/client.test.ts +++ b/packages/core/test/lib/client.test.ts @@ -12,7 +12,6 @@ import { SyncPromise, withMonitor, } from '../../src'; -import { startNewTrace } from '../../src/tracing'; import * as integrationModule from '../../src/integration'; import { _INTERNAL_captureLog } from '../../src/logs/internal'; import { _INTERNAL_captureMetric } from '../../src/metrics/internal'; @@ -2740,7 +2739,7 @@ describe('Client', () => { const returnedResult = withMonitor('test-monitor', callback, { schedule: { type: 'crontab', value: '* * * * *' }, - isolateTrace: true + isolateTrace: true, }); expect(returnedResult).toBe(result); @@ -2753,7 +2752,7 @@ describe('Client', () => { const returnedResult = withMonitor('test-monitor', callback, { schedule: { type: 'crontab', value: '* * * * *' }, - isolateTrace: false + isolateTrace: false, }); expect(returnedResult).toBe(result); @@ -2766,7 +2765,7 @@ describe('Client', () => { const promise = withMonitor('test-monitor', callback, { schedule: { type: 'crontab', value: '* * * * *' }, - isolateTrace: true + isolateTrace: true, }); await expect(promise).resolves.toEqual(result); }); From 4551a8e9692e326dded682138cef7f39cf87f8fc Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Tue, 2 Dec 2025 11:06:06 +0100 Subject: [PATCH 11/11] fix unit tests, fix integration tests --- .../suites/public-api/withMonitor/test.ts | 4 +- packages/core/test/lib/client.test.ts | 90 +++++++++++++------ 2 files changed, 67 insertions(+), 27 deletions(-) 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 index fd66ac4feb8c..e5483682ddfe 100644 --- a/dev-packages/node-integration-tests/suites/public-api/withMonitor/test.ts +++ b/dev-packages/node-integration-tests/suites/public-api/withMonitor/test.ts @@ -46,9 +46,9 @@ describe('withMonitor isolateTrace', () => { 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?.trace_id).toBe(checkIn1InProgress!.contexts?.trace?.trace_id); + expect(checkIn1Ok!.contexts?.trace?.span_id).not.toBe(checkIn2Ok!.contexts?.trace?.span_id); - expect(checkIn2InProgress!.contexts?.trace?.trace_id).not.toBe(checkIn1InProgress!.contexts?.trace?.trace_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/test/lib/client.test.ts b/packages/core/test/lib/client.test.ts index 68807c4d0633..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'; @@ -2733,41 +2734,80 @@ describe('Client', () => { await expect(promise).rejects.toThrowError(error); }); - test('accepts isolateTrace option without error', () => { - const result = 'foo'; - const callback = vi.fn().mockReturnValue(result); + describe('isolateTrace', () => { + const startNewTraceSpy = vi.spyOn(traceModule, 'startNewTrace').mockImplementation(cb => cb()); - const returnedResult = withMonitor('test-monitor', callback, { - schedule: { type: 'crontab', value: '* * * * *' }, - isolateTrace: true, + beforeEach(() => { + startNewTraceSpy.mockClear(); }); - expect(returnedResult).toBe(result); - expect(callback).toHaveBeenCalledTimes(1); - }); + it('starts a new trace when isolateTrace is true (sync)', () => { + const result = 'foo'; + const callback = vi.fn().mockReturnValue(result); - test('works with isolateTrace set to false', () => { - const result = 'foo'; - const callback = vi.fn().mockReturnValue(result); + const returnedResult = withMonitor('test-monitor', callback, { + schedule: { type: 'crontab', value: '* * * * *' }, + isolateTrace: true, + }); - const returnedResult = withMonitor('test-monitor', callback, { - schedule: { type: 'crontab', value: '* * * * *' }, - isolateTrace: false, + expect(returnedResult).toBe(result); + expect(callback).toHaveBeenCalledTimes(1); + expect(startNewTraceSpy).toHaveBeenCalledTimes(1); }); - expect(returnedResult).toBe(result); - expect(callback).toHaveBeenCalledTimes(1); - }); + it('starts a new trace when isolateTrace is true (async)', async () => { + const result = 'foo'; + const callback = vi.fn().mockResolvedValue(result); - test('handles isolateTrace with asynchronous operations', 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); + }); - const promise = withMonitor('test-monitor', callback, { - schedule: { type: 'crontab', value: '* * * * *' }, - isolateTrace: true, + 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(); }); - await expect(promise).resolves.toEqual(result); }); });