ref: change getSession param errorSampleRate to allowBuffering

billyvg · billyvg · commit 5d0e02409b02 · 2023-04-25T10:52:02.000+02:00
diff --git a/packages/replay/src/replay.ts b/packages/replay/src/replay.ts
@@ -230,7 +230,7 @@ export class ReplayContainer implements ReplayContainerInterface {
       currentSession: this.session,
       // This is intentional: create a new session-based replay when calling `start()`
       sessionSampleRate: 1,
-      errorSampleRate: 0,
+      allowBuffering: false,
     });
 
     session.previousSessionId = previousSessionId;
@@ -255,8 +255,7 @@ export class ReplayContainer implements ReplayContainerInterface {
       stickySession: Boolean(this._options.stickySession),
       currentSession: this.session,
       sessionSampleRate: 0,
-      // This is intentional: create a new buffer-based replay when calling `startBuffering()`
-      errorSampleRate: 1,
+      allowBuffering: true,
     });
 
     session.previousSessionId = previousSessionId;
@@ -596,7 +595,7 @@ export class ReplayContainer implements ReplayContainerInterface {
       stickySession: Boolean(this._options.stickySession),
       currentSession: this.session,
       sessionSampleRate: this._options.sessionSampleRate,
-      errorSampleRate: this._options.errorSampleRate,
+      allowBuffering: this._options.errorSampleRate > 0,
     });
 
     // If session was newly created (i.e. was not loaded from storage), then
diff --git a/packages/replay/src/session/Session.ts b/packages/replay/src/session/Session.ts
@@ -1,7 +1,6 @@
 import { uuid4 } from '@sentry/utils';
 
 import type { Sampled, Session } from '../types';
-import { isSampled } from '../util/isSampled';
 
 /**
  * Get a session with defaults & applied sampling.
@@ -24,10 +23,3 @@ export function makeSession(session: Partial<Session> & { sampled: Sampled }): S
     shouldRefresh: true,
   };
 }
-
-/**
- * Get the sampled status for a session based on sample rates & current sampled status.
- */
-export function getSessionSampleType(sessionSampleRate: number, errorSampleRate: number): Sampled {
-  return isSampled(sessionSampleRate) ? 'session' : errorSampleRate > 0 ? 'buffer' : false;
-}
diff --git a/packages/replay/src/session/createSession.ts b/packages/replay/src/session/createSession.ts
@@ -1,16 +1,24 @@
 import { logger } from '@sentry/utils';
 
-import type { Session, SessionOptions } from '../types';
+import type { Sampled,Session, SessionOptions } from '../types';
+import { isSampled } from '../util/isSampled';
 import { saveSession } from './saveSession';
-import { getSessionSampleType, makeSession } from './Session';
+import { makeSession } from './Session';
+
+/**
+ * Get the sampled status for a session based on sample rates & current sampled status.
+ */
+export function getSessionSampleType(sessionSampleRate: number, allowBuffering: boolean): Sampled {
+  return isSampled(sessionSampleRate) ? 'session' : allowBuffering ? 'buffer' : false;
+}
 
 /**
  * Create a new session, which in its current implementation is a Sentry event
  * that all replays will be saved to as attachments. Currently, we only expect
  * one of these Sentry events per "replay session".
  */
-export function createSession({ sessionSampleRate, errorSampleRate, stickySession = false }: SessionOptions): Session {
-  const sampled = getSessionSampleType(sessionSampleRate, errorSampleRate);
+export function createSession({ sessionSampleRate, allowBuffering, stickySession = false }: SessionOptions): Session {
+  const sampled = getSessionSampleType(sessionSampleRate, allowBuffering);
   const session = makeSession({
     sampled,
   });
diff --git a/packages/replay/src/session/getSession.ts b/packages/replay/src/session/getSession.ts
@@ -23,7 +23,7 @@ export function getSession({
   currentSession,
   stickySession,
   sessionSampleRate,
-  errorSampleRate,
+  allowBuffering,
 }: GetSessionParams): { type: 'new' | 'saved'; session: Session } {
   // If session exists and is passed, use it instead of always hitting session storage
   const session = currentSession || (stickySession && fetchSession());
@@ -50,7 +50,7 @@ export function getSession({
   const newSession = createSession({
     stickySession,
     sessionSampleRate,
-    errorSampleRate,
+    allowBuffering,
   });
 
   return { type: 'new', session: newSession };
diff --git a/packages/replay/src/types.ts b/packages/replay/src/types.ts
@@ -182,32 +182,39 @@ export interface WorkerResponse {
 
 export type AddEventResult = void;
 
-export interface SampleRates {
+interface SessionAndPluginOptions {
   /**
    * The sample rate for session-long replays. 1.0 will record all sessions and
    * 0 will record none.
    */
   sessionSampleRate: number;
 
   /**
-   * The sample rate for sessions that has had an error occur. This is
-   * independent of `sessionSampleRate`.
+   * If false, will create a new session per pageload. Otherwise, saves session
+   * to Session Storage.
    */
-  errorSampleRate: number;
+  stickySession: boolean;
 }
 
 /**
  * Session options that are configurable by the integration configuration
  */
-export interface SessionOptions extends SampleRates {
+export interface SessionOptions extends SessionAndPluginOptions {
   /**
-   * If false, will create a new session per pageload. Otherwise, saves session
-   * to Session Storage.
+   * Should buffer recordings to be saved later either by error sampling, or by
+   * manually calling `flush()`. This is only a factor if not sampled for a
+   * session-based replay.
    */
-  stickySession: boolean;
+  allowBuffering: boolean;
 }
 
-export interface ReplayPluginOptions extends SessionOptions {
+export interface ReplayPluginOptions extends SessionAndPluginOptions {
+  /**
+   * The sample rate for each error event. This is only a factor if not sampled
+   * for a session-based replay.
+   */
+  errorSampleRate: number;
+
   /**
    * The amount of time to wait before sending a replay
    */
diff --git a/packages/replay/test/unit/session/createSession.test.ts b/packages/replay/test/unit/session/createSession.test.ts
@@ -34,7 +34,7 @@ describe('Unit | session | createSession', () => {
     const newSession = createSession({
       stickySession: false,
       sessionSampleRate: 1.0,
-      errorSampleRate: 0,
+      allowBuffering: false,
     });
     expect(captureEventMock).not.toHaveBeenCalled();
 
@@ -49,7 +49,7 @@ describe('Unit | session | createSession', () => {
     const newSession = createSession({
       stickySession: true,
       sessionSampleRate: 1.0,
-      errorSampleRate: 0,
+      allowBuffering: false,
     });
     expect(captureEventMock).not.toHaveBeenCalled();
 
diff --git a/packages/replay/test/unit/session/getSession.test.ts b/packages/replay/test/unit/session/getSession.test.ts
@@ -12,9 +12,9 @@ jest.mock('@sentry/utils', () => {
   };
 });
 
-const SAMPLE_RATES = {
+const SAMPLE_OPTIONS = {
   sessionSampleRate: 1.0,
-  errorSampleRate: 0,
+  allowBuffering: false,
 };
 
 function createMockSession(when: number = Date.now()) {
@@ -48,7 +48,7 @@ describe('Unit | session | getSession', () => {
         maxSessionLife: MAX_SESSION_LIFE,
       },
       stickySession: false,
-      ...SAMPLE_RATES,
+      ...SAMPLE_OPTIONS,
     });
 
     expect(FetchSession.fetchSession).not.toHaveBeenCalled();
@@ -76,7 +76,7 @@ describe('Unit | session | getSession', () => {
         maxSessionLife: MAX_SESSION_LIFE,
       },
       stickySession: false,
-      ...SAMPLE_RATES,
+      ...SAMPLE_OPTIONS,
     });
 
     expect(FetchSession.fetchSession).not.toHaveBeenCalled();
@@ -92,7 +92,7 @@ describe('Unit | session | getSession', () => {
         maxSessionLife: MAX_SESSION_LIFE,
       },
       stickySession: false,
-      ...SAMPLE_RATES,
+      ...SAMPLE_OPTIONS,
       currentSession: makeSession({
         id: 'old_session_id',
         lastActivity: Date.now() - 1001,
@@ -119,7 +119,7 @@ describe('Unit | session | getSession', () => {
       },
       stickySession: true,
       sessionSampleRate: 1.0,
-      errorSampleRate: 0.0,
+      allowBuffering: false,
     });
 
     expect(FetchSession.fetchSession).toHaveBeenCalled();
@@ -156,7 +156,7 @@ describe('Unit | session | getSession', () => {
       },
       stickySession: true,
       sessionSampleRate: 1.0,
-      errorSampleRate: 0.0,
+      allowBuffering: false,
     });
 
     expect(FetchSession.fetchSession).toHaveBeenCalled();
@@ -182,7 +182,7 @@ describe('Unit | session | getSession', () => {
         maxSessionLife: MAX_SESSION_LIFE,
       },
       stickySession: true,
-      ...SAMPLE_RATES,
+      ...SAMPLE_OPTIONS,
     });
 
     expect(FetchSession.fetchSession).toHaveBeenCalled();
@@ -201,7 +201,7 @@ describe('Unit | session | getSession', () => {
         maxSessionLife: MAX_SESSION_LIFE,
       },
       stickySession: false,
-      ...SAMPLE_RATES,
+      ...SAMPLE_OPTIONS,
       currentSession: makeSession({
         id: 'test_session_uuid_2',
         lastActivity: +new Date() - 500,
diff --git a/packages/replay/test/unit/session/sessionSampling.test.ts b/packages/replay/test/unit/session/sessionSampling.test.ts
@@ -1,25 +1,26 @@
-import { getSessionSampleType, makeSession } from '../../../src/session/Session';
+import { getSessionSampleType} from '../../../src/session/createSession';
+import { makeSession } from '../../../src/session/Session';
 
 describe('Unit | session | sessionSampling', () => {
   it('does not sample', function () {
     const newSession = makeSession({
-      sampled: getSessionSampleType(0, 0),
+      sampled: getSessionSampleType(0, false),
     });
 
     expect(newSession.sampled).toBe(false);
   });
 
   it('samples using `sessionSampleRate`', function () {
     const newSession = makeSession({
-      sampled: getSessionSampleType(1.0, 0),
+      sampled: getSessionSampleType(1.0, false),
     });
 
     expect(newSession.sampled).toBe('session');
   });
 
   it('samples using `errorSampleRate`', function () {
     const newSession = makeSession({
-      sampled: getSessionSampleType(0, 1),
+      sampled: getSessionSampleType(0, true),
     });
 
     expect(newSession.sampled).toBe('buffer');
@@ -32,4 +33,4 @@ describe('Unit | session | sessionSampling', () => {
 
     expect(newSession.sampled).toBe('session');
   });
-});
+})
