From b76f15d3776c0ad25988003e0c76bc835d7b6823 Mon Sep 17 00:00:00 2001 From: emily-shen <69125074+emily-shen@users.noreply.github.com> Date: Fri, 15 Nov 2024 13:32:50 +0000 Subject: [PATCH 1/5] feat: add telemetry commands (#7236) * add telemetry commands * changeset * fix and test dates * update changeset * add global/project status * default true * remove changeset * update wrangler telemetry status --- .../wrangler/src/__tests__/metrics.test.ts | 267 ++++++++++++++---- packages/wrangler/src/index.ts | 2 + packages/wrangler/src/metrics/commands.ts | 81 ++++++ .../wrangler/src/metrics/metrics-config.ts | 37 +-- 4 files changed, 308 insertions(+), 79 deletions(-) create mode 100644 packages/wrangler/src/metrics/commands.ts diff --git a/packages/wrangler/src/__tests__/metrics.test.ts b/packages/wrangler/src/__tests__/metrics.test.ts index 4da693aaf533..b4f4fe4956de 100644 --- a/packages/wrangler/src/__tests__/metrics.test.ts +++ b/packages/wrangler/src/__tests__/metrics.test.ts @@ -7,17 +7,18 @@ import { CI } from "../is-ci"; import { logger } from "../logger"; import { getMetricsConfig, getMetricsDispatcher } from "../metrics"; import { - CURRENT_METRICS_DATE, readMetricsConfig, USER_ID_CACHE_PATH, writeMetricsConfig, } from "../metrics/metrics-config"; import { writeAuthConfigFile } from "../user"; import { mockConsoleMethods } from "./helpers/mock-console"; -import { clearDialogs, mockConfirm } from "./helpers/mock-dialogs"; +import { clearDialogs } from "./helpers/mock-dialogs"; import { useMockIsTTY } from "./helpers/mock-istty"; import { msw, mswSuccessOauthHandlers } from "./helpers/msw"; import { runInTempDir } from "./helpers/run-in-tmp"; +import { runWrangler } from "./helpers/run-wrangler"; +import { writeWranglerToml } from "./helpers/write-wrangler-toml"; import type { MockInstance } from "vitest"; declare const global: { SPARROW_SOURCE_KEY: string | undefined }; @@ -270,7 +271,7 @@ describe("metrics", () => { }); it("should return enabled true if the user on this device previously agreed to send metrics", async () => { - await writeMetricsConfig({ + writeMetricsConfig({ permission: { enabled: true, date: new Date(2022, 6, 4), @@ -287,7 +288,7 @@ describe("metrics", () => { }); it("should return enabled false if the user on this device previously refused to send metrics", async () => { - await writeMetricsConfig({ + writeMetricsConfig({ permission: { enabled: false, date: new Date(2022, 6, 4), @@ -303,49 +304,11 @@ describe("metrics", () => { }); }); - it("should accept and store permission granting to send metrics if the user agrees", async () => { - mockConfirm({ - text: "Would you like to help improve Wrangler by sending usage metrics to Cloudflare?", - result: true, - }); - expect( - await getMetricsConfig({ - sendMetrics: undefined, - offline: false, - }) - ).toMatchObject({ - enabled: true, - }); - expect((await readMetricsConfig()).permission).toMatchObject({ - enabled: true, - }); - }); - - it("should accept and store permission declining to send metrics if the user declines", async () => { - mockConfirm({ - text: "Would you like to help improve Wrangler by sending usage metrics to Cloudflare?", - result: false, - }); - expect( - await getMetricsConfig({ - sendMetrics: undefined, - offline: false, - }) - ).toMatchObject({ - enabled: false, - }); - expect((await readMetricsConfig()).permission).toMatchObject({ - enabled: false, - }); - }); - - it("should ignore the config if the permission date is older than the current metrics date", async () => { - mockConfirm({ - text: "Would you like to help improve Wrangler by sending usage metrics to Cloudflare?", - result: false, - }); + it("should print a message if the permission date is older than the current metrics date", async () => { + vi.useFakeTimers(); + vi.setSystemTime(new Date(2024, 11, 12)); const OLD_DATE = new Date(2000); - await writeMetricsConfig({ + writeMetricsConfig({ permission: { enabled: true, date: OLD_DATE }, }); expect( @@ -354,38 +317,33 @@ describe("metrics", () => { offline: false, }) ).toMatchObject({ - enabled: false, + enabled: true, }); - const { permission } = await readMetricsConfig(); - expect(permission?.enabled).toBe(false); + const { permission } = readMetricsConfig(); + expect(permission?.enabled).toBe(true); // The date should be updated to today's date - expect(permission?.date).toEqual(CURRENT_METRICS_DATE); + expect(permission?.date).toEqual(new Date(2024, 11, 12)); expect(std.out).toMatchInlineSnapshot(` - "Usage metrics tracking has changed since you last granted permission. - Your choice has been saved in the following file: test-xdg-config/metrics.json. - - You can override the user level setting for a project in \`wrangler.toml\`: - - - to disable sending metrics for a project: \`send_metrics = false\` - - to enable sending metrics for a project: \`send_metrics = true\`" + "Usage metrics tracking has changed since you last granted permission." `); + vi.useRealTimers(); }); }); describe("deviceId", () => { it("should return a deviceId found in the config file", async () => { - await writeMetricsConfig({ deviceId: "XXXX-YYYY-ZZZZ" }); + writeMetricsConfig({ deviceId: "XXXX-YYYY-ZZZZ" }); const { deviceId } = await getMetricsConfig({ sendMetrics: true, offline: false, }); expect(deviceId).toEqual("XXXX-YYYY-ZZZZ"); - expect((await readMetricsConfig()).deviceId).toEqual(deviceId); + expect(readMetricsConfig().deviceId).toEqual(deviceId); }); it("should create and store a new deviceId if none is found in the config file", async () => { - await writeMetricsConfig({}); + writeMetricsConfig({}); const { deviceId } = await getMetricsConfig({ sendMetrics: true, offline: false, @@ -393,14 +351,14 @@ describe("metrics", () => { expect(deviceId).toMatch( /[0-9a-fA-F]{8}-([0-9a-fA-F]{4}-){3}[0-9a-fA-F]{12}/ ); - expect((await readMetricsConfig()).deviceId).toEqual(deviceId); + expect(readMetricsConfig().deviceId).toEqual(deviceId); }); }); describe("userId", () => { const userRequests = mockUserRequest(); it("should return a userId found in a cache file", async () => { - await saveToConfigCache(USER_ID_CACHE_PATH, { + saveToConfigCache(USER_ID_CACHE_PATH, { userId: "CACHED_USER_ID", }); const { userId } = await getMetricsConfig({ @@ -434,6 +392,191 @@ describe("metrics", () => { }); }); }); + + describe("telemetry commands", () => { + beforeEach(() => { + vi.useFakeTimers(); + vi.setSystemTime(new Date(2024, 11, 12)); + }); + afterEach(() => { + vi.useRealTimers(); + }); + describe("wrangler telemetry status", () => { + it("prints the current telemetry status based on the cached metrics config", async () => { + writeMetricsConfig({ + permission: { + enabled: true, + date: new Date(2022, 6, 4), + }, + }); + await runWrangler("telemetry status"); + expect(std.out).toMatchInlineSnapshot(` + "Status: Enabled + + To configure telemetry globally on this machine, you can run \`wrangler telemetry disable / enable\`. + You can override this for individual projects with the environment variable \`WRANGLER_SEND_METRICS=true/false\`. + " + `); + writeMetricsConfig({ + permission: { + enabled: false, + date: new Date(2022, 6, 4), + }, + }); + await runWrangler("telemetry status"); + expect(std.out).toMatchInlineSnapshot(` + "Status: Enabled + + To configure telemetry globally on this machine, you can run \`wrangler telemetry disable / enable\`. + You can override this for individual projects with the environment variable \`WRANGLER_SEND_METRICS=true/false\`. + + Status: Disabled + + To configure telemetry globally on this machine, you can run \`wrangler telemetry disable / enable\`. + You can override this for individual projects with the environment variable \`WRANGLER_SEND_METRICS=true/false\`. + " + `); + }); + + it("shows wrangler.toml as the source with send_metrics is present", async () => { + writeMetricsConfig({ + permission: { + enabled: true, + date: new Date(2022, 6, 4), + }, + }); + writeWranglerToml({ send_metrics: false }); + await runWrangler("telemetry status"); + expect(std.out).toMatchInlineSnapshot(` + "Status: Disabled (set by wrangler.toml) + + To configure telemetry globally on this machine, you can run \`wrangler telemetry disable / enable\`. + You can override this for individual projects with the environment variable \`WRANGLER_SEND_METRICS=true/false\`. + " + `); + }); + + it("shows environment variable as the source if used", async () => { + writeMetricsConfig({ + permission: { + enabled: true, + date: new Date(2022, 6, 4), + }, + }); + vi.stubEnv("WRANGLER_SEND_METRICS", "false"); + await runWrangler("telemetry status"); + expect(std.out).toMatchInlineSnapshot(` + "Status: Disabled (set by environment variable) + + To configure telemetry globally on this machine, you can run \`wrangler telemetry disable / enable\`. + You can override this for individual projects with the environment variable \`WRANGLER_SEND_METRICS=true/false\`. + " + `); + }); + + it("defaults to enabled if metrics config is not set", async () => { + writeMetricsConfig({}); + await runWrangler("telemetry status"); + expect(std.out).toMatchInlineSnapshot(` + "Status: Enabled + + To configure telemetry globally on this machine, you can run \`wrangler telemetry disable / enable\`. + You can override this for individual projects with the environment variable \`WRANGLER_SEND_METRICS=true/false\`. + " + `); + }); + + it("prioritises environment variable over send_metrics", async () => { + writeMetricsConfig({ + permission: { + enabled: true, + date: new Date(2022, 6, 4), + }, + }); + writeWranglerToml({ send_metrics: true }); + vi.stubEnv("WRANGLER_SEND_METRICS", "false"); + await runWrangler("telemetry status"); + expect(std.out).toMatchInlineSnapshot(` + "Status: Disabled (set by environment variable) + + To configure telemetry globally on this machine, you can run \`wrangler telemetry disable / enable\`. + You can override this for individual projects with the environment variable \`WRANGLER_SEND_METRICS=true/false\`. + " + `); + }); + }); + + it("disables telemetry when `wrangler telemetry disable` is run", async () => { + writeMetricsConfig({ + permission: { + enabled: true, + date: new Date(2022, 6, 4), + }, + }); + await runWrangler("telemetry disable"); + expect(std.out).toMatchInlineSnapshot(` + "Status: Disabled + + Wrangler is no longer collecting telemetry about your usage. + " + `); + expect(readMetricsConfig()).toMatchObject({ + permission: { + enabled: false, + date: new Date(2024, 11, 12), + }, + }); + }); + + it("enables telemetry when `wrangler telemetry enable` is run", async () => { + writeMetricsConfig({ + permission: { + enabled: false, + date: new Date(2022, 6, 4), + }, + }); + await runWrangler("telemetry enable"); + expect(std.out).toMatchInlineSnapshot(` + "Status: Enabled + + Wrangler is now collecting telemetry about your usage. Thank you for helping make Wrangler better 🧡 + " + `); + expect(readMetricsConfig()).toMatchObject({ + permission: { + enabled: true, + date: new Date(2024, 11, 12), + }, + }); + }); + + it("doesn't overwrite c3 telemetry config", async () => { + writeMetricsConfig({ + c3permission: { + enabled: false, + date: new Date(2022, 6, 4), + }, + }); + await runWrangler("telemetry enable"); + expect(std.out).toMatchInlineSnapshot(` + "Status: Enabled + + Wrangler is now collecting telemetry about your usage. Thank you for helping make Wrangler better 🧡 + " + `); + const config = readMetricsConfig(); + expect(config).toMatchObject({ + c3permission: { + enabled: false, + date: new Date(2022, 6, 4), + }, + permission: { + enabled: true, + date: new Date(2024, 11, 12), + }, + }); + }); + }); }); function mockUserRequest() { diff --git a/packages/wrangler/src/index.ts b/packages/wrangler/src/index.ts index 53c9cfb2dcbe..dd2c290571d2 100644 --- a/packages/wrangler/src/index.ts +++ b/packages/wrangler/src/index.ts @@ -49,6 +49,7 @@ import "./dev"; import "./kv"; import "./workflows"; import "./user/commands"; +import "./metrics/commands"; import { demandSingleValue } from "./core"; import { logBuildFailure, logger, LOGGER_LEVELS } from "./logger"; import { mTlsCertificateCommands } from "./mtls-certificate/cli"; @@ -584,6 +585,7 @@ export function createCLIParser(argv: string[]) { register.registerNamespace("login"); register.registerNamespace("logout"); register.registerNamespace("whoami"); + register.registerNamespace("telemetry"); /******************************************************/ /* DEPRECATED COMMANDS */ diff --git a/packages/wrangler/src/metrics/commands.ts b/packages/wrangler/src/metrics/commands.ts new file mode 100644 index 000000000000..34e1ec4c763f --- /dev/null +++ b/packages/wrangler/src/metrics/commands.ts @@ -0,0 +1,81 @@ +import chalk from "chalk"; +import { defineCommand, defineNamespace } from "../core"; +import { getWranglerSendMetricsFromEnv } from "../environment-variables/misc-variables"; +import { logger } from "../logger"; +import { readMetricsConfig, updateMetricsPermission } from "./metrics-config"; + +defineNamespace({ + command: "wrangler telemetry", + metadata: { + description: "📈 Configure whether Wrangler collects telemetry", + owner: "Workers: Authoring and Testing", + status: "stable", + hidden: true, + }, +}); + +defineCommand({ + command: "wrangler telemetry disable", + metadata: { + description: "Disable Wrangler telemetry collection", + owner: "Workers: Authoring and Testing", + status: "stable", + }, + async handler() { + updateMetricsPermission(false); + logTelemetryStatus(false); + logger.log( + "Wrangler is no longer collecting telemetry about your usage.\n" + ); + }, +}); + +defineCommand({ + command: "wrangler telemetry enable", + metadata: { + description: "Enable Wrangler telemetry collection", + owner: "Workers: Authoring and Testing", + status: "stable", + }, + async handler() { + updateMetricsPermission(true); + logTelemetryStatus(true); + logger.log( + "Wrangler is now collecting telemetry about your usage. Thank you for helping make Wrangler better 🧡\n" + ); + }, +}); + +defineCommand({ + command: "wrangler telemetry status", + metadata: { + description: "Check whether Wrangler telemetry collection is enabled", + owner: "Workers: Authoring and Testing", + status: "stable", + }, + async handler(_, { config }) { + const savedConfig = readMetricsConfig(); + const sendMetricsEnv = getWranglerSendMetricsFromEnv(); + if (config.send_metrics !== undefined || sendMetricsEnv !== undefined) { + const resolvedPermission = + sendMetricsEnv !== undefined + ? sendMetricsEnv === "true" + : config.send_metrics; + logger.log( + `Status: ${resolvedPermission ? chalk.green("Enabled") : chalk.red("Disabled")} (set by ${sendMetricsEnv !== undefined ? "environment variable" : "wrangler.toml"})\n` + ); + } else { + logTelemetryStatus(savedConfig.permission?.enabled ?? true); + } + logger.log( + "To configure telemetry globally on this machine, you can run `wrangler telemetry disable / enable`.\n" + + "You can override this for individual projects with the environment variable `WRANGLER_SEND_METRICS=true/false`.\n" + ); + }, +}); + +const logTelemetryStatus = (enabled: boolean) => { + logger.log( + `Status: ${enabled ? chalk.green("Enabled") : chalk.red("Disabled")}\n` + ); +}; diff --git a/packages/wrangler/src/metrics/metrics-config.ts b/packages/wrangler/src/metrics/metrics-config.ts index 258846bffd74..235db05fa222 100644 --- a/packages/wrangler/src/metrics/metrics-config.ts +++ b/packages/wrangler/src/metrics/metrics-config.ts @@ -3,7 +3,6 @@ import { mkdirSync, readFileSync, writeFileSync } from "node:fs"; import path from "node:path"; import { fetchResult } from "../cfetch"; import { getConfigCache, saveToConfigCache } from "../config-cache"; -import { confirm } from "../dialogs"; import { getWranglerSendMetricsFromEnv } from "../environment-variables/misc-variables"; import { getGlobalWranglerConfigPath } from "../global-wrangler-config-path"; import { isNonInteractiveOrCI } from "../is-interactive"; @@ -106,27 +105,16 @@ export async function getMetricsConfig({ return { enabled: false, deviceId, userId }; } - // Otherwise, let's ask the user and store the result in the metrics config. - const enabled = await confirm( - "Would you like to help improve Wrangler by sending usage metrics to Cloudflare?" - ); - logger.log( - `Your choice has been saved in the following file: ${path.relative( - process.cwd(), - getMetricsConfigPath() - )}.\n\n` + - " You can override the user level setting for a project in `wrangler.toml`:\n\n" + - " - to disable sending metrics for a project: `send_metrics = false`\n" + - " - to enable sending metrics for a project: `send_metrics = true`" - ); + // Otherwise, default to true writeMetricsConfig({ + ...config, permission: { - enabled, - date: CURRENT_METRICS_DATE, + enabled: true, + date: new Date(), }, deviceId, }); - return { enabled, deviceId, userId }; + return { enabled: true, deviceId, userId }; } /** @@ -158,6 +146,15 @@ export function readMetricsConfig(): MetricsConfigFile { } } +export function updateMetricsPermission(enabled: boolean) { + const config = readMetricsConfig(); + config.permission = { + enabled, + date: new Date(), + }; + writeMetricsConfig(config); +} + /** * Get the path to the metrics config file. */ @@ -175,6 +172,12 @@ export interface MetricsConfigFile { /** The date that this permission was set. */ date: Date; }; + c3permission?: { + /** True if c3 should send metrics to Cloudflare. */ + enabled: boolean; + /** The date that this permission was set. */ + date: Date; + }; /** A unique UUID that identifies this device for metrics purposes. */ deviceId?: string; } From 61acb6644a192a77bde975c750a5056f3039146d Mon Sep 17 00:00:00 2001 From: emily-shen <69125074+emily-shen@users.noreply.github.com> Date: Mon, 18 Nov 2024 19:19:59 +0000 Subject: [PATCH 2/5] feat: add `wrangler metrics` as an alias for `wrangler telemetry` (#7284) * add metrics alias * tests * use each to test alias --- .../wrangler/src/__tests__/metrics.test.ts | 103 +++++------------- packages/wrangler/src/metrics/commands.ts | 7 +- 2 files changed, 35 insertions(+), 75 deletions(-) diff --git a/packages/wrangler/src/__tests__/metrics.test.ts b/packages/wrangler/src/__tests__/metrics.test.ts index b4f4fe4956de..eb16fd992b6a 100644 --- a/packages/wrangler/src/__tests__/metrics.test.ts +++ b/packages/wrangler/src/__tests__/metrics.test.ts @@ -393,7 +393,7 @@ describe("metrics", () => { }); }); - describe("telemetry commands", () => { + describe.each(["metrics", "telemetry"])("%s commands", (cmd) => { beforeEach(() => { vi.useFakeTimers(); vi.setSystemTime(new Date(2024, 11, 12)); @@ -401,7 +401,7 @@ describe("metrics", () => { afterEach(() => { vi.useRealTimers(); }); - describe("wrangler telemetry status", () => { + describe(`${cmd} status`, () => { it("prints the current telemetry status based on the cached metrics config", async () => { writeMetricsConfig({ permission: { @@ -409,14 +409,9 @@ describe("metrics", () => { date: new Date(2022, 6, 4), }, }); - await runWrangler("telemetry status"); - expect(std.out).toMatchInlineSnapshot(` - "Status: Enabled - - To configure telemetry globally on this machine, you can run \`wrangler telemetry disable / enable\`. - You can override this for individual projects with the environment variable \`WRANGLER_SEND_METRICS=true/false\`. - " - `); + await runWrangler(`${cmd} status`); + expect(std.out).toContain("Status: Enabled"); + expect(std.out).not.toContain("Status: Disabled"); writeMetricsConfig({ permission: { enabled: false, @@ -424,18 +419,7 @@ describe("metrics", () => { }, }); await runWrangler("telemetry status"); - expect(std.out).toMatchInlineSnapshot(` - "Status: Enabled - - To configure telemetry globally on this machine, you can run \`wrangler telemetry disable / enable\`. - You can override this for individual projects with the environment variable \`WRANGLER_SEND_METRICS=true/false\`. - - Status: Disabled - - To configure telemetry globally on this machine, you can run \`wrangler telemetry disable / enable\`. - You can override this for individual projects with the environment variable \`WRANGLER_SEND_METRICS=true/false\`. - " - `); + expect(std.out).toContain("Status: Disabled"); }); it("shows wrangler.toml as the source with send_metrics is present", async () => { @@ -446,14 +430,8 @@ describe("metrics", () => { }, }); writeWranglerToml({ send_metrics: false }); - await runWrangler("telemetry status"); - expect(std.out).toMatchInlineSnapshot(` - "Status: Disabled (set by wrangler.toml) - - To configure telemetry globally on this machine, you can run \`wrangler telemetry disable / enable\`. - You can override this for individual projects with the environment variable \`WRANGLER_SEND_METRICS=true/false\`. - " - `); + await runWrangler(`${cmd} status`); + expect(std.out).toContain("Status: Disabled (set by wrangler.toml)"); }); it("shows environment variable as the source if used", async () => { @@ -464,26 +442,16 @@ describe("metrics", () => { }, }); vi.stubEnv("WRANGLER_SEND_METRICS", "false"); - await runWrangler("telemetry status"); - expect(std.out).toMatchInlineSnapshot(` - "Status: Disabled (set by environment variable) - - To configure telemetry globally on this machine, you can run \`wrangler telemetry disable / enable\`. - You can override this for individual projects with the environment variable \`WRANGLER_SEND_METRICS=true/false\`. - " - `); + await runWrangler(`${cmd} status`); + expect(std.out).toContain( + "Status: Disabled (set by environment variable)" + ); }); it("defaults to enabled if metrics config is not set", async () => { writeMetricsConfig({}); - await runWrangler("telemetry status"); - expect(std.out).toMatchInlineSnapshot(` - "Status: Enabled - - To configure telemetry globally on this machine, you can run \`wrangler telemetry disable / enable\`. - You can override this for individual projects with the environment variable \`WRANGLER_SEND_METRICS=true/false\`. - " - `); + await runWrangler(`${cmd} status`); + expect(std.out).toContain("Status: Enabled"); }); it("prioritises environment variable over send_metrics", async () => { @@ -495,31 +463,24 @@ describe("metrics", () => { }); writeWranglerToml({ send_metrics: true }); vi.stubEnv("WRANGLER_SEND_METRICS", "false"); - await runWrangler("telemetry status"); - expect(std.out).toMatchInlineSnapshot(` - "Status: Disabled (set by environment variable) - - To configure telemetry globally on this machine, you can run \`wrangler telemetry disable / enable\`. - You can override this for individual projects with the environment variable \`WRANGLER_SEND_METRICS=true/false\`. - " - `); + await runWrangler(`${cmd} status`); + expect(std.out).toContain( + "Status: Disabled (set by environment variable)" + ); }); }); - it("disables telemetry when `wrangler telemetry disable` is run", async () => { + it(`disables telemetry when "wrangler ${cmd} disable" is run`, async () => { writeMetricsConfig({ permission: { enabled: true, date: new Date(2022, 6, 4), }, }); - await runWrangler("telemetry disable"); - expect(std.out).toMatchInlineSnapshot(` - "Status: Disabled + await runWrangler(`${cmd} disable`); + expect(std.out).toContain(`Status: Disabled - Wrangler is no longer collecting telemetry about your usage. - " - `); +Wrangler is no longer collecting telemetry about your usage.`); expect(readMetricsConfig()).toMatchObject({ permission: { enabled: false, @@ -528,20 +489,17 @@ describe("metrics", () => { }); }); - it("enables telemetry when `wrangler telemetry enable` is run", async () => { + it(`enables telemetry when "wrangler ${cmd} enable" is run`, async () => { writeMetricsConfig({ permission: { enabled: false, date: new Date(2022, 6, 4), }, }); - await runWrangler("telemetry enable"); - expect(std.out).toMatchInlineSnapshot(` - "Status: Enabled + await runWrangler(`${cmd} enable`); + expect(std.out).toContain(`Status: Enabled - Wrangler is now collecting telemetry about your usage. Thank you for helping make Wrangler better 🧡 - " - `); +Wrangler is now collecting telemetry about your usage. Thank you for helping make Wrangler better 🧡`); expect(readMetricsConfig()).toMatchObject({ permission: { enabled: true, @@ -557,13 +515,10 @@ describe("metrics", () => { date: new Date(2022, 6, 4), }, }); - await runWrangler("telemetry enable"); - expect(std.out).toMatchInlineSnapshot(` - "Status: Enabled + await runWrangler(`${cmd} enable`); + expect(std.out).toContain(`Status: Enabled - Wrangler is now collecting telemetry about your usage. Thank you for helping make Wrangler better 🧡 - " - `); +Wrangler is now collecting telemetry about your usage. Thank you for helping make Wrangler better 🧡`); const config = readMetricsConfig(); expect(config).toMatchObject({ c3permission: { diff --git a/packages/wrangler/src/metrics/commands.ts b/packages/wrangler/src/metrics/commands.ts index 34e1ec4c763f..05bfe0af2ebe 100644 --- a/packages/wrangler/src/metrics/commands.ts +++ b/packages/wrangler/src/metrics/commands.ts @@ -1,5 +1,5 @@ import chalk from "chalk"; -import { defineCommand, defineNamespace } from "../core"; +import { defineAlias, defineCommand, defineNamespace } from "../core"; import { getWranglerSendMetricsFromEnv } from "../environment-variables/misc-variables"; import { logger } from "../logger"; import { readMetricsConfig, updateMetricsPermission } from "./metrics-config"; @@ -14,6 +14,11 @@ defineNamespace({ }, }); +defineAlias({ + command: "wrangler metrics", + aliasOf: "wrangler telemetry", +}); + defineCommand({ command: "wrangler telemetry disable", metadata: { From 10f86fb503d540c6ca21fff03aec3bf2f98b5af6 Mon Sep 17 00:00:00 2001 From: emily-shen <69125074+emily-shen@users.noreply.github.com> Date: Tue, 19 Nov 2024 12:14:40 +0000 Subject: [PATCH 3/5] feat: send metrics for command start/complete/error (#7267) * stop collecting userId in telemetry Co-authored-by: emily-shen * implement telemetry collection * infer errorType based on the constructor name * implement common event properties * log common event properties Co-authored-by: Edmund Hung * respect metric enabled/disabled * remove dispatcher.identify * include SPARROW_SOURCE_KEY in PR pre-release build * fix tests * ensure debug log covers the request failed message * replace SPARROW_SOURCE_KEY regardless whethe env exists --------- Co-authored-by: Edmund Hung Co-authored-by: emily-shen Co-authored-by: Edmund Hung --- .../create-pullrequest-prerelease.yml | 2 + packages/wrangler/package.json | 2 + packages/wrangler/scripts/bundle.ts | 6 +- .../wrangler/src/__tests__/metrics.test.ts | 243 ++++-------------- packages/wrangler/src/config/index.ts | 71 +++-- packages/wrangler/src/dev.ts | 1 - packages/wrangler/src/index.ts | 54 +++- packages/wrangler/src/metrics/helpers.ts | 29 +++ .../wrangler/src/metrics/metrics-config.ts | 64 +---- .../src/metrics/metrics-dispatcher.ts | 110 +++++--- packages/wrangler/src/metrics/send-event.ts | 50 ++++ pnpm-lock.yaml | 6 + 12 files changed, 312 insertions(+), 326 deletions(-) create mode 100644 packages/wrangler/src/metrics/helpers.ts diff --git a/.github/workflows/create-pullrequest-prerelease.yml b/.github/workflows/create-pullrequest-prerelease.yml index acdb65fe2a2c..9d6d378bf7e6 100644 --- a/.github/workflows/create-pullrequest-prerelease.yml +++ b/.github/workflows/create-pullrequest-prerelease.yml @@ -57,6 +57,8 @@ jobs: run: node .github/prereleases/2-build-pack-upload.mjs env: NODE_ENV: "production" + # this is the "test/staging" key for sparrow analytics + SPARROW_SOURCE_KEY: "5adf183f94b3436ba78d67f506965998" ALGOLIA_APP_ID: ${{ secrets.ALGOLIA_APP_ID }} ALGOLIA_PUBLIC_KEY: ${{ secrets.ALGOLIA_PUBLIC_KEY }} SENTRY_DSN: "https://9edbb8417b284aa2bbead9b4c318918b@sentry10.cfdata.org/583" diff --git a/packages/wrangler/package.json b/packages/wrangler/package.json index 9c049ef70b5a..f2b580cd0e91 100644 --- a/packages/wrangler/package.json +++ b/packages/wrangler/package.json @@ -86,6 +86,7 @@ "selfsigned": "^2.0.1", "source-map": "^0.6.1", "unenv": "npm:unenv-nightly@2.0.0-20241111-080453-894aa31", + "which-pm-runs": "^1.1.0", "workerd": "1.20241106.1", "xxhash-wasm": "^1.0.1" }, @@ -115,6 +116,7 @@ "@types/shell-quote": "^1.7.2", "@types/signal-exit": "^3.0.1", "@types/supports-color": "^8.1.1", + "@types/which-pm-runs": "^1.0.0", "@types/ws": "^8.5.7", "@types/yargs": "^17.0.22", "@vitest/ui": "catalog:default", diff --git a/packages/wrangler/scripts/bundle.ts b/packages/wrangler/scripts/bundle.ts index 9fbe71d94700..4e23e3072ab3 100644 --- a/packages/wrangler/scripts/bundle.ts +++ b/packages/wrangler/scripts/bundle.ts @@ -44,9 +44,9 @@ async function buildMain(flags: BuildFlags = {}) { __RELATIVE_PACKAGE_PATH__, "import.meta.url": "import_meta_url", "process.env.NODE_ENV": `'${process.env.NODE_ENV || "production"}'`, - ...(process.env.SPARROW_SOURCE_KEY - ? { SPARROW_SOURCE_KEY: `"${process.env.SPARROW_SOURCE_KEY}"` } - : {}), + "process.env.SPARROW_SOURCE_KEY": JSON.stringify( + process.env.SPARROW_SOURCE_KEY ?? "" + ), ...(process.env.ALGOLIA_APP_ID ? { ALGOLIA_APP_ID: `"${process.env.ALGOLIA_APP_ID}"` } : {}), diff --git a/packages/wrangler/src/__tests__/metrics.test.ts b/packages/wrangler/src/__tests__/metrics.test.ts index eb16fd992b6a..b9c430438a52 100644 --- a/packages/wrangler/src/__tests__/metrics.test.ts +++ b/packages/wrangler/src/__tests__/metrics.test.ts @@ -1,135 +1,56 @@ -import { mkdirSync } from "node:fs"; import { http, HttpResponse } from "msw"; import { vi } from "vitest"; -import { version as wranglerVersion } from "../../package.json"; -import { purgeConfigCaches, saveToConfigCache } from "../config-cache"; import { CI } from "../is-ci"; import { logger } from "../logger"; -import { getMetricsConfig, getMetricsDispatcher } from "../metrics"; +import { getOS, getWranglerVersion } from "../metrics/helpers"; import { + getMetricsConfig, readMetricsConfig, - USER_ID_CACHE_PATH, writeMetricsConfig, } from "../metrics/metrics-config"; -import { writeAuthConfigFile } from "../user"; +import { getMetricsDispatcher } from "../metrics/metrics-dispatcher"; import { mockConsoleMethods } from "./helpers/mock-console"; -import { clearDialogs } from "./helpers/mock-dialogs"; import { useMockIsTTY } from "./helpers/mock-istty"; -import { msw, mswSuccessOauthHandlers } from "./helpers/msw"; +import { msw } from "./helpers/msw"; import { runInTempDir } from "./helpers/run-in-tmp"; import { runWrangler } from "./helpers/run-wrangler"; import { writeWranglerToml } from "./helpers/write-wrangler-toml"; import type { MockInstance } from "vitest"; -declare const global: { SPARROW_SOURCE_KEY: string | undefined }; +vi.mock("../metrics/helpers"); vi.unmock("../metrics/metrics-config"); describe("metrics", () => { - const ORIGINAL_SPARROW_SOURCE_KEY = global.SPARROW_SOURCE_KEY; const std = mockConsoleMethods(); runInTempDir(); beforeEach(async () => { - global.SPARROW_SOURCE_KEY = "MOCK_KEY"; + vi.stubEnv("SPARROW_SOURCE_KEY", "MOCK_KEY"); logger.loggerLevel = "debug"; - // Create a node_modules directory to store config-cache files - mkdirSync("node_modules"); }); + afterEach(() => { - global.SPARROW_SOURCE_KEY = ORIGINAL_SPARROW_SOURCE_KEY; - purgeConfigCaches(); - clearDialogs(); + vi.unstubAllEnvs(); }); describe("getMetricsDispatcher()", () => { - const MOCK_DISPATCHER_OPTIONS = { - // By setting this to true we avoid the `getMetricsConfig()` logic in these tests. - sendMetrics: true, - offline: false, - }; - - // These tests should never hit the `/user` API endpoint. - const userRequests = mockUserRequest(); - afterEach(() => { - expect(userRequests.count).toBe(0); - }); - - describe("identify()", () => { - it("should send a request to the default URL", async () => { - const request = mockMetricRequest( - { - event: "identify", - properties: { - category: "Workers", - wranglerVersion, - os: process.platform + ":" + process.arch, - a: 1, - b: 2, - }, - }, - { "Sparrow-Source-Key": "MOCK_KEY" }, - "identify" - ); - const dispatcher = await getMetricsDispatcher(MOCK_DISPATCHER_OPTIONS); - await dispatcher.identify({ a: 1, b: 2 }); - - expect(request.count).toBe(1); - expect(std.debug).toMatchInlineSnapshot( - `"Metrics dispatcher: Posting data {\\"type\\":\\"identify\\",\\"name\\":\\"identify\\",\\"properties\\":{\\"a\\":1,\\"b\\":2}}"` - ); - expect(std.out).toMatchInlineSnapshot(`""`); - expect(std.warn).toMatchInlineSnapshot(`""`); - expect(std.err).toMatchInlineSnapshot(`""`); - }); - - it("should write a debug log if the dispatcher is disabled", async () => { - const requests = mockMetricRequest({}, {}, "identify"); - const dispatcher = await getMetricsDispatcher({ - ...MOCK_DISPATCHER_OPTIONS, - sendMetrics: false, - }); - await dispatcher.identify({ a: 1, b: 2 }); - await flushPromises(); - - expect(requests.count).toBe(0); - expect(std.debug).toMatchInlineSnapshot( - `"Metrics dispatcher: Dispatching disabled - would have sent {\\"type\\":\\"identify\\",\\"name\\":\\"identify\\",\\"properties\\":{\\"a\\":1,\\"b\\":2}}."` - ); - expect(std.out).toMatchInlineSnapshot(`""`); - expect(std.warn).toMatchInlineSnapshot(`""`); - expect(std.err).toMatchInlineSnapshot(`""`); + beforeEach(() => { + vi.mocked(getOS).mockReturnValue("foo:bar"); + vi.mocked(getWranglerVersion).mockReturnValue("1.2.3"); + vi.useFakeTimers({ + now: new Date(2024, 11, 12), }); - - it("should write a debug log if the request fails", async () => { - msw.use( - http.post("*/identify", async () => { - return HttpResponse.error(); - }) - ); - - const dispatcher = await getMetricsDispatcher(MOCK_DISPATCHER_OPTIONS); - await dispatcher.identify({ a: 1, b: 2 }); - await flushPromises(); - expect(std.debug).toMatchInlineSnapshot(` - "Metrics dispatcher: Posting data {\\"type\\":\\"identify\\",\\"name\\":\\"identify\\",\\"properties\\":{\\"a\\":1,\\"b\\":2}} - Metrics dispatcher: Failed to send request: Failed to fetch" - `); - expect(std.out).toMatchInlineSnapshot(`""`); - expect(std.warn).toMatchInlineSnapshot(`""`); - expect(std.err).toMatchInlineSnapshot(`""`); + writeMetricsConfig({ + permission: { + enabled: true, + date: new Date(2024, 11, 11), + }, + deviceId: "f82b1f46-eb7b-4154-aa9f-ce95f23b2288", }); + }); - it("should write a warning log if no source key has been provided", async () => { - global.SPARROW_SOURCE_KEY = undefined; - const dispatcher = await getMetricsDispatcher(MOCK_DISPATCHER_OPTIONS); - await dispatcher.identify({ a: 1, b: 2 }); - expect(std.debug).toMatchInlineSnapshot( - `"Metrics dispatcher: Source Key not provided. Be sure to initialize before sending events. { type: 'identify', name: 'identify', properties: { a: 1, b: 2 } }"` - ); - expect(std.out).toMatchInlineSnapshot(`""`); - expect(std.warn).toMatchInlineSnapshot(`""`); - expect(std.err).toMatchInlineSnapshot(`""`); - }); + afterEach(() => { + vi.useRealTimers(); }); describe("sendEvent()", () => { @@ -139,8 +60,8 @@ describe("metrics", () => { event: "some-event", properties: { category: "Workers", - wranglerVersion, - os: process.platform + ":" + process.arch, + wranglerVersion: "1.2.3", + os: "foo:bar", a: 1, b: 2, }, @@ -150,12 +71,14 @@ describe("metrics", () => { }, "event" ); - const dispatcher = await getMetricsDispatcher(MOCK_DISPATCHER_OPTIONS); + const dispatcher = await getMetricsDispatcher({ + sendMetrics: true, + }); await dispatcher.sendEvent("some-event", { a: 1, b: 2 }); expect(requests.count).toBe(1); expect(std.debug).toMatchInlineSnapshot( - `"Metrics dispatcher: Posting data {\\"type\\":\\"event\\",\\"name\\":\\"some-event\\",\\"properties\\":{\\"a\\":1,\\"b\\":2}}"` + `"Metrics dispatcher: Posting data {\\"deviceId\\":\\"f82b1f46-eb7b-4154-aa9f-ce95f23b2288\\",\\"event\\":\\"some-event\\",\\"timestamp\\":1733961600000,\\"properties\\":{\\"category\\":\\"Workers\\",\\"wranglerVersion\\":\\"1.2.3\\",\\"os\\":\\"foo:bar\\",\\"a\\":1,\\"b\\":2}}"` ); expect(std.out).toMatchInlineSnapshot(`""`); expect(std.warn).toMatchInlineSnapshot(`""`); @@ -164,17 +87,14 @@ describe("metrics", () => { it("should write a debug log if the dispatcher is disabled", async () => { const requests = mockMetricRequest({}, {}, "event"); - const dispatcher = await getMetricsDispatcher({ - ...MOCK_DISPATCHER_OPTIONS, sendMetrics: false, }); await dispatcher.sendEvent("some-event", { a: 1, b: 2 }); - await flushPromises(); expect(requests.count).toBe(0); expect(std.debug).toMatchInlineSnapshot( - `"Metrics dispatcher: Dispatching disabled - would have sent {\\"type\\":\\"event\\",\\"name\\":\\"some-event\\",\\"properties\\":{\\"a\\":1,\\"b\\":2}}."` + `"Metrics dispatcher: Dispatching disabled - would have sent {\\"deviceId\\":\\"f82b1f46-eb7b-4154-aa9f-ce95f23b2288\\",\\"event\\":\\"some-event\\",\\"timestamp\\":1733961600000,\\"properties\\":{\\"category\\":\\"Workers\\",\\"wranglerVersion\\":\\"1.2.3\\",\\"os\\":\\"foo:bar\\",\\"a\\":1,\\"b\\":2}}."` ); expect(std.out).toMatchInlineSnapshot(`""`); expect(std.warn).toMatchInlineSnapshot(`""`); @@ -187,27 +107,35 @@ describe("metrics", () => { return HttpResponse.error(); }) ); - const dispatcher = await getMetricsDispatcher(MOCK_DISPATCHER_OPTIONS); + const dispatcher = await getMetricsDispatcher({ + sendMetrics: true, + }); await dispatcher.sendEvent("some-event", { a: 1, b: 2 }); await flushPromises(); - expect(std.debug).toMatchInlineSnapshot(` - "Metrics dispatcher: Posting data {\\"type\\":\\"event\\",\\"name\\":\\"some-event\\",\\"properties\\":{\\"a\\":1,\\"b\\":2}} + + expect(std.debug).toMatchInlineSnapshot( + ` + "Metrics dispatcher: Posting data {\\"deviceId\\":\\"f82b1f46-eb7b-4154-aa9f-ce95f23b2288\\",\\"event\\":\\"some-event\\",\\"timestamp\\":1733961600000,\\"properties\\":{\\"category\\":\\"Workers\\",\\"wranglerVersion\\":\\"1.2.3\\",\\"os\\":\\"foo:bar\\",\\"a\\":1,\\"b\\":2}} Metrics dispatcher: Failed to send request: Failed to fetch" - `); + ` + ); expect(std.out).toMatchInlineSnapshot(`""`); expect(std.warn).toMatchInlineSnapshot(`""`); expect(std.err).toMatchInlineSnapshot(`""`); }); it("should write a warning log if no source key has been provided", async () => { - global.SPARROW_SOURCE_KEY = undefined; + vi.stubEnv("SPARROW_SOURCE_KEY", undefined); + const requests = mockMetricRequest({}, {}, "event"); - const dispatcher = await getMetricsDispatcher(MOCK_DISPATCHER_OPTIONS); + const dispatcher = await getMetricsDispatcher({ + sendMetrics: true, + }); await dispatcher.sendEvent("some-event", { a: 1, b: 2 }); expect(requests.count).toBe(0); expect(std.debug).toMatchInlineSnapshot( - `"Metrics dispatcher: Source Key not provided. Be sure to initialize before sending events. { type: 'event', name: 'some-event', properties: { a: 1, b: 2 } }"` + `"Metrics dispatcher: Source Key not provided. Be sure to initialize before sending events {\\"deviceId\\":\\"f82b1f46-eb7b-4154-aa9f-ce95f23b2288\\",\\"event\\":\\"some-event\\",\\"timestamp\\":1733961600000,\\"properties\\":{\\"category\\":\\"Workers\\",\\"wranglerVersion\\":\\"1.2.3\\",\\"os\\":\\"foo:bar\\",\\"a\\":1,\\"b\\":2}}"` ); expect(std.out).toMatchInlineSnapshot(`""`); expect(std.warn).toMatchInlineSnapshot(`""`); @@ -246,14 +174,10 @@ describe("metrics", () => { }); it("should return the sendMetrics argument for enabled if it is defined", async () => { - expect( - await getMetricsConfig({ sendMetrics: false, offline: false }) - ).toMatchObject({ + expect(await getMetricsConfig({ sendMetrics: false })).toMatchObject({ enabled: false, }); - expect( - await getMetricsConfig({ sendMetrics: true, offline: false }) - ).toMatchObject({ + expect(await getMetricsConfig({ sendMetrics: true })).toMatchObject({ enabled: true, }); }); @@ -263,7 +187,6 @@ describe("metrics", () => { expect( await getMetricsConfig({ sendMetrics: undefined, - offline: false, }) ).toMatchObject({ enabled: false, @@ -280,7 +203,6 @@ describe("metrics", () => { expect( await getMetricsConfig({ sendMetrics: undefined, - offline: false, }) ).toMatchObject({ enabled: true, @@ -297,7 +219,6 @@ describe("metrics", () => { expect( await getMetricsConfig({ sendMetrics: undefined, - offline: false, }) ).toMatchObject({ enabled: false, @@ -314,7 +235,6 @@ describe("metrics", () => { expect( await getMetricsConfig({ sendMetrics: undefined, - offline: false, }) ).toMatchObject({ enabled: true, @@ -336,7 +256,6 @@ describe("metrics", () => { writeMetricsConfig({ deviceId: "XXXX-YYYY-ZZZZ" }); const { deviceId } = await getMetricsConfig({ sendMetrics: true, - offline: false, }); expect(deviceId).toEqual("XXXX-YYYY-ZZZZ"); expect(readMetricsConfig().deviceId).toEqual(deviceId); @@ -346,7 +265,6 @@ describe("metrics", () => { writeMetricsConfig({}); const { deviceId } = await getMetricsConfig({ sendMetrics: true, - offline: false, }); expect(deviceId).toMatch( /[0-9a-fA-F]{8}-([0-9a-fA-F]{4}-){3}[0-9a-fA-F]{12}/ @@ -354,43 +272,6 @@ describe("metrics", () => { expect(readMetricsConfig().deviceId).toEqual(deviceId); }); }); - - describe("userId", () => { - const userRequests = mockUserRequest(); - it("should return a userId found in a cache file", async () => { - saveToConfigCache(USER_ID_CACHE_PATH, { - userId: "CACHED_USER_ID", - }); - const { userId } = await getMetricsConfig({ - sendMetrics: true, - offline: false, - }); - expect(userId).toEqual("CACHED_USER_ID"); - expect(userRequests.count).toBe(0); - }); - - it("should fetch the userId from Cloudflare and store it in a cache file", async () => { - writeAuthConfigFile({ oauth_token: "DUMMY_TOKEN" }); - const { userId } = await getMetricsConfig({ - sendMetrics: true, - offline: false, - }); - await flushPromises(); - - expect(userId).toEqual("MOCK_USER_ID"); - expect(userRequests.count).toBe(1); - }); - - it("should not fetch the userId from Cloudflare if running in `offline` mode", async () => { - writeAuthConfigFile({ oauth_token: "DUMMY_TOKEN" }); - const { userId } = await getMetricsConfig({ - sendMetrics: true, - offline: true, - }); - expect(userId).toBe(undefined); - expect(userRequests.count).toBe(0); - }); - }); }); describe.each(["metrics", "telemetry"])("%s commands", (cmd) => { @@ -534,31 +415,6 @@ Wrangler is now collecting telemetry about your usage. Thank you for helping mak }); }); -function mockUserRequest() { - const requests = { count: 0 }; - beforeEach(() => { - msw.use( - ...mswSuccessOauthHandlers, - http.get("*/user", () => { - requests.count++; - return HttpResponse.json( - { - success: true, - errors: [], - messages: [], - result: { id: "MOCK_USER_ID" }, - }, - { status: 200 } - ); - }) - ); - }); - afterEach(() => { - requests.count = 0; - }); - return requests; -} - function mockMetricRequest( body: unknown, header: unknown, @@ -582,6 +438,9 @@ function mockMetricRequest( } // Forces a tick to allow the non-awaited fetch promise to resolve. -function flushPromises(): Promise { - return new Promise((resolve) => setTimeout(resolve, 0)); +async function flushPromises(): Promise { + await Promise.all([ + new Promise((resolve) => setTimeout(resolve, 0)), + vi.advanceTimersToNextTimerAsync(), + ]); } diff --git a/packages/wrangler/src/config/index.ts b/packages/wrangler/src/config/index.ts index b1973e748b63..296f5fd15cd8 100644 --- a/packages/wrangler/src/config/index.ts +++ b/packages/wrangler/src/config/index.ts @@ -55,35 +55,13 @@ export function readConfig( requirePagesConfig?: boolean, hideWarnings: boolean = false ): Config { - const isJsonConfigEnabled = - getFlag("JSON_CONFIG_FILE") ?? args.experimentalJsonConfig; - let rawConfig: RawConfig = {}; - - if (!configPath) { - configPath = findWranglerToml(process.cwd(), isJsonConfigEnabled); - } + const { + rawConfig, + configPath: updateConfigPath, + isJsonConfigEnabled, + } = parseConfig(configPath, args, requirePagesConfig); - try { - // Load the configuration from disk if available - if (configPath?.endsWith("toml")) { - rawConfig = parseTOML(readFileSync(configPath), configPath); - } else if (configPath?.endsWith("json") || configPath?.endsWith("jsonc")) { - rawConfig = parseJSONC(readFileSync(configPath), configPath); - } - } catch (e) { - // Swallow parsing errors if we require a pages config file. - // At this point, we can't tell if the user intended to provide a Pages config file (and so should see the parsing error) or not (and so shouldn't). - // We err on the side of swallowing the error so as to not break existing projects - if (requirePagesConfig) { - logger.error(e); - throw new FatalError( - "Your wrangler.toml is not a valid Pages config file", - EXIT_CODE_INVALID_PAGES_CONFIG - ); - } else { - throw e; - } - } + configPath = updateConfigPath; /** * Check if configuration file belongs to a Pages project. @@ -154,6 +132,43 @@ export function readConfig( return config; } +export const parseConfig = ( + configPath: string | undefined, + // Include command specific args as well as the wrangler global flags + args: ReadConfigCommandArgs, + requirePagesConfig?: boolean +) => { + const isJsonConfigEnabled = + getFlag("JSON_CONFIG_FILE") ?? args.experimentalJsonConfig; + let rawConfig: RawConfig = {}; + + if (!configPath) { + configPath = findWranglerToml(process.cwd(), isJsonConfigEnabled); + } + + try { + // Load the configuration from disk if available + if (configPath?.endsWith("toml")) { + rawConfig = parseTOML(readFileSync(configPath), configPath); + } else if (configPath?.endsWith("json") || configPath?.endsWith("jsonc")) { + rawConfig = parseJSONC(readFileSync(configPath), configPath); + } + } catch (e) { + // Swallow parsing errors if we require a pages config file. + // At this point, we can't tell if the user intended to provide a Pages config file (and so should see the parsing error) or not (and so shouldn't). + // We err on the side of swallowing the error so as to not break existing projects + if (requirePagesConfig) { + logger.error(e); + throw new FatalError( + "Your wrangler.toml is not a valid Pages config file", + EXIT_CODE_INVALID_PAGES_CONFIG + ); + } else { + throw e; + } + } + return { rawConfig, configPath, isJsonConfigEnabled }; +}; /** * Modifies the provided config to support python workers, if the entrypoint is a .py file */ diff --git a/packages/wrangler/src/dev.ts b/packages/wrangler/src/dev.ts index aa516bd8e863..f251ef4e5bf2 100644 --- a/packages/wrangler/src/dev.ts +++ b/packages/wrangler/src/dev.ts @@ -752,7 +752,6 @@ export async function startDev(args: StartDevOptions) { }, { sendMetrics: devEnv.config.latestConfig?.sendMetrics, - offline: !args.remote, } ); diff --git a/packages/wrangler/src/index.ts b/packages/wrangler/src/index.ts index dd2c290571d2..af7c161e8d83 100644 --- a/packages/wrangler/src/index.ts +++ b/packages/wrangler/src/index.ts @@ -7,7 +7,7 @@ import makeCLI from "yargs"; import { version as wranglerVersion } from "../package.json"; import { ai } from "./ai"; import { cloudchamber } from "./cloudchamber"; -import { loadDotEnv } from "./config"; +import { loadDotEnv, parseConfig } from "./config"; import { createCommandRegister } from "./core/register-commands"; import { d1 } from "./d1"; import { deleteHandler, deleteOptions } from "./delete"; @@ -52,6 +52,7 @@ import "./user/commands"; import "./metrics/commands"; import { demandSingleValue } from "./core"; import { logBuildFailure, logger, LOGGER_LEVELS } from "./logger"; +import { getMetricsDispatcher } from "./metrics"; import { mTlsCertificateCommands } from "./mtls-certificate/cli"; import { writeOutput } from "./output"; import { pages } from "./pages"; @@ -671,8 +672,11 @@ export function createCLIParser(argv: string[]) { export async function main(argv: string[]): Promise { setupSentry(); + const startTime = Date.now(); const wrangler = createCLIParser(argv); - + let command: string | undefined; + let metricsArgs: Record | undefined; + let dispatcher: ReturnType | undefined; // Register Yargs middleware to record command as Sentry breadcrumb let recordedCommand = false; const wranglerWithMiddleware = wrangler.middleware((args) => { @@ -683,15 +687,43 @@ export async function main(argv: string[]): Promise { recordedCommand = true; // `args._` doesn't include any positional arguments (e.g. script name, // key to fetch) or flags - addBreadcrumb(`wrangler ${args._.join(" ")}`); + + try { + const { rawConfig } = parseConfig(args.config, args, false); + dispatcher = getMetricsDispatcher({ + sendMetrics: rawConfig.send_metrics, + }); + } catch (e) { + // If we can't parse the config, we can't send metrics + logger.debug("Failed to parse config. Disabling metrics dispatcher.", e); + } + + command = `wrangler ${args._.join(" ")}`; + metricsArgs = args; + addBreadcrumb(command); + void dispatcher?.sendNewEvent("wrangler command started", { + command, + args, + }); }, /* applyBeforeValidation */ true); let cliHandlerThrew = false; try { await wranglerWithMiddleware.parse(); + + const durationMs = Date.now() - startTime; + + void dispatcher?.sendNewEvent("wrangler command completed", { + command, + args: metricsArgs, + durationMs, + durationSeconds: durationMs / 1000, + durationMinutes: durationMs / 1000 / 60, + }); } catch (e) { cliHandlerThrew = true; let mayReport = true; + let errorType: string | undefined; logger.log(""); // Just adds a bit of space if (e instanceof CommandLineArgsError) { @@ -702,6 +734,7 @@ export async function main(argv: string[]): Promise { await createCLIParser([...argv, "--help"]).parse(); } else if (isAuthenticationError(e)) { mayReport = false; + errorType = "AuthenticationError"; logger.log(formatMessage(e)); const envAuth = getAuthFromEnv(); if (envAuth !== undefined && "apiToken" in envAuth) { @@ -748,9 +781,12 @@ export async function main(argv: string[]): Promise { ); } else if (isBuildFailure(e)) { mayReport = false; + errorType = "BuildFailure"; + logBuildFailure(e.errors, e.warnings); } else if (isBuildFailureFromCause(e)) { mayReport = false; + errorType = "BuildFailure"; logBuildFailure(e.cause.errors, e.cause.warnings); } else { let loggableException = e; @@ -791,6 +827,18 @@ export async function main(argv: string[]): Promise { await captureGlobalException(e); } + const durationMs = Date.now() - startTime; + + void dispatcher?.sendNewEvent("wrangler command errored", { + command, + args: metricsArgs, + durationMs, + durationSeconds: durationMs / 1000, + durationMinutes: durationMs / 1000 / 60, + errorType: + errorType ?? (e instanceof Error ? e.constructor.name : undefined), + }); + throw e; } finally { try { diff --git a/packages/wrangler/src/metrics/helpers.ts b/packages/wrangler/src/metrics/helpers.ts new file mode 100644 index 000000000000..173d0363ab2a --- /dev/null +++ b/packages/wrangler/src/metrics/helpers.ts @@ -0,0 +1,29 @@ +import whichPmRuns from "which-pm-runs"; +import { version as wranglerVersion } from "../../package.json"; + +export function getWranglerVersion() { + return wranglerVersion; +} + +export function getPackageManager() { + return whichPmRuns()?.name; +} + +export function getPlatform() { + const platform = process.platform; + + switch (platform) { + case "win32": + return "Windows"; + case "darwin": + return "Mac OS"; + case "linux": + return "Linux"; + default: + return `Others: ${platform}`; + } +} + +export function getOS() { + return process.platform + ":" + process.arch; +} diff --git a/packages/wrangler/src/metrics/metrics-config.ts b/packages/wrangler/src/metrics/metrics-config.ts index 235db05fa222..8dd6d496c001 100644 --- a/packages/wrangler/src/metrics/metrics-config.ts +++ b/packages/wrangler/src/metrics/metrics-config.ts @@ -1,13 +1,10 @@ import { randomUUID } from "node:crypto"; import { mkdirSync, readFileSync, writeFileSync } from "node:fs"; import path from "node:path"; -import { fetchResult } from "../cfetch"; -import { getConfigCache, saveToConfigCache } from "../config-cache"; import { getWranglerSendMetricsFromEnv } from "../environment-variables/misc-variables"; import { getGlobalWranglerConfigPath } from "../global-wrangler-config-path"; import { isNonInteractiveOrCI } from "../is-interactive"; import { logger } from "../logger"; -import { getAPIToken } from "../user"; /** * The date that the metrics being gathered was last updated in a way that would require @@ -19,7 +16,6 @@ import { getAPIToken } from "../user"; * gathering. */ export const CURRENT_METRICS_DATE = new Date(2022, 6, 4); -export const USER_ID_CACHE_PATH = "user-id.json"; export interface MetricsConfigOptions { /** @@ -28,10 +24,6 @@ export interface MetricsConfigOptions { * Otherwise, infer the enabled value from the user configuration. */ sendMetrics?: boolean; - /** - * When true, do not make any CF API requests. - */ - offline?: boolean; } /** @@ -42,8 +34,6 @@ export interface MetricsConfig { enabled: boolean; /** A UID that identifies this user and machine pair for Wrangler. */ deviceId: string; - /** The currently logged in user - undefined if not logged in. */ - userId: string | undefined; } /** @@ -62,13 +52,11 @@ export interface MetricsConfig { * If the current process is not interactive then we will cannot prompt the user and just assume * we cannot send metrics if there is no cached or project-level preference available. */ -export async function getMetricsConfig({ +export function getMetricsConfig({ sendMetrics, - offline = false, -}: MetricsConfigOptions): Promise { +}: MetricsConfigOptions): MetricsConfig { const config = readMetricsConfig(); const deviceId = getDeviceId(config); - const userId = await getUserId(offline); // If the WRANGLER_SEND_METRICS environment variable has been set use that // and ignore everything else. @@ -77,21 +65,20 @@ export async function getMetricsConfig({ return { enabled: sendMetricsEnv.toLowerCase() === "true", deviceId, - userId, }; } // If the project is explicitly set the `send_metrics` options in `wrangler.toml` // then use that and ignore any user preference. if (sendMetrics !== undefined) { - return { enabled: sendMetrics, deviceId, userId }; + return { enabled: sendMetrics, deviceId }; } // Get the user preference from the metrics config. const permission = config.permission; if (permission !== undefined) { if (new Date(permission.date) >= CURRENT_METRICS_DATE) { - return { enabled: permission.enabled, deviceId, userId }; + return { enabled: permission.enabled, deviceId }; } else if (permission.enabled) { logger.log( "Usage metrics tracking has changed since you last granted permission." @@ -102,7 +89,7 @@ export async function getMetricsConfig({ // We couldn't get the metrics permission from the project-level nor the user-level config. // If we are not interactive or in a CI build then just bail out. if (isNonInteractiveOrCI()) { - return { enabled: false, deviceId, userId }; + return { enabled: false, deviceId }; } // Otherwise, default to true @@ -114,7 +101,7 @@ export async function getMetricsConfig({ }, deviceId, }); - return { enabled: true, deviceId, userId }; + return { enabled: true, deviceId }; } /** @@ -196,42 +183,3 @@ function getDeviceId(config: MetricsConfigFile) { } return deviceId; } - -/** - * Returns the ID of the current user, which will be sent with each event. - * - * The ID is retrieved from the CF API `/user` endpoint if the user is authenticated and then - * stored in the `node_modules/.cache`. - * - * If it is not possible to retrieve the ID (perhaps the user is not logged in) then we just use - * `undefined`. - */ -async function getUserId(offline: boolean) { - // Get the userId from the cache. - // If it has not been found in the cache and we are not offline then make an API call to get it. - // If we can't work in out then just use `anonymous`. - let userId = getConfigCache<{ userId: string }>(USER_ID_CACHE_PATH).userId; - if (userId === undefined && !offline) { - userId = await fetchUserId(); - if (userId !== undefined) { - saveToConfigCache(USER_ID_CACHE_PATH, { userId }); - } - } - return userId; -} - -/** - * Ask the Cloudflare API for the User ID of the current user. - * - * We will only do this if we are not "offline", e.g. not running `wrangler dev --local`. - * Quietly return undefined if anything goes wrong. - */ -async function fetchUserId(): Promise { - try { - return getAPIToken() - ? (await fetchResult<{ id: string }>("/user")).id - : undefined; - } catch (e) { - return undefined; - } -} diff --git a/packages/wrangler/src/metrics/metrics-dispatcher.ts b/packages/wrangler/src/metrics/metrics-dispatcher.ts index 8ba5de4f82ee..8a6bf0081447 100644 --- a/packages/wrangler/src/metrics/metrics-dispatcher.ts +++ b/packages/wrangler/src/metrics/metrics-dispatcher.ts @@ -1,15 +1,28 @@ import { fetch } from "undici"; -import { version as wranglerVersion } from "../../package.json"; import { logger } from "../logger"; -import { getMetricsConfig } from "./metrics-config"; +import { + getOS, + getPackageManager, + getPlatform, + getWranglerVersion, +} from "./helpers"; +import { getMetricsConfig, readMetricsConfig } from "./metrics-config"; import type { MetricsConfigOptions } from "./metrics-config"; +import type { CommonEventProperties, Events } from "./send-event"; -// The SPARROW_SOURCE_KEY is provided at esbuild time as a `define` for production and beta -// releases. Otherwise it is left undefined, which automatically disables metrics requests. -declare const SPARROW_SOURCE_KEY: string; const SPARROW_URL = "https://sparrow.cloudflare.com"; -export async function getMetricsDispatcher(options: MetricsConfigOptions) { +export function getMetricsDispatcher(options: MetricsConfigOptions) { + // The SPARROW_SOURCE_KEY will be provided at build time through esbuild's `define` option + // No events will be sent if the env `SPARROW_SOURCE_KEY` is not provided and the value will be set to an empty string instead. + const SPARROW_SOURCE_KEY = process.env.SPARROW_SOURCE_KEY ?? ""; + const wranglerVersion = getWranglerVersion(); + const platform = getPlatform(); + const packageManager = getPackageManager(); + const isFirstUsage = readMetricsConfig().permission === undefined; + const amplitude_session_id = Date.now(); + let amplitude_event_id = 0; + return { /** * Dispatch a event to the analytics target. @@ -19,62 +32,77 @@ export async function getMetricsDispatcher(options: MetricsConfigOptions) { * - additional properties are camelCased */ async sendEvent(name: string, properties: Properties = {}): Promise { - await dispatch({ type: "event", name, properties }); + await dispatch({ + name, + properties: { + category: "Workers", + wranglerVersion, + os: getOS(), + ...properties, + }, + }); }, - /** - * Dispatch a user profile information to the analytics target. - * - * This call can be used to inform the analytics target of relevant properties associated - * with the current user. - */ - async identify(properties: Properties): Promise { - await dispatch({ type: "identify", name: "identify", properties }); + async sendNewEvent( + name: EventName, + properties: Omit< + Extract["properties"], + keyof CommonEventProperties + > + ): Promise { + const commonEventProperties: CommonEventProperties = { + amplitude_session_id, + amplitude_event_id: amplitude_event_id++, + wranglerVersion, + platform, + packageManager, + isFirstUsage, + }; + + await dispatch({ + name, + properties: { + ...commonEventProperties, + properties, + }, + }); }, }; async function dispatch(event: { - type: "identify" | "event"; name: string; properties: Properties; }): Promise { - if (!SPARROW_SOURCE_KEY) { + const metricsConfig = getMetricsConfig(options); + const body = { + deviceId: metricsConfig.deviceId, + event: event.name, + timestamp: Date.now(), + properties: event.properties, + }; + + if (!metricsConfig.enabled) { logger.debug( - "Metrics dispatcher: Source Key not provided. Be sure to initialize before sending events.", - event + `Metrics dispatcher: Dispatching disabled - would have sent ${JSON.stringify( + body + )}.` ); return; } - // Lazily get the config for this dispatcher only when an event is being dispatched. - // We must await this since it might trigger user interaction that would break other UI - // in Wrangler if it was allowed to run in parallel. - const metricsConfig = await getMetricsConfig(options); - if (!metricsConfig.enabled) { + if (!SPARROW_SOURCE_KEY) { logger.debug( - `Metrics dispatcher: Dispatching disabled - would have sent ${JSON.stringify( - event - )}.` + "Metrics dispatcher: Source Key not provided. Be sure to initialize before sending events", + JSON.stringify(body) ); return; } - logger.debug(`Metrics dispatcher: Posting data ${JSON.stringify(event)}`); - const body = JSON.stringify({ - deviceId: metricsConfig.deviceId, - userId: metricsConfig.userId, - event: event.name, - properties: { - category: "Workers", - wranglerVersion, - os: process.platform + ":" + process.arch, - ...event.properties, - }, - }); + logger.debug(`Metrics dispatcher: Posting data ${JSON.stringify(body)}`); // Do not await this fetch call. // Just fire-and-forget, otherwise we might slow down the rest of Wrangler. - fetch(`${SPARROW_URL}/api/v1/${event.type}`, { + fetch(`${SPARROW_URL}/api/v1/event`, { method: "POST", headers: { Accept: "*/*", @@ -83,7 +111,7 @@ export async function getMetricsDispatcher(options: MetricsConfigOptions) { }, mode: "cors", keepalive: true, - body, + body: JSON.stringify(body), }).catch((e) => { logger.debug( "Metrics dispatcher: Failed to send request:", diff --git a/packages/wrangler/src/metrics/send-event.ts b/packages/wrangler/src/metrics/send-event.ts index bdde7d86eea4..2e224edd5f9e 100644 --- a/packages/wrangler/src/metrics/send-event.ts +++ b/packages/wrangler/src/metrics/send-event.ts @@ -120,3 +120,53 @@ export async function sendMetricsEvent( logger.debug("Error sending metrics event", err); } } + +export type CommonEventProperties = { + /** The version of the wrangler client that is sending the event. */ + wranglerVersion: string; + /** + * The platform that the wrangler client is running on. + */ + platform: string; + /** + * The package manager that the wrangler client is using. + */ + packageManager: string | undefined; + /** + * Whether this is the first time the user has used the wrangler client. + */ + isFirstUsage: boolean; + + amplitude_session_id: number; + amplitude_event_id: number; +}; + +export type Events = + | { + name: "wrangler command started"; + properties: CommonEventProperties & { + command: string; + args: Record; + }; + } + | { + name: "wrangler command completed"; + properties: CommonEventProperties & { + command: string | undefined; + args: Record | undefined; + durationMs: number; + durationMinutes: number; + durationSeconds: number; + }; + } + | { + name: "wrangler command errored"; + properties: CommonEventProperties & { + command: string | undefined; + args: Record | undefined; + durationMs: number; + durationMinutes: number; + durationSeconds: number; + errorType: string | undefined; + }; + }; diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index f63655fae88c..b6a17680edb6 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -1714,6 +1714,9 @@ importers: unenv: specifier: npm:unenv-nightly@2.0.0-20241111-080453-894aa31 version: unenv-nightly@2.0.0-20241111-080453-894aa31 + which-pm-runs: + specifier: ^1.1.0 + version: 1.1.0 workerd: specifier: 1.20241106.1 version: 1.20241106.1 @@ -1800,6 +1803,9 @@ importers: '@types/supports-color': specifier: ^8.1.1 version: 8.1.1 + '@types/which-pm-runs': + specifier: ^1.0.0 + version: 1.0.0 '@types/ws': specifier: ^8.5.7 version: 8.5.10 From 4799bba69bdff28c0dd74b7b660567af32e677f5 Mon Sep 17 00:00:00 2001 From: emily-shen <69125074+emily-shen@users.noreply.github.com> Date: Wed, 20 Nov 2024 13:29:24 +0000 Subject: [PATCH 4/5] fix nested properties (#7300) --- packages/wrangler/src/metrics/metrics-dispatcher.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/wrangler/src/metrics/metrics-dispatcher.ts b/packages/wrangler/src/metrics/metrics-dispatcher.ts index 8a6bf0081447..9a66966c12a9 100644 --- a/packages/wrangler/src/metrics/metrics-dispatcher.ts +++ b/packages/wrangler/src/metrics/metrics-dispatcher.ts @@ -63,7 +63,7 @@ export function getMetricsDispatcher(options: MetricsConfigOptions) { name, properties: { ...commonEventProperties, - properties, + ...properties, }, }); }, From ee37f8cf0a5ecc6051b011e5d888c4b3ba7187c0 Mon Sep 17 00:00:00 2001 From: Edmund Hung Date: Wed, 20 Nov 2024 21:46:59 +0000 Subject: [PATCH 5/5] wait for all events sent with timeout --- .../wrangler/src/__tests__/metrics.test.ts | 37 ++++++++++++++----- packages/wrangler/src/index.ts | 5 +++ .../src/metrics/metrics-dispatcher.ts | 23 ++++++++---- 3 files changed, 48 insertions(+), 17 deletions(-) diff --git a/packages/wrangler/src/__tests__/metrics.test.ts b/packages/wrangler/src/__tests__/metrics.test.ts index b9c430438a52..279fc431c580 100644 --- a/packages/wrangler/src/__tests__/metrics.test.ts +++ b/packages/wrangler/src/__tests__/metrics.test.ts @@ -142,6 +142,27 @@ describe("metrics", () => { expect(std.err).toMatchInlineSnapshot(`""`); }); }); + + it("should keep track of all requests made", async () => { + const requests = mockMetricRequest({}, {}, "event"); + const dispatcher = await getMetricsDispatcher({ + sendMetrics: true, + }); + + void dispatcher.sendEvent("some-event", { a: 1, b: 2 }); + expect(dispatcher.requests.length).toBe(1); + + expect(requests.count).toBe(0); + await Promise.allSettled(dispatcher.requests); + expect(requests.count).toBe(1); + + void dispatcher.sendEvent("another-event", { c: 3, d: 4 }); + expect(dispatcher.requests.length).toBe(2); + + expect(requests.count).toBe(1); + await Promise.allSettled(dispatcher.requests); + expect(requests.count).toBe(2); + }); }); describe("getMetricsConfig()", () => { @@ -422,16 +443,12 @@ function mockMetricRequest( ) { const requests = { count: 0 }; msw.use( - http.post( - `*/${endpoint}`, - async ({ request }) => { - requests.count++; - expect(await request.json()).toEqual(body); - expect(request.headers).toContain(header); - return HttpResponse.json({}, { status: 200 }); - }, - { once: true } - ) + http.post(`*/${endpoint}`, async ({ request }) => { + requests.count++; + expect(await request.json()).toEqual(body); + expect(request.headers).toContain(header); + return HttpResponse.json({}, { status: 200 }); + }) ); return requests; diff --git a/packages/wrangler/src/index.ts b/packages/wrangler/src/index.ts index af7c161e8d83..874b4f78f86e 100644 --- a/packages/wrangler/src/index.ts +++ b/packages/wrangler/src/index.ts @@ -1,5 +1,6 @@ import module from "node:module"; import os from "node:os"; +import { setTimeout } from "node:timers/promises"; import TOML from "@iarna/toml"; import chalk from "chalk"; import { ProxyAgent, setGlobalDispatcher } from "undici"; @@ -855,6 +856,10 @@ export async function main(argv: string[]): Promise { } await closeSentry(); + await Promise.race([ + await Promise.allSettled(dispatcher?.requests ?? []), + setTimeout(1000), // Ensure we don't hang indefinitely + ]); } catch (e) { logger.error(e); // Only re-throw if we haven't already re-thrown an exception from a diff --git a/packages/wrangler/src/metrics/metrics-dispatcher.ts b/packages/wrangler/src/metrics/metrics-dispatcher.ts index 9a66966c12a9..2c8fb4a1bca3 100644 --- a/packages/wrangler/src/metrics/metrics-dispatcher.ts +++ b/packages/wrangler/src/metrics/metrics-dispatcher.ts @@ -16,6 +16,7 @@ export function getMetricsDispatcher(options: MetricsConfigOptions) { // The SPARROW_SOURCE_KEY will be provided at build time through esbuild's `define` option // No events will be sent if the env `SPARROW_SOURCE_KEY` is not provided and the value will be set to an empty string instead. const SPARROW_SOURCE_KEY = process.env.SPARROW_SOURCE_KEY ?? ""; + const requests: Array> = []; const wranglerVersion = getWranglerVersion(); const platform = getPlatform(); const packageManager = getPackageManager(); @@ -67,6 +68,10 @@ export function getMetricsDispatcher(options: MetricsConfigOptions) { }, }); }, + + get requests() { + return requests; + }, }; async function dispatch(event: { @@ -102,7 +107,7 @@ export function getMetricsDispatcher(options: MetricsConfigOptions) { // Do not await this fetch call. // Just fire-and-forget, otherwise we might slow down the rest of Wrangler. - fetch(`${SPARROW_URL}/api/v1/event`, { + const request = fetch(`${SPARROW_URL}/api/v1/event`, { method: "POST", headers: { Accept: "*/*", @@ -112,12 +117,16 @@ export function getMetricsDispatcher(options: MetricsConfigOptions) { mode: "cors", keepalive: true, body: JSON.stringify(body), - }).catch((e) => { - logger.debug( - "Metrics dispatcher: Failed to send request:", - (e as Error).message - ); - }); + }) + .then(() => {}) + .catch((e) => { + logger.debug( + "Metrics dispatcher: Failed to send request:", + (e as Error).message + ); + }); + + requests.push(request); } }