[v3 backport] Backport tail-based logging from #11135 and #11346 (#11138)

* Implement tail-based logging for realish preview (#11135)

* Turn on tail logs by default (#11346)

* Turn on tail logs by default

* Isolate inspector changes to remote only

* Create tricky-starfishes-tap.md

* Isolate hotkeys

* fix types

* fix types

* fix changeset

---------

Co-authored-by: Somhairle MacLeòid <smacleod@cloudflare.com>

Author: devin-ai-integration[bot]

.changeset/rotten-fireants-listen.md

@@ -0,0 +1,5 @@
+---
+"wrangler": patch
+---
+
+Implement tail-based logging for `wrangler dev` remote mode, behind the `--x-tail-tags` flag. This will become the default in the future.

.changeset/tricky-starfishes-tap.md

@@ -0,0 +1,7 @@
+---
+"wrangler": patch
+---
+
+We're soon going to make backend changes that mean that `wrangler dev --remote` sessions will no longer have an associated inspector connection. In advance of these backend changes, we've enabled a new `wrangler tail`-based logging strategy for `wrangler dev --remote`. For now, you can revert to the previous logging strategy with `wrangler dev --remote --no-x-tail-logs`, but in future it will not be possible to revert.
+
+The impact of this will be that logs that were previously available via devtools will now be provided directly to the Wrangler console and it will no longer be possible to interact with the remote Worker via the devtools console.

packages/wrangler/e2e/dev-with-resources.test.ts

@@ -13,7 +13,7 @@ const inspectorPort = await getPort();
 
 const RUNTIMES = [
 	{ flags: "", runtime: "local" },
-	{ flags: "--remote", runtime: "remote" },
+	{ flags: "--remote --x-tail-logs", runtime: "remote" },
 ] as const;
 
 // WebAssembly module containing single `func add(i32, i32): i32` export.
@@ -47,6 +47,9 @@ describe.sequential.each(RUNTIMES)("Core: $flags", ({ runtime, flags }) => {
 				name = "${workerName}"
 				main = "src/index.ts"
 				compatibility_date = "2023-01-01"
+				# TODO: This is a workaround for an EWC bug where remote dev workers only log properly if they have bindings.
+				#       Remove the below line when MR:7727 is merged
+				version_metadata = { binding = "METADATA" }
 			`,
 			"src/index.ts": dedent`
 				export default {
@@ -79,8 +82,10 @@ describe.sequential.each(RUNTIMES)("Core: $flags", ({ runtime, flags }) => {
 			expect(text).toContain("Error: monkey");
 			expect(text).toContain("src/index.ts:7:10");
 		}
-		await worker.readUntil(/Error: monkey/, 30_000);
-		await worker.readUntil(/src\/index\.ts:7:10/, 30_000);
+		await worker.readUntil(/monkey/, 30_000);
+		if (isLocal) {
+			await worker.readUntil(/src\/index\.ts:7:10/, 30_000);
+		}
 	});
 
 	it("works with basic service worker", async () => {
@@ -89,6 +94,9 @@ describe.sequential.each(RUNTIMES)("Core: $flags", ({ runtime, flags }) => {
 				name = "${workerName}"
 				main = "src/index.ts"
 				compatibility_date = "2023-01-01"
+				# TODO: This is a workaround for an EWC bug where remote dev workers only log properly if they have bindings.
+				#       Remove the below line when MR:7727 is merged
+				version_metadata = { binding = "METADATA" }
 			`,
 			"src/index.ts": dedent`
 				addEventListener("fetch", (event) => {
@@ -118,8 +126,10 @@ describe.sequential.each(RUNTIMES)("Core: $flags", ({ runtime, flags }) => {
 			expect(text).toContain("Error: monkey");
 			expect(text).toContain("src/index.ts:6:9");
 		}
-		await worker.readUntil(/Error: monkey/, 30_000);
-		await worker.readUntil(/src\/index\.ts:6:9/, 30_000);
+		await worker.readUntil(/monkey/, 30_000);
+		if (isLocal) {
+			await worker.readUntil(/src\/index\.ts:6:9/, 30_000);
+		}
 	});
 
 	it.todo("workers with no bundle");
@@ -166,30 +176,33 @@ describe.sequential.each(RUNTIMES)("Core: $flags", ({ runtime, flags }) => {
 		});
 	});
 
-	it("starts inspector and allows debugging", async () => {
-		await helper.seed({
-			"wrangler.toml": dedent`
+	it.skipIf(runtime === "remote")(
+		"starts inspector and allows debugging",
+		async () => {
+			await helper.seed({
+				"wrangler.toml": dedent`
 				name = "${workerName}"
 				main = "src/index.ts"
 				compatibility_date = "2023-01-01"
 			`,
-			"src/index.ts": dedent`
+				"src/index.ts": dedent`
 				export default {
 					fetch(request, env, ctx) { return new Response("body"); }
 				}
 			`,
-		});
-		const worker = helper.runLongLived(
-			`wrangler dev ${flags} --port ${port} --inspector-port ${inspectorPort}`
-		);
-		await worker.waitForReady();
-		const inspectorUrl = new URL(`ws://127.0.0.1:${inspectorPort}`);
-		const ws = new WebSocket(inspectorUrl);
-		await events.once(ws, "open");
-		ws.close();
-		// TODO(soon): once we have inspector proxy worker, write basic tests here,
-		//  messages currently too non-deterministic to do this reliably
-	});
+			});
+			const worker = helper.runLongLived(
+				`wrangler dev ${flags} --port ${port} --inspector-port ${inspectorPort}`
+			);
+			await worker.waitForReady();
+			const inspectorUrl = new URL(`ws://127.0.0.1:${inspectorPort}`);
+			const ws = new WebSocket(inspectorUrl);
+			await events.once(ws, "open");
+			ws.close();
+			// TODO(soon): once we have inspector proxy worker, write basic tests here,
+			//  messages currently too non-deterministic to do this reliably
+		}
+	);
 
 	it("starts https server", async () => {
 		await helper.seed({

packages/wrangler/e2e/startWorker.test.ts

@@ -108,6 +108,8 @@ describe.each(OPTIONS)("DevEnv (remote: $remote)", ({ remote }) => {
 		});
 
 		const inspectorUrl = await worker.inspectorUrl;
+		assert(inspectorUrl, "missing inspectorUrl");
+
 		const res = await undici.fetch(`http://${inspectorUrl.host}/json`);
 
 		await expect(res.json()).resolves.toBeInstanceOf(Array);
@@ -177,6 +179,7 @@ describe.each(OPTIONS)("DevEnv (remote: $remote)", ({ remote }) => {
 		});
 
 		const inspectorUrl = await worker.inspectorUrl;
+		assert(inspectorUrl, "missing inspectorUrl");
 
 		let ws = new WebSocket(inspectorUrl.href, {
 			setHost: false,
@@ -236,6 +239,8 @@ describe.each(OPTIONS)("DevEnv (remote: $remote)", ({ remote }) => {
 		});
 
 		const inspectorUrl = await worker.inspectorUrl;
+		assert(inspectorUrl, "missing inspectorUrl");
+
 		const ws = new WebSocket(inspectorUrl.href);
 
 		const consoleApiMessages: DevToolsEvent<"Runtime.consoleAPICalled">[] =

packages/wrangler/src/__tests__/api/startDevWorker/LocalRuntimeController.test.ts

@@ -534,6 +534,7 @@ describe("LocalRuntimeController", () => {
 			});
 			const event = await waitForReloadComplete(controller);
 			const url = urlFromParts(event.proxyData.userWorkerUrl);
+			assert(event.proxyData.userWorkerInspectorUrl);
 			const inspectorUrl = urlFromParts(event.proxyData.userWorkerInspectorUrl);
 
 			// Connect inspector WebSocket

packages/wrangler/src/api/dev.ts

@@ -224,6 +224,7 @@ export async function unstable_dev(
 		experimentalImagesLocalMode: imagesLocalMode ?? false,
 		enableIpc: options?.experimental?.enableIpc,
 		experimentalAssetsRpc: false,
+		experimentalTailLogs: true,
 	};
 
 	//outside of test mode, rebuilds work fine, but only one instance of wrangler will work at a time

packages/wrangler/src/api/startDevWorker/ConfigController.ts

@@ -317,6 +317,9 @@ async function resolveConfig(
 			metadata: input.unsafe?.metadata ?? unsafe?.metadata,
 		},
 		assets: assetsOptions,
+		experimental: {
+			tailLogs: !!input.experimental?.tailLogs,
+		},
 	} satisfies StartDevWorkerOptions;
 
 	if (resolved.legacy.legacyAssets && resolved.legacy.site) {

packages/wrangler/src/api/startDevWorker/ProxyController.ts

@@ -64,7 +64,9 @@ export class ProxyController extends Controller<ProxyControllerEventMap> {
 
 		const cert =
 			this.latestConfig.dev?.server?.secure ||
-			this.latestConfig.dev?.inspector?.secure
+			(this.inspectorEnabled &&
+				this.latestConfig.dev?.inspector &&
+				this.latestConfig.dev?.inspector?.secure)
 				? getHttpsOptions(
 						this.latestConfig.dev.server?.httpsKeyPath,
 						this.latestConfig.dev.server?.httpsCertPath
@@ -108,44 +110,6 @@ export class ProxyController extends Controller<ProxyControllerEventMap> {
 						PROXY_CONTROLLER_AUTH_SECRET: this.secret,
 					},
 
-					// no need to use file-system, so don't
-					cache: false,
-					unsafeEphemeralDurableObjects: true,
-				},
-				{
-					name: "InspectorProxyWorker",
-					compatibilityDate: "2023-12-18",
-					compatibilityFlags: [
-						"nodejs_compat",
-						"increase_websocket_message_size",
-					],
-					modulesRoot: path.dirname(inspectorProxyWorkerPath),
-					modules: [{ type: "ESModule", path: inspectorProxyWorkerPath }],
-					durableObjects: {
-						DURABLE_OBJECT: {
-							className: "InspectorProxyWorker",
-							unsafePreventEviction: true,
-						},
-					},
-					serviceBindings: {
-						PROXY_CONTROLLER: async (req): Promise<Response> => {
-							const body =
-								(await req.json()) as InspectorProxyWorkerOutgoingRequestBody;
-
-							return this.onInspectorProxyWorkerRequest(body);
-						},
-					},
-					bindings: {
-						PROXY_CONTROLLER_AUTH_SECRET: this.secret,
-					},
-
-					unsafeDirectSockets: [
-						{
-							host: this.latestConfig.dev?.inspector?.hostname,
-							port: this.latestConfig.dev?.inspector?.port ?? 0,
-						},
-					],
-
 					// no need to use file-system, so don't
 					cache: false,
 					unsafeEphemeralDurableObjects: true,
@@ -165,6 +129,48 @@ export class ProxyController extends Controller<ProxyControllerEventMap> {
 			liveReload: false,
 		};
 
+		if (this.inspectorEnabled) {
+			assert(this.latestConfig.dev?.inspector);
+			proxyWorkerOptions.workers.push({
+				name: "InspectorProxyWorker",
+				compatibilityDate: "2023-12-18",
+				compatibilityFlags: [
+					"nodejs_compat",
+					"increase_websocket_message_size",
+				],
+				modulesRoot: path.dirname(inspectorProxyWorkerPath),
+				modules: [{ type: "ESModule", path: inspectorProxyWorkerPath }],
+				durableObjects: {
+					DURABLE_OBJECT: {
+						className: "InspectorProxyWorker",
+						unsafePreventEviction: true,
+					},
+				},
+				serviceBindings: {
+					PROXY_CONTROLLER: async (req): Promise<Response> => {
+						const body =
+							(await req.json()) as InspectorProxyWorkerOutgoingRequestBody;
+
+						return this.onInspectorProxyWorkerRequest(body);
+					},
+				},
+				bindings: {
+					PROXY_CONTROLLER_AUTH_SECRET: this.secret,
+				},
+
+				unsafeDirectSockets: [
+					{
+						host: this.latestConfig.dev?.inspector?.hostname,
+						port: this.latestConfig.dev?.inspector?.port ?? 0,
+					},
+				],
+
+				// no need to use file-system, so don't
+				cache: false,
+				unsafeEphemeralDurableObjects: true,
+			});
+		}
+
 		const proxyWorkerOptionsChanged = didMiniflareOptionsChange(
 			this.proxyWorkerOptions,
 			proxyWorkerOptions
@@ -193,9 +199,14 @@ export class ProxyController extends Controller<ProxyControllerEventMap> {
 		if (willInstantiateMiniflareInstance) {
 			void Promise.all([
 				proxyWorker.ready,
-				proxyWorker.unsafeGetDirectURL("InspectorProxyWorker"),
+				!this.inspectorEnabled
+					? Promise.resolve(undefined)
+					: proxyWorker.unsafeGetDirectURL("InspectorProxyWorker"),
 			])
 				.then(([url, inspectorUrl]) => {
+					if (!this.inspectorEnabled) {
+						return [url, undefined];
+					}
 					// Don't connect the inspector proxy worker until we have a valid ready Miniflare instance.
 					// Otherwise, tearing down the ProxyController immediately after setting it up
 					// will result in proxyWorker.ready throwing, but reconnectInspectorProxyWorker hanging for ever,
@@ -206,6 +217,7 @@ export class ProxyController extends Controller<ProxyControllerEventMap> {
 					]);
 				})
 				.then(([url, inspectorUrl]) => {
+					assert(url);
 					this.emitReadyEvent(proxyWorker, url, inspectorUrl);
 				})
 				.catch((error) => {
@@ -367,6 +379,14 @@ export class ProxyController extends Controller<ProxyControllerEventMap> {
 		}
 	}
 
+	get inspectorEnabled() {
+		if (this.latestConfig?.dev.remote) {
+			// In `wrangler dev --remote`, only enable the inspector if the `--x-tail-logs` flag is disabled
+			return !this.latestConfig?.experimental?.tailLogs;
+		}
+		return true;
+	}
+
 	// ******************
 	//   Event Handlers
 	// ******************
@@ -386,7 +406,9 @@ export class ProxyController extends Controller<ProxyControllerEventMap> {
 		this.latestConfig = data.config;
 
 		void this.sendMessageToProxyWorker({ type: "pause" });
-		void this.sendMessageToInspectorProxyWorker({ type: "reloadStart" });
+		if (this.inspectorEnabled) {
+			void this.sendMessageToInspectorProxyWorker({ type: "reloadStart" });
+		}
 	}
 	onReloadComplete(data: ReloadCompleteEvent) {
 		this.latestConfig = data.config;
@@ -397,10 +419,12 @@ export class ProxyController extends Controller<ProxyControllerEventMap> {
 			proxyData: data.proxyData,
 		});
 
-		void this.sendMessageToInspectorProxyWorker({
-			type: "reloadComplete",
-			proxyData: data.proxyData,
-		});
+		if (this.inspectorEnabled) {
+			void this.sendMessageToInspectorProxyWorker({
+				type: "reloadComplete",
+				proxyData: data.proxyData,
+			});
+		}
 	}
 	onProxyWorkerMessage(message: ProxyWorkerOutgoingRequestBody) {
 		switch (message.type) {
@@ -522,7 +546,11 @@ export class ProxyController extends Controller<ProxyControllerEventMap> {
 	//   Event Dispatchers
 	// *********************
 
-	emitReadyEvent(proxyWorker: Miniflare, url: URL, inspectorUrl: URL) {
+	emitReadyEvent(
+		proxyWorker: Miniflare,
+		url: URL,
+		inspectorUrl: URL | undefined
+	) {
 		const data: ReadyEvent = {
 			type: "ready",
 			proxyWorker,