From a776819fbe3e07188b6be88ec48c4dda4a130c98 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] 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 acdb65fe2a2c1..9d6d378bf7e63 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 948655742497a..300acdd64cf8b 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-20241024-111401-d4156ac", + "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 9fbe71d947000..4e23e3072ab37 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 eb16fd992b6ab..b9c430438a529 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 b1973e748b631..296f5fd15cd8d 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 aa516bd8e8635..f251ef4e5bf27 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 dd2c290571d2f..af7c161e8d831 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 0000000000000..173d0363ab2a9 --- /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 235db05fa222d..8dd6d496c0017 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 8ba5de4f82ee3..8a6bf00814470 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 bdde7d86eea43..2e224edd5f9e2 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 715d69468bc78..be99977c0d5db 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -1708,6 +1708,9 @@ importers: unenv: specifier: npm:unenv-nightly@2.0.0-20241024-111401-d4156ac version: unenv-nightly@2.0.0-20241024-111401-d4156ac + which-pm-runs: + specifier: ^1.1.0 + version: 1.1.0 workerd: specifier: 1.20241106.1 version: 1.20241106.1 @@ -1794,6 +1797,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