From 0c7b42dc6780e687e416137539f55a3a427d1d10 Mon Sep 17 00:00:00 2001 From: Nate Moore Date: Mon, 28 Aug 2023 16:03:01 -0500 Subject: [PATCH] Update telemetry notice (#8234) * chore: enable telemetry notice * chore: update telemetry date, notify * chore(telemetry): refactor telemetry notices * chore: add changeset * chore: improve debugging * chore: update debug * fix: logical error * chore(lint): remove unused input * chore: improve telemetry debug * chore: improve telemetry tests * chore: allow isCI to be stubbed * chore: stub process.env * chore: stub process.env * chore: add env to class * chore: act like we're not on CI * test: didn't commit the env stub properly * chore: tweak wording --- .changeset/breezy-books-notice.md | 6 ++ packages/astro/src/cli/index.ts | 13 ++-- packages/astro/src/cli/telemetry/index.ts | 7 ++ packages/astro/src/core/messages.ts | 28 ++++---- packages/telemetry/src/index.ts | 35 +++++++--- packages/telemetry/test/index.test.js | 78 ++++++++++++++++++++++- 6 files changed, 133 insertions(+), 34 deletions(-) create mode 100644 .changeset/breezy-books-notice.md diff --git a/.changeset/breezy-books-notice.md b/.changeset/breezy-books-notice.md new file mode 100644 index 000000000000..586b405ea6f1 --- /dev/null +++ b/.changeset/breezy-books-notice.md @@ -0,0 +1,6 @@ +--- +'@astrojs/telemetry': patch +'astro': patch +--- + +Update telemetry notice diff --git a/packages/astro/src/cli/index.ts b/packages/astro/src/cli/index.ts index fdf43201fb37..0421258a51ae 100644 --- a/packages/astro/src/cli/index.ts +++ b/packages/astro/src/cli/index.ts @@ -109,6 +109,11 @@ async function runCommand(cmd: string, flags: yargs.Arguments) { await update(subcommand, { flags }); return; } + case 'sync': { + const { sync } = await import('./sync/index.js'); + const exitCode = await sync({ flags }); + return process.exit(exitCode); + } } // In verbose/debug mode, we log the debug logs asap before any potential errors could appear @@ -122,6 +127,9 @@ async function runCommand(cmd: string, flags: yargs.Arguments) { process.env.NODE_ENV = cmd === 'dev' ? 'development' : 'production'; } + const { notify } = await import('./telemetry/index.js'); + await notify(); + // These commands uses the logging and user config. All commands are assumed to have been handled // by the end of this switch statement. switch (cmd) { @@ -161,11 +169,6 @@ async function runCommand(cmd: string, flags: yargs.Arguments) { return process.exit(checkServer ? 1 : 0); } } - case 'sync': { - const { sync } = await import('./sync/index.js'); - const exitCode = await sync({ flags }); - return process.exit(exitCode); - } } // No command handler matched! This is unexpected. diff --git a/packages/astro/src/cli/telemetry/index.ts b/packages/astro/src/cli/telemetry/index.ts index 5ff71f5bf781..1c9703a9c658 100644 --- a/packages/astro/src/cli/telemetry/index.ts +++ b/packages/astro/src/cli/telemetry/index.ts @@ -7,6 +7,13 @@ interface TelemetryOptions { flags: yargs.Arguments; } +export async function notify() { + await telemetry.notify(() => { + console.log(msg.telemetryNotice() + '\n'); + return true; + }) +} + export async function update(subcommand: string, { flags }: TelemetryOptions) { const isValid = ['enable', 'disable', 'reset'].includes(subcommand); diff --git a/packages/astro/src/core/messages.ts b/packages/astro/src/core/messages.ts index d0c5a5e88404..9feecb26adb9 100644 --- a/packages/astro/src/core/messages.ts +++ b/packages/astro/src/core/messages.ts @@ -1,4 +1,3 @@ -import boxen from 'boxen'; import { bgCyan, bgGreen, @@ -107,34 +106,29 @@ export function serverStart({ } export function telemetryNotice() { - const headline = yellow(`Astro now collects ${bold('anonymous')} usage data.`); - const why = `This ${bold('optional program')} will help shape our roadmap.`; - const more = `For more info, visit ${underline('https://astro.build/telemetry')}`; - const box = boxen([headline, why, '', more].join('\n'), { - margin: 0, - padding: 1, - borderStyle: 'round', - borderColor: 'yellow', - }); - return box; + const headline = `${cyan('◆')} Astro collects completely anonymous usage data.`; + const why = dim(' This optional program helps shape our roadmap.') + const disable = dim(' Run `npm run astro telemetry disable` to opt-out.'); + const details = ` Details: ${underline('https://astro.build/telemetry')}`; + return [headline, why, disable, details].map(v => ' ' + v).join('\n'); } export function telemetryEnabled() { - return `\n ${green('◉')} Anonymous telemetry is ${bgGreen( + return `${green('◉')} Anonymous telemetry is now ${bgGreen( black(' enabled ') - )}. Thank you for improving Astro!\n`; + )}\n ${dim('Thank you for improving Astro!')}\n`; } export function telemetryDisabled() { - return `\n ${yellow('◯')} Anonymous telemetry is ${bgYellow( + return `${yellow('◯')} Anonymous telemetry is now ${bgYellow( black(' disabled ') - )}. We won't share any usage data.\n`; + )}\n ${dim('We won\'t ever record your usage data.')}\n`; } export function telemetryReset() { - return `\n ${cyan('◆')} Anonymous telemetry has been ${bgCyan( + return `${cyan('◆')} Anonymous telemetry has been ${bgCyan( black(' reset ') - )}. You may be prompted again.\n`; + )}\n ${dim('You may be prompted again.')}\n`; } export function fsStrictWarning() { diff --git a/packages/telemetry/src/index.ts b/packages/telemetry/src/index.ts index 3d08cd52914e..b63ff9c233e3 100644 --- a/packages/telemetry/src/index.ts +++ b/packages/telemetry/src/index.ts @@ -10,6 +10,9 @@ import { getSystemInfo, type SystemInfo } from './system-info.js'; export type AstroTelemetryOptions = { astroVersion: string; viteVersion: string }; export type TelemetryEvent = { eventName: string; payload: Record }; +// In the event of significant policy changes, update this! +const VALID_TELEMETRY_NOTICE_DATE = '2023-08-25'; + type EventMeta = SystemInfo; interface EventContext extends ProjectInfo { anonymousId: string; @@ -20,6 +23,8 @@ export class AstroTelemetry { private _anonymousProjectInfo: ProjectInfo | undefined; private config = new GlobalConfig({ name: 'astro' }); private debug = debug('astro:telemetry'); + private isCI = isCI; + private env = process.env; private get astroVersion() { return this.opts.astroVersion; @@ -28,10 +33,10 @@ export class AstroTelemetry { return this.opts.viteVersion; } private get ASTRO_TELEMETRY_DISABLED() { - return process.env.ASTRO_TELEMETRY_DISABLED; + return this.env.ASTRO_TELEMETRY_DISABLED; } private get TELEMETRY_DISABLED() { - return process.env.TELEMETRY_DISABLED; + return this.env.TELEMETRY_DISABLED; } constructor(private opts: AstroTelemetryOptions) { @@ -47,7 +52,7 @@ export class AstroTelemetry { */ private getConfigWithFallback(key: string, getValue: () => T): T { const currentValue = this.config.get(key); - if (currentValue) { + if (currentValue !== undefined) { return currentValue; } const newValue = getValue(); @@ -75,7 +80,7 @@ export class AstroTelemetry { private get anonymousProjectInfo(): ProjectInfo { // NOTE(fks): this value isn't global, so it can't use getConfigWithFallback(). - this._anonymousProjectInfo = this._anonymousProjectInfo || getProjectInfo(isCI); + this._anonymousProjectInfo = this._anonymousProjectInfo || getProjectInfo(this.isCI); return this._anonymousProjectInfo; } @@ -94,19 +99,29 @@ export class AstroTelemetry { return this.config.clear(); } - async notify(callback: () => Promise) { - if (this.isDisabled || isCI) { + isValidNotice() { + if (!this.notifyDate) return false; + const current = Number(this.notifyDate); + const valid = new Date(VALID_TELEMETRY_NOTICE_DATE).valueOf(); + + return current > valid; + } + + async notify(callback: () => boolean | Promise) { + if (this.isDisabled || this.isCI) { + this.debug(`[notify] telemetry has been disabled`); return; } // The end-user has already been notified about our telemetry integration! // Don't bother them about it again. - // In the event of significant changes, we should invalidate old dates. - if (this.notifyDate) { + if (this.isValidNotice()) { + this.debug(`[notify] last notified on ${this.notifyDate}`) return; } const enabled = await callback(); - this.config.set(KEY.TELEMETRY_NOTIFY_DATE, Date.now().toString()); + this.config.set(KEY.TELEMETRY_NOTIFY_DATE, new Date().valueOf().toString()); this.config.set(KEY.TELEMETRY_ENABLED, enabled); + this.debug(`[notify] telemetry has been ${enabled ? 'enabled' : 'disabled'}`) } async record(event: TelemetryEvent | TelemetryEvent[] = []) { @@ -117,7 +132,7 @@ export class AstroTelemetry { // Skip recording telemetry if the feature is disabled if (this.isDisabled) { - this.debug('telemetry disabled'); + this.debug('[record] telemetry has been disabled'); return Promise.resolve(); } diff --git a/packages/telemetry/test/index.test.js b/packages/telemetry/test/index.test.js index 29ade53f90e8..a8929329e795 100644 --- a/packages/telemetry/test/index.test.js +++ b/packages/telemetry/test/index.test.js @@ -1,9 +1,83 @@ import { expect } from 'chai'; import { AstroTelemetry } from '../dist/index.js'; -describe('AstroTelemetry', () => { +function setup() { + const config = new Map(); + const telemetry = new AstroTelemetry({ version: '0.0.0-test.1' }); + const logs = []; + // Stub isCI to false so we can test user-facing behavior + telemetry.isCI = false; + // Stub process.env to properly test in Astro's own CI + telemetry.env = {}; + // Override config so we can inspect it + telemetry.config = config; + // Override debug so we can inspect it + telemetry.debug.enabled = true; + telemetry.debug.log = (...args) => logs.push(args); + + return { telemetry, config, logs } +} +describe('AstroTelemetry', () => { + let oldCI; + before(() => { + oldCI = process.env.CI; + // Stub process.env.CI to `false` + process.env.CI = 'false'; + }) + after(() => { + process.env.CI = oldCI; + }) it('initializes when expected arguments are given', () => { - const telemetry = new AstroTelemetry({ version: '0.0.0-test.1' }); + const { telemetry } = setup(); expect(telemetry).to.be.instanceOf(AstroTelemetry); }); + it('does not record event if disabled', async () => { + const { telemetry, config, logs } = setup(); + telemetry.setEnabled(false); + const [key] = Array.from(config.keys()); + expect(key).not.to.be.undefined; + expect(config.get(key)).to.be.false; + expect(telemetry.enabled).to.be.false; + expect(telemetry.isDisabled).to.be.true; + const result = await telemetry.record(['TEST']); + expect(result).to.be.undefined; + const [log] = logs; + expect(log).not.to.be.undefined; + expect(logs.join('')).to.match(/disabled/); + }); + it('records event if enabled', async () => { + const { telemetry, config, logs } = setup(); + telemetry.setEnabled(true); + const [key] = Array.from(config.keys()); + expect(key).not.to.be.undefined; + expect(config.get(key)).to.be.true; + expect(telemetry.enabled).to.be.true; + expect(telemetry.isDisabled).to.be.false; + await telemetry.record(['TEST']); + expect(logs.length).to.equal(2); + }); + it('respects disable from notify', async () => { + const { telemetry, config, logs } = setup(); + await telemetry.notify(() => false); + const [key] = Array.from(config.keys()); + expect(key).not.to.be.undefined; + expect(config.get(key)).to.be.false; + expect(telemetry.enabled).to.be.false; + expect(telemetry.isDisabled).to.be.true; + const [log] = logs; + expect(log).not.to.be.undefined; + expect(logs.join('')).to.match(/disabled/); + }); + it('respects enable from notify', async () => { + const { telemetry, config, logs } = setup(); + await telemetry.notify(() => true); + const [key] = Array.from(config.keys()); + expect(key).not.to.be.undefined; + expect(config.get(key)).to.be.true; + expect(telemetry.enabled).to.be.true; + expect(telemetry.isDisabled).to.be.false; + const [log] = logs; + expect(log).not.to.be.undefined; + expect(logs.join('')).to.match(/enabled/); + }); });