From 02de103435689c552e231a2ae2249adeb5f60a8b Mon Sep 17 00:00:00 2001
From: Zeb Piasecki <zeb@cloudflare.com>
Date: Thu, 19 Sep 2024 12:02:17 -0400
Subject: [PATCH] fix: disable observability on deploy if not explicitly
 defined in config (#6776)

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.
---
 .changeset/old-mugs-begin.md                  |  7 ++++++
 .../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 +-
 5 files changed, 48 insertions(+), 4 deletions(-)
 create mode 100644 .changeset/old-mugs-begin.md

diff --git a/.changeset/old-mugs-begin.md b/.changeset/old-mugs-begin.md
new file mode 100644
index 000000000000..ca72815c07f7
--- /dev/null
+++ b/.changeset/old-mugs-begin.md
@@ -0,0 +1,7 @@
+---
+"wrangler": patch
+---
+
+fix: disable observability on deploy if not explicitly defined in config
+
+When deploying a Worker that has observability enabled in the deployed version but not specified in the `wrangler.toml` Wrangler will now set observability to disabled for the new version to match the `wrangler.toml` as the source of truth.
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;