Merge pull request #20144 from storybookjs/tom/sb-1064-dont-send-init-events-if-opt-ing-out

Telemetry: Don't send boot event when cliOptions.disableTelemetry is passed

code/lib/cli/src/build.ts

@@ -15,7 +15,9 @@ export const build = async (cliOptions: any) => {
       cache,
       packageJson: readUpSync({ cwd: __dirname }).packageJson,
     };
-    await withTelemetry('build', { presetOptions: options }, () => buildStaticStandalone(options));
+    await withTelemetry('build', { cliOptions, presetOptions: options }, () =>
+      buildStaticStandalone(options)
+    );
   } catch (err) {
     logger.error(err);
     process.exit(1);

code/lib/cli/src/dev.ts

@@ -17,7 +17,9 @@ export const dev = async (cliOptions: any) => {
       cache,
       packageJson: readUpSync({ cwd: __dirname }).packageJson,
     };
-    await withTelemetry('dev', { presetOptions: options }, () => buildDevStandalone(options));
+    await withTelemetry('dev', { cliOptions, presetOptions: options }, () =>
+      buildDevStandalone(options)
+    );
   } catch (error) {
     // this is a weird bugfix, somehow 'node-pre-gyp' is polluting the npmLog header
     npmLog.heading = '';

code/lib/core-server/src/withTelemetry.test.ts

@@ -10,15 +10,25 @@ jest.mock('prompts');
 jest.mock('@storybook/core-common');
 jest.mock('@storybook/telemetry');
 
+const cliOptions = {};
+
 it('works in happy path', async () => {
   const run = jest.fn();
 
-  await withTelemetry('dev', {}, run);
+  await withTelemetry('dev', { cliOptions }, run);
 
   expect(telemetry).toHaveBeenCalledTimes(1);
   expect(telemetry).toHaveBeenCalledWith('boot', { eventType: 'dev' }, { stripMetadata: true });
 });
 
+it('does not send boot when cli option is passed', async () => {
+  const run = jest.fn();
+
+  await withTelemetry('dev', { cliOptions: { disableTelemetry: true } }, run);
+
+  expect(telemetry).toHaveBeenCalledTimes(0);
+});
+
 describe('when command fails', () => {
   const error = new Error('An Error!');
   const run = jest.fn(async () => {
@@ -26,13 +36,21 @@ describe('when command fails', () => {
   });
 
   it('sends boot message', async () => {
-    await expect(async () => withTelemetry('dev', {}, run)).rejects.toThrow(error);
+    await expect(async () => withTelemetry('dev', { cliOptions }, run)).rejects.toThrow(error);
 
     expect(telemetry).toHaveBeenCalledWith('boot', { eventType: 'dev' }, { stripMetadata: true });
   });
 
+  it('does not send boot when cli option is passed', async () => {
+    await expect(async () =>
+      withTelemetry('dev', { cliOptions: { disableTelemetry: true } }, run)
+    ).rejects.toThrow(error);
+
+    expect(telemetry).toHaveBeenCalledTimes(0);
+  });
+
   it('sends error message when no options are passed', async () => {
-    await expect(async () => withTelemetry('dev', {}, run)).rejects.toThrow(error);
+    await expect(async () => withTelemetry('dev', { cliOptions }, run)).rejects.toThrow(error);
 
     expect(telemetry).toHaveBeenCalledTimes(2);
     expect(telemetry).toHaveBeenCalledWith(
@@ -47,7 +65,7 @@ describe('when command fails', () => {
       withTelemetry('dev', { cliOptions: { disableTelemetry: true } }, run)
     ).rejects.toThrow(error);
 
-    expect(telemetry).toHaveBeenCalledTimes(1);
+    expect(telemetry).toHaveBeenCalledTimes(0);
     expect(telemetry).not.toHaveBeenCalledWith(
       'error',
       { eventType: 'dev', error },
@@ -60,7 +78,7 @@ describe('when command fails', () => {
       apply: async () => ({ enableCrashReports: false } as any),
     });
     await expect(async () =>
-      withTelemetry('dev', { presetOptions: {} as any }, run)
+      withTelemetry('dev', { cliOptions: {} as any, presetOptions: {} as any }, run)
     ).rejects.toThrow(error);
 
     expect(telemetry).toHaveBeenCalledTimes(2);
@@ -77,7 +95,7 @@ describe('when command fails', () => {
     });
 
     await expect(async () =>
-      withTelemetry('dev', { presetOptions: {} as any }, run)
+      withTelemetry('dev', { cliOptions: {} as any, presetOptions: {} as any }, run)
     ).rejects.toThrow(error);
 
     expect(telemetry).toHaveBeenCalledTimes(2);
@@ -94,7 +112,7 @@ describe('when command fails', () => {
     });
 
     await expect(async () =>
-      withTelemetry('dev', { presetOptions: {} as any }, run)
+      withTelemetry('dev', { cliOptions: {} as any, presetOptions: {} as any }, run)
     ).rejects.toThrow(error);
 
     expect(telemetry).toHaveBeenCalledTimes(1);
@@ -111,7 +129,7 @@ describe('when command fails', () => {
     });
 
     await expect(async () =>
-      withTelemetry('dev', { presetOptions: {} as any }, run)
+      withTelemetry('dev', { cliOptions: {} as any, presetOptions: {} as any }, run)
     ).rejects.toThrow(error);
 
     expect(telemetry).toHaveBeenCalledTimes(2);
@@ -129,7 +147,7 @@ describe('when command fails', () => {
     jest.mocked(cache.get).mockResolvedValueOnce(false);
 
     await expect(async () =>
-      withTelemetry('dev', { presetOptions: {} as any }, run)
+      withTelemetry('dev', { cliOptions: {} as any, presetOptions: {} as any }, run)
     ).rejects.toThrow(error);
 
     expect(telemetry).toHaveBeenCalledTimes(2);
@@ -147,7 +165,7 @@ describe('when command fails', () => {
     jest.mocked(cache.get).mockResolvedValueOnce(true);
 
     await expect(async () =>
-      withTelemetry('dev', { presetOptions: {} as any }, run)
+      withTelemetry('dev', { cliOptions: {} as any, presetOptions: {} as any }, run)
     ).rejects.toThrow(error);
 
     expect(telemetry).toHaveBeenCalledTimes(2);
@@ -166,7 +184,7 @@ describe('when command fails', () => {
     jest.mocked(prompts).mockResolvedValueOnce({ enableCrashReports: false });
 
     await expect(async () =>
-      withTelemetry('dev', { presetOptions: {} as any }, run)
+      withTelemetry('dev', { cliOptions: {} as any, presetOptions: {} as any }, run)
     ).rejects.toThrow(error);
 
     expect(telemetry).toHaveBeenCalledTimes(2);
@@ -185,7 +203,7 @@ describe('when command fails', () => {
     jest.mocked(prompts).mockResolvedValueOnce({ enableCrashReports: true });
 
     await expect(async () =>
-      withTelemetry('dev', { presetOptions: {} as any }, run)
+      withTelemetry('dev', { cliOptions: {} as any, presetOptions: {} as any }, run)
     ).rejects.toThrow(error);
 
     expect(telemetry).toHaveBeenCalledTimes(2);
@@ -201,7 +219,7 @@ describe('when command fails', () => {
     jest.mocked(loadAllPresets).mockRejectedValueOnce(error);
 
     await expect(async () =>
-      withTelemetry('dev', { presetOptions: {} as any }, run)
+      withTelemetry('dev', { cliOptions: {} as any, presetOptions: {} as any }, run)
     ).rejects.toThrow(error);
 
     expect(telemetry).toHaveBeenCalledTimes(1);

code/lib/core-server/src/withTelemetry.ts

@@ -5,7 +5,7 @@ import { telemetry, getPrecedingUpgrade } from '@storybook/telemetry';
 import type { EventType } from '@storybook/telemetry';
 
 type TelemetryOptions = {
-  cliOptions?: CLIOptions;
+  cliOptions: CLIOptions;
   presetOptions?: Parameters<typeof loadAllPresets>[0];
 };
 
@@ -29,7 +29,7 @@ const promptCrashReports = async () => {
 type ErrorLevel = 'none' | 'error' | 'full';
 
 async function getErrorLevel({ cliOptions, presetOptions }: TelemetryOptions): Promise<ErrorLevel> {
-  if (cliOptions?.disableTelemetry) return 'none';
+  if (cliOptions.disableTelemetry) return 'none';
 
   // If we are running init or similar, we just have to go with true here
   if (!presetOptions) return 'full';
@@ -63,7 +63,8 @@ export async function withTelemetry(
   options: TelemetryOptions,
   run: () => Promise<void>
 ) {
-  telemetry('boot', { eventType }, { stripMetadata: true });
+  if (!options.cliOptions.disableTelemetry)
+    telemetry('boot', { eventType }, { stripMetadata: true });
 
   try {
     await run();
@@ -78,7 +79,7 @@ export async function withTelemetry(
           { eventType, precedingUpgrade, error: errorLevel === 'full' ? error : undefined },
           {
             immediate: true,
-            configDir: options.cliOptions?.configDir || options.presetOptions?.configDir,
+            configDir: options.cliOptions.configDir || options.presetOptions?.configDir,
             enableCrashReports: errorLevel === 'full',
           }
         );

docs/configure/telemetry.md

@@ -150,6 +150,12 @@ You may opt-out of the telemetry by setting Storybook's configuration element `d
 
 <!-- prettier-ignore-end -->
 
+<div class="aside">
+  💡 There is a <code>boot</code> event that contains no metadata at all (simply used to ensure the telemetry is working). It is sent prior to evaluating your <code>main.js</code>, so it is unaffected by the <code>disableTelemetry</code> option. If you want to ensure that event is not sent, be sure to use the <code>STORYBOOK_DISABLE_TELEMETRY</code> environment variable.
+</div>
+
+
+
 ## Crash reports (disabled by default)
 
 In addition to general usage telemetry, you may also choose to share crash reports. Storybook will then sanitize the error object (removing all user paths) and append it to the telemetry event. To enable crash reporting, you can set the `enableCrashReports` configuration element to `true`, using the `--enable-crash-reports` flag, or set the `STORYBOOK_ENABLE_CRASH_REPORTS` environment variable to `1`. For example: