Skip to content

Commit

Permalink
fix: disable observability on deploy if not explicitly defined in con…
Browse files Browse the repository at this point in the history
…fig (#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.
  • Loading branch information
zebp authored Sep 19, 2024
1 parent b45e326 commit 02de103
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 02de103

Please sign in to comment.