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.

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

Telemetry: Add precedingUpgrade data to dev/build/error events #20136

Merged
merged 3 commits into from
Dec 7, 2022
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
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
15 changes: 9 additions & 6 deletions code/lib/core-server/src/build-static.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import { dedent } from 'ts-dedent';
import global from 'global';

import { logger } from '@storybook/node-logger';
import { telemetry } from '@storybook/telemetry';
import { telemetry, getPrecedingUpgrade } from '@storybook/telemetry';
import type {
BuilderOptions,
CLIOptions,
Expand Down Expand Up @@ -173,11 +173,14 @@ export async function buildStaticStandalone(
effects.push(
initializedStoryIndexGenerator.then(async (generator) => {
const storyIndex = await generator?.getIndex();
const payload = storyIndex
? {
storyIndex: summarizeIndex(storyIndex),
}
: undefined;
const payload = {
precedingUpgrade: await getPrecedingUpgrade(),
};
if (storyIndex) {
Object.assign(payload, {
storyIndex: summarizeIndex(storyIndex),
});
}
await telemetry('build', payload, { configDir: options.configDir });
})
);
Expand Down
17 changes: 10 additions & 7 deletions code/lib/core-server/src/dev-server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import type {

import { normalizeStories, logConfig } from '@storybook/core-common';

import { telemetry } from '@storybook/telemetry';
import { telemetry, getPrecedingUpgrade } from '@storybook/telemetry';
import { getMiddleware } from './utils/middleware';
import { getServerAddresses } from './utils/server-address';
import { getServer } from './utils/server-init';
Expand Down Expand Up @@ -156,12 +156,15 @@ async function doTelemetry(
initializedStoryIndexGenerator.then(async (generator) => {
const storyIndex = await generator?.getIndex();
const { versionCheck, versionUpdates } = options;
const payload = storyIndex
? {
versionStatus: versionUpdates ? versionStatus(versionCheck) : 'disabled',
storyIndex: summarizeIndex(storyIndex),
}
: undefined;
const payload = {
precedingUpgrade: await getPrecedingUpgrade(),
};
if (storyIndex) {
Object.assign(payload, {
versionStatus: versionUpdates ? versionStatus(versionCheck) : 'disabled',
storyIndex: summarizeIndex(storyIndex),
});
}
telemetry('dev', payload, { configDir: options.configDir });
});
}
Expand Down
20 changes: 10 additions & 10 deletions code/lib/core-server/src/withTelemetry.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -63,10 +63,10 @@ describe('when command fails', () => {
withTelemetry('dev', { presetOptions: {} as any }, run)
).rejects.toThrow(error);

expect(telemetry).toHaveBeenCalledTimes(1);
expect(telemetry).not.toHaveBeenCalledWith(
expect(telemetry).toHaveBeenCalledTimes(2);
expect(telemetry).toHaveBeenCalledWith(
'error',
{ eventType: 'dev', error },
{ eventType: 'dev' },
expect.objectContaining({})
);
});
Expand All @@ -88,7 +88,7 @@ describe('when command fails', () => {
);
});

it('does not send error message when telemetry is disabled', async () => {
it('does not send full error message when telemetry is disabled', async () => {
jest.mocked(loadAllPresets).mockResolvedValueOnce({
apply: async () => ({ disableTelemetry: true } as any),
});
Expand Down Expand Up @@ -132,10 +132,10 @@ describe('when command fails', () => {
withTelemetry('dev', { presetOptions: {} as any }, run)
).rejects.toThrow(error);

expect(telemetry).toHaveBeenCalledTimes(1);
expect(telemetry).not.toHaveBeenCalledWith(
expect(telemetry).toHaveBeenCalledTimes(2);
expect(telemetry).toHaveBeenCalledWith(
'error',
{ eventType: 'dev', error },
{ eventType: 'dev' },
expect.objectContaining({})
);
});
Expand Down Expand Up @@ -169,10 +169,10 @@ describe('when command fails', () => {
withTelemetry('dev', { presetOptions: {} as any }, run)
).rejects.toThrow(error);

expect(telemetry).toHaveBeenCalledTimes(1);
expect(telemetry).not.toHaveBeenCalledWith(
expect(telemetry).toHaveBeenCalledTimes(2);
expect(telemetry).toHaveBeenCalledWith(
'error',
{ eventType: 'dev', error },
{ eventType: 'dev' },
expect.objectContaining({})
);
});
Expand Down
29 changes: 17 additions & 12 deletions code/lib/core-server/src/withTelemetry.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import prompts from 'prompts';
import type { CLIOptions, CoreConfig } from '@storybook/types';
import { loadAllPresets, cache } from '@storybook/core-common';
import { telemetry } from '@storybook/telemetry';
import { telemetry, getPrecedingUpgrade } from '@storybook/telemetry';
import type { EventType } from '@storybook/telemetry';

type TelemetryOptions = {
Expand All @@ -26,11 +26,13 @@ const promptCrashReports = async () => {
return enableCrashReports;
};

async function shouldSendError({ cliOptions, presetOptions }: TelemetryOptions) {
if (cliOptions?.disableTelemetry) return false;
type ErrorLevel = 'none' | 'error' | 'full';

async function getErrorLevel({ cliOptions, presetOptions }: TelemetryOptions): Promise<ErrorLevel> {
if (cliOptions?.disableTelemetry) return 'none';

// If we are running init or similar, we just have to go with true here
if (!presetOptions) return true;
if (!presetOptions) return 'full';

// should we load the preset?
const presets = await loadAllPresets({
Expand All @@ -42,18 +44,18 @@ async function shouldSendError({ cliOptions, presetOptions }: TelemetryOptions)
// If the user has chosen to enable/disable crash reports in main.js
// or disabled telemetry, we can return that
const core = await presets.apply<CoreConfig>('core');
if (core?.enableCrashReports !== undefined) return core.enableCrashReports;
if (core?.disableTelemetry) return false;
if (core?.enableCrashReports !== undefined) return core.enableCrashReports ? 'full' : 'error';
if (core?.disableTelemetry) return 'none';

// Deal with typo, remove in future version (7.1?)
const valueFromCache =
(await cache.get('enableCrashReports')) ?? (await cache.get('enableCrashreports'));
if (valueFromCache !== undefined) return valueFromCache;
if (valueFromCache !== undefined) return valueFromCache ? 'full' : 'error';

const valueFromPrompt = await promptCrashReports();
if (valueFromPrompt !== undefined) return valueFromPrompt;
if (valueFromPrompt !== undefined) return valueFromPrompt ? 'full' : 'error';

return true;
return 'full';
}

export async function withTelemetry(
Expand All @@ -67,14 +69,17 @@ export async function withTelemetry(
await run();
} catch (error) {
try {
if (await shouldSendError(options)) {
const errorLevel = await getErrorLevel(options);
if (errorLevel !== 'none') {
const precedingUpgrade = await getPrecedingUpgrade();

await telemetry(
'error',
{ eventType, error },
{ eventType, precedingUpgrade, error: errorLevel === 'full' ? error : undefined },
{
immediate: true,
configDir: options.cliOptions?.configDir || options.presetOptions?.configDir,
enableCrashReports: true,
enableCrashReports: errorLevel === 'full',
}
);
}
Expand Down
175 changes: 175 additions & 0 deletions code/lib/telemetry/src/event-cache.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,175 @@
import { getPrecedingUpgrade } from './event-cache';

expect.addSnapshotSerializer({
print: (val: any) => JSON.stringify(val, null, 2),
test: (val) => typeof val !== 'string',
});

describe('event-cache', () => {
const init = { body: { eventType: 'init', eventId: 'init' }, timestamp: 1 };
const upgrade = { body: { eventType: 'upgrade', eventId: 'upgrade' }, timestamp: 2 };
const dev = { body: { eventType: 'dev', eventId: 'dev' }, timestamp: 3 };
const build = { body: { eventType: 'build', eventId: 'build' }, timestamp: 3 };
const error = { body: { eventType: 'build', eventId: 'error' }, timestamp: 4 };
const versionUpdate = {
body: { eventType: 'version-update', eventId: 'version-update' },
timestamp: 5,
};

describe('data handling', () => {
it('errors', async () => {
const preceding = await getPrecedingUpgrade({
init: { timestamp: 1, body: { ...init.body, error: {} } },
});
expect(preceding).toMatchInlineSnapshot(`
{
"timestamp": 1,
"eventType": "init",
"eventId": "init"
}
`);
});

it('session IDs', async () => {
const preceding = await getPrecedingUpgrade({
init: { timestamp: 1, body: { ...init.body, sessionId: 100 } },
});
expect(preceding).toMatchInlineSnapshot(`
{
"timestamp": 1,
"eventType": "init",
"eventId": "init",
"sessionId": 100
}
`);
});

it('extra fields', async () => {
const preceding = await getPrecedingUpgrade({
init: { timestamp: 1, body: { ...init.body, foobar: 'baz' } },
});
expect(preceding).toMatchInlineSnapshot(`
{
"timestamp": 1,
"eventType": "init",
"eventId": "init"
}
`);
});
});

describe('no intervening dev events', () => {
it('no upgrade events', async () => {
const preceding = await getPrecedingUpgrade({});
expect(preceding).toBeUndefined();
});

it('init', async () => {
const preceding = await getPrecedingUpgrade({ init });
expect(preceding).toMatchInlineSnapshot(`
{
"timestamp": 1,
"eventType": "init",
"eventId": "init"
}
`);
});

it('upgrade', async () => {
const preceding = await getPrecedingUpgrade({ upgrade });
expect(preceding).toMatchInlineSnapshot(`
{
"timestamp": 2,
"eventType": "upgrade",
"eventId": "upgrade"
}
`);
});

it('both init and upgrade', async () => {
const preceding = await getPrecedingUpgrade({ init, upgrade });
expect(preceding).toMatchInlineSnapshot(`
{
"timestamp": 2,
"eventType": "upgrade",
"eventId": "upgrade"
}
`);
});
});

describe('intervening dev events', () => {
it('no upgrade events', async () => {
const preceding = await getPrecedingUpgrade({ dev });
expect(preceding).toBeUndefined();
});

it('init', async () => {
const preceding = await getPrecedingUpgrade({ init, dev });
expect(preceding).toBeUndefined();
});

it('upgrade', async () => {
const preceding = await getPrecedingUpgrade({ upgrade, dev });
expect(preceding).toBeUndefined();
});

it('init followed by upgrade', async () => {
const preceding = await getPrecedingUpgrade({ init, upgrade, dev });
expect(preceding).toBeUndefined();
});

it('both init and upgrade with intervening dev', async () => {
const secondUpgrade = {
body: { eventType: 'upgrade', eventId: 'secondUpgrade' },
timestamp: 4,
};
const preceding = await getPrecedingUpgrade({ init, dev, upgrade: secondUpgrade });
expect(preceding).toMatchInlineSnapshot(`
{
"timestamp": 4,
"eventType": "upgrade",
"eventId": "secondUpgrade"
}
`);
});

it('both init and upgrade with non-intervening dev', async () => {
const earlyDev = {
body: { eventType: 'dev', eventId: 'earlyDev' },
timestamp: -1,
};
const preceding = await getPrecedingUpgrade({ dev: earlyDev, init, upgrade });
expect(preceding).toMatchInlineSnapshot(`
{
"timestamp": 2,
"eventType": "upgrade",
"eventId": "upgrade"
}
`);
});
});

describe('intervening other events', () => {
it('build', async () => {
const preceding = await getPrecedingUpgrade({ upgrade, build });
expect(preceding).toBeUndefined();
});

it('error', async () => {
const preceding = await getPrecedingUpgrade({ upgrade, error });
expect(preceding).toBeUndefined();
});

it('version-update', async () => {
const preceding = await getPrecedingUpgrade({ upgrade, 'version-update': versionUpdate });
expect(preceding).toMatchInlineSnapshot(`
{
"timestamp": 2,
"eventType": "upgrade",
"eventId": "upgrade"
}
`);
});
});
});
Loading