Add EdgeZero canary routing (rollout_pct flag + FNV-1a bucket assignment) (PR 19)

[prk-Jr]

Summary

• Adds edgezero_rollout_pct (0–100) to the trusted_server_config store so EdgeZero traffic can be shifted incrementally without a deploy
• Uses FNV-1a 32-bit hash of client_ip for deterministic, sticky per-user bucket assignment (0–99); routing predicate is bucket < rollout_pct
• Fail-safe defaults: read error or invalid value → 0 (all legacy); absent key (backward compat) → 100 (all EdgeZero); rollout_pct = 0 is the immediate no-deploy rollback

Changes

File	Change
crates/trusted-server-adapter-fastly/src/main.rs	Added parse_rollout_pct, fnv1a_bucket, canary_routes_to_edgezero, read_rollout_pct; updated main() with canary dispatch; 11 unit tests (pinned FNV-1a vectors, distribution, boundary routing)
fastly.toml	Added edgezero_rollout_pct = "0" to local Viceroy config store with ops comment
docs/internal/EDGEZERO_MIGRATION.md	New ops runbook: config store keys, failure modes, pre-flight activation, 4 canary stages, pass/fail thresholds, rollback procedure, monitoring

Closes

Closes #500

Test plan

• cargo test-fastly — 65 + 956 + 21 + 3 tests green
• cargo test-axum — 31 tests green
• cargo clippy-fastly && cargo clippy-axum — no warnings
• cargo fmt --all -- --check — clean
• JS tests: cd crates/js/lib && npx vitest run — 291 tests green
• JS format: cd crates/js/lib && npm run format — clean
• Docs format: cd docs && npm run format — clean
• WASM build: cargo build --package trusted-server-adapter-fastly --release --target wasm32-wasip1
• Manual testing via fastly compute serve with edgezero_rollout_pct = "1" and "0" (rollback)

Checklist

• Changes follow CLAUDE.md conventions
• No unwrap() in production code — use expect("should ...")
• Uses log macros (not println!)
• New code has tests
• No secrets or credentials committed

[ChristianPavilonis]

Summary

One low-severity docs/ops note: the canary runbook references routing log lines that are emitted at debug level, so production verification may require a debug/log-level deployment or equivalent observability setup.

[ChristianPavilonis]

Automated review:

Review Summary

Reviewed PR #727 against feature/edgezero-pr19-spin-adapter, focusing on the Fastly entry-point canary routing, config-store failure behavior, rollout docs, and CI/review context. I did not find any blocking correctness, security, data-loss, authorization, or severe compatibility issues. I left one P2 inline comment about strengthening coverage for the safety-critical entry-point dispatch matrix.

Findings

P0 / Blockers

None.

P1 / High

None.

P2 / Medium

1 inline comment submitted.

P3 / Low

None.

CI / Existing Reviews

All reported PR checks are passing. Existing inline feedback about absent-key logging, config-store branch tests, degenerate rollout hashing, naming, and debug-level log documentation appears to have been addressed in later commits. Local cargo test-fastly rollout -- --nocapture could not run in this environment because Viceroy failed to satisfy fastly_http_downstream::downstream_bot_analyzed; CI's Fastly job is green.

[ChristianPavilonis]

Automated review:

Review Summary

Reviewed PR #727 against feature/edgezero-pr19-spin-adapter, focusing on the Fastly canary dispatch, rollout percentage parsing/defaults, config-store failure behavior, tests, local Fastly config, and the migration runbook. I did not find any blocking correctness, security, data-loss, authorization, or severe compatibility issues. I left one P2 inline comment about making the production canary observability path explicit before operators rely on the runbook.

Findings

P0 / Blockers

None.

P1 / High

None.

P2 / Medium

1 inline comment submitted.

P3 / Low

None.

CI / Existing Reviews

All reported PR checks are passing. Existing review feedback about absent-key log volume, degenerate rollout hashing, config-store branch coverage, naming, and debug-level log documentation appears addressed by later commits. Local cargo test-fastly rollout -- --nocapture compiled the changed Fastly test binary but could not execute under the installed Viceroy because it lacks fastly_http_downstream::downstream_bot_analyzed; CI's Fastly test job is green.

[ChristianPavilonis]

Automated review:

Review Summary

Reviewed PR #727 against feature/edgezero-pr19-spin-adapter, focusing on the Fastly canary dispatch, rollout percentage parsing/defaults, config-store failure behavior, local Fastly config, tests, and the EdgeZero migration runbook. I did not find any blocking correctness, security, data-loss, authorization, or severe compatibility issues. I left one P2 inline comment about a runbook validation signal that is not actually emitted with the current logger configuration.

Findings

P0 / Blockers

None.

P1 / High

None.

P2 / Medium

1 inline comment submitted.

P3 / Low

None.

CI / Existing Reviews

All reported PR checks are passing. Existing review feedback about absent-key logging, degenerate rollout hashing, config-store branch coverage, dispatch-matrix tests, naming, and production route observability appears addressed in later commits. Local cargo test-fastly rollout -- --nocapture compiled the Fastly test binary but could not execute under the installed Viceroy 0.16.4 because it lacks fastly_http_downstream::downstream_bot_analyzed; CI's Fastly job is green.

[ChristianPavilonis]

Automated review:

Review Summary

Reviewed PR #727 against feature/edgezero-pr19-spin-adapter, focusing on the Fastly canary routing path, rollout percentage parsing/fallbacks, logger-level changes, tests, local config, and EdgeZero migration documentation. I did not find any blocking correctness, security, data-loss, authorization, or severe compatibility issues. I left one P2 inline comment about stale operational guidance duplicated in the implementation plan.

Findings

P0 / Blockers

None.

P1 / High

None.

P2 / Medium

1 inline comment submitted.

P3 / Low

None.

CI / Existing Reviews

All reported PR checks are passing. Existing automated-review feedback about absent-key log volume, config-store branch coverage, degenerate rollout hashing, dispatch-matrix tests, production route observability, and local Viceroy debug logs appears addressed in later commits. Local cargo test -p trusted-server-adapter-fastly --target wasm32-wasip1 rollout_pct --no-run passed; a native cargo test -p trusted-server-adapter-fastly rollout_pct -- --nocapture attempt failed at link time on Fastly hostcall symbols, which is expected for the Fastly adapter outside the wasm/Viceroy path and not counted as a PR finding.

[aram356]

Summary

Re-review of the canary routing changes against the PR base (feature/edgezero-pr19-spin-adapter). The prior review's findings on the per-request warn flood, degenerate-rollout short-circuit, predicate rename, read_rollout_pct test coverage, and debug-log observability are all resolved. Two findings remain open (both carried over) and three new minor items are noted inline. CI is green (12/12).

Blocking

🔧 wrench

• Fail-open default for absent edgezero_rollout_pct: absent key → 100 (full cutover) is the only failure branch that does not fail to legacy. Recommend defaulting to 0. (main.rs absent branch + doc table; runbook failure-modes table)
• Runbook overstates stickiness: "sticky per user" should read "sticky per client IP" — NAT sharing and IP changes break the per-user assumption. (docs/internal/EDGEZERO_MIGRATION.md:19)

Non-blocking

🤔 thinking

• Two config-store reads per request — is_edgezero_enabled then read_rollout_pct; consolidatable into one read. (main.rs:316)

⛏ nitpick

• resolve_max_level doc says "raises" but the override can also lower the level. (logging.rs:44)

📝 note

• Env-var override safety rests on a Fastly-platform property — must not be copied verbatim into the axum/spin/cloudflare adapters. (logging.rs:25)

CI Status

• fmt: PASS
• clippy: PASS
• rust tests: PASS
• js tests: PASS

[ChristianPavilonis]

Automated review:

Review Summary

Reviewed PR #727 against feature/edgezero-pr19-spin-adapter, focusing on the Fastly canary routing implementation, rollout defaults, config-store failure behavior, local Fastly config, migration runbook, implementation plan docs, and CI/review context. I did not find any blocking correctness, security, data-loss, authorization, or severe compatibility issues. I left two P2 inline comments about stale operational guidance that still contradicts the current fail-safe absent-key behavior.

Findings

P0 / Blockers

None.

P1 / High

None.

P2 / Medium

2 inline comments submitted.

P3 / Low

None.

CI / Existing Reviews

All reported PR checks are passing. Existing review feedback about fail-open absent-key behavior, sticky-per-user wording, route-decision observability, logging override wording/scope, and stale monitoring guidance appears addressed in code/runbook and replies; however, some duplicated guidance in fastly.toml and the implementation plan remains stale. Locally, cargo test -p trusted-server-adapter-fastly --target wasm32-wasip1 read_rollout_pct --no-run completed successfully.

[ChristianPavilonis]

Automated review:

Review Summary

Reviewed PR #727 against feature/edgezero-pr19-spin-adapter, focusing on the Fastly canary-routing code, fail-safe rollout defaults, logging changes, local config, migration runbook, implementation-plan documentation, CI, and existing review feedback. I did not find any blocking correctness, security, data-loss, authorization, or severe compatibility issues. I left two P2 inline comments on stale plan text that still contradicts the implemented fail-safe and per-client-IP semantics.

Findings

P0 / Blockers

None.

P1 / High

None.

P2 / Medium

2 inline comments submitted.

P3 / Low

None.

CI / Existing Reviews

All reported PR checks are passing. Existing feedback about absent-key fail-open behavior, route-decision observability, logging override wording/scope, and stale monitoring guidance appears addressed in the runtime code, fastly.toml, and the canonical migration runbook; the remaining issues are duplicated/stale text in the implementation-plan document. Locally, cargo test -p trusted-server-adapter-fastly --target wasm32-wasip1 read_rollout_pct --no-run and cargo fmt --all -- --check passed; cd docs && npm run format -- --check could not run because prettier is not installed in this environment, while CI's docs format check is green.

[ChristianPavilonis]

Automated review: P2: This snippet still documents the old fail-open absent-key behavior.

The table immediately above says an absent edgezero_rollout_pct returns 0, the implementation now returns 0, and the final invariant says absent/invalid/unreadable must never route traffic to EdgeZero. This embedded code block still says Ok(None) => 100, so anyone reusing the implementation plan would reintroduce the exact fail-open behavior this PR just fixed: deleting or omitting the key would trigger full EdgeZero cutover.

Suggested fix: update this arm to return 0 (and preferably include the same debug log/comment as the real implementation), or mark the whole implementation snippet as historical/superseded so it cannot be copied as current guidance.

[ChristianPavilonis]

Automated review: P2: This invariant still overstates stickiness as per-user.

The runtime hashes client_ip, and the canonical runbook now correctly says routing is sticky per client IP, not per user: NAT/CGNAT users can share a bucket, and a user whose IP changes can be re-bucketed and switch paths. This final invariant still promises that the same user always gets the same path and that split-session bugs are prevented, which contradicts the implemented routing key and the fixed runbook wording.

Suggested fix: align this invariant with the runbook, e.g. client IP hash is deterministic; the same client IP gets the same path at a given rollout_pct, but NAT sharing and IP changes can re-bucket users.

[ChristianPavilonis]

Review Summary

🔧 Requesting changes. The runtime fail-safe rollout behavior is much improved and CI is green, but local canary validation still relies on a log-level mechanism that Viceroy/Compute guest code will not actually see. I also left accept-ready suggestions for stale plan snippets and validation commands so the PR artifacts stay safe to reuse.

Additional note folded into the body because the exact insertion point is unchanged context in the diff: max_level is currently applied only to the inner log_fastly::Logger. Please also set the same level on the outer fern::Dispatch (e.g. fern::Dispatch::new().level(max_level)) so production debug! route decisions are filtered before timestamp/format construction, not just dropped by the child logger.

[ChristianPavilonis]

🔧 wrench — EDGEZERO_LOG_LEVEL is not visible to Viceroy guest code.

std::env::var(LOG_LEVEL_ENV) reads the guest WASI environment, but fastly compute serve/Viceroy does not pass arbitrary shell environment variables like EDGEZERO_LOG_LEVEL into the Compute guest. That means the runbook command EDGEZERO_LOG_LEVEL=debug fastly compute serve still initializes at Info, so the route-decision debug! lines remain unavailable in the only documented branch-level validation path.

One minimal way to make the local validation path real is to key off the Fastly-provided local hostname and keep production at the safe default:

Suggested change

	let configured = std::env::var(LOG_LEVEL_ENV).ok();
	let configured = std::env::var(LOG_LEVEL_ENV).ok().or_else(|| {
	(std::env::var("FASTLY_HOSTNAME").as_deref() == Ok("localhost"))
	.then_some("debug".to_owned())
	});

[ChristianPavilonis]

🔧 wrench — make the local log validation instructions match a guest-visible signal.

After changing the logger to use a Viceroy/Compute-provided signal (for example FASTLY_HOSTNAME=localhost) instead of an arbitrary shell env var, this runbook should stop telling operators to set EDGEZERO_LOG_LEVEL, which the guest will not see.

Suggested change

	> For local pre-production validation under Viceroy, start the simulator with
	> `EDGEZERO_LOG_LEVEL=debug` (the logger defaults to `Info`, which suppresses
	> these lines) — for example `EDGEZERO_LOG_LEVEL=debug fastly compute serve`.
	> For local pre-production validation under Viceroy, start the simulator normally.
	> Viceroy exposes `FASTLY_HOSTNAME=localhost` to guest code, and the Fastly
	> logger raises the route-decision level in that local environment while
	> production stays at `Info`.

[ChristianPavilonis]

🔧 wrench — this embedded snippet still has the old fail-open absent-key behavior.

The table above, the runtime code, and the runbook all say an absent edgezero_rollout_pct fails safe to 0; this snippet still returns 100. Because this file is an agent handoff, copying it later would reintroduce the exact full-cutover footgun the PR fixed.

Suggested change

	Ok(None) => 100,
	Ok(None) => {
	log::debug!(
	"edgezero_rollout_pct key absent, defaulting to 0 (legacy path — fail safe)"
	);
	0
	}

[ChristianPavilonis]

🔧 wrench — keep the invariant to per-client-IP stickiness.

The runtime hashes client_ip, not a durable user identifier. NAT/CGNAT users can share a bucket, and a user whose IP changes can re-bucket, so the plan should not promise that the same user always gets the same path.

Suggested change

	- **Sticky routing** — client IP hash is deterministic; the same user always gets the same path at a given `rollout_pct`, preventing split-session bugs.
	- **Sticky routing** — client IP hash is deterministic; the same client IP gets the same path at a given `rollout_pct`, but NAT sharing and IP changes can re-bucket users.

[ChristianPavilonis]

🔧 wrench — use the target-matched Fastly clippy gate.

This host-targeted clippy command conflicts with CLAUDE.md's Fastly gate. Also, the surrounding | tail/| head validation commands should be removed or wrapped in set -o pipefail, otherwise a failed cargo/npm command can look green because the final pipeline command succeeded.

Suggested change

	cargo clippy -p trusted-server-adapter-fastly --all-targets --all-features -- -D warnings 2>&1 | tail -20
	cargo clippy-fastly

[ChristianPavilonis]

🔧 wrench — don't pipe validation through tail without pipefail.

As written, a failing test command can still exit successfully if tail succeeds. Keep the command's real exit status visible.

Suggested change

	cargo test-fastly 2>&1 | tail -20
	cargo test-fastly

[ChristianPavilonis]

🔧 wrench — final verification should use the Fastly clippy alias and preserve exit status.

This is the PR handoff gate, so it should match the repo's target-specific CI command and not hide failures behind tail.

Suggested change

	cargo clippy -p trusted-server-adapter-fastly --all-targets --all-features -- -D warnings 2>&1 | tail -10
	cargo clippy-fastly