feat(core): Deprecate APIs around RequestSessions (#14566)

Author: lforst

dev-packages/node-integration-tests/suites/sessions/server.ts

@@ -13,6 +13,7 @@ import express from 'express';
 
 const app = express();
 
+// eslint-disable-next-line deprecation/deprecation
 const flusher = (Sentry.getClient() as Sentry.NodeClient)['_sessionFlusher'] as SessionFlusher;
 
 // Flush after 2 seconds (to avoid waiting for the default 60s)

docs/migration/draft-v9-migration-guide.md

@@ -47,6 +47,10 @@
 - Deprecated `addTracingHeadersToFetchRequest` method - this was only meant for internal use and is not needed anymore.
 - Deprecated `generatePropagationContext()` in favor of using `generateTraceId()` directly.
 - Deprecated `spanId` field on `propagationContext` - this field will be removed in v9, and should neither be read or set anymore.
+- Deprecated `RequestSession` type. No replacements.
+- Deprecated `RequestSessionStatus` type. No replacements.
+- Deprecated `SessionFlusherLike` type. No replacements.
+- Deprecated `SessionFlusher`. No replacements.
 
 ## `@sentry/nestjs`
 
@@ -69,6 +73,9 @@
 - **The `@sentry/types` package has been deprecated. Import everything from `@sentry/core` instead.**
 
 - Deprecated `Request` in favor of `RequestEventData`.
+- Deprecated `RequestSession`. No replacements.
+- Deprecated `RequestSessionStatus`. No replacements.
+- Deprecated `SessionFlusherLike`. No replacements.
 
 ## `@sentry/nuxt`
 

packages/core/src/index.ts

@@ -47,6 +47,7 @@ export {
 export { setAsyncContextStrategy } from './asyncContext';
 export { getMainCarrier } from './carrier';
 export { makeSession, closeSession, updateSession } from './session';
+// eslint-disable-next-line deprecation/deprecation
 export { SessionFlusher } from './sessionflusher';
 export { Scope } from './scope';
 export { notifyEventProcessors } from './eventProcessors';

packages/core/src/scope.ts

@@ -94,6 +94,7 @@ class ScopeClass implements ScopeInterface {
   protected _session?: Session;
 
   /** Request Mode Session Status */
+  // eslint-disable-next-line deprecation/deprecation
   protected _requestSession?: RequestSession;
 
   /** The client on this scope */
@@ -230,13 +231,15 @@ class ScopeClass implements ScopeInterface {
   /**
    * @inheritDoc
    */
+  // eslint-disable-next-line deprecation/deprecation
   public getRequestSession(): RequestSession | undefined {
     return this._requestSession;
   }
 
   /**
    * @inheritDoc
    */
+  // eslint-disable-next-line deprecation/deprecation
   public setRequestSession(requestSession?: RequestSession): this {
     this._requestSession = requestSession;
     return this;
@@ -358,7 +361,8 @@ class ScopeClass implements ScopeInterface {
 
     const [scopeInstance, requestSession] =
       scopeToMerge instanceof Scope
-        ? [scopeToMerge.getScopeData(), scopeToMerge.getRequestSession()]
+        ? // eslint-disable-next-line deprecation/deprecation
+          [scopeToMerge.getScopeData(), scopeToMerge.getRequestSession()]
         : isPlainObject(scopeToMerge)
           ? [captureContext as ScopeContext, (captureContext as ScopeContext).requestSession]
           : [];

packages/core/src/server-runtime-client.ts

@@ -42,6 +42,7 @@ export interface ServerRuntimeClientOptions extends ClientOptions<BaseTransportO
 export class ServerRuntimeClient<
   O extends ClientOptions & ServerRuntimeClientOptions = ServerRuntimeClientOptions,
 > extends BaseClient<O> {
+  // eslint-disable-next-line deprecation/deprecation
   protected _sessionFlusher: SessionFlusher | undefined;
 
   /**
@@ -80,10 +81,12 @@ export class ServerRuntimeClient<
    */
   // eslint-disable-next-line @typescript-eslint/no-explicit-any
   public captureException(exception: any, hint?: EventHint, scope?: Scope): string {
-    // Check if the flag `autoSessionTracking` is enabled, and if `_sessionFlusher` exists because it is initialised only
-    // when the `requestHandler` middleware is used, and hence the expectation is to have SessionAggregates payload
-    // sent to the Server only when the `requestHandler` middleware is used
+    // Check if `_sessionFlusher` exists because it is initialized (defined) only when the `autoSessionTracking` is enabled.
+    // The expectation is that session aggregates are only sent when `autoSessionTracking` is enabled.
+    // TODO(v9): Our goal in the future is to not have the `autoSessionTracking` option and instead rely on integrations doing the creation and sending of sessions. We will not have a central kill-switch for sessions.
+    // TODO(v9): This should move into the httpIntegration.
     if (this._options.autoSessionTracking && this._sessionFlusher) {
+      // eslint-disable-next-line deprecation/deprecation
       const requestSession = getIsolationScope().getRequestSession();
 
       // Necessary checks to ensure this is code block is executed only within a request
@@ -100,16 +103,18 @@ export class ServerRuntimeClient<
    * @inheritDoc
    */
   public captureEvent(event: Event, hint?: EventHint, scope?: Scope): string {
-    // Check if the flag `autoSessionTracking` is enabled, and if `_sessionFlusher` exists because it is initialised only
-    // when the `requestHandler` middleware is used, and hence the expectation is to have SessionAggregates payload
-    // sent to the Server only when the `requestHandler` middleware is used
+    // Check if `_sessionFlusher` exists because it is initialized only when the `autoSessionTracking` is enabled.
+    // The expectation is that session aggregates are only sent when `autoSessionTracking` is enabled.
+    // TODO(v9): Our goal in the future is to not have the `autoSessionTracking` option and instead rely on integrations doing the creation and sending of sessions. We will not have a central kill-switch for sessions.
+    // TODO(v9): This should move into the httpIntegration.
     if (this._options.autoSessionTracking && this._sessionFlusher) {
       const eventType = event.type || 'exception';
       const isException =
         eventType === 'exception' && event.exception && event.exception.values && event.exception.values.length > 0;
 
       // If the event is of type Exception, then a request session should be captured
       if (isException) {
+        // eslint-disable-next-line deprecation/deprecation
         const requestSession = getIsolationScope().getRequestSession();
 
         // Ensure that this is happening within the bounds of a request, and make sure not to override
@@ -134,12 +139,19 @@ export class ServerRuntimeClient<
     return super.close(timeout);
   }
 
-  /** Method that initialises an instance of SessionFlusher on Client */
+  /**
+   * Initializes an instance of SessionFlusher on the client which will aggregate and periodically flush session data.
+   *
+   * NOTICE: This method will implicitly create an interval that is periodically called.
+   * To clean up this resources, call `.close()` when you no longer intend to use the client.
+   * Not doing so will result in a memory leak.
+   */
   public initSessionFlusher(): void {
     const { release, environment } = this._options;
     if (!release) {
-      DEBUG_BUILD && logger.warn('Cannot initialise an instance of SessionFlusher if no release is provided!');
+      DEBUG_BUILD && logger.warn('Cannot initialize an instance of SessionFlusher if no release is provided!');
     } else {
+      // eslint-disable-next-line deprecation/deprecation
       this._sessionFlusher = new SessionFlusher(this, {
         release,
         environment,
@@ -214,6 +226,8 @@ export class ServerRuntimeClient<
   /**
    * Method responsible for capturing/ending a request session by calling `incrementSessionStatusCount` to increment
    * appropriate session aggregates bucket
+   *
+   * @deprecated This method should not be used or extended. It's functionality will move into the `httpIntegration` and not be part of any public API.
    */
   protected _captureRequestSession(): void {
     if (!this._sessionFlusher) {

packages/core/src/sessionflusher.ts

@@ -14,8 +14,10 @@ type ReleaseHealthAttributes = {
 };
 
 /**
- * @inheritdoc
+ * @deprecated `SessionFlusher` is deprecated and will be removed in the next major version of the SDK.
  */
+// TODO(v9): The goal for the SessionFlusher is to become a stupidly simple mechanism to aggregate "Sessions" (actually "RequestSessions"). It should probably live directly inside the Http integration/instrumentation.
+// eslint-disable-next-line deprecation/deprecation
 export class SessionFlusher implements SessionFlusherLike {
   public readonly flushTimeout: number;
   private _pendingAggregates: Map<number, AggregationCounts>;
@@ -80,12 +82,14 @@ export class SessionFlusher implements SessionFlusherLike {
       return;
     }
     const isolationScope = getIsolationScope();
+    // eslint-disable-next-line deprecation/deprecation
     const requestSession = isolationScope.getRequestSession();
 
     if (requestSession && requestSession.status) {
       this._incrementSessionStatusCount(requestSession.status, new Date());
       // This is not entirely necessarily but is added as a safe guard to indicate the bounds of a request and so in
       // case captureRequestSession is called more than once to prevent double count
+      // eslint-disable-next-line deprecation/deprecation
       isolationScope.setRequestSession(undefined);
       /* eslint-enable @typescript-eslint/no-unsafe-member-access */
     }
@@ -95,6 +99,7 @@ export class SessionFlusher implements SessionFlusherLike {
    * Increments status bucket in pendingAggregates buffer (internal state) corresponding to status of
    * the session received
    */
+  // eslint-disable-next-line deprecation/deprecation
   private _incrementSessionStatusCount(status: RequestSessionStatus, date: Date): number {
     // Truncate minutes and seconds on Session Started attribute to have one minute bucket keys
     const sessionStartedTrunc = new Date(date).setSeconds(0, 0);

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

@@ -104,8 +104,11 @@ export type {
   Session,
   SessionContext,
   SessionStatus,
+  // eslint-disable-next-line deprecation/deprecation
   RequestSession,
+  // eslint-disable-next-line deprecation/deprecation
   RequestSessionStatus,
+  // eslint-disable-next-line deprecation/deprecation
   SessionFlusherLike,
   SerializedSession,
 } from './session';

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

@@ -23,6 +23,7 @@ export interface ScopeContext {
   contexts: Contexts;
   tags: { [key: string]: Primitive };
   fingerprint: string[];
+  // eslint-disable-next-line deprecation/deprecation
   requestSession: RequestSession;
   propagationContext: PropagationContext;
 }
@@ -168,12 +169,18 @@ export interface Scope {
 
   /**
    * Returns the `RequestSession` if there is one
+   *
+   * @deprecated Use `getSession()` and `setSession()` instead of `getRequestSession()` and `setRequestSession()`;
    */
+  // eslint-disable-next-line deprecation/deprecation
   getRequestSession(): RequestSession | undefined;
 
   /**
    * Sets the `RequestSession` on the scope
+   *
+   * @deprecated Use `getSession()` and `setSession()` instead of `getRequestSession()` and `setRequestSession()`;
    */
+  // eslint-disable-next-line deprecation/deprecation
   setRequestSession(requestSession?: RequestSession): this;
 
   /**

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

@@ -1,6 +1,10 @@
 import type { User } from './user';
 
+/**
+ * @deprecated This type is deprecated and will be removed in the next major version of the SDK.
+ */
 export interface RequestSession {
+  // eslint-disable-next-line deprecation/deprecation
   status?: RequestSessionStatus;
 }
 
@@ -35,6 +39,10 @@ export interface Session {
 export type SessionContext = Partial<Session>;
 
 export type SessionStatus = 'ok' | 'exited' | 'crashed' | 'abnormal';
+
+/**
+ * @deprecated This type is deprecated and will be removed in the next major version of the SDK.
+ */
 export type RequestSessionStatus = 'ok' | 'errored' | 'crashed';
 
 /** JSDoc */
@@ -46,6 +54,9 @@ export interface SessionAggregates {
   aggregates: Array<AggregationCounts>;
 }
 
+/**
+ * @deprecated This type is deprecated and will be removed in the next major version of the SDK.
+ */
 export interface SessionFlusherLike {
   /**
    * Increments the Session Status bucket in SessionAggregates Object corresponding to the status of the session

packages/core/test/lib/scope.test.ts

@@ -261,8 +261,10 @@ describe('Scope', () => {
 
     test('_requestSession clone', () => {
       const parentScope = new Scope();
+      // eslint-disable-next-line deprecation/deprecation
       parentScope.setRequestSession({ status: 'errored' });
       const scope = parentScope.clone();
+      // eslint-disable-next-line deprecation/deprecation
       expect(parentScope.getRequestSession()).toEqual(scope.getRequestSession());
     });
 
@@ -288,15 +290,19 @@ describe('Scope', () => {
       // Test that ensures if the status value of `status` of `_requestSession` is changed in a child scope
       // that it should also change in parent scope because we are copying the reference to the object
       const parentScope = new Scope();
+      // eslint-disable-next-line deprecation/deprecation
       parentScope.setRequestSession({ status: 'errored' });
 
       const scope = parentScope.clone();
+      // eslint-disable-next-line deprecation/deprecation
       const requestSession = scope.getRequestSession();
       if (requestSession) {
         requestSession.status = 'ok';
       }
 
+      // eslint-disable-next-line deprecation/deprecation
       expect(parentScope.getRequestSession()).toEqual({ status: 'ok' });
+      // eslint-disable-next-line deprecation/deprecation
       expect(scope.getRequestSession()).toEqual({ status: 'ok' });
     });
 
@@ -316,6 +322,7 @@ describe('Scope', () => {
     scope.setUser({ id: '1' });
     scope.setFingerprint(['abcd']);
     scope.addBreadcrumb({ message: 'test' });
+    // eslint-disable-next-line deprecation/deprecation
     scope.setRequestSession({ status: 'ok' });
     expect(scope['_extra']).toEqual({ a: 2 });
     scope.clear();
@@ -349,6 +356,7 @@ describe('Scope', () => {
       scope.setUser({ id: '1337' });
       scope.setLevel('info');
       scope.setFingerprint(['foo']);
+      // eslint-disable-next-line deprecation/deprecation
       scope.setRequestSession({ status: 'ok' });
     });
 
@@ -453,6 +461,7 @@ describe('Scope', () => {
         level: 'warning' as const,
         tags: { bar: '3', baz: '4' },
         user: { id: '42' },
+        // eslint-disable-next-line deprecation/deprecation
         requestSession: { status: 'errored' as RequestSessionStatus },
         propagationContext: {
           traceId: '8949daf83f4a4a70bee4c1eb9ab242ed',

packages/core/test/lib/sessionflusher.test.ts

@@ -19,6 +19,7 @@ describe('Session Flusher', () => {
   });
 
   test('test incrementSessionStatusCount updates the internal SessionFlusher state', () => {
+    // eslint-disable-next-line deprecation/deprecation
     const flusher = new SessionFlusher(mockClient, { release: '1.0.0', environment: 'dev' });
 
     const date = new Date('2021-04-08T12:18:23.043Z');
@@ -42,6 +43,7 @@ describe('Session Flusher', () => {
   });
 
   test('test undefined attributes are excluded, on incrementSessionStatusCount call', () => {
+    // eslint-disable-next-line deprecation/deprecation
     const flusher = new SessionFlusher(mockClient, { release: '1.0.0' });
 
     const date = new Date('2021-04-08T12:18:23.043Z');
@@ -55,6 +57,7 @@ describe('Session Flusher', () => {
   });
 
   test('flush is called every 60 seconds after initialisation of an instance of SessionFlusher', () => {
+    // eslint-disable-next-line deprecation/deprecation
     const flusher = new SessionFlusher(mockClient, { release: '1.0.0', environment: 'dev' });
     const flusherFlushFunc = jest.spyOn(flusher, 'flush');
     jest.advanceTimersByTime(59000);
@@ -68,6 +71,7 @@ describe('Session Flusher', () => {
   });
 
   test('sendSessions is called on flush if sessions were captured', () => {
+    // eslint-disable-next-line deprecation/deprecation
     const flusher = new SessionFlusher(mockClient, { release: '1.0.0', environment: 'dev' });
     const flusherFlushFunc = jest.spyOn(flusher, 'flush');
     const date = new Date('2021-04-08T12:18:23.043Z');
@@ -88,6 +92,7 @@ describe('Session Flusher', () => {
   });
 
   test('sendSessions is not called on flush if no sessions were captured', () => {
+    // eslint-disable-next-line deprecation/deprecation
     const flusher = new SessionFlusher(mockClient, { release: '1.0.0', environment: 'dev' });
     const flusherFlushFunc = jest.spyOn(flusher, 'flush');
 
@@ -98,12 +103,14 @@ describe('Session Flusher', () => {
   });
 
   test('calling close on SessionFlusher should disable SessionFlusher', () => {
+    // eslint-disable-next-line deprecation/deprecation
     const flusher = new SessionFlusher(mockClient, { release: '1.0.x' });
     flusher.close();
     expect((flusher as any)._isEnabled).toEqual(false);
   });
 
   test('calling close on SessionFlusher will force call flush', () => {
+    // eslint-disable-next-line deprecation/deprecation
     const flusher = new SessionFlusher(mockClient, { release: '1.0.x' });
     const flusherFlushFunc = jest.spyOn(flusher, 'flush');
     const date = new Date('2021-04-08T12:18:23.043Z');

packages/node/src/integrations/http/SentryHttpInstrumentation.ts

@@ -148,6 +148,7 @@ export class SentryHttpInstrumentation extends InstrumentationBase<SentryHttpIns
 
         const client = getClient<NodeClient>();
         if (client && client.getOptions().autoSessionTracking) {
+          // eslint-disable-next-line deprecation/deprecation
           isolationScope.setRequestSession({ status: 'ok' });
         }
 

packages/node/src/integrations/tracing/express.ts

@@ -129,6 +129,7 @@ export function expressErrorHandler(options?: ExpressHandlerOptions): ExpressMid
         // running in SessionAggregates mode
         const isSessionAggregatesMode = client['_sessionFlusher'] !== undefined;
         if (isSessionAggregatesMode) {
+          // eslint-disable-next-line deprecation/deprecation
           const requestSession = getIsolationScope().getRequestSession();
           // If an error bubbles to the `errorHandler`, then this is an unhandled error, and should be reported as a
           // Crashed session. The `_requestSession.status` is checked to ensure that this error is happening within

packages/node/src/sdk/index.ts

@@ -156,6 +156,7 @@ function _init(
 
   logger.log(`Running in ${isCjs() ? 'CommonJS' : 'ESM'} mode.`);
 
+  // TODO(V9): Remove this code since all of the logic should be in an integration
   if (options.autoSessionTracking) {
     startSessionTracking();
   }