Repository navigation
Confidence 4/5 · No issues found
🟢 Confidence 4/5 · likely safe to merge
service.url must still resolve without public: true, since both hostnames' CNAMEs come from it and the vendored alchemy source cannot confirm it.
quality 98/100 · 1 note · tests not needed · risk high · 2/2 new units observable
Moves both regional ingress paths onto ingest's single ALB: ingest owns the load balancer and listener, electric attaches a host rule and its own SNI certificate. The wiring reads correctly and the doc drift is the only defect I could confirm; the alchemy assumptions are unverifiable here.
createMapleIngestnow ownsAWS.ELBv2.LoadBalancer/Listener(ingest-lb,ingest-listener) and forwards to ingest at priority 50000createMapleElectrictakes ingest'slistener,albSecurityGroupIdand a requiredhostname, droppingpublic: trueand its own ALB groupalchemy.run.tscreates electric only whendomains.electricis set, and does not deploy it in previews or dev
Findings
🔵 Note · F1 · Runbook this file points to still documents electric's own ALB and group
maintainability · apps/electric/alchemy.run.ts:33
The header comment still points at docs/electric-sync.md as the runbook, but that doc (lines 176-179) still describes Electric "with its own cluster, ALB, security groups and certificate inside the ingest fleet's VPC", and docs/infra.md's ECS.Service pitfall note still explains the certificateArn/listenerPort pairing this diff removes. The next operator or on-call following it will reason about ALBs and a security group that no longer exist, including the manual electric-lb cleanup the deploy notes call out. Update those two sections with the shared listener, the host rule and the Cloudflare-only group.
What was checked
- Electric's rule (priority 10) still wins over ingest's catch-all (
apps/ingest/alchemy.run.ts:510), and every host reaching the 443 listener still forwards to ingest domains.electricanddomains.ingestare set together for prd and prd-eu only, so the HTTP-80 listener path never carries electric's hostname- Both
ingestandelectricsynced to Maple in the last 6h, so the shared ALB's path stays observable
Observability coverage: 2 of 2 changes observable
| Change | Kind | Observable | Evidence |
|---|---|---|---|
| ingest ALB listener + electric host rule (shared ingress) | load balancer entrypoint | yes | ALB is infrastructure in front of already-instrumented apps; ingest (247.8k req) and electric-sync reported spans in the last 6h |
| cloudflare CNAMEs for ingest and electric hostnames | DNS wiring | yes | Traffic still arrives at the same instrumented services through the proxied hostnames |
53fb08b · Updated on every push. Reply "won't fix" to dismiss a finding, or mention @maple-review-bot to ask about one.
Annotations
Check notice on line 33 in apps/electric/alchemy.run.ts
maple-review-bot / Maple / review
maintainability: Runbook this file points to still documents electric's own ALB and group
The header comment still points at `docs/electric-sync.md` as the runbook, but that doc (lines 176-179) still describes Electric "with its own cluster, ALB, security groups and certificate inside the ingest fleet's VPC", and `docs/infra.md`'s `ECS.Service` pitfall note still explains the `certificateArn`/`listenerPort` pairing this diff removes. The next operator or on-call following it will reason about ALBs and a security group that no longer exist, including the manual `electric-lb` cleanup the deploy notes call out. Update those two sections with the shared listener, the host rule and the Cloudflare-only group.