From fcba0f0199b53e2077aae36b4ea2b287482d5c5c Mon Sep 17 00:00:00 2001 From: Bjorn Lu Date: Fri, 30 Jun 2023 17:28:17 +0800 Subject: [PATCH] Refactor telemetry usage flow (#7522) --- packages/astro/src/@types/astro.ts | 1 - packages/astro/src/cli/index.ts | 8 +++----- packages/astro/src/cli/telemetry.ts | 6 +++--- packages/astro/src/core/add/index.ts | 4 +--- packages/astro/src/core/build/index.ts | 2 -- packages/astro/src/core/config/settings.ts | 1 - packages/astro/src/core/dev/container.ts | 8 +------- packages/astro/src/core/dev/dev.ts | 5 ++--- packages/astro/src/core/preview/index.ts | 2 -- .../astro/src/core/render/dev/environment.ts | 1 - packages/astro/src/core/render/environment.ts | 2 -- .../src/vite-plugin-astro-server/request.ts | 4 +--- packages/astro/test/test-utils.js | 20 ++++++------------- .../test/units/routing/route-matching.test.js | 1 - packages/astro/test/units/shiki/shiki.test.js | 2 +- 15 files changed, 18 insertions(+), 49 deletions(-) diff --git a/packages/astro/src/@types/astro.ts b/packages/astro/src/@types/astro.ts index c544fd8b8c84..ef958d2a89e1 100644 --- a/packages/astro/src/@types/astro.ts +++ b/packages/astro/src/@types/astro.ts @@ -1375,7 +1375,6 @@ export interface AstroSettings { tsConfig: TsConfigJson | undefined; tsConfigPath: string | undefined; watchFiles: string[]; - forceDisableTelemetry: boolean; timer: AstroTimer; } diff --git a/packages/astro/src/cli/index.ts b/packages/astro/src/cli/index.ts index 250729dd9b80..4f0fa941cd94 100644 --- a/packages/astro/src/cli/index.ts +++ b/packages/astro/src/cli/index.ts @@ -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)); @@ -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 }); } } @@ -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'); @@ -224,7 +223,6 @@ async function runCommand(cmd: string, flags: yargs.Arguments) { return await build(settings, { flags, logging, - telemetry, teardownCompiler: true, mode: flags.mode, }); @@ -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 } diff --git a/packages/astro/src/cli/telemetry.ts b/packages/astro/src/cli/telemetry.ts index a8ec1c174f7c..7a5bfd469c7b 100644 --- a/packages/astro/src/cli/telemetry.ts +++ b/packages/astro/src/cli/telemetry.ts @@ -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) { diff --git a/packages/astro/src/core/add/index.ts b/packages/astro/src/core/add/index.ts index 7af09b4de3a2..1ce9163bf00f 100644 --- a/packages/astro/src/core/add/index.ts +++ b/packages/astro/src/core/add/index.ts @@ -1,4 +1,3 @@ -import type { AstroTelemetry } from '@astrojs/telemetry'; import boxen from 'boxen'; import { diffWords } from 'diff'; import { execa } from 'execa'; @@ -30,7 +29,6 @@ import { wrapDefaultExport } from './wrapper.js'; export interface AddOptions { logging: LogOptions; flags: yargs.Arguments; - telemetry: AstroTelemetry; cwd?: string; } @@ -83,7 +81,7 @@ async function getRegistry(): Promise { 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({ diff --git a/packages/astro/src/core/build/index.ts b/packages/astro/src/core/build/index.ts index ca1c98aff29a..037c462fde1e 100644 --- a/packages/astro/src/core/build/index.ts +++ b/packages/astro/src/core/build/index.ts @@ -1,4 +1,3 @@ -import type { AstroTelemetry } from '@astrojs/telemetry'; import type { AstroConfig, AstroSettings, ManifestData, RuntimeMode } from '../../@types/astro'; import fs from 'fs'; @@ -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. diff --git a/packages/astro/src/core/config/settings.ts b/packages/astro/src/core/config/settings.ts index c5e9def8ce43..8040b3fbe46f 100644 --- a/packages/astro/src/core/config/settings.ts +++ b/packages/astro/src/core/config/settings.ts @@ -104,7 +104,6 @@ export function createBaseSettings(config: AstroConfig): AstroSettings { scripts: [], clientDirectives: getDefaultClientDirectives(), watchFiles: [], - forceDisableTelemetry: false, timer: new AstroTimer(), }; } diff --git a/packages/astro/src/core/dev/container.ts b/packages/astro/src/core/dev/container.ts index b65a2c48bbb6..4a1d6cc6b252 100644 --- a/packages/astro/src/core/dev/container.ts +++ b/packages/astro/src/core/dev/container.ts @@ -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 { @@ -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({ @@ -149,7 +143,7 @@ export async function runInContainer( params: CreateContainerParams, callback: (container: Container) => Promise | void ) { - const container = await createContainer({ ...params, disableTelemetry: true }); + const container = await createContainer(params); try { await callback(container); } finally { diff --git a/packages/astro/src/core/dev/dev.ts b/packages/astro/src/core/dev/dev.ts index 8dbaabddf884..5308eaf6b3ca 100644 --- a/packages/astro/src/core/dev/dev.ts +++ b/packages/astro/src/core/dev/dev.ts @@ -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'; @@ -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'; @@ -18,7 +18,6 @@ export interface DevOptions { configFlagPath: string | undefined; flags?: yargs.Arguments; logging: LogOptions; - telemetry: AstroTelemetry; handleConfigError: (error: Error) => void; isRestart?: boolean; } @@ -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({ diff --git a/packages/astro/src/core/preview/index.ts b/packages/astro/src/core/preview/index.ts index faaa67428ab0..492afe03f427 100644 --- a/packages/astro/src/core/preview/index.ts +++ b/packages/astro/src/core/preview/index.ts @@ -1,4 +1,3 @@ -import type { AstroTelemetry } from '@astrojs/telemetry'; import { cyan } from 'kleur/colors'; import { createRequire } from 'module'; import { pathToFileURL } from 'url'; @@ -12,7 +11,6 @@ import { getResolvedHostForHttpServer } from './util.js'; interface PreviewOptions { logging: LogOptions; - telemetry: AstroTelemetry; flags?: Arguments; } diff --git a/packages/astro/src/core/render/dev/environment.ts b/packages/astro/src/core/render/dev/environment.ts index ea11ee99959c..ec7cd4534d13 100644 --- a/packages/astro/src/core/render/dev/environment.ts +++ b/packages/astro/src/core/render/dev/environment.ts @@ -32,7 +32,6 @@ export function createDevelopmentEnvironment( site: settings.config.site, ssr: isServerLikeOutput(settings.config), streaming: true, - telemetry: Boolean(settings.forceDisableTelemetry), }); return { diff --git a/packages/astro/src/core/render/environment.ts b/packages/astro/src/core/render/environment.ts index a141ff728a51..7cd1ae3d8799 100644 --- a/packages/astro/src/core/render/environment.ts +++ b/packages/astro/src/core/render/environment.ts @@ -23,7 +23,6 @@ export interface Environment { site?: string; ssr: boolean; streaming: boolean; - telemetry?: boolean; } export type CreateEnvironmentArgs = Environment; @@ -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, }); } diff --git a/packages/astro/src/vite-plugin-astro-server/request.ts b/packages/astro/src/vite-plugin-astro-server/request.ts index 3c5da0575b67..197033b048c9 100644 --- a/packages/astro/src/vite-plugin-astro-server/request.ts +++ b/packages/astro/src/vite-plugin-astro-server/request.ts @@ -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); diff --git a/packages/astro/test/test-utils.js b/packages/astro/test/test-utils.js index 583a6c61e4d9..953ebf6daeeb 100644 --- a/packages/astro/test/test-utils.js +++ b/packages/astro/test/test-utils.js @@ -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 @@ -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(/^\/?/, '/')}`; @@ -184,7 +180,7 @@ 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) => { @@ -192,7 +188,7 @@ export async function loadFixture(inlineConfig) { }, 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; @@ -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; diff --git a/packages/astro/test/units/routing/route-matching.test.js b/packages/astro/test/units/routing/route-matching.test.js index 46536176533f..fca2064ea0e4 100644 --- a/packages/astro/test/units/routing/route-matching.test.js +++ b/packages/astro/test/units/routing/route-matching.test.js @@ -135,7 +135,6 @@ describe('Route matching', () => { output: 'hybrid', adapter: testAdapter(), }, - disableTelemetry: true, }); settings = container.settings; diff --git a/packages/astro/test/units/shiki/shiki.test.js b/packages/astro/test/units/shiki/shiki.test.js index 21f93dce819b..0d67dda35f0a 100644 --- a/packages/astro/test/units/shiki/shiki.test.js +++ b/packages/astro/test/units/shiki/shiki.test.js @@ -9,7 +9,7 @@ describe('', () => { 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'); });