Improve asset-worker tracing and add tracing into router-worker (#7887)

* Add more spans into asset-worker fetch handling

* Add tracing observability into router-worker

.changeset/nice-mice-mix.md

@@ -0,0 +1,5 @@
+---
+"@cloudflare/workers-shared": patch
+---
+
+chore: add more observability into asset-worker for the Workers team to better insight into how requests are handled.

.changeset/plenty-guests-look.md

@@ -0,0 +1,5 @@
+---
+"@cloudflare/workers-shared": patch
+---
+
+chore: add tracing into router-worker so the Workers team can have a better insight into into how requests are handled.

packages/workers-shared/asset-worker/src/handler.ts

@@ -27,15 +27,30 @@ export const handleRequest = async (
 	const intent = await getIntent(decodedPathname, configuration, exists);
 
 	if (!intent) {
-		return new NotFoundResponse();
+		return env.JAEGER.enterSpan("no_intent", (span) => {
+			span.setTags({
+				decodedPathname,
+				configuration: JSON.stringify(configuration),
+				status: 404,
+			});
+
+			return new NotFoundResponse();
+		});
 	}
 
 	// if there was a POST etc. to a route without an asset
 	// this should be passed onto a user worker if one exists
 	// so prioritise returning a 404 over 405?
 	const method = request.method.toUpperCase();
 	if (!["GET", "HEAD"].includes(method)) {
-		return new MethodNotAllowedResponse();
+		return env.JAEGER.enterSpan("method_not_allowed", (span) => {
+			span.setTags({
+				method,
+				status: 405,
+			});
+
+			return new MethodNotAllowedResponse();
+		});
 	}
 
 	const decodedDestination = intent.redirect ?? decodedPathname;
@@ -47,11 +62,29 @@ export const handleRequest = async (
 	 * We combine this with other redirects (e.g. for html_handling) to avoid multiple redirects.
 	 */
 	if ((encodedDestination !== pathname && intent.asset) || intent.redirect) {
-		return new TemporaryRedirectResponse(encodedDestination + search);
+		return env.JAEGER.enterSpan("redirect", (span) => {
+			span.setTags({
+				originalPath: pathname,
+				location:
+					encodedDestination !== pathname
+						? encodedDestination
+						: intent.redirect ?? "<unknown>",
+				status: 307,
+			});
+
+			return new TemporaryRedirectResponse(encodedDestination + search);
+		});
 	}
 
 	if (!intent.asset) {
-		return new InternalServerErrorResponse(new Error("Unknown action"));
+		return env.JAEGER.enterSpan("unknown_action", (span) => {
+			span.setTags({
+				pathname,
+				status: 500,
+			});
+
+			return new InternalServerErrorResponse(new Error("Unknown action"));
+		});
 	}
 
 	const asset = await env.JAEGER.enterSpan("getByETag", async (span) => {
@@ -70,16 +103,31 @@ export const handleRequest = async (
 	const weakETag = `W/${strongETag}`;
 	const ifNoneMatch = request.headers.get("If-None-Match") || "";
 	if ([weakETag, strongETag].includes(ifNoneMatch)) {
-		return new NotModifiedResponse(null, { headers });
-	}
+		return env.JAEGER.enterSpan("matched_etag", (span) => {
+			span.setTags({
+				matchedEtag: ifNoneMatch,
+				status: 304,
+			});
 
-	const body = method === "HEAD" ? null : asset.readableStream;
-	switch (intent.asset.status) {
-		case 404:
-			return new NotFoundResponse(body, { headers });
-		case 200:
-			return new OkResponse(body, { headers });
+			return new NotModifiedResponse(null, { headers });
+		});
 	}
+
+	return env.JAEGER.enterSpan("response", (span) => {
+		span.setTags({
+			etag: intent.asset.eTag,
+			status: intent.asset.status,
+			head: method === "HEAD",
+		});
+
+		const body = method === "HEAD" ? null : asset.readableStream;
+		switch (intent.asset.status) {
+			case 404:
+				return new NotFoundResponse(body, { headers });
+			case 200:
+				return new OkResponse(body, { headers });
+		}
+	});
 };
 
 type Intent =
@@ -90,6 +138,7 @@ type Intent =
 	| { asset: null; redirect: string }
 	| null;
 
+// TODO: Trace this
 export const getIntent = async (
 	pathname: string,
 	configuration: Required<AssetConfig>,

packages/workers-shared/asset-worker/src/index.ts

@@ -1,13 +1,13 @@
 import { WorkerEntrypoint } from "cloudflare:workers";
 import { PerformanceTimer } from "../../utils/performance";
 import { setupSentry } from "../../utils/sentry";
+import { mockJaegerBinding } from "../../utils/tracing";
 import { Analytics } from "./analytics";
 import { AssetsManifest } from "./assets-manifest";
 import { applyConfigurationDefaults } from "./configuration";
 import { decodePath, getIntent, handleRequest } from "./handler";
 import { InternalServerErrorResponse } from "./responses";
 import { getAssetWithMetadataFromKV } from "./utils/kv";
-import { mockJaegerBinding } from "./utils/mocks";
 import type {
 	AssetWorkerConfig,
 	ColoMetadata,
@@ -164,6 +164,7 @@ export default class extends WorkerEntrypoint<Env> {
 		}
 	}
 
+	// TODO: Trace unstable methods
 	async unstable_canFetch(request: Request): Promise<boolean> {
 		const url = new URL(request.url);
 		const decodedPathname = decodePath(url.pathname);

packages/workers-shared/asset-worker/tests/handler.test.ts

@@ -1,7 +1,7 @@
 import { vi } from "vitest";
+import { mockJaegerBinding } from "../../utils/tracing";
 import { applyConfigurationDefaults } from "../src/configuration";
 import { handleRequest } from "../src/handler";
-import { mockJaegerBinding } from "../src/utils/mocks";
 import type { AssetConfig } from "../../utils/types";
 
 describe("[Asset Worker] `handleRequest`", () => {
@@ -222,7 +222,7 @@ describe("[Asset Worker] `handleRequest`", () => {
 		const response2 = await handleRequest(
 			new Request("https://example.com/%A0%A0"),
 			// @ts-expect-error Empty config default to using mocked jaeger
-			{},
+			mockEnv,
 			configuration,
 			exists,
 			getByEtag

packages/workers-shared/router-worker/src/index.ts

@@ -1,8 +1,13 @@
 import { PerformanceTimer } from "../../utils/performance";
 import { setupSentry } from "../../utils/sentry";
+import { mockJaegerBinding } from "../../utils/tracing";
 import { Analytics, DISPATCH_TYPE } from "./analytics";
 import type AssetWorker from "../../asset-worker/src/index";
-import type { RoutingConfig, UnsafePerformanceTimer } from "../../utils/types";
+import type {
+	JaegerTracing,
+	RoutingConfig,
+	UnsafePerformanceTimer,
+} from "../../utils/types";
 import type { ColoMetadata, Environment, ReadyAnalytics } from "./types";
 
 interface Env {
@@ -16,6 +21,7 @@ interface Env {
 	COLO_METADATA: ColoMetadata;
 	UNSAFE_PERFORMANCE: UnsafePerformanceTimer;
 	VERSION_METADATA: WorkerVersionMetadata;
+	JAEGER: JaegerTracing;
 
 	SENTRY_ACCESS_CLIENT_ID: string;
 	SENTRY_ACCESS_CLIENT_SECRET: string;
@@ -29,6 +35,10 @@ export default {
 		const startTimeMs = performance.now();
 
 		try {
+			if (!env.JAEGER) {
+				env.JAEGER = mockJaegerBinding();
+			}
+
 			sentry = setupSentry(
 				request,
 				ctx,
@@ -63,27 +73,48 @@ export default {
 			// User's configuration indicates they want user-Worker to run ahead of any
 			// assets. Do not provide any fallback logic.
 			if (env.CONFIG.invoke_user_worker_ahead_of_assets) {
-				if (!env.CONFIG.has_user_worker) {
-					throw new Error(
-						"Fetch for user worker without having a user worker binding"
-					);
-				}
-				return env.USER_WORKER.fetch(maybeSecondRequest);
+				return env.JAEGER.enterSpan("invoke_user_worker_first", async () => {
+					if (!env.CONFIG.has_user_worker) {
+						throw new Error(
+							"Fetch for user worker without having a user worker binding"
+						);
+					}
+					return env.USER_WORKER.fetch(maybeSecondRequest);
+				});
 			}
 
 			// Otherwise, we try to first fetch assets, falling back to user-Worker.
 			if (env.CONFIG.has_user_worker) {
-				if (await env.ASSET_WORKER.unstable_canFetch(request)) {
-					analytics.setData({ dispatchtype: DISPATCH_TYPE.ASSETS });
-					return env.ASSET_WORKER.fetch(maybeSecondRequest);
-				} else {
-					analytics.setData({ dispatchtype: DISPATCH_TYPE.WORKER });
-					return env.USER_WORKER.fetch(maybeSecondRequest);
-				}
+				return env.JAEGER.enterSpan("has_user_worker", async (span) => {
+					if (await env.ASSET_WORKER.unstable_canFetch(request)) {
+						span.setTags({
+							asset: true,
+							dispatchType: DISPATCH_TYPE.ASSETS,
+						});
+
+						analytics.setData({ dispatchtype: DISPATCH_TYPE.ASSETS });
+						return env.ASSET_WORKER.fetch(maybeSecondRequest);
+					} else {
+						span.setTags({
+							asset: false,
+							dispatchType: DISPATCH_TYPE.WORKER,
+						});
+
+						analytics.setData({ dispatchtype: DISPATCH_TYPE.WORKER });
+						return env.USER_WORKER.fetch(maybeSecondRequest);
+					}
+				});
 			}
 
-			analytics.setData({ dispatchtype: DISPATCH_TYPE.ASSETS });
-			return env.ASSET_WORKER.fetch(request);
+			return env.JAEGER.enterSpan("assets_only", async (span) => {
+				span.setTags({
+					asset: true,
+					dispatchType: DISPATCH_TYPE.ASSETS,
+				});
+
+				analytics.setData({ dispatchtype: DISPATCH_TYPE.ASSETS });
+				return env.ASSET_WORKER.fetch(request);
+			});
 		} catch (err) {
 			if (err instanceof Error) {
 				analytics.setData({ error: err.message });

packages/workers-shared/asset-worker/src/utils/mocks.ts → packages/workers-shared/utils/tracing.ts

@@ -1,4 +1,4 @@
-import type { JaegerTracing, Span } from "../../../utils/types";
+import type { JaegerTracing, Span } from "./types";
 
 export function mockJaegerBindingSpan(): Span {
 	return {