Skip to content

Commit

Permalink
Refactor telemetry usage flow (#7522)
Browse files Browse the repository at this point in the history
  • Loading branch information
bluwy authored Jun 30, 2023
1 parent 5b1745b commit fcba0f0
Show file tree
Hide file tree
Showing 15 changed files with 18 additions and 49 deletions.
1 change: 0 additions & 1 deletion packages/astro/src/@types/astro.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1375,7 +1375,6 @@ export interface AstroSettings {
tsConfig: TsConfigJson | undefined;
tsConfigPath: string | undefined;
watchFiles: string[];
forceDisableTelemetry: boolean;
timer: AstroTimer;
}

Expand Down
8 changes: 3 additions & 5 deletions packages/astro/src/cli/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ async function runCommand(cmd: string, flags: yargs.Arguments) {

telemetry.record(event.eventCliSession(cmd));
const packages = flags._.slice(3) as string[];
return await add(packages, { cwd: root, flags, logging, telemetry });
return await add(packages, { cwd: root, flags, logging });
}
case 'docs': {
telemetry.record(event.eventCliSession(cmd));
Expand All @@ -170,7 +170,7 @@ async function runCommand(cmd: string, flags: yargs.Arguments) {
// Do not track session start, since the user may be trying to enable,
// disable, or modify telemetry settings.
const subcommand = flags._[3]?.toString();
return await telemetryHandler.update(subcommand, { flags, telemetry });
return await telemetryHandler.update(subcommand, { flags });
}
}

Expand Down Expand Up @@ -209,7 +209,6 @@ async function runCommand(cmd: string, flags: yargs.Arguments) {
configFlagPath,
flags,
logging,
telemetry,
handleConfigError(e) {
handleConfigError(e, { cmd, cwd: root, flags, logging });
info(logging, 'astro', 'Continuing with previous valid configuration\n');
Expand All @@ -224,7 +223,6 @@ async function runCommand(cmd: string, flags: yargs.Arguments) {
return await build(settings, {
flags,
logging,
telemetry,
teardownCompiler: true,
mode: flags.mode,
});
Expand Down Expand Up @@ -256,7 +254,7 @@ async function runCommand(cmd: string, flags: yargs.Arguments) {
case 'preview': {
const { default: preview } = await import('../core/preview/index.js');

const server = await preview(settings, { logging, telemetry, flags });
const server = await preview(settings, { logging, flags });
if (server) {
return await server.closed(); // keep alive until the server is closed
}
Expand Down
6 changes: 3 additions & 3 deletions packages/astro/src/cli/telemetry.ts
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
/* eslint-disable no-console */
import type { AstroTelemetry } from '@astrojs/telemetry';
import type yargs from 'yargs-parser';
import * as msg from '../core/messages.js';
import { telemetry } from '../events/index.js';

export interface TelemetryOptions {
flags: yargs.Arguments;
telemetry: AstroTelemetry;
}

export async function update(subcommand: string, { flags, telemetry }: TelemetryOptions) {
export async function update(subcommand: string, { flags }: TelemetryOptions) {
const isValid = ['enable', 'disable', 'reset'].includes(subcommand);

if (flags.help || flags.h || !isValid) {
Expand Down
4 changes: 1 addition & 3 deletions packages/astro/src/core/add/index.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import type { AstroTelemetry } from '@astrojs/telemetry';
import boxen from 'boxen';
import { diffWords } from 'diff';
import { execa } from 'execa';
Expand Down Expand Up @@ -30,7 +29,6 @@ import { wrapDefaultExport } from './wrapper.js';
export interface AddOptions {
logging: LogOptions;
flags: yargs.Arguments;
telemetry: AstroTelemetry;
cwd?: string;
}

Expand Down Expand Up @@ -83,7 +81,7 @@ async function getRegistry(): Promise<string> {
return stdout || 'https://registry.npmjs.org';
}

export default async function add(names: string[], { cwd, flags, logging, telemetry }: AddOptions) {
export default async function add(names: string[], { cwd, flags, logging }: AddOptions) {
applyPolyfill();
if (flags.help || names.length === 0) {
printHelp({
Expand Down
2 changes: 0 additions & 2 deletions packages/astro/src/core/build/index.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import type { AstroTelemetry } from '@astrojs/telemetry';
import type { AstroConfig, AstroSettings, ManifestData, RuntimeMode } from '../../@types/astro';

import fs from 'fs';
Expand Down Expand Up @@ -26,7 +25,6 @@ import { getTimeStat } from './util.js';
export interface BuildOptions {
mode?: RuntimeMode;
logging: LogOptions;
telemetry: AstroTelemetry;
/**
* Teardown the compiler WASM instance after build. This can improve performance when
* building once, but may cause a performance hit if building multiple times in a row.
Expand Down
1 change: 0 additions & 1 deletion packages/astro/src/core/config/settings.ts
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,6 @@ export function createBaseSettings(config: AstroConfig): AstroSettings {
scripts: [],
clientDirectives: getDefaultClientDirectives(),
watchFiles: [],
forceDisableTelemetry: false,
timer: new AstroTimer(),
};
}
Expand Down
8 changes: 1 addition & 7 deletions packages/astro/src/core/dev/container.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,6 @@ export interface CreateContainerParams {
// The string passed to --config and the resolved path
configFlag?: string;
configFlagPath?: string;
disableTelemetry?: boolean;
}

export async function createContainer(params: CreateContainerParams = {}): Promise<Container> {
Expand All @@ -55,13 +54,8 @@ export async function createContainer(params: CreateContainerParams = {}): Promi
logging = defaultLogging,
settings = await createDefaultDevSettings(params.userConfig, params.root),
fs = nodeFs,
disableTelemetry,
} = params;

if (disableTelemetry) {
settings.forceDisableTelemetry = true;
}

// Initialize
applyPolyfill();
settings = await runHookConfigSetup({
Expand Down Expand Up @@ -149,7 +143,7 @@ export async function runInContainer(
params: CreateContainerParams,
callback: (container: Container) => Promise<void> | void
) {
const container = await createContainer({ ...params, disableTelemetry: true });
const container = await createContainer(params);
try {
await callback(container);
} finally {
Expand Down
5 changes: 2 additions & 3 deletions packages/astro/src/core/dev/dev.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import type { AstroTelemetry } from '@astrojs/telemetry';
import type http from 'http';
import { cyan } from 'kleur/colors';
import type { AddressInfo } from 'net';
Expand All @@ -7,6 +6,7 @@ import type * as vite from 'vite';
import type yargs from 'yargs-parser';
import type { AstroSettings } from '../../@types/astro';
import { attachContentServerListeners } from '../../content/index.js';
import { telemetry } from '../../events/index.js';
import { info, warn, type LogOptions } from '../logger/core.js';
import * as msg from '../messages.js';
import { printHelp } from '../messages.js';
Expand All @@ -18,7 +18,6 @@ export interface DevOptions {
configFlagPath: string | undefined;
flags?: yargs.Arguments;
logging: LogOptions;
telemetry: AstroTelemetry;
handleConfigError: (error: Error) => void;
isRestart?: boolean;
}
Expand Down Expand Up @@ -56,7 +55,7 @@ export default async function dev(
}

const devStart = performance.now();
await options.telemetry.record([]);
await telemetry.record([]);

// Create a container which sets up the Vite server.
const restart = await createContainerWithAutomaticRestart({
Expand Down
2 changes: 0 additions & 2 deletions packages/astro/src/core/preview/index.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import type { AstroTelemetry } from '@astrojs/telemetry';
import { cyan } from 'kleur/colors';
import { createRequire } from 'module';
import { pathToFileURL } from 'url';
Expand All @@ -12,7 +11,6 @@ import { getResolvedHostForHttpServer } from './util.js';

interface PreviewOptions {
logging: LogOptions;
telemetry: AstroTelemetry;
flags?: Arguments;
}

Expand Down
1 change: 0 additions & 1 deletion packages/astro/src/core/render/dev/environment.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@ export function createDevelopmentEnvironment(
site: settings.config.site,
ssr: isServerLikeOutput(settings.config),
streaming: true,
telemetry: Boolean(settings.forceDisableTelemetry),
});

return {
Expand Down
2 changes: 0 additions & 2 deletions packages/astro/src/core/render/environment.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ export interface Environment {
site?: string;
ssr: boolean;
streaming: boolean;
telemetry?: boolean;
}

export type CreateEnvironmentArgs = Environment;
Expand Down Expand Up @@ -53,6 +52,5 @@ export function createBasicEnvironment(options: CreateBasicEnvironmentArgs): Env
routeCache: new RouteCache(options.logging, mode),
ssr: options.ssr ?? true,
streaming: options.streaming ?? true,
telemetry: false,
});
}
4 changes: 1 addition & 3 deletions packages/astro/src/vite-plugin-astro-server/request.ts
Original file line number Diff line number Diff line change
Expand Up @@ -91,9 +91,7 @@ export async function handleRequest(
// Our error should already be complete, but let's try to add a bit more through some guesswork
const errorWithMetadata = collectErrorMetadata(err, config.root);

if (env.telemetry !== false) {
telemetry.record(eventError({ cmd: 'dev', err: errorWithMetadata, isFatal: false }));
}
telemetry.record(eventError({ cmd: 'dev', err: errorWithMetadata, isFatal: false }));

error(env.logging, null, msg.formatErrorMessage(errorWithMetadata));
handle500Response(moduleLoader, res, errorWithMetadata);
Expand Down
20 changes: 6 additions & 14 deletions packages/astro/test/test-utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,9 @@ polyfill(globalThis, {
exclude: 'window document',
});

// Disable telemetry when running tests
process.env.ASTRO_TELEMETRY_DISABLED = true;

/**
* @typedef {import('undici').Response} Response
* @typedef {import('../src/core/dev/dev').DedvServer} DevServer
Expand Down Expand Up @@ -146,13 +149,6 @@ export async function loadFixture(inlineConfig) {
return settings;
};

/** @type {import('@astrojs/telemetry').AstroTelemetry} */
const telemetry = {
record() {
return Promise.resolve();
},
};

const resolveUrl = (url) =>
`http://${config.server.host || 'localhost'}:${config.server.port}${url.replace(/^\/?/, '/')}`;

Expand Down Expand Up @@ -184,15 +180,15 @@ export async function loadFixture(inlineConfig) {
return {
build: async (opts = {}) => {
process.env.NODE_ENV = 'production';
return build(await getSettings(), { logging, telemetry, ...opts });
return build(await getSettings(), { logging, ...opts });
},
sync: async (opts) => sync(await getSettings(), { logging, fs, ...opts }),
check: async (opts) => {
return await check(await getSettings(), { logging, ...opts });
},
startDevServer: async (opts = {}) => {
process.env.NODE_ENV = 'development';
devServer = await dev(await getSettings(), { logging, telemetry, ...opts });
devServer = await dev(await getSettings(), { logging, ...opts });
config.server.host = parseAddressToHost(devServer.address.address); // update host
config.server.port = devServer.address.port; // update port
return devServer;
Expand All @@ -214,11 +210,7 @@ export async function loadFixture(inlineConfig) {
},
preview: async (opts = {}) => {
process.env.NODE_ENV = 'production';
const previewServer = await preview(await getSettings(), {
logging,
telemetry,
...opts,
});
const previewServer = await preview(await getSettings(), { logging, ...opts });
config.server.host = parseAddressToHost(previewServer.host); // update host
config.server.port = previewServer.port; // update port
return previewServer;
Expand Down
1 change: 0 additions & 1 deletion packages/astro/test/units/routing/route-matching.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,6 @@ describe('Route matching', () => {
output: 'hybrid',
adapter: testAdapter(),
},
disableTelemetry: true,
});
settings = container.settings;

Expand Down
2 changes: 1 addition & 1 deletion packages/astro/test/units/shiki/shiki.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ describe('<Code />', () => {
let container;
let mod;
before(async () => {
container = await createContainer({ root, disableTelemetry: true });
container = await createContainer({ root });
const loader = createViteLoader(container.viteServer);
mod = await loader.import('astro/components/Shiki.js');
});
Expand Down

0 comments on commit fcba0f0

Please sign in to comment.