From 1dc35cd8f62289047fe4d30d523b942f3041753a Mon Sep 17 00:00:00 2001 From: Tom Coleman Date: Thu, 8 Dec 2022 10:22:02 +1100 Subject: [PATCH 1/5] Don't send boot event when `cliOptions.disableTelemetry` is passed --- code/lib/cli/src/build.ts | 4 +++- code/lib/cli/src/dev.ts | 4 +++- .../lib/core-server/src/withTelemetry.test.ts | 24 ++++++++++++++++--- code/lib/core-server/src/withTelemetry.ts | 9 +++---- 4 files changed, 32 insertions(+), 9 deletions(-) diff --git a/code/lib/cli/src/build.ts b/code/lib/cli/src/build.ts index f9b9c2ca1dd..2110be45d4f 100644 --- a/code/lib/cli/src/build.ts +++ b/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); diff --git a/code/lib/cli/src/dev.ts b/code/lib/cli/src/dev.ts index a6aa573c16b..b50da746622 100644 --- a/code/lib/cli/src/dev.ts +++ b/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 = ''; diff --git a/code/lib/core-server/src/withTelemetry.test.ts b/code/lib/core-server/src/withTelemetry.test.ts index 121b60718af..a5fb0f1103e 100644 --- a/code/lib/core-server/src/withTelemetry.test.ts +++ b/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( diff --git a/code/lib/core-server/src/withTelemetry.ts b/code/lib/core-server/src/withTelemetry.ts index a59e6fd4a18..78179a202b4 100644 --- a/code/lib/core-server/src/withTelemetry.ts +++ b/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[0]; }; @@ -29,7 +29,7 @@ const promptCrashReports = async () => { type ErrorLevel = 'none' | 'error' | 'full'; async function getErrorLevel({ cliOptions, presetOptions }: TelemetryOptions): Promise { - 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 ) { - 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', } ); From 2c3528f2c1f01ac4fc8962f052aa6fb1ffe21901 Mon Sep 17 00:00:00 2001 From: Tom Coleman Date: Thu, 8 Dec 2022 10:25:55 +1100 Subject: [PATCH 2/5] Add a note to docs about `boot` --- docs/configure/telemetry.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/docs/configure/telemetry.md b/docs/configure/telemetry.md index 17abc3f6d6e..fdc9b5ec262 100644 --- a/docs/configure/telemetry.md +++ b/docs/configure/telemetry.md @@ -150,6 +150,12 @@ You may opt-out of the telemetry by setting Storybook's configuration element `d +
+ 💡 There is a single boot event that contains no metadata at all (simply used to ensure the telemetry is working) that is sent prior to evaluating your main.js, so unaffected by the disableTelemetry option. If you want to ensure that event is not sent either, be sure to use the STORYBOOK_DISABLE_TELEMETRY environment variable. +
+ + + ## 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: From cdf36a6f22fd81d01d6c1f884e34d671d5b9fb32 Mon Sep 17 00:00:00 2001 From: Michael Shilman Date: Thu, 8 Dec 2022 09:01:35 +0800 Subject: [PATCH 3/5] Tweak copy --- docs/configure/telemetry.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/configure/telemetry.md b/docs/configure/telemetry.md index fdc9b5ec262..74f6253851a 100644 --- a/docs/configure/telemetry.md +++ b/docs/configure/telemetry.md @@ -151,7 +151,7 @@ You may opt-out of the telemetry by setting Storybook's configuration element `d
- 💡 There is a single boot event that contains no metadata at all (simply used to ensure the telemetry is working) that is sent prior to evaluating your main.js, so unaffected by the disableTelemetry option. If you want to ensure that event is not sent either, be sure to use the STORYBOOK_DISABLE_TELEMETRY environment variable. + 💡 There is a boot event that contains no metadata at all (simply used to ensure the telemetry is working). It is sent prior to evaluating your main.js, so it is unaffected by the disableTelemetry option. If you want to ensure that event is not sent, be sure to use the STORYBOOK_DISABLE_TELEMETRY environment variable.
From b973a49f4ec763a51f6938f8605b152737c8d213 Mon Sep 17 00:00:00 2001 From: Tom Coleman Date: Tue, 13 Dec 2022 08:00:49 +1100 Subject: [PATCH 4/5] Fix types --- code/lib/core-server/src/withTelemetry.test.ts | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/code/lib/core-server/src/withTelemetry.test.ts b/code/lib/core-server/src/withTelemetry.test.ts index a5fb0f1103e..64de5a82ed5 100644 --- a/code/lib/core-server/src/withTelemetry.test.ts +++ b/code/lib/core-server/src/withTelemetry.test.ts @@ -78,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); @@ -95,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); @@ -112,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); @@ -129,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); @@ -147,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); @@ -165,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); @@ -184,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); @@ -203,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); @@ -219,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); From bd55e2b4815c90fff938f935fa659bf0ef623523 Mon Sep 17 00:00:00 2001 From: Tom Coleman Date: Tue, 13 Dec 2022 11:35:35 +1100 Subject: [PATCH 5/5] Fix test failure --- code/lib/core-server/src/withTelemetry.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/code/lib/core-server/src/withTelemetry.test.ts b/code/lib/core-server/src/withTelemetry.test.ts index 64de5a82ed5..a63df263dea 100644 --- a/code/lib/core-server/src/withTelemetry.test.ts +++ b/code/lib/core-server/src/withTelemetry.test.ts @@ -65,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 },