feat(api): add /healthz + /readyz probes, deprecate /health + /ready - #213
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR migrates health probe endpoints from legacy naming ( ChangesKubernetes Health Probe Endpoints
🎯 3 (Moderate) | ⏱️ ~25 minutes Note 🎁 Summarized by CodeRabbit FreeYour organization is on the Free plan. CodeRabbit will generate a high-level summary and a walkthrough for each pull request. For a comprehensive line-by-line review, please upgrade your subscription to CodeRabbit Pro by visiting https://app.coderabbit.ai/login. Comment |
|
/gemini review |
EricAndrechek
left a comment
There was a problem hiding this comment.
A quick google make it appear as though k8s has deprecated or started phasing out /healthz and recommending /readyz and /livez instead – considering we are starting from scratch here we should probably conform with the current recommendations. I also don't know what the norms are for what each endpoint must return, or what cases they return success/failures in, so additional implementation work may be needed to get it all conforming properly.
Then we also need to work out whether we need /healthz for backwards compatability with older k8s versions or not, and how (if at all) it will differ.
We also need to think about which if any of these endpoints is what we'd want to use for the docker compose health checks, and which our own e2e orchestrator uses, and what if anything should be returned along with those routes.
I also am thinking/wondering, but am unsure on the hosting and deployment conventions, if /readyz, /livez, /healthz etc. are usually on the same port/exposed to traffic like this or not. As we have it now as I understand it, if WaveHouse's port is forwarded to the internet, these k8s endpoints become public for anyone to query, whereas (I think I can't remember for sure) we had it setup so that the prometheus metrics, for instance, could optionally be hosted on a different port. Ideally it'd be the same port, or a competent devops person deploying this would filter those paths out at the reverse proxy layer so they were only addressable internally, but unsure if we should build that in as a feature too or not, worth looking into what the k8s norm is.
I DO think, however, we should have a public facing /health and/or /live or /ready or similar, that simply returns 200 OK or an error and likely no content (so we don't need to do JSON encoding on each request and don't need to cache anything), specifically for use in the SDK to check if a server is online before sending data, or to check if a particular server is healthy and accepting data when choosing between multiple in a distributed setup.
Happy to talk about this all more in person if needed today.
Deploying with
|
| Status | Name | Latest Commit | Updated (UTC) |
|---|---|---|---|
| 🚫 Deployment cancelled View logs |
wavehouse-docs | fe26e9f | Jun 03 2026, 03:20 PM |
EricAndrechek
left a comment
There was a problem hiding this comment.
Only want /v1/health accessible and used by API, not readiness.
Resolves the CHANGELOG.md [Unreleased] conflict: keeps this branch's "Lint/format coverage" + docs-deploy-from-CI-job entries alongside main's new per-table-batching (#192) and health-endpoints (#213) entries, and reconciles the stale "Project policies CardGrid" wording (that homepage element was replaced by the closer call-to-action band on this branch). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Summary
Adopts the Kubernetes-convention health-probe names from #144:
GET /healthz(liveness) andGET /readyz(readiness) are now the canonical paths. The existing/healthand/readyare kept as deprecated aliases of the same handlers for the v0.1.x line and are slated for removal in v0.2.0 — so nothing breaks for anyone already probing the old paths. Response bodies are unchanged (/healthz→{"status":"ok"},/readyz→{"status":"ready"}, same boot-gated 503s as before).WaveHouse's own tooling is repointed at the canonical names so it doesn't ride an alias we've announced for removal: the distroless
healthsubcommand, the e2e orchestrator's readiness wait, the SDK e2e setup probe, and both DockerfileHEALTHCHECKs. Theprometheus.pathreserved-collision check now also covers/healthz+/readyz, and the tracing span-skip middleware excl probe paths. Docs,AGENTS.md, andCHANGELOG.mdupdated to match.Part 2 of #144 is deferred: the per-dependency drill-down probes under
/healthz/{clickhouse,nats,schema,dlq}and the richer aggregate{"ready", "checks"}readiness body are left for a follow-up. This PR is a pure rename + alias.Related Issues
Part of #144 (part 1:
/healthz+/readyzrename + aliases). Intentionally notCloses— part 2 (per-dependency probes, aggregate readiness body) is deferred.