ensure initial checkin and terminal checkin have same trace

Author: Lms24

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<void>((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<void>(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<void>((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<void>(resolve => {
+      setTimeout(() => {
+        resolve();
+      }, 100);
+    });
+  },
+  {
+    schedule: { type: 'crontab', value: '* * * * *' },
+    isolateTrace: true,
+  },
+);
 
 setTimeout(() => {
   process.exit();

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);
   });
-});
+});

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<T>(
   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<T>(
     finishCheckIn('ok');
 
     return maybePromiseResult;
-  });
+  }
+
+  return withIsolationScope(() => (upsertMonitorConfig?.isolateTrace ? startNewTrace(runCallback) : runCallback()));
 }
 
 /**