Skip to content

Commit

Permalink
fix: disable observability on deploy if not explicitly defined in config
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
zebp committed Sep 19, 2024
1 parent 7e0d83d commit 76d4929
Show file tree
Hide file tree
Showing 5 changed files with 48 additions and 4 deletions.
7 changes: 7 additions & 0 deletions .changeset/old-mugs-begin.md
Original file line number Diff line number Diff line change
@@ -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.
23 changes: 23 additions & 0 deletions packages/wrangler/src/__tests__/deploy.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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"
`);
});
});
});

Expand Down
13 changes: 12 additions & 1 deletion packages/wrangler/src/__tests__/helpers/mock-upload-worker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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";

Expand Down Expand Up @@ -35,6 +36,7 @@ export function mockUploadWorkerRequest(
};
useOldUploadApi?: boolean;
expectedObservability?: CfWorkerInit["observability"];
expectedSettingsPatch?: Partial<NonVersionedScriptSettings>;
} = {}
) {
const expectedScriptName = (options.expectedScriptName ??= "test-name");
Expand Down Expand Up @@ -178,6 +180,7 @@ export function mockUploadWorkerRequest(
expectedDispatchNamespace,
useOldUploadApi,
expectedObservability,
expectedSettingsPatch,
} = options;
if (env && !legacyEnv) {
msw.use(
Expand Down Expand Up @@ -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({}));
}
)
);
}
Expand Down
7 changes: 5 additions & 2 deletions packages/wrangler/src/deploy/deploy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<{
Expand Down
2 changes: 1 addition & 1 deletion packages/wrangler/src/versions/api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ export async function createDeployment(
);
}

type NonVersionedScriptSettings = {
export type NonVersionedScriptSettings = {
logpush: boolean;
tail_consumers: TailConsumer[];
observability: Observability;
Expand Down

0 comments on commit 76d4929

Please sign in to comment.