Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

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

Merged
merged 6 commits into from
Dec 13, 2022
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
1dc35cd
Don't send boot event when `cliOptions.disableTelemetry` is passed
tmeasday Dec 7, 2022
2c3528f
Add a note to docs about `boot`
tmeasday Dec 7, 2022
cdf36a6
Tweak copy
shilman Dec 8, 2022
b973a49
Fix types
tmeasday Dec 12, 2022
bd55e2b
Fix test failure
tmeasday Dec 13, 2022
615703c
Merge remote-tracking branch 'origin/next' into tom/sb-1064-dont-send…
tmeasday Dec 13, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion code/lib/cli/src/build.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
4 changes: 3 additions & 1 deletion code/lib/cli/src/dev.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 = '';
Expand Down
44 changes: 31 additions & 13 deletions code/lib/core-server/src/withTelemetry.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,29 +10,47 @@ 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 () => {
throw error;
});

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(
Expand All @@ -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 },
Expand All @@ -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);
Expand All @@ -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);
Expand All @@ -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);
Expand All @@ -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);
Expand All @@ -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);
Expand All @@ -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);
Expand All @@ -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);
Expand All @@ -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);
Expand All @@ -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);
Expand Down
9 changes: 5 additions & 4 deletions code/lib/core-server/src/withTelemetry.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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];
};

Expand All @@ -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';
Expand Down Expand Up @@ -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();
Expand All @@ -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',
}
);
Expand Down
6 changes: 6 additions & 0 deletions docs/configure/telemetry.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down