From f7bf04a779cb8a82cd4577835b2fbdf02fa737de Mon Sep 17 00:00:00 2001
From: Zeb Piasecki <zeb@cloudflare.com>
Date: Thu, 19 Sep 2024 10:15:17 -0400
Subject: [PATCH] fix: disable observability on deploy if not explicitly
 defined in config

Changes the deploy behavior deploys using the new versions API to disable observability if it isn't explicitly defined in the user's wrangler.toml.
---
 .../wrangler/src/__tests__/deploy.test.ts     | 23 +++++++++++++++++++
 .../__tests__/helpers/mock-upload-worker.ts   | 13 ++++++++++-
 packages/wrangler/src/deploy/deploy.ts        |  7 ++++--
 packages/wrangler/src/versions/api.ts         |  2 +-
 4 files changed, 41 insertions(+), 4 deletions(-)

diff --git a/packages/wrangler/src/__tests__/deploy.test.ts b/packages/wrangler/src/__tests__/deploy.test.ts
index e3f2c1ddc417..b586daf1e336 100644
--- a/packages/wrangler/src/__tests__/deploy.test.ts
+++ b/packages/wrangler/src/__tests__/deploy.test.ts
@@ -10880,6 +10880,29 @@ export default{
 				Current Version ID: Galaxy-Class"
 			`);
 		});
+
+		it("should disable observability if not explicitly defined", async () => {
+			writeWranglerToml({});
+			await fs.promises.writeFile("index.js", `export default {};`);
+			mockSubDomainRequest();
+			mockUploadWorkerRequest({
+				expectedSettingsPatch: {
+					observability: {
+						enabled: false,
+					},
+				},
+			});
+
+			await runWrangler("publish index.js");
+			expect(std.out).toMatchInlineSnapshot(`
+				"Total Upload: xx KiB / gzip: xx KiB
+				Worker Startup Time: 100 ms
+				Uploaded test-name (TIMINGS)
+				Deployed test-name triggers (TIMINGS)
+				  https://test-name.test-sub-domain.workers.dev
+				Current Version ID: Galaxy-Class"
+			`);
+		});
 	});
 });
 
diff --git a/packages/wrangler/src/__tests__/helpers/mock-upload-worker.ts b/packages/wrangler/src/__tests__/helpers/mock-upload-worker.ts
index 1f22347d8b7e..ae86fd27b0be 100644
--- a/packages/wrangler/src/__tests__/helpers/mock-upload-worker.ts
+++ b/packages/wrangler/src/__tests__/helpers/mock-upload-worker.ts
@@ -3,6 +3,7 @@ import { createFetchResult, msw } from "./msw";
 import { serialize, toString } from "./serialize-form-data-entry";
 import type { WorkerMetadata } from "../../deployment-bundle/create-worker-upload-form";
 import type { CfWorkerInit } from "../../deployment-bundle/worker";
+import type { NonVersionedScriptSettings } from "../../versions/api";
 import type { AssetConfig } from "@cloudflare/workers-shared";
 import type { HttpResponseResolver } from "msw";
 
@@ -35,6 +36,7 @@ export function mockUploadWorkerRequest(
 		};
 		useOldUploadApi?: boolean;
 		expectedObservability?: CfWorkerInit["observability"];
+		expectedSettingsPatch?: Partial<NonVersionedScriptSettings>;
 	} = {}
 ) {
 	const expectedScriptName = (options.expectedScriptName ??= "test-name");
@@ -178,6 +180,7 @@ export function mockUploadWorkerRequest(
 		expectedDispatchNamespace,
 		useOldUploadApi,
 		expectedObservability,
+		expectedSettingsPatch,
 	} = options;
 	if (env && !legacyEnv) {
 		msw.use(
@@ -212,7 +215,15 @@ export function mockUploadWorkerRequest(
 			),
 			http.patch(
 				"*/accounts/:accountId/workers/scripts/:scriptName/script-settings",
-				() => HttpResponse.json(createFetchResult({}))
+				async ({ request }) => {
+					const body = await request.json();
+
+					if ("expectedSettingsPatch" in options) {
+						expect(body).toEqual(expectedSettingsPatch);
+					}
+
+					return HttpResponse.json(createFetchResult({}));
+				}
 			)
 		);
 	}
diff --git a/packages/wrangler/src/deploy/deploy.ts b/packages/wrangler/src/deploy/deploy.ts
index c16f1aa8457a..b180c5eda373 100644
--- a/packages/wrangler/src/deploy/deploy.ts
+++ b/packages/wrangler/src/deploy/deploy.ts
@@ -829,11 +829,14 @@ See https://developers.cloudflare.com/workers/platform/compatibility-dates for m
 					versionMap.set(versionResult.id, 100);
 					await createDeployment(accountId, scriptName, versionMap, undefined);
 
-					// Update tail consumers and logpush settings
+					// Update tail consumers, logpush, and observability settings
 					await patchNonVersionedScriptSettings(accountId, scriptName, {
 						tail_consumers: worker.tail_consumers,
 						logpush: worker.logpush,
-						observability: worker.observability,
+						// If the user hasn't specified observability assume that they want it disabled if they have it on.
+						// This is a no-op in the event that they don't have observability enabled, but will remove observability
+						// if it has been removed from their wrangler.toml
+						observability: worker.observability ?? { enabled: false },
 					});
 
 					const { available_on_subdomain } = await fetchResult<{
diff --git a/packages/wrangler/src/versions/api.ts b/packages/wrangler/src/versions/api.ts
index 718c5befc722..52f41314d30d 100644
--- a/packages/wrangler/src/versions/api.ts
+++ b/packages/wrangler/src/versions/api.ts
@@ -125,7 +125,7 @@ export async function createDeployment(
 	);
 }
 
-type NonVersionedScriptSettings = {
+export type NonVersionedScriptSettings = {
 	logpush: boolean;
 	tail_consumers: TailConsumer[];
 	observability: Observability;