WO-250 add workers-observability logs configuration (#7173)

* WO-250 add workers-observability logs configuration * WO-250 correctly validate nested observability configuration * run lint fix * WO-250 addressing PR feedback to avoid mutation of argument --------- Co-authored-by: Thomas Ankcorn <[email protected]>
cloudflare · Nov 14, 2024 · b6cbfbd · b6cbfbd
1 parent b100713
commit b6cbfbd
Show file tree

Hide file tree

Showing 6 changed files with 204 additions and 8 deletions.
diff --git a/.changeset/clever-grapes-sparkle.md b/.changeset/clever-grapes-sparkle.md
@@ -0,0 +1,15 @@
+---
+"wrangler": minor
+---
+
+Adds [observability.logs] settings to wrangler. This setting lets developers control the settings for logs as an independent dataset enabling more dataset types in the future. The most specific setting will win if any of the datasets are not enabled.
+
+It also adds the following setting to the logs config
+
+- `invocation_logs` - set to false to disable invocation logs. Defaults to true.
+
+```toml
+[observability.logs]
+enabled = true
+invocation_logs = false
+```
diff --git a/packages/wrangler/src/__tests__/configuration.test.ts b/packages/wrangler/src/__tests__/configuration.test.ts
@@ -1167,6 +1167,7 @@ describe("normalizeAndValidateConfig()", () => {
 				  - Expected \\"logpush\\" to be of type boolean but got \\"INVALID\\".
 				  - Expected \\"upload_source_maps\\" to be of type boolean but got \\"INVALID\\".
 				  - Expected \\"observability.enabled\\" to be of type boolean but got \\"INVALID\\".
+				  - Expected \\"observability.logs.enabled\\" to be of type boolean but got undefined.
 				  - Expected \\"observability.head_sampling_rate\\" to be of type number but got \\"INVALID\\"."
 			`);
 		});
@@ -5485,11 +5486,57 @@ describe("normalizeAndValidateConfig()", () => {
 				expect(diagnostics.hasErrors()).toBe(true);
 				expect(diagnostics.renderErrors()).toMatchInlineSnapshot(`
 					"Processing wrangler configuration:
-					  - \\"observability.enabled\\" is a required field.
+					  - \\"observability.enabled\\" or \\"observability.logs.enabled\\" is required.
 					  - Expected \\"observability.head_sampling_rate\\" to be of type number but got true."
 				`);
 			});
 
+			it("should error on invalid observability.logs", () => {
+				const { diagnostics } = normalizeAndValidateConfig(
+					{
+						observability: {
+							enabled: true,
+							logs: "enabled",
+						},
+					} as unknown as RawConfig,
+					undefined,
+					{ env: undefined }
+				);
+
+				expect(diagnostics.hasWarnings()).toBe(false);
+				expect(diagnostics.renderWarnings()).toMatchInlineSnapshot(`
+					"Processing wrangler configuration:
+					"
+				`);
+
+				expect(diagnostics.hasErrors()).toBe(true);
+				expect(diagnostics.renderErrors()).toMatchInlineSnapshot(`
+					"Processing wrangler configuration:
+					  - Expected \\"observability.logs\\" to be of type object but got \\"enabled\\"."
+				`);
+			});
+
+			it("should not error on nested [observability.logs] config only", () => {
+				const { diagnostics } = normalizeAndValidateConfig(
+					{
+						observability: {
+							logs: {
+								enabled: true,
+							},
+						},
+					} as unknown as RawConfig,
+					undefined,
+					{ env: undefined }
+				);
+
+				expect(diagnostics.hasWarnings()).toBe(false);
+				expect(diagnostics.renderWarnings()).toMatchInlineSnapshot(`
+					"Processing wrangler configuration:
+					"
+				`);
+
+				expect(diagnostics.hasErrors()).toBe(false);
+			});
 			it("should error on a sampling rate out of range", () => {
 				const { diagnostics } = normalizeAndValidateConfig(
 					{

diff --git a/packages/wrangler/src/__tests__/deploy.test.ts b/packages/wrangler/src/__tests__/deploy.test.ts
@@ -11420,6 +11420,43 @@ export default{
 			`);
 		});
 
+		it("should allow uploading workers with nested observability logs setting", async () => {
+			writeWranglerToml({
+				observability: {
+					enabled: true,
+					head_sampling_rate: 0.5,
+					logs: {
+						enabled: true,
+						head_sampling_rate: 0.3,
+						invocation_logs: false,
+					},
+				},
+			});
+			await fs.promises.writeFile("index.js", `export default {};`);
+			mockSubDomainRequest();
+			mockUploadWorkerRequest({
+				expectedObservability: {
+					enabled: true,
+					head_sampling_rate: 0.5,
+					logs: {
+						enabled: true,
+						head_sampling_rate: 0.3,
+						invocation_logs: 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"
+			`);
+		});
+
 		it("should disable observability if not explicitly defined", async () => {
 			writeWranglerToml({});
 			await fs.promises.writeFile("index.js", `export default {};`);

diff --git a/packages/wrangler/src/config/environment.ts b/packages/wrangler/src/config/environment.ts
@@ -946,4 +946,11 @@ export interface Observability {
 	enabled: boolean;
 	/** The sampling rate */
 	head_sampling_rate?: number;
+	logs?: {
+		enabled: boolean;
+		/** The sampling rate */
+		head_sampling_rate?: number;
+		/** Set to false to disable invocation logs */
+		invocation_logs?: boolean;
+	};
 }
diff --git a/packages/wrangler/src/config/validation-helpers.ts b/packages/wrangler/src/config/validation-helpers.ts
@@ -434,6 +434,43 @@ export const validateRequiredProperty = (
 	return true;
 };
 
+/**
+ * Validate that at least one of the properties in the list is required.
+ */
+export const validateAtLeastOnePropertyRequired = (
+	diagnostics: Diagnostics,
+	container: string,
+	properties: {
+		key: string;
+		value: unknown;
+		type: TypeofType;
+	}[]
+): boolean => {
+	const containerPath = container ? `${container}.` : "";
+
+	if (properties.every((property) => property.value === undefined)) {
+		diagnostics.errors.push(
+			`${properties.map(({ key }) => `"${containerPath}${key}"`).join(" or ")} is required.`
+		);
+		return false;
+	}
+
+	const errors = [];
+	for (const prop of properties) {
+		if (typeof prop.value === prop.type) {
+			return true;
+		}
+		errors.push(
+			`Expected "${containerPath}${prop.key}" to be of type ${prop.type} but got ${JSON.stringify(
+				prop.value
+			)}.`
+		);
+	}
+
+	diagnostics.errors.push(...errors);
+	return false;
+};
+
 /**
  * Validate that, if the optional field exists, then it has the expected type.
  */

diff --git a/packages/wrangler/src/config/validation.ts b/packages/wrangler/src/config/validation.ts
@@ -24,6 +24,7 @@ import {
 	isValidName,
 	notInheritable,
 	validateAdditionalProperties,
+	validateAtLeastOnePropertyRequired,
 	validateOptionalProperty,
 	validateOptionalTypedArray,
 	validateRequiredProperty,
@@ -3341,14 +3342,22 @@ const validateObservability: ValidatorFn = (diagnostics, field, value) => {
 	const val = value as Observability;
 	let isValid = true;
 
+	/**
+	 * One of observability.enabled or observability.logs.enabled must be defined
+	 */
 	isValid =
-		validateRequiredProperty(
-			diagnostics,
-			field,
-			"enabled",
-			val.enabled,
-			"boolean"
-		) && isValid;
+		validateAtLeastOnePropertyRequired(diagnostics, field, [
+			{
+				key: "enabled",
+				value: val.enabled,
+				type: "boolean",
+			},
+			{
+				key: "logs.enabled",
+				value: val.logs?.enabled,
+				type: "boolean",
+			},
+		]) && isValid;
 
 	isValid =
 		validateOptionalProperty(
@@ -3359,12 +3368,56 @@ const validateObservability: ValidatorFn = (diagnostics, field, value) => {
 			"number"
 		) && isValid;
 
+	isValid =
+		validateOptionalProperty(diagnostics, field, "logs", val.logs, "object") &&
+		isValid;
+
 	isValid =
 		validateAdditionalProperties(diagnostics, field, Object.keys(val), [
 			"enabled",
 			"head_sampling_rate",
+			"logs",
 		]) && isValid;
 
+	/**
+	 * Validate the optional nested logs configuration
+	 */
+	if (typeof val.logs === "object") {
+		isValid =
+			validateRequiredProperty(
+				diagnostics,
+				field,
+				"logs.enabled",
+				val.logs.enabled,
+				"boolean"
+			) && isValid;
+
+		isValid =
+			validateOptionalProperty(
+				diagnostics,
+				field,
+				"logs.head_sampling_rate",
+				val.logs.head_sampling_rate,
+				"number"
+			) && isValid;
+
+		isValid =
+			validateOptionalProperty(
+				diagnostics,
+				field,
+				"logs.invocation_logs",
+				val.logs.invocation_logs,
+				"boolean"
+			) && isValid;
+
+		isValid =
+			validateAdditionalProperties(diagnostics, field, Object.keys(val.logs), [
+				"enabled",
+				"head_sampling_rate",
+				"invocation_logs",
+			]) && isValid;
+	}
+
 	const samplingRate = val?.head_sampling_rate;
 
 	if (samplingRate && (samplingRate < 0 || samplingRate > 1)) {