feat(node): Stop sending the process session once request sessions are tracked

dev-packages/bun-integration-tests/node-suites/excludes.ts

@@ -180,6 +180,7 @@ const FETCH_INTEGRATION_DIFFERS = [
 // that sets `emit` back to the `emit` it had before its first request gets no spans after that.
 const HTTP_SERVER_OPTIONS_IGNORED = [
   'suites/sessions/exited-session-aggregate/test.ts',
+  'suites/sessions/server-without-process-session/test.ts',
   'suites/tracing/httpIntegration/test.ts',
 ];
 

dev-packages/node-integration-tests/suites/sessions/process-session/instrument.mjs

@@ -0,0 +1,8 @@
+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,
+});

dev-packages/node-integration-tests/suites/sessions/process-session/scenario.mjs

@@ -0,0 +1,3 @@
+import * as Sentry from '@sentry/node';
+
+Sentry.captureException(new Error('Scenario error'));

dev-packages/node-integration-tests/suites/sessions/process-session/test.ts

@@ -0,0 +1,18 @@
+import { afterAll, test } from 'vitest';
+import { join } from 'path';
+import { cleanupChildProcesses, createRunner } from '../../../utils/runner';
+
+afterAll(() => {
+  cleanupChildProcesses();
+});
+
+test('sends a process session for a process that does not serve requests', async () => {
+  await createRunner(__dirname, 'scenario.mjs')
+    .withInstrument(join(__dirname, 'instrument.mjs'))
+    .ignore('event')
+    .unignore('session')
+    .expect({ session: { init: true, status: 'ok', errors: 1 } })
+    .expect({ session: { init: false, status: 'exited', errors: 1 } })
+    .start()
+    .completed();
+});

dev-packages/node-integration-tests/suites/sessions/server-without-process-session/server-with-startup-error.mjs

@@ -0,0 +1,20 @@
+import * as Sentry from '@sentry/node';
+import { startExpressServerAndSendPortToRunner } from '@sentry-internal/node-integration-tests';
+import express from 'express';
+
+Sentry.captureException(new Error('Startup error'));
+// Event processing is async; make sure the error has updated the process session before the first request.
+await Sentry.flush();
+
+const app = express();
+
+app.get('/test/success', (_req, res) => {
+  res.send('Success!');
+});
+
+app.get('/test/error_handled', (_req, res) => {
+  Sentry.captureException(new Error('Request error'));
+  res.send('Handled!');
+});
+
+startExpressServerAndSendPortToRunner(app);

dev-packages/node-integration-tests/suites/sessions/server-without-process-session/test.ts

@@ -0,0 +1,56 @@
+import { afterAll, expect, test } from 'vitest';
+import { join } from 'path';
+import { cleanupChildProcesses, createRunner } from '../../../utils/runner';
+import type { SessionAggregates } from '@sentry/core';
+
+afterAll(() => {
+  cleanupChildProcesses();
+});
+
+function sumAggregates(agg: SessionAggregates): { exited: number; errored: number; crashed: number } {
+  return agg.aggregates.reduce(
+    (acc, b) => ({
+      exited: acc.exited + (b.exited ?? 0),
+      errored: acc.errored + (b.errored ?? 0),
+      crashed: acc.crashed + (b.crashed ?? 0),
+    }),
+    { exited: 0, errored: 0, crashed: 0 },
+  );
+}
+
+test('does not send a process session once requests are tracked as request sessions', async () => {
+  const runner = createRunner(__dirname, '..', 'server.mjs')
+    .withInstrument(join(__dirname, '..', 'instrument.mjs'))
+    .ignore('transaction', 'event')
+    .unignore('session', 'sessions')
+    .expect({
+      sessions: agg => {
+        expect(sumAggregates(agg)).toEqual({ exited: 1, errored: 1, crashed: 1 });
+      },
+    })
+    .start();
+
+  runner.makeRequest('get', '/test/success');
+  runner.makeRequest('get', '/test/error_handled');
+  runner.makeRequest('get', '/test/error_unhandled', { expectError: true });
+  await runner.completed();
+});
+
+test('counts an error before the first request in the process session, and later errors in the aggregate', async () => {
+  const runner = createRunner(__dirname, 'server-with-startup-error.mjs')
+    .withInstrument(join(__dirname, '..', 'instrument.mjs'))
+    .ignore('transaction', 'event')
+    .unignore('session', 'sessions')
+    .expect({ session: { init: true, status: 'ok', errors: 1 } })
+    .expect({ session: { init: false, status: 'exited', errors: 1 } })
+    .expect({
+      sessions: agg => {
+        expect(sumAggregates(agg)).toEqual({ exited: 1, errored: 1, crashed: 0 });
+      },
+    })
+    .start();
+
+  await runner.makeRequest('get', '/test/success');
+  await runner.makeRequest('get', '/test/error_handled');
+  await runner.completed();
+});

packages/core/src/client.ts

@@ -883,6 +883,12 @@ export abstract class Client<O extends ClientOptions = ClientOptions> {
     callback: (breadcrumb: Breadcrumb, hint: XhrBreadcrumbHint | FetchBreadcrumbHint) => void,
   ): () => void;
 
+  /**
+   * A hook that is called when an incoming request starts being tracked as a request session.
+   * @returns {() => void} A function that, when executed, removes the registered callback.
+   */
+  public on(hook: 'startRequestSession', callback: () => void): () => void;
+
   /**
    * A hook that is called when the client is flushing
    * @returns {() => void} A function that, when executed, removes the registered callback.
@@ -1194,6 +1200,11 @@ export abstract class Client<O extends ClientOptions = ClientOptions> {
     hint: XhrBreadcrumbHint | FetchBreadcrumbHint,
   ): void;
 
+  /**
+   * Emit a hook event when an incoming request starts being tracked as a request session.
+   */
+  public emit(hook: 'startRequestSession'): void;
+
   /**
    * Emit a hook event for client flush
    */

packages/core/src/integrations/http/record-request-session.ts

@@ -33,9 +33,13 @@ export function recordRequestSession(
     sessionFlushingDelayMS?: number;
   },
 ): void {
+  // Errors in this request are counted by the request session aggregate, so they must not also
+  // update a process session inherited from the parent isolation scope.
+  requestIsolationScope.setSession();
   requestIsolationScope.setSDKProcessingMetadata({
     requestSession: { status: 'ok' },
   });
+  client.emit('startRequestSession');
 
   response.once('close', () => {
     const requestSession = requestIsolationScope.getScopeData().sdkProcessingMetadata.requestSession;

packages/node/src/integrations/processSession.ts

@@ -1,15 +1,20 @@
-import { defineIntegration, endSession, getIsolationScope, startSession } from '@sentry/core';
+import { closeSession, defineIntegration, endSession, getIsolationScope, startSession } from '@sentry/core';
 
 const INTEGRATION_NAME = 'ProcessSession' as const;
 
 /**
  * Records a Session for the current process to track release health.
+ *
+ * Once the process starts serving requests that are tracked as request sessions, release health is
+ * reported through session aggregates instead, and the process session is discarded.
+ * This integration exists so that we capture some form of session health data even when the process is not serving requests.
  */
 export const processSessionIntegration = defineIntegration(() => {
   return {
     name: INTEGRATION_NAME,
-    setupOnce() {
+    setup(client) {
       startSession();
+      const processIsolationScope = getIsolationScope();
 
       // Emitted in the case of healthy sessions, error of `mechanism.handled: true` and unhandledrejections because
       // The 'beforeExit' event is not emitted for conditions causing explicit termination,
@@ -26,6 +31,22 @@ export const processSessionIntegration = defineIntegration(() => {
           endSession();
         }
       });
+
+      const unsubscribe = client.on('startRequestSession', () => {
+        unsubscribe();
+
+        const session = processIsolationScope.getSession();
+        if (!session) {
+          return;
+        }
+
+        // A session that was already sent upstream is closed so it does not linger as never-ended.
+        if (!session.init && session.status === 'ok') {
+          closeSession(session);
+          client.captureSession(session);
+        }
+        processIsolationScope.setSession();
+      });
     },
   };
 });

packages/node/test/integrations/httpServerIntegration.test.ts

@@ -1,5 +1,5 @@
 import type { Client } from '@sentry/core';
-import { createTransport, Scope, withScope } from '@sentry/core';
+import { createTransport, makeSession, Scope, withScope } from '@sentry/core';
 import { ServerRuntimeClient } from '@sentry/core/server';
 import { EventEmitter } from 'stream';
 import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
@@ -125,6 +125,27 @@ describe('recordRequestSession()', () => {
       ],
     });
   });
+
+  it('removes a session inherited from the parent isolation scope', () => {
+    const client = createTestClient();
+    const requestIsolationScope = new Scope();
+    requestIsolationScope.setSession(makeSession());
+
+    recordRequestSession(client, { requestIsolationScope, response: new EventEmitter() });
+
+    expect(requestIsolationScope.getSession()).toBeUndefined();
+  });
+
+  it('emits the "startRequestSession" hook for every recorded request', () => {
+    const client = createTestClient();
+    const onStartRequestSession = vi.fn();
+    client.on('startRequestSession', onStartRequestSession);
+
+    simulateRequest(client, 'ok');
+    simulateRequest(client, 'ok');
+
+    expect(onStartRequestSession).toHaveBeenCalledTimes(2);
+  });
 });
 
 function simulateRequest(client: Client, status: 'ok' | 'errored' | 'crashed') {

packages/node/test/integrations/processSession.test.ts

@@ -1,4 +1,4 @@
-import { getIsolationScope, setCurrentClient } from '@sentry/core';
+import { getIsolationScope, makeSession, setCurrentClient } from '@sentry/core';
 import { beforeEach, describe, expect, it, vi } from 'vitest';
 import { processSessionIntegration } from '../../src/integrations/processSession';
 import { NodeClient } from '../../src/sdk/client';
@@ -24,7 +24,7 @@ describe('processSessionIntegration', () => {
       return process;
     }) as never);
 
-    processSessionIntegration().setupOnce!();
+    processSessionIntegration().setup!(client);
     processOn.mockRestore();
   });
 
@@ -62,6 +62,56 @@ describe('processSessionIntegration', () => {
     expect(sendSession).not.toHaveBeenCalled();
   });
 
+  describe('when requests start being tracked as request sessions', () => {
+    it('removes a session that was never sent, without sending it', () => {
+      client.emit('startRequestSession');
+
+      expect(getIsolationScope().getSession()).toBeUndefined();
+      expect(sendSession).not.toHaveBeenCalled();
+    });
+
+    it('closes and removes a session that was already sent', () => {
+      const session = getIsolationScope().getSession()!;
+      session.init = false;
+      session.errors = 1;
+
+      client.emit('startRequestSession');
+
+      expect(sendSession).toHaveBeenCalledTimes(1);
+      expect(sendSession).toHaveBeenCalledWith(expect.objectContaining({ status: 'exited', errors: 1 }));
+      expect(getIsolationScope().getSession()).toBeUndefined();
+    });
+
+    it('removes an already-crashed session without updating it', () => {
+      const session = getIsolationScope().getSession()!;
+      session.init = false;
+      session.status = 'crashed';
+
+      client.emit('startRequestSession');
+
+      expect(sendSession).not.toHaveBeenCalled();
+      expect(getIsolationScope().getSession()).toBeUndefined();
+    });
+
+    it('only reacts to the first request session', () => {
+      client.emit('startRequestSession');
+      const laterSession = makeSession();
+      getIsolationScope().setSession(laterSession);
+
+      client.emit('startRequestSession');
+
+      expect(getIsolationScope().getSession()).toBe(laterSession);
+    });
+
+    it('does not send a session on exit afterwards', () => {
+      client.emit('startRequestSession');
+
+      beforeExitHandler();
+
+      expect(sendSession).not.toHaveBeenCalled();
+    });
+  });
+
   it('does nothing when no session is on the scope', () => {
     getIsolationScope().setSession(undefined);
     sendSession.mockClear();