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

Author: natemoo-re

.changeset/breezy-books-notice.md

@@ -0,0 +1,6 @@
+---
+'@astrojs/telemetry': patch
+'astro': patch
+---
+
+Update telemetry notice

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.

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);
 

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() {

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<string, any> };
 
+// 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<T>(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<boolean>) {
-		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<boolean>) {
+		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();
 		}
 

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/);
+	});
 });