From ac4928c50169d4b01eddb4c9d09109cb3cc6380d Mon Sep 17 00:00:00 2001 From: emily-shen <69125074+emily-shen@users.noreply.github.com> Date: Wed, 20 Nov 2024 13:22:08 +0000 Subject: [PATCH 1/7] add banner --- .../wrangler/src/metrics/metrics-config.ts | 2 + .../src/metrics/metrics-dispatcher.ts | 48 ++++++++++++++++++- 2 files changed, 49 insertions(+), 1 deletion(-) diff --git a/packages/wrangler/src/metrics/metrics-config.ts b/packages/wrangler/src/metrics/metrics-config.ts index 8dd6d496c001..306ebaa5b4f5 100644 --- a/packages/wrangler/src/metrics/metrics-config.ts +++ b/packages/wrangler/src/metrics/metrics-config.ts @@ -158,6 +158,8 @@ export interface MetricsConfigFile { enabled: boolean; /** The date that this permission was set. */ date: Date; + /** Version number the banner was last shown - only show on version update */ + bannerLastShown?: string; }; c3permission?: { /** True if c3 should send metrics to Cloudflare. */ diff --git a/packages/wrangler/src/metrics/metrics-dispatcher.ts b/packages/wrangler/src/metrics/metrics-dispatcher.ts index 9a66966c12a9..8429d8eebf5d 100644 --- a/packages/wrangler/src/metrics/metrics-dispatcher.ts +++ b/packages/wrangler/src/metrics/metrics-dispatcher.ts @@ -1,3 +1,4 @@ +import chalk from "chalk"; import { fetch } from "undici"; import { logger } from "../logger"; import { @@ -6,7 +7,11 @@ import { getPlatform, getWranglerVersion, } from "./helpers"; -import { getMetricsConfig, readMetricsConfig } from "./metrics-config"; +import { + getMetricsConfig, + readMetricsConfig, + writeMetricsConfig, +} from "./metrics-config"; import type { MetricsConfigOptions } from "./metrics-config"; import type { CommonEventProperties, Events } from "./send-event"; @@ -43,6 +48,12 @@ export function getMetricsDispatcher(options: MetricsConfigOptions) { }); }, + /** + * Dispatches `wrangler command started / completed / errored` events + * + * This happens on every command execution, and will (hopefully) replace sendEvent soon. + * However to prevent disruption, we're adding under `sendNewEvent` for now. + */ async sendNewEvent( name: EventName, properties: Omit< @@ -50,6 +61,9 @@ export function getMetricsDispatcher(options: MetricsConfigOptions) { keyof CommonEventProperties > ): Promise { + if (name === "wrangler command started") { + printMetricsBanner(); + } const commonEventProperties: CommonEventProperties = { amplitude_session_id, amplitude_event_id: amplitude_event_id++, @@ -119,6 +133,38 @@ export function getMetricsDispatcher(options: MetricsConfigOptions) { ); }); } + + function printMetricsBanner() { + const metricsConfig = readMetricsConfig(); + const lastShown = metricsConfig.permission?.bannerLastShown; + if ( + metricsConfig.permission?.enabled && + (!lastShown || isNewVersion(lastShown, wranglerVersion)) + ) { + logger.log( + chalk.gray( + `\nCloudflare collects anonymous telemetry about your usage of Wrangler. Learn more at https://github.com/cloudflare/workers-sdk/tree/main/telemetry.md` + ) + ); + metricsConfig.permission.bannerLastShown = wranglerVersion; + writeMetricsConfig(metricsConfig); + } + } } export type Properties = Record; + +const isNewVersion = (stored: string, current: string) => { + const storedVersion = stored.split("."); + const currentVersion = current.split("."); + for (let i = 0; i < storedVersion.length; i++) { + const storedSegment = parseInt(storedVersion[i]); + const currentSegment = parseInt(currentVersion[i]); + if (currentSegment > storedSegment) { + return true; + } else if (currentSegment < storedSegment) { + return false; + } + } + return false; +}; From e0466f6060406f5336bb9c2a057ce2fbe3c94449 Mon Sep 17 00:00:00 2001 From: emily-shen <69125074+emily-shen@users.noreply.github.com> Date: Wed, 20 Nov 2024 17:46:12 +0000 Subject: [PATCH 2/7] abort if telemetry disable --- .../wrangler/src/__tests__/metrics.test.ts | 56 ++++++++++++------- .../src/metrics/metrics-dispatcher.ts | 6 ++ 2 files changed, 43 insertions(+), 19 deletions(-) diff --git a/packages/wrangler/src/__tests__/metrics.test.ts b/packages/wrangler/src/__tests__/metrics.test.ts index b9c430438a52..9e3bf3d77c11 100644 --- a/packages/wrangler/src/__tests__/metrics.test.ts +++ b/packages/wrangler/src/__tests__/metrics.test.ts @@ -68,8 +68,7 @@ describe("metrics", () => { }, { "Sparrow-Source-Key": "MOCK_KEY", - }, - "event" + } ); const dispatcher = await getMetricsDispatcher({ sendMetrics: true, @@ -86,7 +85,7 @@ describe("metrics", () => { }); it("should write a debug log if the dispatcher is disabled", async () => { - const requests = mockMetricRequest({}, {}, "event"); + const requests = mockMetricRequest({}, {}); const dispatcher = await getMetricsDispatcher({ sendMetrics: false, }); @@ -127,7 +126,7 @@ describe("metrics", () => { it("should write a warning log if no source key has been provided", async () => { vi.stubEnv("SPARROW_SOURCE_KEY", undefined); - const requests = mockMetricRequest({}, {}, "event"); + const requests = mockMetricRequest({}, {}); const dispatcher = await getMetricsDispatcher({ sendMetrics: true, }); @@ -370,6 +369,32 @@ Wrangler is no longer collecting telemetry about your usage.`); }); }); + it(`doesn't send telemetry when running "wrangler ${cmd} disable"`, async () => { + const requests = mockMetricRequest({}, {}); + writeMetricsConfig({ + permission: { + enabled: true, + date: new Date(2022, 6, 4), + }, + }); + await runWrangler(`${cmd} disable`); + expect(requests.count).toBe(0); + expect(std.debug).not.toContain("Metrics dispatcher: Posting data"); + }); + + it(`does send telemetry when running "wrangler ${cmd} enable"`, async () => { + const requests = mockMetricRequest({}, {}); + writeMetricsConfig({ + permission: { + enabled: true, + date: new Date(2022, 6, 4), + }, + }); + await runWrangler(`${cmd} enable`); + expect(requests.count).toBe(2); + expect(std.debug).toContain("Metrics dispatcher: Posting data"); + }); + it(`enables telemetry when "wrangler ${cmd} enable" is run`, async () => { writeMetricsConfig({ permission: { @@ -415,23 +440,16 @@ Wrangler is now collecting telemetry about your usage. Thank you for helping mak }); }); -function mockMetricRequest( - body: unknown, - header: unknown, - endpoint: "identify" | "event" -) { +function mockMetricRequest(body: unknown, header: unknown) { 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(`*/event`, async ({ request }) => { + requests.count++; + + expect(await request.json()).toBe(body); + expect(request.headers).toContain(header); + return HttpResponse.json({}, { status: 200 }); + }) ); return requests; diff --git a/packages/wrangler/src/metrics/metrics-dispatcher.ts b/packages/wrangler/src/metrics/metrics-dispatcher.ts index 8429d8eebf5d..4c765f9fbf1b 100644 --- a/packages/wrangler/src/metrics/metrics-dispatcher.ts +++ b/packages/wrangler/src/metrics/metrics-dispatcher.ts @@ -61,6 +61,12 @@ export function getMetricsDispatcher(options: MetricsConfigOptions) { keyof CommonEventProperties > ): Promise { + if ( + properties.command === "wrangler telemetry disable" || + properties.command === "wrangler metrics disable" + ) { + return; + } if (name === "wrangler command started") { printMetricsBanner(); } From 5bddb675482e51d31fd1a8018af6dd856ed7b3ea Mon Sep 17 00:00:00 2001 From: emily-shen <69125074+emily-shen@users.noreply.github.com> Date: Thu, 21 Nov 2024 11:19:24 +0000 Subject: [PATCH 3/7] basic sendNewEvent tests --- .../wrangler/src/__tests__/metrics.test.ts | 83 +++++++++++++++++++ 1 file changed, 83 insertions(+) diff --git a/packages/wrangler/src/__tests__/metrics.test.ts b/packages/wrangler/src/__tests__/metrics.test.ts index 9e3bf3d77c11..f0f635ae75e2 100644 --- a/packages/wrangler/src/__tests__/metrics.test.ts +++ b/packages/wrangler/src/__tests__/metrics.test.ts @@ -1,5 +1,7 @@ import { http, HttpResponse } from "msw"; import { vi } from "vitest"; +import { defineCommand, defineNamespace } from "../core"; +import { UserError } from "../errors"; import { CI } from "../is-ci"; import { logger } from "../logger"; import { getOS, getWranglerVersion } from "../metrics/helpers"; @@ -141,6 +143,87 @@ describe("metrics", () => { expect(std.err).toMatchInlineSnapshot(`""`); }); }); + + describe("sendNewEvent()", () => { + beforeAll(() => { + // register a no-op test command + defineNamespace({ + command: "wrangler command", + metadata: { + description: "test command namespace", + owner: "Workers: Authoring and Testing", + status: "stable", + }, + }); + + defineCommand({ + command: "wrangler command subcommand", + metadata: { + description: "test command", + owner: "Workers: Authoring and Testing", + status: "stable", + }, + args: { + positional: { + type: "string", + demandOption: true, + }, + optional: { + type: "string", + }, + }, + positionalArgs: ["positional"], + handler(args, ctx) { + ctx.logger.log("Ran wrangler command subcommand"); + if (args.positional === "error") { + throw new UserError("oh no"); + } + }, + }); + }); + it("should send a started and completed event", async () => { + const requests = mockMetricRequest({}, {}); + + await runWrangler("command subcommand positional"); + + expect(requests.count).toBe(2); + + // command started + expect(std.debug).toContain( + `Metrics dispatcher: Posting data {"deviceId":"f82b1f46-eb7b-4154-aa9f-ce95f23b2288","event":"wrangler command started","timestamp":1733961600000,"properties":{"amplitude_session_id":1733961600000,"amplitude_event_id":0,"wranglerVersion":"1.2.3","isFirstUsage":false,"command":"wrangler command subcommand","args":{"_":["command","subcommand"],"experimental-versions":true,"x-versions":true,"experimental-gradual-rollouts":true,"xVersions":true,"experimentalGradualRollouts":true,"experimentalVersions":true,"$0":"wrangler","positional":"positional"}}}` + ); + // command completed + expect(std.debug).toContain( + `Metrics dispatcher: Posting data {"deviceId":"f82b1f46-eb7b-4154-aa9f-ce95f23b2288","event":"wrangler command completed","timestamp":1733961600000,"properties":{"amplitude_session_id":1733961600000,"amplitude_event_id":1,"wranglerVersion":"1.2.3","isFirstUsage":false,"command":"wrangler command subcommand","args":{"_":["command","subcommand"],"experimental-versions":true,"x-versions":true,"experimental-gradual-rollouts":true,"xVersions":true,"experimentalGradualRollouts":true,"experimentalVersions":true,"$0":"wrangler","positional":"positional"},"durationMs":0,"durationSeconds":0,"durationMinutes":0}}` + ); + expect(std.out).toMatchInlineSnapshot(` + " + Cloudflare collects anonymous telemetry about your usage of Wrangler. Learn more at https://github.com/cloudflare/workers-sdk/tree/main/telemetry.md + Ran wrangler command subcommand" + `); + expect(std.warn).toMatchInlineSnapshot(`""`); + expect(std.err).toMatchInlineSnapshot(`""`); + }); + + it("should send a started and errored event", async () => { + const requests = mockMetricRequest({}, {}); + + await expect(runWrangler("command subcommand error")).rejects.toThrow( + "oh no" + ); + + expect(requests.count).toBe(2); + + // command started + expect(std.debug).toContain( + `Metrics dispatcher: Posting data {"deviceId":"f82b1f46-eb7b-4154-aa9f-ce95f23b2288","event":"wrangler command started","timestamp":1733961600000,"properties":{"amplitude_session_id":1733961600000,"amplitude_event_id":0,"wranglerVersion":"1.2.3","isFirstUsage":false,"command":"wrangler command subcommand","args":{"_":["command","subcommand"],"experimental-versions":true,"x-versions":true,"experimental-gradual-rollouts":true,"xVersions":true,"experimentalGradualRollouts":true,"experimentalVersions":true,"$0":"wrangler","positional":"error"}}}` + ); + // command completed + expect(std.debug).toContain( + `Metrics dispatcher: Posting data {"deviceId":"f82b1f46-eb7b-4154-aa9f-ce95f23b2288","event":"wrangler command errored","timestamp":1733961600000,"properties":{"amplitude_session_id":1733961600000,"amplitude_event_id":1,"wranglerVersion":"1.2.3","isFirstUsage":false,"command":"wrangler command subcommand","args":{"_":["command","subcommand"],"experimental-versions":true,"x-versions":true,"experimental-gradual-rollouts":true,"xVersions":true,"experimentalGradualRollouts":true,"experimentalVersions":true,"$0":"wrangler","positional":"error"},"durationMs":0,"durationSeconds":0,"durationMinutes":0,"errorType":"UserError"}}` + ); + }); + }); }); describe("getMetricsConfig()", () => { From 94747defa1fb552c5047d2317c2f8c3ceb345289 Mon Sep 17 00:00:00 2001 From: emily-shen <69125074+emily-shen@users.noreply.github.com> Date: Thu, 21 Nov 2024 11:55:57 +0000 Subject: [PATCH 4/7] banner tests --- .../wrangler/src/__tests__/metrics.test.ts | 90 +++++++++++++++++++ 1 file changed, 90 insertions(+) diff --git a/packages/wrangler/src/__tests__/metrics.test.ts b/packages/wrangler/src/__tests__/metrics.test.ts index f0f635ae75e2..a7b6a2fc61c0 100644 --- a/packages/wrangler/src/__tests__/metrics.test.ts +++ b/packages/wrangler/src/__tests__/metrics.test.ts @@ -223,6 +223,96 @@ describe("metrics", () => { `Metrics dispatcher: Posting data {"deviceId":"f82b1f46-eb7b-4154-aa9f-ce95f23b2288","event":"wrangler command errored","timestamp":1733961600000,"properties":{"amplitude_session_id":1733961600000,"amplitude_event_id":1,"wranglerVersion":"1.2.3","isFirstUsage":false,"command":"wrangler command subcommand","args":{"_":["command","subcommand"],"experimental-versions":true,"x-versions":true,"experimental-gradual-rollouts":true,"xVersions":true,"experimentalGradualRollouts":true,"experimentalVersions":true,"$0":"wrangler","positional":"error"},"durationMs":0,"durationSeconds":0,"durationMinutes":0,"errorType":"UserError"}}` ); }); + + describe("banner", () => { + beforeEach(() => { + vi.mocked(getWranglerVersion).mockReturnValue("1.2.3"); + }); + it("should print the banner if current version is newer than stored version", async () => { + writeMetricsConfig({ + permission: { + enabled: true, + date: new Date(2022, 6, 4), + bannerLastShown: "1.2.1", + }, + }); + + const requests = mockMetricRequest({}, {}); + + await runWrangler("command subcommand positional"); + expect(std.out).toMatchInlineSnapshot(` + " + Cloudflare collects anonymous telemetry about your usage of Wrangler. Learn more at https://github.com/cloudflare/workers-sdk/tree/main/telemetry.md + Ran wrangler command subcommand" + `); + + expect(requests.count).toBe(2); + }); + it("should not print the banner if current version is the same as the stored version", async () => { + writeMetricsConfig({ + permission: { + enabled: true, + date: new Date(2022, 6, 4), + bannerLastShown: "1.2.3", + }, + }); + const requests = mockMetricRequest({}, {}); + await runWrangler("command subcommand positional"); + expect(std.out).toMatchInlineSnapshot(` + "Ran wrangler command subcommand" + `); + expect(requests.count).toBe(2); + }); + it("should not print the banner if current version is older than the stored version", async () => { + writeMetricsConfig({ + permission: { + enabled: true, + date: new Date(2022, 6, 4), + bannerLastShown: "1.2.4", + }, + }); + const requests = mockMetricRequest({}, {}); + await runWrangler("command subcommand positional"); + expect(std.out).toMatchInlineSnapshot(` + "Ran wrangler command subcommand" + `); + expect(requests.count).toBe(2); + }); + it("should print the banner if nothing is stored under bannerLastShown and then store the current version", async () => { + writeMetricsConfig({ + permission: { + enabled: true, + date: new Date(2022, 6, 4), + }, + }); + const requests = mockMetricRequest({}, {}); + await runWrangler("command subcommand positional"); + expect(std.out).toMatchInlineSnapshot(` + " + Cloudflare collects anonymous telemetry about your usage of Wrangler. Learn more at https://github.com/cloudflare/workers-sdk/tree/main/telemetry.md + Ran wrangler command subcommand" + `); + expect(requests.count).toBe(2); + const { permission } = readMetricsConfig(); + expect(permission?.bannerLastShown).toEqual("1.2.3"); + }); + it("should not print the banner if telemetry permission is disabled", async () => { + writeMetricsConfig({ + permission: { + enabled: false, + date: new Date(2022, 6, 4), + }, + }); + const requests = mockMetricRequest({}, {}); + await runWrangler("command subcommand positional"); + expect(std.out).toMatchInlineSnapshot(` + "Ran wrangler command subcommand" + `); + expect(requests.count).toBe(0); + const { permission } = readMetricsConfig(); + expect(permission?.bannerLastShown).toBeUndefined(); + }); + }); }); }); From c19b487d21f711889fe9e5529028e128c14fc094 Mon Sep 17 00:00:00 2001 From: emily-shen <69125074+emily-shen@users.noreply.github.com> Date: Thu, 21 Nov 2024 16:29:49 +0000 Subject: [PATCH 5/7] settle promises before exiting --- .../wrangler/src/__tests__/metrics.test.ts | 32 ++++++++++++------- packages/wrangler/src/index.ts | 5 +++ .../src/metrics/metrics-dispatcher.ts | 23 +++++++++---- 3 files changed, 42 insertions(+), 18 deletions(-) diff --git a/packages/wrangler/src/__tests__/metrics.test.ts b/packages/wrangler/src/__tests__/metrics.test.ts index a7b6a2fc61c0..bd9556d44cc7 100644 --- a/packages/wrangler/src/__tests__/metrics.test.ts +++ b/packages/wrangler/src/__tests__/metrics.test.ts @@ -144,6 +144,27 @@ describe("metrics", () => { }); }); + it("should keep track of all requests made", async () => { + const requests = mockMetricRequest({}, {}); + 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("sendNewEvent()", () => { beforeAll(() => { // register a no-op test command @@ -354,17 +375,6 @@ describe("metrics", () => { }); }); - it("should return enabled false if the process is not interactive", async () => { - setIsTTY(false); - expect( - await getMetricsConfig({ - sendMetrics: undefined, - }) - ).toMatchObject({ - enabled: false, - }); - }); - it("should return enabled true if the user on this device previously agreed to send metrics", async () => { writeMetricsConfig({ permission: { 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 4c765f9fbf1b..2adcb29451b9 100644 --- a/packages/wrangler/src/metrics/metrics-dispatcher.ts +++ b/packages/wrangler/src/metrics/metrics-dispatcher.ts @@ -21,6 +21,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(); @@ -87,6 +88,10 @@ export function getMetricsDispatcher(options: MetricsConfigOptions) { }, }); }, + + get requests() { + return requests; + }, }; async function dispatch(event: { @@ -122,7 +127,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: "*/*", @@ -132,12 +137,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); } function printMetricsBanner() { From 8814b3eb73e85e78d3fada42cc2110d3a0b2d364 Mon Sep 17 00:00:00 2001 From: emily-shen <69125074+emily-shen@users.noreply.github.com> Date: Thu, 21 Nov 2024 16:34:04 +0000 Subject: [PATCH 6/7] remove unnecessary banner logic --- packages/wrangler/src/metrics/metrics-dispatcher.ts | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/packages/wrangler/src/metrics/metrics-dispatcher.ts b/packages/wrangler/src/metrics/metrics-dispatcher.ts index 2adcb29451b9..f1dbd4e3f71c 100644 --- a/packages/wrangler/src/metrics/metrics-dispatcher.ts +++ b/packages/wrangler/src/metrics/metrics-dispatcher.ts @@ -68,9 +68,7 @@ export function getMetricsDispatcher(options: MetricsConfigOptions) { ) { return; } - if (name === "wrangler command started") { - printMetricsBanner(); - } + printMetricsBanner(); const commonEventProperties: CommonEventProperties = { amplitude_session_id, amplitude_event_id: amplitude_event_id++, From 57eca6ccfd98cfe174537d2ec59ff1195f2af952 Mon Sep 17 00:00:00 2001 From: emily-shen <69125074+emily-shen@users.noreply.github.com> Date: Thu, 21 Nov 2024 17:17:33 +0000 Subject: [PATCH 7/7] just check if version is different --- .../wrangler/src/__tests__/metrics.test.ts | 17 +---------------- .../wrangler/src/metrics/metrics-dispatcher.ts | 18 +----------------- 2 files changed, 2 insertions(+), 33 deletions(-) diff --git a/packages/wrangler/src/__tests__/metrics.test.ts b/packages/wrangler/src/__tests__/metrics.test.ts index bd9556d44cc7..abda95a1aaf8 100644 --- a/packages/wrangler/src/__tests__/metrics.test.ts +++ b/packages/wrangler/src/__tests__/metrics.test.ts @@ -249,7 +249,7 @@ describe("metrics", () => { beforeEach(() => { vi.mocked(getWranglerVersion).mockReturnValue("1.2.3"); }); - it("should print the banner if current version is newer than stored version", async () => { + it("should print the banner if current version is different to the stored version", async () => { writeMetricsConfig({ permission: { enabled: true, @@ -284,21 +284,6 @@ describe("metrics", () => { `); expect(requests.count).toBe(2); }); - it("should not print the banner if current version is older than the stored version", async () => { - writeMetricsConfig({ - permission: { - enabled: true, - date: new Date(2022, 6, 4), - bannerLastShown: "1.2.4", - }, - }); - const requests = mockMetricRequest({}, {}); - await runWrangler("command subcommand positional"); - expect(std.out).toMatchInlineSnapshot(` - "Ran wrangler command subcommand" - `); - expect(requests.count).toBe(2); - }); it("should print the banner if nothing is stored under bannerLastShown and then store the current version", async () => { writeMetricsConfig({ permission: { diff --git a/packages/wrangler/src/metrics/metrics-dispatcher.ts b/packages/wrangler/src/metrics/metrics-dispatcher.ts index f1dbd4e3f71c..8ea528cc5c27 100644 --- a/packages/wrangler/src/metrics/metrics-dispatcher.ts +++ b/packages/wrangler/src/metrics/metrics-dispatcher.ts @@ -149,10 +149,9 @@ export function getMetricsDispatcher(options: MetricsConfigOptions) { function printMetricsBanner() { const metricsConfig = readMetricsConfig(); - const lastShown = metricsConfig.permission?.bannerLastShown; if ( metricsConfig.permission?.enabled && - (!lastShown || isNewVersion(lastShown, wranglerVersion)) + metricsConfig.permission?.bannerLastShown !== wranglerVersion ) { logger.log( chalk.gray( @@ -166,18 +165,3 @@ export function getMetricsDispatcher(options: MetricsConfigOptions) { } export type Properties = Record; - -const isNewVersion = (stored: string, current: string) => { - const storedVersion = stored.split("."); - const currentVersion = current.split("."); - for (let i = 0; i < storedVersion.length; i++) { - const storedSegment = parseInt(storedVersion[i]); - const currentSegment = parseInt(currentVersion[i]); - if (currentSegment > storedSegment) { - return true; - } else if (currentSegment < storedSegment) { - return false; - } - } - return false; -};