Skip to content

feat(router): add the Go HTTP server, JSON logging and healthcheck (not shipped) - #10327

Open
rabeckett wants to merge 1 commit into
mainfrom
rabeckett-router-go-http
Open

rabeckett wants to merge 1 commit into
mainfrom
rabeckett-router-go-http

Conversation

@rabeckett

Copy link
Copy Markdown
Collaborator

Closes #10326

Part of #10261 (step 3 of 7).

Summary

This PR adds the Go router's serve command: the four HTTP endpoints on net/http, structured JSON logs on stderr, and a healthcheck subcommand that the Go image will use instead of Python. It follows D28 in the port plan: use net/http as it is, and record where its transport behavior differs from the Python service.

Nothing ships yet. The image still runs Python.

Changes

  • Go code in containers/router/go/ (about 540 lines, standard library only):
    • server.go: the handler for /healthz, /capabilities, /v1/route and /v1/classify, with the Python service's success bodies and error envelopes. It also holds the limits: 64 requests in flight (/healthz included, then a bare 503), a 1,048,576-byte body limit (413 payload_too_large), and the server timeouts. A handler panic becomes a 500 internal_error, or an aborted connection if the response has already started.
    • serve.go: startup (load and validate the tables, check the bind address, listen), and shutdown on SIGTERM or SIGINT with a 0.5 s drain before closing the remaining connections.
    • logging.go: one JSON record per line with a fixed field set, and an allowlist so request data never reaches a log record.
    • cli.go: the serve options and environment variables shared with the Python CLI, and the healthcheck [--url URL] command (exit 0 only on a 204, 1 s timeout, no proxy, no redirects).
  • Go tests (about 1,400 lines):
    • Ports of test_http.py and of the serve tests in test_cli.py: the endpoints and envelopes, content-type and method handling, body limit (declared and streamed), the concurrency limit, panics, the bind and log-level validation, startup failures and their exit codes, and shutdown that drains or aborts an in-flight request.
    • Shutdown is tested both in process (SIGINT) and, on Linux, with a real SIGTERM to a child process.
    • A test sends request data through every endpoint and error path and checks that none of it appears in a response or log record.
    • Python's deadline tests have no port: the 504 is replaced by the server timeouts (see below).
  • Docs:
    • the router README documents Go serve (limits, timeouts, shutdown, the log events table and what's never logged) and healthcheck, and says that differential.py doesn't compare HTTP;
    • the parity README's decisions table has the new rows below.

CI already runs the Go job (gofmt, vet, tests, with -race and govulncheck on Linux) from #10313, so test-router.yml doesn't change.

Decisions for maintainers

These rows are added to the decisions table in tests/fixtures/parity/README.md, and merging this PR approves them. All are net/http behavior that the port keeps rather than reimplements:

  • Paths that aren't clean, such as //healthz, get a 307 redirect to the clean path instead of a 404.
  • There's no 504 request_timeout. Server timeouts (10 s for headers, 30 s for the request, 30 s for the response) close the connection instead. openapi.yaml drops the 504 at the cut-over.
  • OPTIONS * gets an empty 200 instead of a 404.
  • An absolute-form target such as GET http://host/healthz is routed by its path instead of getting a 404.
  • An invalid chunked body closes the connection instead of getting a bare 400.

Notes for reviewers

  • Aborted requests aren't logged. If a request's body never fully arrives (a broken chunk, a client disconnect or the 30 s read timeout), the connection closes without a request record, because no status was sent. Python logs nothing structured for this either, and the api-proxy logs its own side. A slow handler isn't an abort: if the api-proxy gives up while the router is still computing, the request is still logged. The OpenTelemetry step can surface aborts as error spans.
  • Each overload 503 gets its own ERROR request record. D28 dropped the rate-limited overload log, so a burst of overload produces a burst of log lines.

Validation

In containers/router, with Go 1.27.2 and Python 3.12 with the locked dependencies:

  • Windows: gofmt -l, go vet and go test pass in go/; statement coverage is 96%. The uncovered lines are mostly unreachable encoding-failure branches and the Linux-only SIGTERM name.
  • Linux arm64 (WSL): gofmt -l, go vet, go test and go test -race pass, including the real SIGTERM test.
  • pytest: 1,180 passed. ruff check, ruff format --check and python tests/parity_corpus.py --check pass. The Python code is unchanged; these cover the README files that the Python tests read.
  • govulncheck wasn't run locally; CI runs it. The module has no dependencies outside the standard library.

…ot shipped)

Implement `serve` in the Go port on net/http as it is (D28): the four
endpoints with the Python service's bodies and error envelopes, a
64-request in-flight limit, the 1 MiB body limit, server timeouts, panic
recovery and a 0.5 s drain on SIGTERM/SIGINT. Add structured JSON logs on
stderr and a `healthcheck` subcommand that doesn't load tables.

Record the net/http transport behavior the port takes in the parity
decisions table and document serve, healthcheck and the log format in the
router README. The image still runs the Python service.

Part of #10261.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI max review requested due to automatic review settings October 20, 2026 05:55
@github-actions

Copy link
Copy Markdown
Contributor

✅ Coverage Check Passed

Overall Coverage

Metric Base PR Delta
Lines 93.40% 93.41% ➡️ +0.01%
Statements 91.96% 91.97% 📈 +0.01%
Functions 91.16% 91.16% ➡️ +0.00%
Branches 86.00% 86.01% 📈 +0.01%
📁 Per-file Coverage Changes (1 files)
File Lines (Before → After) Statements (Before → After)
src/nvx/one-shot-adapter.ts 82.9% → 83.4% (+0.56%) 80.2% → 80.7% (+0.52%)

Coverage comparison generated by scripts/ci/compare-coverage.ts

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

Transport and privacy guarantees still need verification with the pinned Go toolchain.

3 open issues
What changed in this PR

Adds an HTTP adapter to the router’s existing Go core while leaving the production image on Python.

Changes:

  • Implements HTTP endpoints, request limits, timeouts, startup validation and graceful shutdown.
  • Adds structured JSON logging and a standalone healthcheck command.
  • Adds HTTP, privacy and lifecycle tests, plus Go-port documentation.
File Description
containers/​router/​tests/​fixtures/​parity/​README.md Records transport differences from Python.
containers/​router/​README.md Documents Go serving, logging and healthchecks.
containers/​router/​go/​server.go Implements HTTP handling and safeguards.
containers/​router/​go/​server_test.go Tests endpoints, limits, errors and privacy.
containers/​router/​go/​serve.go Implements startup, bind validation and healthchecks.
containers/​router/​go/​serve_test.go Tests configuration, startup and shutdown.
containers/​router/​go/​logging.go Provides JSON logging and sanitization helpers.
containers/​router/​go/​logging_test.go Tests logging format and privacy helpers.
containers/​router/​go/​cli.go Adds serve and healthcheck commands.
containers/​router/​go/​cli_test.go Updates CLI coverage and shutdown-test support.

Review effort: Max

envelopes of the Python service:

- At most 64 requests are in flight, `/healthz` included. Further requests get a bare 503.
- A declared or streamed body over 1,048,576 bytes gets 413 `payload_too_large`.
Comment on lines +825 to +827
if strings.Contains(string(r.body), canary) || strings.Contains(strings.Join(slices.Collect(maps.Keys(r.header)), ","), canary) {
t.Errorf("%s %s echoes the canary: %s", method, target, r.body)
}
| `serve` startup errors | Plain one-line text | JSON log records; other commands keep plain text |
| Trailing slash | Starlette redirects `/healthz/` to `/healthz` with 307 | 404 `not_found`, like any unknown path |
| Unsupported transfer coding | Uvicorn answers a bare 400 | Go's HTTP/1.1 server answers a bare 501, as RFC 9112 recommends |
| Paths that aren't clean, such as `//healthz` and `/a/../healthz` | 404 `not_found` | `ServeMux` redirects to the cleaned path with 307 |

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Router: add the Go HTTP server, structured JSON logging and a healthcheck subcommand (not shipped)

2 participants