Skip to content

Commit 2f56dcf

Browse files
committed
fix: sync observability on versions deploy
In addition to logpush and tail_consumers, patch observability settings on version deploy. This still doesn't quite match the behavior of "wrangler deploy", which will disable observability if it is not defined in wrangler.toml (as of #6776), but at least this is more correct for now.
1 parent 924ec18 commit 2f56dcf

File tree

3 files changed

+85
-3
lines changed

3 files changed

+85
-3
lines changed

.changeset/chilled-apes-repeat.md

+7
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
---
2+
"wrangler": patch
3+
---
4+
5+
fix: synchronize observability settings during `wrangler versions deploy`
6+
7+
When running `wrangler versions deploy`, Wrangler will now update `observability` settings in addition to `logpush` and `tail_consumers`. Unlike `wrangler deploy`, it will not disable observability when `observability` is undefined in `wrangler.toml`.

packages/wrangler/src/__tests__/versions/versions.deploy.test.ts

+65-2
Original file line numberDiff line numberDiff line change
@@ -704,17 +704,78 @@ describe("versions deploy", () => {
704704
705705
│ Synced non-versioned settings:
706706
│ logpush: true
707+
│ observability: <skipped>
707708
│ tail_consumers: <skipped>
708709
709710
╰ SUCCESS Deployed test-name version 00000000-0000-0000-0000-000000000000 at 100% (TIMINGS)"
710711
`);
712+
});
713+
714+
test("with observability disabled in wrangler.toml", async () => {
715+
writeWranglerToml({
716+
observability: {
717+
enabled: false,
718+
},
719+
});
711720

712-
expect(normalizeOutput(std.out)).toContain("logpush: true");
721+
const result = runWrangler(
722+
"versions deploy 10000000-0000-0000-0000-000000000000 --yes --experimental-gradual-rollouts"
723+
);
724+
725+
await expect(result).resolves.toBeUndefined();
726+
727+
expect(normalizeOutput(std.out)).toMatchInlineSnapshot(`
728+
"╭ Deploy Worker Versions by splitting traffic between multiple versions
729+
730+
├ Fetching latest deployment
731+
732+
├ Your current deployment has 2 version(s):
733+
734+
│ (10%) 00000000-0000-0000-0000-000000000000
735+
│ Created: TIMESTAMP
736+
│ Tag: -
737+
│ Message: -
738+
739+
│ (90%) 00000000-0000-0000-0000-000000000000
740+
│ Created: TIMESTAMP
741+
│ Tag: -
742+
│ Message: -
743+
744+
├ Fetching deployable versions
745+
746+
├ Which version(s) do you want to deploy?
747+
├ 1 Worker Version(s) selected
748+
749+
├ Worker Version 1: 00000000-0000-0000-0000-000000000000
750+
│ Created: TIMESTAMP
751+
│ Tag: -
752+
│ Message: -
753+
754+
├ What percentage of traffic should Worker Version 1 receive?
755+
├ 100% of traffic
756+
757+
├ Add a deployment message (skipped)
758+
759+
├ Deploying 1 version(s)
760+
761+
├ Syncing non-versioned settings
762+
763+
│ Synced non-versioned settings:
764+
│ logpush: <skipped>
765+
│ observability: enabled: false
766+
│ tail_consumers: <skipped>
767+
768+
╰ SUCCESS Deployed test-name version 00000000-0000-0000-0000-000000000000 at 100% (TIMINGS)"
769+
`);
713770
});
714771

715-
test("with logpush and tail_consumers in wrangler.toml", async () => {
772+
test("with logpush, tail_consumers, and observability in wrangler.toml", async () => {
716773
writeWranglerToml({
717774
logpush: false,
775+
observability: {
776+
enabled: true,
777+
head_sampling_rate: 0.5,
778+
},
718779
tail_consumers: [
719780
{ service: "worker-1" },
720781
{ service: "worker-2", environment: "preview" },
@@ -766,6 +827,8 @@ describe("versions deploy", () => {
766827
767828
│ Synced non-versioned settings:
768829
│ logpush: false
830+
│ observability: enabled: true
831+
│ head_sampling_rate: 0.5
769832
│ tail_consumers: worker-1
770833
│ worker-2 (preview)
771834
│ worker-3 (staging)

packages/wrangler/src/versions/deploy.ts

+13-1
Original file line numberDiff line numberDiff line change
@@ -558,11 +558,12 @@ async function promptPercentages(
558558
async function maybePatchSettings(
559559
accountId: string,
560560
workerName: string,
561-
config: Pick<Config, "logpush" | "tail_consumers">
561+
config: Pick<Config, "logpush" | "tail_consumers" | "observability">
562562
) {
563563
const maybeUndefinedSettings = {
564564
logpush: config.logpush,
565565
tail_consumers: config.tail_consumers,
566+
observability: config.observability, // TODO reconcile with how regular deploy handles empty state
566567
};
567568
const definedSettings = Object.fromEntries(
568569
Object.entries(maybeUndefinedSettings).filter(
@@ -587,9 +588,20 @@ async function maybePatchSettings(
587588
},
588589
});
589590

591+
const observability = [];
592+
if (patchedSettings.observability) {
593+
observability.push(`enabled: ${patchedSettings.observability.enabled}`);
594+
if (patchedSettings.observability.head_sampling_rate) {
595+
observability.push(
596+
`head_sampling_rate: ${patchedSettings.observability.head_sampling_rate}`
597+
);
598+
}
599+
}
590600
const formattedSettings = formatLabelledValues(
591601
{
592602
logpush: String(patchedSettings.logpush ?? "<skipped>"),
603+
observability:
604+
observability.length > 0 ? observability.join("\n") : "<skipped>",
593605
tail_consumers:
594606
patchedSettings.tail_consumers
595607
?.map((tc) =>

0 commit comments

Comments
 (0)