ref(replay): Extract sample decision for session

mydea · mydea · commit ec0276ee2ff3 · 2022-12-06T10:17:46.000+01:00
diff --git a/packages/replay/src/session/Session.ts b/packages/replay/src/session/Session.ts
@@ -1,6 +1,5 @@
 import { uuid4 } from '@sentry/utils';
 
-import { SampleRates } from '../types';
 import { isSampled } from '../util/isSampled';
 
 type Sampled = false | 'session' | 'error';
@@ -38,13 +37,13 @@ export interface Session {
 /**
  * Get a session with defaults & applied sampling.
  */
-export function makeSession(session: Partial<Session>, { sessionSampleRate, errorSampleRate }: SampleRates): Session {
+export function makeSession(session: Partial<Session> & { sampled: Sampled }): Session {
   const now = new Date().getTime();
   const id = session.id || uuid4();
   const started = session.started || now;
   const lastActivity = session.lastActivity || now;
   const segmentId = session.segmentId || 0;
-  const sampled = sampleSession(session.sampled, { sessionSampleRate, errorSampleRate });
+  const sampled = session.sampled;
 
   return {
     id,
@@ -55,7 +54,14 @@ export function makeSession(session: Partial<Session>, { sessionSampleRate, erro
   };
 }
 
-function sampleSession(sampled: Sampled | undefined, { sessionSampleRate, errorSampleRate }: SampleRates): Sampled {
+/**
+ * Get the sampled status for a session based on sample rates & current sampled status.
+ */
+export function sampleSession(
+  sampled: Sampled | undefined,
+  sessionSampleRate: number,
+  errorSampleRate: number,
+): Sampled {
   if (typeof sampled !== 'undefined') {
     return sampled;
   }
diff --git a/packages/replay/src/session/createSession.ts b/packages/replay/src/session/createSession.ts
@@ -2,21 +2,18 @@ import { logger } from '@sentry/utils';
 
 import { SessionOptions } from '../types';
 import { saveSession } from './saveSession';
-import { makeSession, Session } from './Session';
+import { makeSession, sampleSession, Session } from './Session';
 
 /**
  * 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 session = makeSession(
-    {},
-    {
-      errorSampleRate,
-      sessionSampleRate,
-    },
-  );
+  const sampled = sampleSession(undefined, sessionSampleRate, errorSampleRate);
+  const session = makeSession({
+    sampled,
+  });
 
   __DEBUG_BUILD__ && logger.log(`[Replay] Creating new session: ${session.id}`);
 
diff --git a/packages/replay/src/session/fetchSession.ts b/packages/replay/src/session/fetchSession.ts
@@ -1,6 +1,6 @@
 import { REPLAY_SESSION_KEY, WINDOW } from '../constants';
 import { SampleRates } from '../types';
-import { makeSession, Session } from './Session';
+import { makeSession, sampleSession, Session } from './Session';
 
 /**
  * Fetches a session from storage
@@ -20,9 +20,10 @@ export function fetchSession({ sessionSampleRate, errorSampleRate }: SampleRates
       return null;
     }
 
-    const sessionObj = JSON.parse(sessionStringFromStorage);
+    const sessionObj = JSON.parse(sessionStringFromStorage) as Partial<Session>;
+    const sampled = sampleSession(sessionObj.sampled, sessionSampleRate, errorSampleRate);
 
-    return makeSession(sessionObj, { sessionSampleRate, errorSampleRate });
+    return makeSession({ ...sessionObj, sampled });
   } catch {
     return null;
   }
diff --git a/packages/replay/test/unit/session/Session.test.ts b/packages/replay/test/unit/session/Session.test.ts
@@ -26,7 +26,7 @@ jest.mock('@sentry/utils', () => {
 import * as Sentry from '@sentry/browser';
 
 import { WINDOW } from '../../../src/constants';
-import { makeSession } from '../../../src/session/Session';
+import { makeSession, sampleSession } from '../../../src/session/Session';
 
 type CaptureEventMockType = jest.MockedFunction<typeof Sentry.captureEvent>;
 
@@ -39,51 +39,33 @@ afterEach(() => {
 });
 
 it('does not sample', function () {
-  const newSession = makeSession(
-    {},
-    {
-      sessionSampleRate: 0.0,
-      errorSampleRate: 0.0,
-    },
-  );
+  const newSession = makeSession({
+    sampled: sampleSession(undefined, 0, 0),
+  });
 
   expect(newSession.sampled).toBe(false);
 });
 
 it('samples using `sessionSampleRate`', function () {
-  const newSession = makeSession(
-    {},
-    {
-      sessionSampleRate: 1.0,
-      errorSampleRate: 0.0,
-    },
-  );
+  const newSession = makeSession({
+    sampled: sampleSession(undefined, 1.0, 0),
+  });
 
   expect(newSession.sampled).toBe('session');
 });
 
 it('samples using `errorSampleRate`', function () {
-  const newSession = makeSession(
-    {},
-    {
-      sessionSampleRate: 0,
-      errorSampleRate: 1.0,
-    },
-  );
+  const newSession = makeSession({
+    sampled: sampleSession(undefined, 0, 1),
+  });
 
   expect(newSession.sampled).toBe('error');
 });
 
 it('does not run sampling function if existing session was sampled', function () {
-  const newSession = makeSession(
-    {
-      sampled: 'session',
-    },
-    {
-      sessionSampleRate: 0,
-      errorSampleRate: 0,
-    },
-  );
+  const newSession = makeSession({
+    sampled: 'session',
+  });
 
   expect(newSession.sampled).toBe('session');
 });
diff --git a/packages/replay/test/unit/session/fetchSession.test.ts b/packages/replay/test/unit/session/fetchSession.test.ts
@@ -23,14 +23,44 @@ const SAMPLE_RATES = {
 it('fetches a valid and sampled session', function () {
   WINDOW.sessionStorage.setItem(
     REPLAY_SESSION_KEY,
-    '{"id":"fd09adfc4117477abc8de643e5a5798a","sampled": true,"started":1648827162630,"lastActivity":1648827162658}',
+    '{"id":"fd09adfc4117477abc8de643e5a5798a","sampled": "session","started":1648827162630,"lastActivity":1648827162658}',
   );
 
   expect(fetchSession(SAMPLE_RATES)).toEqual({
     id: 'fd09adfc4117477abc8de643e5a5798a',
     lastActivity: 1648827162658,
     segmentId: 0,
-    sampled: true,
+    sampled: 'session',
+    started: 1648827162630,
+  });
+});
+
+it('fetches an unsampled session', function () {
+  WINDOW.sessionStorage.setItem(
+    REPLAY_SESSION_KEY,
+    '{"id":"fd09adfc4117477abc8de643e5a5798a","sampled": false,"started":1648827162630,"lastActivity":1648827162658}',
+  );
+
+  expect(fetchSession(SAMPLE_RATES)).toEqual({
+    id: 'fd09adfc4117477abc8de643e5a5798a',
+    lastActivity: 1648827162658,
+    segmentId: 0,
+    sampled: false,
+    started: 1648827162630,
+  });
+});
+
+it('auto-fixes a session without sampled', function () {
+  WINDOW.sessionStorage.setItem(
+    REPLAY_SESSION_KEY,
+    '{"id":"fd09adfc4117477abc8de643e5a5798a","started":1648827162630,"lastActivity":1648827162658}',
+  );
+
+  expect(fetchSession(SAMPLE_RATES)).toEqual({
+    id: 'fd09adfc4117477abc8de643e5a5798a',
+    lastActivity: 1648827162658,
+    segmentId: 0,
+    sampled: 'session',
     started: 1648827162630,
   });
 });
diff --git a/packages/replay/test/unit/session/getSession.test.ts b/packages/replay/test/unit/session/getSession.test.ts
@@ -18,16 +18,13 @@ const SAMPLE_RATES = {
 };
 
 function createMockSession(when: number = new Date().getTime()) {
-  return makeSession(
-    {
-      id: 'test_session_id',
-      segmentId: 0,
-      lastActivity: when,
-      started: when,
-      sampled: 'session',
-    },
-    { ...SAMPLE_RATES },
-  );
+  return makeSession({
+    id: 'test_session_id',
+    segmentId: 0,
+    lastActivity: when,
+    started: when,
+    sampled: 'session',
+  });
 }
 
 beforeAll(() => {
@@ -84,15 +81,13 @@ it('creates a non-sticky session, when one is expired', function () {
     expiry: 1000,
     stickySession: false,
     ...SAMPLE_RATES,
-    currentSession: makeSession(
-      {
-        id: 'old_session_id',
-        lastActivity: new Date().getTime() - 1001,
-        started: new Date().getTime() - 1001,
-        segmentId: 0,
-      },
-      { ...SAMPLE_RATES },
-    ),
+    currentSession: makeSession({
+      id: 'old_session_id',
+      lastActivity: new Date().getTime() - 1001,
+      started: new Date().getTime() - 1001,
+      segmentId: 0,
+      sampled: 'session',
+    }),
   });
 
   expect(FetchSession.fetchSession).not.toHaveBeenCalled();
@@ -180,15 +175,13 @@ it('fetches a non-expired non-sticky session', function () {
     expiry: 1000,
     stickySession: false,
     ...SAMPLE_RATES,
-    currentSession: makeSession(
-      {
-        id: 'test_session_id_2',
-        lastActivity: +new Date() - 500,
-        started: +new Date() - 500,
-        segmentId: 0,
-      },
-      { ...SAMPLE_RATES },
-    ),
+    currentSession: makeSession({
+      id: 'test_session_id_2',
+      lastActivity: +new Date() - 500,
+      started: +new Date() - 500,
+      segmentId: 0,
+      sampled: 'session',
+    }),
   });
 
   expect(FetchSession.fetchSession).not.toHaveBeenCalled();
diff --git a/packages/replay/test/unit/session/saveSession.test.ts b/packages/replay/test/unit/session/saveSession.test.ts
@@ -11,16 +11,13 @@ afterEach(() => {
 });
 
 it('saves a valid session', function () {
-  const session = makeSession(
-    {
-      id: 'fd09adfc4117477abc8de643e5a5798a',
-      segmentId: 0,
-      started: 1648827162630,
-      lastActivity: 1648827162658,
-      sampled: 'session',
-    },
-    { sessionSampleRate: 1.0, errorSampleRate: 0 },
-  );
+  const session = makeSession({
+    id: 'fd09adfc4117477abc8de643e5a5798a',
+    segmentId: 0,
+    started: 1648827162630,
+    lastActivity: 1648827162658,
+    sampled: 'session',
+  });
   saveSession(session);
 
   expect(WINDOW.sessionStorage.getItem(REPLAY_SESSION_KEY)).toEqual(JSON.stringify(session));
diff --git a/packages/replay/test/unit/util/isSessionExpired.test.ts b/packages/replay/test/unit/util/isSessionExpired.test.ts
@@ -2,16 +2,14 @@ import { makeSession } from '../../../src/session/Session';
 import { isSessionExpired } from '../../../src/util/isSessionExpired';
 
 function createSession(extra?: Record<string, any>) {
-  return makeSession(
-    {
-      // Setting started/lastActivity to 0 makes it use the default, which is `Date.now()`
-      started: 1,
-      lastActivity: 1,
-      segmentId: 0,
-      ...extra,
-    },
-    { sessionSampleRate: 1.0, errorSampleRate: 0 },
-  );
+  return makeSession({
+    // Setting started/lastActivity to 0 makes it use the default, which is `Date.now()`
+    started: 1,
+    lastActivity: 1,
+    segmentId: 0,
+    sampled: 'session',
+    ...extra,
+  });
 }
 
 it('session last activity is older than expiry time', function () {
