Add additional metrics (#7549)

* Add additional metrics
* Detect Pages & Workers CI
* Filter out default args (e.g. --x-versions, --x-dev-env, and --latest)

* Create selfish-yaks-hope.md

* fix format

* extract CI check

* sanitise wrangler login

Author: penalosa

.changeset/selfish-yaks-hope.md

@@ -0,0 +1,8 @@
+---
+"wrangler": patch
+---
+
+Expand metrics collection to:
+
+- Detect Pages & Workers CI
+- Filter out default args (e.g. `--x-versions`, `--x-dev-env`, and `--latest`) by only including args that were in `argv`

packages/wrangler/src/__tests__/metrics.test.ts

@@ -186,15 +186,11 @@ describe("metrics", () => {
 				isFirstUsage: false,
 				configFileType: "toml",
 				isCI: false,
+				isPagesCI: false,
+				isWorkersCI: false,
 				isInteractive: true,
-				argsUsed: [
-					"j",
-					"search",
-					"xGradualRollouts",
-					"xJsonConfig",
-					"xVersions",
-				],
-				argsCombination: "j, search, xGradualRollouts, xJsonConfig, xVersions",
+				argsUsed: [],
+				argsCombination: "",
 				command: "wrangler docs",
 				args: {
 					xJsonConfig: true,
@@ -343,6 +339,50 @@ describe("metrics", () => {
 				expect(std.debug).toContain('isCI":true');
 			});
 
+			it("should mark isPagesCI as true if running in Pages CI", async () => {
+				vi.stubEnv("CF_PAGES", "1");
+				const requests = mockMetricRequest();
+
+				await runWrangler("docs arg");
+
+				expect(requests.count).toBe(2);
+				expect(std.debug).toContain('isPagesCI":true');
+			});
+
+			it("should mark isWorkersCI as true if running in Workers CI", async () => {
+				vi.stubEnv("WORKERS_CI", "1");
+				const requests = mockMetricRequest();
+
+				await runWrangler("docs arg");
+
+				expect(requests.count).toBe(2);
+				expect(std.debug).toContain('isWorkersCI":true');
+			});
+
+			it("should not send arguments with wrangler login", async () => {
+				const requests = mockMetricRequest();
+
+				await expect(
+					runWrangler("login username password")
+				).rejects.toThrowErrorMatchingInlineSnapshot(
+					`[Error: Unknown arguments: username, password]`
+				);
+
+				expect(requests.count).toBe(2);
+				expect(std.debug).toContain('"argsCombination":""');
+				expect(std.debug).toContain('"command":"wrangler login"');
+			});
+
+			it("should include args provided by the user", async () => {
+				const requests = mockMetricRequest();
+
+				await runWrangler("docs arg --search 'some search term'");
+
+				expect(requests.count).toBe(2);
+				// Notably, this _doesn't_ include default args (e.g. --x-versions)
+				expect(std.debug).toContain('argsCombination":"search"');
+			});
+
 			it("should mark as non-interactive if running in non-interactive environment", async () => {
 				setIsTTY(false);
 				const requests = mockMetricRequest();

packages/wrangler/src/__tests__/vitest.setup.ts

@@ -135,8 +135,9 @@ vi.mock("../user/generate-auth-url", () => {
 	};
 });
 
-vi.mock("../is-ci", () => {
-	return { CI: { isCI: vi.fn().mockImplementation(() => false) } };
+vi.mock("../is-ci", async (importOriginal) => {
+	const original = await importOriginal<typeof import("../is-ci")>();
+	return { ...original, CI: { isCI: vi.fn().mockImplementation(() => false) } };
 });
 
 vi.mock("../user/generate-random-state", () => {

packages/wrangler/src/index.ts

@@ -1169,10 +1169,14 @@ export async function main(argv: string[]): Promise<void> {
 		addBreadcrumb(command);
 		// NB despite 'applyBeforeValidation = true', this runs *after* yargs 'validates' options,
 		// e.g. if a required arg is missing, yargs will error out before we send any events :/
-		dispatcher?.sendCommandEvent("wrangler command started", {
-			command,
-			args,
-		});
+		dispatcher?.sendCommandEvent(
+			"wrangler command started",
+			{
+				command,
+				args,
+			},
+			argv
+		);
 	}, /* applyBeforeValidation */ true);
 
 	let cliHandlerThrew = false;
@@ -1181,13 +1185,17 @@ export async function main(argv: string[]): Promise<void> {
 
 		const durationMs = Date.now() - startTime;
 
-		dispatcher?.sendCommandEvent("wrangler command completed", {
-			command,
-			args: metricsArgs,
-			durationMs,
-			durationSeconds: durationMs / 1000,
-			durationMinutes: durationMs / 1000 / 60,
-		});
+		dispatcher?.sendCommandEvent(
+			"wrangler command completed",
+			{
+				command,
+				args: metricsArgs,
+				durationMs,
+				durationSeconds: durationMs / 1000,
+				durationMinutes: durationMs / 1000 / 60,
+			},
+			argv
+		);
 	} catch (e) {
 		cliHandlerThrew = true;
 		let mayReport = true;
@@ -1297,15 +1305,19 @@ export async function main(argv: string[]): Promise<void> {
 
 		const durationMs = Date.now() - startTime;
 
-		dispatcher?.sendCommandEvent("wrangler command errored", {
-			command,
-			args: metricsArgs,
-			durationMs,
-			durationSeconds: durationMs / 1000,
-			durationMinutes: durationMs / 1000 / 60,
-			errorType:
-				errorType ?? (e instanceof Error ? e.constructor.name : undefined),
-		});
+		dispatcher?.sendCommandEvent(
+			"wrangler command errored",
+			{
+				command,
+				args: metricsArgs,
+				durationMs,
+				durationSeconds: durationMs / 1000,
+				durationMinutes: durationMs / 1000 / 60,
+				errorType:
+					errorType ?? (e instanceof Error ? e.constructor.name : undefined),
+			},
+			argv
+		);
 
 		throw e;
 	} finally {

packages/wrangler/src/is-ci.ts

@@ -1,5 +1,12 @@
 import isCI from "is-ci";
 
+export function isPagesCI() {
+	return process.env.CF_PAGES === "1";
+}
+
+export function isWorkersCI() {
+	return process.env.WORKERS_CI === "1";
+}
 /**
  * Use this object to find out if we are currently running in a continuous integration environment.
  *
@@ -9,6 +16,6 @@ import isCI from "is-ci";
 export const CI = {
 	/** Is Wrangler currently running in a CI? */
 	isCI() {
-		return isCI;
+		return isCI || isPagesCI() || isWorkersCI();
 	},
 };

packages/wrangler/src/is-interactive.ts

@@ -1,13 +1,13 @@
 import { isInteractive as __isInteractive } from "@cloudflare/cli/interactive";
-import { CI } from "./is-ci";
+import { CI, isPagesCI, isWorkersCI } from "./is-ci";
 
 /**
  * Test whether the process is "interactive".
  * Reasons it may not be interactive: it could be running in CI,
  * or you're piping values from / to another process, etc
  */
 export default function isInteractive(): boolean {
-	if (process.env.CF_PAGES === "1") {
+	if (isPagesCI() || isWorkersCI()) {
 		return false;
 	}
 

packages/wrangler/src/metrics/metrics-dispatcher.ts

@@ -4,7 +4,7 @@ import { configFormat } from "../config";
 import isInteractive from "../is-interactive";
 import { logger } from "../logger";
 import { sniffUserAgent } from "../package-manager";
-import { CI } from "./../is-ci";
+import { CI, isPagesCI, isWorkersCI } from "./../is-ci";
 import {
 	getNodeVersion,
 	getOS,
@@ -73,9 +73,13 @@ export function getMetricsDispatcher(options: MetricsConfigOptions) {
 			properties: Omit<
 				Extract<Events, { name: EventName }>["properties"],
 				keyof CommonEventProperties
-			>
+			>,
+			argv?: string[]
 		) {
 			try {
+				if (properties.command?.startsWith("wrangler login")) {
+					properties.command = "wrangler login";
+				}
 				if (
 					properties.command === "wrangler telemetry disable" ||
 					properties.command === "wrangler metrics disable"
@@ -91,7 +95,7 @@ export function getMetricsDispatcher(options: MetricsConfigOptions) {
 					printMetricsBanner();
 				}
 
-				const argsUsed = sanitiseUserInput(properties.args ?? {});
+				const argsUsed = sanitiseUserInput(properties.args ?? {}, argv);
 				const argsCombination = argsUsed.sort().join(", ");
 				const commonEventProperties: CommonEventProperties = {
 					amplitude_session_id,
@@ -104,6 +108,8 @@ export function getMetricsDispatcher(options: MetricsConfigOptions) {
 					isFirstUsage: readMetricsConfig().permission === undefined,
 					configFileType: configFormat(options.configPath),
 					isCI: CI.isCI(),
+					isPagesCI: isPagesCI(),
+					isWorkersCI: isWorkersCI(),
 					isInteractive: isInteractive(),
 					argsUsed,
 					argsCombination,
@@ -213,11 +219,20 @@ const normalise = (arg: string) => {
 };
 
 const exclude = new Set(["$0", "_"]);
-/** just some pretty naive cleaning so we don't send "experimental-versions", "experimentalVersions", "x-versions" and "xVersions" etc. */
-const sanitiseUserInput = (argsWithValues: Record<string, unknown>) => {
+/**
+ * just some pretty naive cleaning so we don't send duplicates of "experimental-versions", "experimentalVersions", "x-versions" and "xVersions" etc.
+ * optionally, if an argv is provided remove all args that were not specified in argv (which means that default values will be filtered out)
+ */
+const sanitiseUserInput = (
+	argsWithValues: Record<string, unknown>,
+	argv?: string[]
+) => {
 	const result: string[] = [];
 	const args = Object.keys(argsWithValues);
 	for (const arg of args) {
+		if (Array.isArray(argv) && !argv.some((a) => a.includes(arg))) {
+			continue;
+		}
 		if (exclude.has(arg)) {
 			continue;
 		}

packages/wrangler/src/metrics/types.ts

@@ -40,6 +40,14 @@ export type CommonEventProperties = {
 	 * Whether the Wrangler client is running in CI
 	 */
 	isCI: boolean;
+	/**
+	 * Whether the Wrangler client is running in Pages CI
+	 */
+	isPagesCI: boolean;
+	/**
+	 * Whether the Wrangler client is running in Workers CI
+	 */
+	isWorkersCI: boolean;
 	/**
 	 * Whether the Wrangler client is running in an interactive instance
 	 */

packages/wrangler/turbo.json

@@ -36,6 +36,7 @@
 				"EXPERIMENTAL_MIDDLEWARE",
 				"FORMAT_WRANGLER_ERRORS",
 				"CF_PAGES",
+				"WORKERS_CI",
 				"CI",
 				"CF_PAGES_UPLOAD_JWT",
 				"EXPERIMENTAL_MIDDLEWARE",