Merge pull request #1287 from chromaui/CAP-4317-analytics

Send analytics for Storybook build failures

Author: justin-thurman

node-src/index.ts

@@ -276,6 +276,13 @@ async function runBuild(ctx: Context) {
       }
 
       ctx.log.flush();
+
+      try {
+        await ctx.analytics?.shutdown();
+      } catch (error) {
+        // Analytics shutdown should never crash the CLI, but we want to know about it
+        Sentry.captureException(error);
+      }
     }
   } catch (error) {
     const errors = [error].flat(); // GraphQLClient might throw an array of errors

node-src/lib/analytics/events.ts

@@ -0,0 +1,5 @@
+export const AnalyticsEvent = {
+  CLI_STORYBOOK_BUILD_FAILED: 'CLI_STORYBOOK_BUILD_FAILED',
+} as const;
+
+export type AnalyticsEvent = (typeof AnalyticsEvent)[keyof typeof AnalyticsEvent];

node-src/lib/analytics/index.test.ts

@@ -0,0 +1,32 @@
+import { afterEach, describe, expect, it, vi } from 'vitest';
+
+import TestLogger from '../testLogger';
+import { createAnalyticsClient } from './index';
+import { IndexAnalyticsClient } from './indexClient';
+import { LogOnlyAnalyticsClient } from './logOnly';
+
+function makeContext() {
+  return {
+    log: new TestLogger(),
+    client: { runQuery: vi.fn() },
+  } as any;
+}
+
+describe('createAnalyticsClient', () => {
+  afterEach(() => {
+    delete process.env.CHROMATIC_DISABLE_ANALYTICS;
+  });
+
+  it('returns an Index client when CHROMATIC_DISABLE_ANALYTICS is not set', () => {
+    const ctx = makeContext();
+    const client = createAnalyticsClient(ctx);
+    expect(client).toBeInstanceOf(IndexAnalyticsClient);
+  });
+
+  it('returns a log-only client when CHROMATIC_DISABLE_ANALYTICS is set', () => {
+    process.env.CHROMATIC_DISABLE_ANALYTICS = 'true';
+    const ctx = makeContext();
+    const client = createAnalyticsClient(ctx);
+    expect(client).toBeInstanceOf(LogOnlyAnalyticsClient);
+  });
+});

node-src/lib/analytics/index.ts

@@ -0,0 +1,23 @@
+import { Context } from '../../types';
+import { IndexAnalyticsClient } from './indexClient';
+import { LogOnlyAnalyticsClient } from './logOnly';
+import type { AnalyticsClient } from './types';
+
+export type { AnalyticsClient } from './types';
+
+/**
+ * Creates an analytics client.
+ *
+ * @param ctx The context set when executing the CLI.
+ *
+ * @returns An analytics client instance.
+ */
+export function createAnalyticsClient(ctx: Context): AnalyticsClient {
+  if (process.env.CHROMATIC_DISABLE_ANALYTICS === 'true') {
+    ctx.log.debug('[analytics] disabled via CHROMATIC_DISABLE_ANALYTICS, using log-only client');
+    return new LogOnlyAnalyticsClient(ctx.log);
+  }
+
+  ctx.log.debug('[analytics] initializing Index analytics client');
+  return new IndexAnalyticsClient({ client: ctx.client, logger: ctx.log });
+}

node-src/lib/analytics/indexClient.test.ts

@@ -0,0 +1,103 @@
+import * as Sentry from '@sentry/node';
+import { beforeEach, describe, expect, it, vi } from 'vitest';
+
+import TestLogger from '../testLogger';
+import { AnalyticsEvent } from './events';
+import { IndexAnalyticsClient } from './indexClient';
+
+vi.mock('@sentry/node', () => ({
+  captureException: vi.fn(),
+}));
+
+function makeClient(runQuery = vi.fn().mockResolvedValue({ trackEvent: { success: true } })) {
+  const gqlClient = { runQuery } as any;
+  const logger = new TestLogger();
+  const client = new IndexAnalyticsClient({ client: gqlClient, logger });
+  return { client, gqlClient, logger, runQuery };
+}
+
+describe('IndexAnalyticsClient', () => {
+  beforeEach(() => {
+    vi.mocked(Sentry.captureException).mockClear();
+  });
+
+  describe('trackEvent', () => {
+    it('calls runQuery with TrackCLITelemetryEvent mutation and input built from properties', () => {
+      const { client, runQuery } = makeClient();
+
+      client.trackEvent(AnalyticsEvent.CLI_STORYBOOK_BUILD_FAILED, {
+        errorCategory: 'storybook_build_failed',
+      });
+
+      expect(runQuery).toHaveBeenCalledWith(
+        expect.stringMatching(/TrackCLITelemetryEvent/),
+        {
+          input: {
+            event: AnalyticsEvent.CLI_STORYBOOK_BUILD_FAILED,
+            properties: { errorCategory: 'storybook_build_failed' },
+          },
+        },
+        { retries: 0 }
+      );
+    });
+
+    it('swallows GQL errors and reports to Sentry', async () => {
+      const error = new Error('GQL failed');
+      const { client } = makeClient(vi.fn().mockRejectedValue(error));
+
+      expect(() => client.trackEvent(AnalyticsEvent.CLI_STORYBOOK_BUILD_FAILED, {})).not.toThrow();
+
+      await client.shutdown();
+
+      expect(Sentry.captureException).toHaveBeenCalledWith(error);
+    });
+  });
+
+  describe('shutdown', () => {
+    it('awaits all pending in-flight requests', async () => {
+      const resolvers: (() => void)[] = [];
+      const runQuery = vi.fn(
+        () =>
+          new Promise((resolve) => {
+            resolvers.push(() => resolve({ trackEvent: { success: true } }));
+          })
+      );
+      const { client } = makeClient(runQuery);
+
+      client.trackEvent(AnalyticsEvent.CLI_STORYBOOK_BUILD_FAILED, {});
+      client.trackEvent(AnalyticsEvent.CLI_STORYBOOK_BUILD_FAILED, {});
+
+      let resolved = false;
+      const shutdownPromise = client.shutdown().then(() => {
+        resolved = true;
+      });
+
+      // Task flush: force `.then(() => resolved = true)` above to fire if `shutdown` has returned already resolved promise
+      await Promise.resolve();
+      expect(resolved).toBe(false);
+
+      for (const resolve of resolvers) resolve();
+      await shutdownPromise;
+      expect(resolved).toBe(true);
+    });
+
+    it('times out after 5s when requests hang', async () => {
+      vi.useFakeTimers();
+      try {
+        const runQuery = vi.fn(() => new Promise(() => {}));
+        const { client, logger } = makeClient(runQuery);
+
+        client.trackEvent(AnalyticsEvent.CLI_STORYBOOK_BUILD_FAILED, {});
+
+        const shutdownPromise = client.shutdown();
+        vi.advanceTimersByTime(5000);
+        await shutdownPromise;
+        expect(logger.debug).toHaveBeenCalledWith(
+          '[analytics] shutdown timed out before all events flushed'
+        );
+      } finally {
+        vi.useRealTimers();
+      }
+    });
+  });
+});

node-src/lib/analytics/indexClient.ts

@@ -0,0 +1,68 @@
+import * as Sentry from '@sentry/node';
+
+import GraphQLClient from '../../io/graphqlClient';
+import { Logger } from '../log';
+import type { AnalyticsEvent } from './events';
+import type { AnalyticsClient } from './types';
+
+const TrackCLITelemetryEventMutation = `
+  mutation TrackCLITelemetryEvent($input: TrackCLITelemetryEventInput!) {
+    trackCLITelemetryEvent(input: $input)
+  }
+`;
+
+const SHUTDOWN_TIMEOUT_MS = 5000;
+
+interface IndexAnalyticsOptions {
+  client: GraphQLClient;
+  logger: Logger;
+}
+
+/** Analytics client that sends events to the Chromatic Index via GraphQL. */
+export class IndexAnalyticsClient implements AnalyticsClient {
+  private client: GraphQLClient;
+  private logger: Logger;
+  private pending: Promise<unknown>[] = [];
+
+  constructor({ client, logger }: IndexAnalyticsOptions) {
+    this.client = client;
+    this.logger = logger;
+  }
+
+  trackEvent(eventName: AnalyticsEvent, properties?: Record<string, unknown>): void {
+    this.logger.debug(`[analytics] trackEvent: ${eventName}`, JSON.stringify(properties));
+
+    const input = { event: eventName, properties };
+
+    const promise = this.client
+      // Sending analytics is best effort, so we'll skip retries to keep things fast
+      .runQuery(TrackCLITelemetryEventMutation, { input }, { retries: 0 })
+      .catch((err) => {
+        this.logger.debug('[analytics] trackEvent failed', err);
+        Sentry.captureException(err);
+      });
+
+    this.pending.push(promise);
+  }
+
+  async shutdown(): Promise<void> {
+    // Currently no cap on the number of pending events. YAGNI. Will add batching/capping later if it becomes an issue.
+    this.logger.debug(`[analytics] shutdown: flushing ${this.pending.length} pending event(s)`);
+
+    let timeoutId: ReturnType<typeof setTimeout> | undefined;
+    const timeout = new Promise<void>((resolve) => {
+      timeoutId = setTimeout(() => {
+        this.logger.debug('[analytics] shutdown timed out before all events flushed');
+        resolve();
+      }, SHUTDOWN_TIMEOUT_MS);
+    });
+
+    await Promise.race([
+      Promise.all(this.pending).then(() => {
+        this.logger.debug('[analytics] shutdown: finished flushing pending events');
+      }),
+      timeout,
+    ]);
+    if (timeoutId) clearTimeout(timeoutId);
+  }
+}

node-src/lib/analytics/logOnly.test.ts

@@ -0,0 +1,20 @@
+import { describe, expect, it } from 'vitest';
+
+import TestLogger from '../testLogger';
+import { AnalyticsEvent } from './events';
+import { LogOnlyAnalyticsClient } from './logOnly';
+
+describe('LogOnlyAnalyticsClient', () => {
+  it('debug-logs tracked events with properties', () => {
+    const logger = new TestLogger();
+    const client = new LogOnlyAnalyticsClient(logger);
+
+    const properties = { errorCategory: 'storybook_build_failed' };
+    client.trackEvent(AnalyticsEvent.CLI_STORYBOOK_BUILD_FAILED, properties);
+
+    expect(logger.debug).toHaveBeenCalledWith(
+      expect.stringContaining(AnalyticsEvent.CLI_STORYBOOK_BUILD_FAILED),
+      JSON.stringify(properties)
+    );
+  });
+});

node-src/lib/analytics/logOnly.ts

@@ -0,0 +1,20 @@
+import { Logger } from '../log';
+import type { AnalyticsEvent } from './events';
+import type { AnalyticsClient } from './types';
+
+/** Analytics client that debug-logs events. */
+export class LogOnlyAnalyticsClient implements AnalyticsClient {
+  private logger: Logger;
+
+  constructor(logger: Logger) {
+    this.logger = logger;
+  }
+
+  trackEvent(eventName: AnalyticsEvent, properties?: Record<string, unknown>): void {
+    this.logger.debug(`[analytics] trackEvent: ${eventName}`, JSON.stringify(properties));
+  }
+
+  async shutdown(): Promise<void> {
+    this.logger.debug('[analytics] shutdown');
+  }
+}

node-src/lib/analytics/sanitization.test.ts

@@ -0,0 +1,89 @@
+import { describe, expect, it } from 'vitest';
+
+import { sanitizeStackTrace } from './sanitization';
+
+describe('sanitizeStackTrace', () => {
+  const cases: { name: string; input: string | undefined; expected: string }[] = [
+    { name: 'undefined input', input: undefined, expected: '' },
+    { name: 'empty string input', input: '', expected: '' },
+    {
+      name: 'redacts double-quoted content',
+      input: 'Error: "secret value" found',
+      expected: 'Error: <redacted> found',
+    },
+    {
+      name: 'redacts backtick content',
+      input: 'Error: `token abc` found',
+      expected: 'Error: <redacted> found',
+    },
+    {
+      name: 'preserves single-quoted content (apostrophes)',
+      input: "Error: can't find module 'foo'",
+      expected: "Error: can't find module 'foo'",
+    },
+    {
+      name: 'redacts email addresses',
+      input: 'Error: contact user.name+tag@example.co.uk for help',
+      expected: 'Error: contact <email> for help',
+    },
+    {
+      name: 'redacts home path preserving filename and :line:col',
+      input: 'at fn ~/project/src/file.js:10:5',
+      expected: 'at fn <path>/file.js:10:5',
+    },
+    {
+      name: 'redacts home path preserving filename without line:col',
+      input: 'at fn ~/project/src/file.js',
+      expected: 'at fn <path>/file.js',
+    },
+    {
+      name: 'redacts ~user home path preserving filename',
+      input: 'at fn ~user/project/file.js:3:1',
+      expected: 'at fn <path>/file.js:3:1',
+    },
+    {
+      name: 'redacts unix absolute path preserving filename and :line:col',
+      input: 'at fn /Users/user/project/file.js:42:7',
+      expected: 'at fn <path>/file.js:42:7',
+    },
+    {
+      name: 'redacts unix absolute path preserving filename without line:col',
+      input: 'at fn /Users/user/project/file.js',
+      expected: 'at fn <path>/file.js',
+    },
+    {
+      name: 'redacts windows path preserving filename and :line:col',
+      input: String.raw`at fn C:\Users\user\project\file.js:12:4`,
+      expected: 'at fn <path>/file.js:12:4',
+    },
+    {
+      name: 'redacts windows path preserving filename without line:col',
+      input: String.raw`at fn D:\work\file.js`,
+      expected: 'at fn <path>/file.js',
+    },
+    {
+      name: 'preserves filename for single-segment absolute path',
+      input: 'at fn /foo',
+      expected: 'at fn <path>/foo', // not worth the regex complexity to handle this very unlikely weird case
+    },
+    {
+      name: 'does not eat closing paren around path',
+      input: 'at fn (/Users/alice/file.js:1:1)',
+      expected: 'at fn (<path>/file.js:1:1)',
+    },
+    {
+      name: 'handles multi-line stack trace without eating across lines',
+      input: 'at x (/a/first.js:1:1)\n    at y (/b/second.js:2:2)',
+      expected: 'at x (<path>/first.js:1:1)\n    at y (<path>/second.js:2:2)',
+    },
+    {
+      name: 'caps input at 8000 chars before sanitization',
+      input: 'a'.repeat(10_000),
+      expected: 'a'.repeat(8000),
+    },
+  ];
+
+  it.each(cases)('$name', ({ input, expected }) => {
+    expect(sanitizeStackTrace(input)).toBe(expected);
+  });
+});