From c3e19b790bb597b78e0109a162ca8049b5eaf973 Mon Sep 17 00:00:00 2001 From: Taylor <1628134+taylorlee@users.noreply.github.com> Date: Thu, 1 Aug 2024 14:32:35 -0700 Subject: [PATCH] print startup time in version upload (#6392) * [wrangler] log startup time in version upload Same as https://github.com/cloudflare/workers-sdk/pull/6318 , but for version upload * [wrangler] remove bundle size warning Bundle size was a proxy for startup time. Now that we have startup time reported, focus on bundle size is less relevant. Tracked internally as EW-8219 --- .changeset/moody-baboons-wonder.md | 5 +++ .changeset/witty-bats-drum.md | 8 +++++ packages/wrangler/e2e/helpers/normalize.ts | 4 ++- packages/wrangler/e2e/versions.test.ts | 2 ++ .../wrangler/src/__tests__/deploy.test.ts | 32 +------------------ .../src/deployment-bundle/bundle-reporter.ts | 6 ---- packages/wrangler/src/versions/upload.ts | 13 +++++++- packages/wrangler/turbo.json | 2 -- 8 files changed, 31 insertions(+), 41 deletions(-) create mode 100644 .changeset/moody-baboons-wonder.md create mode 100644 .changeset/witty-bats-drum.md diff --git a/.changeset/moody-baboons-wonder.md b/.changeset/moody-baboons-wonder.md new file mode 100644 index 000000000000..aac67787a494 --- /dev/null +++ b/.changeset/moody-baboons-wonder.md @@ -0,0 +1,5 @@ +--- +"wrangler": minor +--- + +feat: log Worker startup time in the `version upload` command diff --git a/.changeset/witty-bats-drum.md b/.changeset/witty-bats-drum.md new file mode 100644 index 000000000000..46ff3695487e --- /dev/null +++ b/.changeset/witty-bats-drum.md @@ -0,0 +1,8 @@ +--- +"wrangler": patch +--- + +fix: remove bundle size warning from Worker deploy commands + +Bundle size was a proxy for startup time. Now that we have startup time +reported, focus on bundle size is less relevant. diff --git a/packages/wrangler/e2e/helpers/normalize.ts b/packages/wrangler/e2e/helpers/normalize.ts index 8da196a633b2..2914109ff0c3 100644 --- a/packages/wrangler/e2e/helpers/normalize.ts +++ b/packages/wrangler/e2e/helpers/normalize.ts @@ -98,7 +98,9 @@ export function normalizeSlashes(str: string): string { * Use this in snapshot tests to be resilient to slight changes in timing of processing. */ export function stripTimings(stdout: string): string { - return stdout.replace(/\(\d+\.\d+ sec\)/g, "(TIMINGS)"); + return stdout + .replace(/\(\d+\.\d+ sec\)/g, "(TIMINGS)") + .replace(/\d+ ms/g, "(TIMINGS)"); } /** diff --git a/packages/wrangler/e2e/versions.test.ts b/packages/wrangler/e2e/versions.test.ts index 8d0eef3e5358..0f13e7733330 100644 --- a/packages/wrangler/e2e/versions.test.ts +++ b/packages/wrangler/e2e/versions.test.ts @@ -60,6 +60,7 @@ describe("versions deploy", { timeout: TIMEOUT }, () => { expect(normalize(upload.stdout)).toMatchInlineSnapshot(` "Total Upload: xx KiB / gzip: xx KiB + Worker Startup Time: (TIMINGS) Worker Version ID: 00000000-0000-0000-0000-000000000000 Uploaded tmp-e2e-worker-00000000-0000-0000-0000-000000000000 (TIMINGS) To deploy this version to production traffic use the command wrangler versions deploy --experimental-versions @@ -174,6 +175,7 @@ describe("versions deploy", { timeout: TIMEOUT }, () => { expect(normalize(upload.stdout)).toMatchInlineSnapshot(` "Total Upload: xx KiB / gzip: xx KiB + Worker Startup Time: (TIMINGS) Worker Version ID: 00000000-0000-0000-0000-000000000000 Uploaded tmp-e2e-worker-00000000-0000-0000-0000-000000000000 (TIMINGS) To deploy this version to production traffic use the command wrangler versions deploy --experimental-versions diff --git a/packages/wrangler/src/__tests__/deploy.test.ts b/packages/wrangler/src/__tests__/deploy.test.ts index f67614e814ad..0928001eb071 100644 --- a/packages/wrangler/src/__tests__/deploy.test.ts +++ b/packages/wrangler/src/__tests__/deploy.test.ts @@ -9403,35 +9403,7 @@ export default{ // keeping these as unit tests to try and keep them snappy, as they often deal with // big files that would take a while to deal with in a full wrangler test - test("should print the bundle size and warn about large scripts when > 1MiB", async () => { - const bigModule = Buffer.alloc(10_000_000); - randomFillSync(bigModule); - await printBundleSize({ name: "index.js", content: "" }, [ - { - name: "index.js", - filePath: undefined, - content: bigModule, - type: "buffer", - }, - ]); - - expect(std).toMatchInlineSnapshot(` - Object { - "debug": "", - "err": "", - "info": "", - "out": "Total Upload: xx KiB / gzip: xx KiB", - "warn": "▲ [WARNING] We recommend keeping your script less than 1MiB (1024 KiB) after gzip. Exceeding this can affect cold start time. Consider using Wrangler's \`--minify\` option to reduce your bundle size. - - ", - } - `); - }); - - test("should not warn about bundle sizes when NO_SCRIPT_SIZE_WARNING is set", async () => { - const previousValue = process.env.NO_SCRIPT_SIZE_WARNING; - process.env.NO_SCRIPT_SIZE_WARNING = "true"; - + test("should print the bundle size", async () => { const bigModule = Buffer.alloc(10_000_000); randomFillSync(bigModule); await printBundleSize({ name: "index.js", content: "" }, [ @@ -9452,8 +9424,6 @@ export default{ "warn": "", } `); - - process.env.NO_SCRIPT_SIZE_WARNING = previousValue; }); test("should print the top biggest dependencies in the bundle when upload fails", () => { diff --git a/packages/wrangler/src/deployment-bundle/bundle-reporter.ts b/packages/wrangler/src/deployment-bundle/bundle-reporter.ts index 471dd04af0d3..150ce74fb88b 100644 --- a/packages/wrangler/src/deployment-bundle/bundle-reporter.ts +++ b/packages/wrangler/src/deployment-bundle/bundle-reporter.ts @@ -40,12 +40,6 @@ export async function printBundleSize( : chalk.green(bundleReport); logger.log(`Total Upload: ${colorizedReport}`); - - if (gzipSize > ALLOWED_INITIAL_MAX && !process.env.NO_SCRIPT_SIZE_WARNING) { - logger.warn( - "We recommend keeping your script less than 1MiB (1024 KiB) after gzip. Exceeding this can affect cold start time. Consider using Wrangler's `--minify` option to reduce your bundle size." - ); - } } export function printOffendingDependencies( diff --git a/packages/wrangler/src/versions/upload.ts b/packages/wrangler/src/versions/upload.ts index 68f7d67095bf..b80b54a289fe 100644 --- a/packages/wrangler/src/versions/upload.ts +++ b/packages/wrangler/src/versions/upload.ts @@ -416,8 +416,11 @@ See https://developers.cloudflare.com/workers/platform/compatibility-dates for m printBindings({ ...withoutStaticAssets, vars: maskedVars }); - if (!props.dryRun) { + if (props.dryRun) { + printBindings({ ...withoutStaticAssets, vars: maskedVars }); + } else { await ensureQueuesExistByConfig(config); + let bindingsPrinted = false; // Upload the script so it has time to propagate. // We can also now tell whether available_on_subdomain is set @@ -431,6 +434,7 @@ See https://developers.cloudflare.com/workers/platform/compatibility-dates for m pipeline_hash: string | null; mutable_pipeline_id: string | null; deployment_id: string | null; + startup_time_ms: number; }>( workerUrl, { @@ -446,8 +450,15 @@ See https://developers.cloudflare.com/workers/platform/compatibility-dates for m }) ); + logger.log("Worker Startup Time:", result.startup_time_ms, "ms"); + bindingsPrinted = true; + printBindings({ ...withoutStaticAssets, vars: maskedVars }); logger.log("Worker Version ID:", result.id); } catch (err) { + if (!bindingsPrinted) { + printBindings({ ...withoutStaticAssets, vars: maskedVars }); + } + helpIfErrorIsSizeOrScriptStartup(err, dependencies); // Apply source mapping to validation startup errors if possible diff --git a/packages/wrangler/turbo.json b/packages/wrangler/turbo.json index b1d136ed3915..a0d7212d7c65 100644 --- a/packages/wrangler/turbo.json +++ b/packages/wrangler/turbo.json @@ -28,7 +28,6 @@ "SPARROW_SOURCE_KEY", "ALGOLIA_APP_ID", "ALGOLIA_PUBLIC_KEY", - "NO_SCRIPT_SIZE_WARNING", "CLOUDFLARE_API_TOKEN", "CLOUDFLARE_ACCOUNT_ID", "WRANGLER_AUTH_DOMAIN", @@ -39,7 +38,6 @@ "CF_PAGES", "CI", "CF_PAGES_UPLOAD_JWT", - "NO_SCRIPT_SIZE_WARNING", "EXPERIMENTAL_MIDDLEWARE", "NO_D1_WARNING", "NO_HYPERDRIVE_WARNING",