Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: disable observability on deploy if not explicitly defined in config #6776

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading