Conversation
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Solid structure and the Cloud Run constraints section is genuinely useful (60s timeout, scale-to-zero/Uplink cold start, egress options). The health-check guidance is correct: Router serves /health on :8088 by default, separate from the :4000 GraphQL port. A few correctness issues need fixing before this is publishable as authoritative guidance.
Blocking:
-
The
apollographql/router#3517reference is mischaracterized. That issue is a container startup failure (/usr/bin/env: 'bash': No such file or directory) specific to Cloud Run; it is not about startup probes or the Router's default port behaviour. It was closed 2026-era (closed 2025-04-11), not "resolved in Router 1.x." Either describe the issue accurately (the bash/entrypoint failure) or drop the claim that it covers probe/port behaviour. The acceptance criterion to link #3517 "where relevant" should reflect what the issue actually says. -
The Rhai snippet will not run as written. Per the Rhai API reference:
request.subgraph.headersis modifiable only inside asubgraph_servicecallback; insupergraph_servicethe only modifiable subgraph-level field isrequest.context. The example must usefn subgraph_service(service, subgraph). Separately, Router's Rhai surface has nofetch()HTTP function, so minting an OIDC token from the metadata server inside Rhai is not possible; this belongs in a coprocessor (which can make HTTP calls). Also, header assignment is indexed (request.subgraph.headers["authorization"] = ...), not a.set()method.
Non-blocking:
-
The
maxScale: "20"annotation carries a comment about per-instance concurrency tuning ("start at 80 and tune"), which actually describescontainerConcurrency: 80two lines down. Move or reword so the concurrency note sits withcontainerConcurrency. -
The pinned image tag
router:v2.10.0is reasonable but worth confirming it is the intended LTS/version for this guide given it ships as an example customers may copy.
| ## Known issue references | ||
|
|
||
| - [`apollographql/router#3517`](https://github.com/apollographql/router/issues/3517) — Cloud Run startup probe + Router default port behaviour. Resolved in Router 1.x but worth verifying on each version bump. | ||
|
|
There was a problem hiding this comment.
router#3517 is a container startup failure (/usr/bin/env: 'bash': No such file or directory) on Cloud Run, not startup-probe or default-port behaviour, and it was not resolved in Router 1.x. Restate accurately or remove the probe/port framing.
There was a problem hiding this comment.
Verified at 2dcd222: the #3517 reference is now recharacterized accurately. The Known issue references section describes it as a Cloud Run container startup failure (/usr/bin/env: 'bash': No such file or directory) caused by the distroless images shipping no shell, and the probe/port framing is gone. This thread can be resolved.
| // router.rhai (excerpt) | ||
| fn supergraph_service(service){ | ||
| let svc = service; | ||
| svc.map_request(|request|{ |
There was a problem hiding this comment.
This will not run. request.subgraph.headers is writable only in a subgraph_service callback, not supergraph_service; and Router's Rhai surface has no fetch() HTTP function. Use a coprocessor for the metadata-server token fetch, or at minimum move header-setting into fn subgraph_service(service, subgraph) using indexed assignment (request.subgraph.headers["authorization"] = ...).
There was a problem hiding this comment.
Verified at 2dcd222: the non-compiling Rhai snippet is gone. The doc now states Rhai is the wrong tool (no outbound HTTP primitive; request.subgraph.headers writable only in subgraph_service) and replaces it with a SubgraphRequest-stage coprocessor that mints the OIDC token from the metadata server and sets the Authorization header. This thread can be resolved.
| metadata: | ||
| annotations: | ||
| # Use the second-gen execution environment — it supports HTTP/2, | ||
| # larger memory, and faster cold starts. First-gen will work but |
There was a problem hiding this comment.
The concurrency comment ("start at 80 and tune from metrics") describes containerConcurrency: 80 below, not maxScale. Move it down or reword to avoid conflating max instance count with per-instance concurrency.
There was a problem hiding this comment.
Verified at 2dcd222: the concurrency note has been moved. The maxScale: "20" comment now describes horizontal autoscaling only, and the per-instance tuning note ("happy at 80-200; start at 80 and tune from metrics") now sits with containerConcurrency: 80. This thread can be resolved.
…(AS-223) Address docs-reviewer feedback on three blocking issues: 1. The OIDC-token Rhai snippet was invalid: Rhai has no fetch() and `request.subgraph.headers` is only writable in `subgraph_service`, not `supergraph_service`. Replace with a SubgraphRequest coprocessor example (Node + Cloud Run metadata server) which is the actual supported shape. 2. router#3517 mischaracterized — it's a container startup failure (`/usr/bin/env: 'bash': No such file or directory`) caused by the distroless Router image shipping no shell, NOT a probe/port issue. Restate accurately and recommend not depending on bash in launch scripts. 3. Move the "start at 80 and tune from metrics" comment from maxScale (instance count) to containerConcurrency (per-instance concurrency) where it belongs. Also repoint legacy /docs/router/... links to the current /docs/graphos/routing/... IA. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Re-reviewed at 2dcd222. All three issues from the prior review are fixed. (1) The #3517 reference is recharacterized accurately as a distroless container startup failure (no shell for bash in the entrypoint), with correct mitigation guidance; the probe/port framing is removed. (2) The non-compiling Rhai snippet is replaced with a correct SubgraphRequest-stage coprocessor pattern; the doc now explains why Rhai cannot do this (no outbound HTTP primitive, and request.subgraph.headers is not writable in supergraph_service). (3) The concurrency comment has been moved off maxScale and now sits with containerConcurrency: 80. The health-check guidance (/health on :8088, not :4000) is correct. Acceptance criteria are met and there are no new blockers. The non-blocking note on confirming router:v2.10.0 as the intended pinned version still stands but does not gate this interim reference. Approving.
Summary
Adds
docs/router-on-gcp-cloud-run.md— Knative service spec + the Cloud Run-specific constraints (60s request timeout, health-check port:8088not:4000, WebSocket lifetime caps, scale-to-zero + Uplink cold-start interaction, egress options).References
apollographql/router#3517for the known Cloud Run startup probe behaviour mentioned in the ticket. Includes a Rhai snippet for Google OIDC service-to-service auth to private subgraphs.Tracks AS-223.
Note
Placement: apollographql/docs archived; apollographql/platform-docs requires SAML grant. Landing here as interim home.
Test plan
🤖 Generated with Claude Code