Skip to content

Commit

Permalink
Revert "Revert "Telemetry: Add precedingUpgrade data to dev/build/err…
Browse files Browse the repository at this point in the history
…or events""
  • Loading branch information
tmeasday authored Dec 8, 2022
1 parent 0cb361f commit b5cbd96
Show file tree
Hide file tree
Showing 8 changed files with 290 additions and 38 deletions.
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/utils/doTelemetry.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import type { CoreConfig, Options } from '@storybook/types';
import { telemetry } from '@storybook/telemetry';
import { telemetry, getPrecedingUpgrade } from '@storybook/telemetry';
import { useStorybookMetadata } from './metadata';
import type { StoryIndexGenerator } from './StoryIndexGenerator';
import { summarizeIndex } from './summarizeIndex';
Expand All @@ -15,12 +15,15 @@ export 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

0 comments on commit b5cbd96

Please sign in to comment.