Merge pull request #22325 from storybookjs/shilman/persistent-session-id

Telemetry: Persist sessionId across runs

code/lib/telemetry/src/session-id.test.ts

@@ -0,0 +1,100 @@
+import { nanoid } from 'nanoid';
+import { cache } from '@storybook/core-common';
+import { resetSessionIdForTest, getSessionId, SESSION_TIMEOUT } from './session-id';
+
+jest.mock('@storybook/core-common', () => {
+  const actual = jest.requireActual('@storybook/core-common');
+  return {
+    ...actual,
+    cache: {
+      get: jest.fn(),
+      set: jest.fn(),
+    },
+  };
+});
+jest.mock('nanoid');
+
+const spy = (x: any) => x as jest.SpyInstance;
+
+describe('getSessionId', () => {
+  beforeEach(() => {
+    jest.clearAllMocks();
+    resetSessionIdForTest();
+  });
+
+  test('returns existing sessionId when cached in memory and does not fetch from disk', async () => {
+    const existingSessionId = 'memory-session-id';
+    resetSessionIdForTest(existingSessionId);
+
+    const sessionId = await getSessionId();
+
+    expect(cache.get).not.toHaveBeenCalled();
+    expect(cache.set).toHaveBeenCalledTimes(1);
+    expect(cache.set).toHaveBeenCalledWith(
+      'session',
+      expect.objectContaining({ id: existingSessionId })
+    );
+    expect(sessionId).toBe(existingSessionId);
+  });
+
+  test('returns existing sessionId when cached on disk and not expired', async () => {
+    const existingSessionId = 'existing-session-id';
+    const existingSession = {
+      id: existingSessionId,
+      lastUsed: Date.now() - SESSION_TIMEOUT + 1000,
+    };
+
+    spy(cache.get).mockResolvedValueOnce(existingSession);
+
+    const sessionId = await getSessionId();
+
+    expect(cache.get).toHaveBeenCalledTimes(1);
+    expect(cache.get).toHaveBeenCalledWith('session');
+    expect(cache.set).toHaveBeenCalledTimes(1);
+    expect(cache.set).toHaveBeenCalledWith(
+      'session',
+      expect.objectContaining({ id: existingSessionId })
+    );
+    expect(sessionId).toBe(existingSessionId);
+  });
+
+  test('generates new sessionId when none exists', async () => {
+    const newSessionId = 'new-session-id';
+    (nanoid as any as jest.SpyInstance).mockReturnValueOnce(newSessionId);
+
+    spy(cache.get).mockResolvedValueOnce(undefined);
+
+    const sessionId = await getSessionId();
+
+    expect(cache.get).toHaveBeenCalledTimes(1);
+    expect(cache.get).toHaveBeenCalledWith('session');
+    expect(nanoid).toHaveBeenCalledTimes(1);
+    expect(cache.set).toHaveBeenCalledTimes(1);
+    expect(cache.set).toHaveBeenCalledWith(
+      'session',
+      expect.objectContaining({ id: newSessionId })
+    );
+    expect(sessionId).toBe(newSessionId);
+  });
+
+  test('generates new sessionId when existing one is expired', async () => {
+    const expiredSessionId = 'expired-session-id';
+    const expiredSession = { id: expiredSessionId, lastUsed: Date.now() - SESSION_TIMEOUT - 1000 };
+    const newSessionId = 'new-session-id';
+    spy(nanoid).mockReturnValueOnce(newSessionId);
+
+    spy(cache.get).mockResolvedValueOnce(expiredSession);
+
+    const sessionId = await getSessionId();
+
+    expect(cache.get).toHaveBeenCalledTimes(1);
+    expect(cache.get).toHaveBeenCalledWith('session');
+    expect(nanoid).toHaveBeenCalledTimes(1);
+    expect(cache.set).toHaveBeenCalledTimes(1);
+    expect(cache.set).toHaveBeenCalledWith(
+      'session',
+      expect.objectContaining({ id: newSessionId })
+    );
+    expect(sessionId).toBe(newSessionId);
+  });
+});

code/lib/telemetry/src/session-id.ts

@@ -0,0 +1,29 @@
+import { nanoid } from 'nanoid';
+import { cache } from '@storybook/core-common';
+
+export const SESSION_TIMEOUT = 1000 * 60 * 60 * 2; // 2h
+
+interface Session {
+  id: string;
+  lastUsed: number;
+}
+
+let sessionId: string | undefined;
+
+export const resetSessionIdForTest = (val: string | undefined = undefined) => {
+  sessionId = val;
+};
+
+export const getSessionId = async () => {
+  const now = Date.now();
+  if (!sessionId) {
+    const session: Session | undefined = await cache.get('session');
+    if (session && session.lastUsed >= now - SESSION_TIMEOUT) {
+      sessionId = session.id;
+    } else {
+      sessionId = nanoid();
+    }
+  }
+  await cache.set('session', { id: sessionId, lastUsed: now });
+  return sessionId;
+};

code/lib/telemetry/src/telemetry.test.ts

@@ -5,6 +5,17 @@ import fetch from 'isomorphic-unfetch';
 import { sendTelemetry } from './telemetry';
 
 jest.mock('isomorphic-unfetch');
+jest.mock('./event-cache', () => {
+  return { set: jest.fn() };
+});
+
+jest.mock('./session-id', () => {
+  return {
+    getSessionId: async () => {
+      return 'session-id';
+    },
+  };
+});
 
 const fetchMock = fetch as jest.Mock;
 
@@ -54,7 +65,11 @@ it('await all pending telemetry when passing in immediate = true', async () => {
   let numberOfResolvedTasks = 0;
 
   fetchMock.mockImplementation(async () => {
-    await Promise.resolve(null);
+    // wait 10ms so that the "fetch" is still running while
+    // getSessionId resolves immediately below. tricky!
+    await new Promise((resolve) => {
+      setTimeout(resolve, 10);
+    });
     numberOfResolvedTasks += 1;
     return { status: 200 };
   });
@@ -72,6 +87,11 @@ it('await all pending telemetry when passing in immediate = true', async () => {
     payload: { foo: 'bar' },
   });
 
+  // wait for getSessionId to finish, but not for fetches
+  await new Promise((resolve) => {
+    setTimeout(resolve, 0);
+  });
+
   expect(fetch).toHaveBeenCalledTimes(2);
   expect(numberOfResolvedTasks).toBe(0);
 

code/lib/telemetry/src/telemetry.ts

@@ -4,18 +4,14 @@ import { nanoid } from 'nanoid';
 import type { Options, TelemetryData } from './types';
 import { getAnonymousProjectId } from './anonymous-id';
 import { set as saveToCache } from './event-cache';
+import { getSessionId } from './session-id';
 
 const URL = process.env.STORYBOOK_TELEMETRY_URL || 'https://storybook.js.org/event-log';
 
 const fetch = retry(originalFetch);
 
 let tasks: Promise<any>[] = [];
 
-// getStorybookMetadata -> packagejson + Main.js
-// event specific data: sessionId, ip, etc..
-// send telemetry
-const sessionId = nanoid();
-
 export const addToGlobalContext = (key: string, value: any) => {
   globalContext[key] = value;
 };
@@ -28,47 +24,59 @@ const globalContext = {
   isTTY: process.stdout.isTTY,
 } as Record<string, any>;
 
+const prepareRequest = async (data: TelemetryData, context: Record<string, any>, options: any) => {
+  const { eventType, payload, metadata, ...rest } = data;
+  const sessionId = await getSessionId();
+  const eventId = nanoid();
+  const body = { ...rest, eventType, eventId, sessionId, metadata, payload, context };
+
+  return fetch(URL, {
+    method: 'POST',
+    body: JSON.stringify(body),
+    headers: { 'Content-Type': 'application/json' },
+    retries: 3,
+    retryOn: [503, 504],
+    retryDelay: (attempt: number) =>
+      2 ** attempt *
+      (typeof options?.retryDelay === 'number' && !Number.isNaN(options?.retryDelay)
+        ? options.retryDelay
+        : 1000),
+  });
+};
+
 export async function sendTelemetry(
   data: TelemetryData,
   options: Partial<Options> = { retryDelay: 1000, immediate: false }
 ) {
+  const { eventType, payload, metadata, ...rest } = data;
+
   // We use this id so we can de-dupe events that arrive at the index multiple times due to the
   // use of retries. There are situations in which the request "5xx"s (or times-out), but
   // the server actually gets the request and stores it anyway.
 
   // flatten the data before we send it
-  const { eventType, payload, metadata, ...rest } = data;
+
   const context = options.stripMetadata
     ? globalContext
     : {
         ...globalContext,
         anonymousId: getAnonymousProjectId(),
       };
-  const eventId = nanoid();
-  const body = { ...rest, eventType, eventId, sessionId, metadata, payload, context };
-  let request: Promise<any>;
 
+  let request: any;
   try {
-    request = fetch(URL, {
-      method: 'POST',
-      body: JSON.stringify(body),
-      headers: { 'Content-Type': 'application/json' },
-      retries: 3,
-      retryOn: [503, 504],
-      retryDelay: (attempt: number) =>
-        2 ** attempt *
-        (typeof options?.retryDelay === 'number' && !Number.isNaN(options?.retryDelay)
-          ? options.retryDelay
-          : 1000),
-    });
-
+    request = prepareRequest(data, context, options);
     tasks.push(request);
     if (options.immediate) {
       await Promise.all(tasks);
     } else {
       await request;
     }
 
+    const sessionId = await getSessionId();
+    const eventId = nanoid();
+    const body = { ...rest, eventType, eventId, sessionId, metadata, payload, context };
+
     await saveToCache(eventType, body);
   } catch (err) {
     //