feat(service): Add /v1/health and /healthz endpoints for health checking#2169
Conversation
|
Staged Fern docs preview: https://nvidia-preview-pr-2169.docs.buildwithfern.com/nemo/guardrails |
|
@greptile-apps Review this PR |
|
@coderabbitai Review this PR |
🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
gh pr view 2169 --repo NVIDIA-NeMo/Guardrails --json title,body,files,additions,deletions
echo "-----DIFF-----"
gh pr diff 2169 --repo NVIDIA-NeMo/GuardrailsLength of output: 18473 🏁 Script executed: #!/bin/bash
set -euo pipefail
sed -n '1,60p' nemoguardrails/server/api.py
echo "---"
sed -n '200,300p' nemoguardrails/server/api.pyLength of output: 5363 🏁 Script executed: #!/bin/bash
set -euo pipefail
rg -n "api_key|Authorization|middleware|dependencies=" nemoguardrails/server/api.py | head -60Length of output: 404 🏁 Script executed: #!/bin/bash
set -euo pipefail
rg -n "prefix" nemoguardrails/server/api.py nemoguardrails/cli/*.py 2>/dev/null | head -40Length of output: 980 🏁 Script executed: #!/bin/bash
set -euo pipefail
rg -n "`@app`\.(get|mount|include_router)|StaticFiles" nemoguardrails/server/api.pyLength of output: 364
What looks good:
Minor observations (non-blocking):
Overall, the implementation matches the stated objectives well and the test/integration evidence in the PR description (local server + Docker) is thorough. No blocking issues found. ✅ Action performedReview finished.
|
Greptile SummaryThis PR adds shallow health checks for the Guardrails server. The main changes are:
|
| Filename | Overview |
|---|---|
| Dockerfile | Adds a configurable Docker healthcheck URL and probes it without proxy handling. |
| nemoguardrails/server/api.py | Adds shallow health routes that return application/health+json with a pass status. |
| docs/deployment/using-docker.mdx | Documents when the container healthcheck applies and how to override its URL. |
| docs/run-rails/using-fastapi-server/run-guardrails-server.mdx | Documents the server health endpoints and their behavior with path prefixes. |
| fern/openapi.yml | Adds OpenAPI entries for both health endpoints. |
| tests/server/test_health.py | Adds tests for the new health endpoint contract and alias. |
Reviews (7): Last reviewed commit: "Bypass proxies in requests library healt..." | Re-trigger Greptile
This comment was marked as outdated.
This comment was marked as outdated.
📝 WalkthroughWalkthroughAdds ChangesHealth Check Flow
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 6✅ Passed checks (6 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/run-rails/using-fastapi-server/run-guardrails-server.mdx`:
- Around line 229-245: Add both documented health endpoints, GET /v1/health and
GET /healthz, to the “Reference → Guardrails API Server” section of
docs/index.yml alongside the existing GET / entry, preserving the current API
reference structure.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 52ecf7d1-d55a-4860-b036-9260a0c64bd4
📒 Files selected for processing (4)
Dockerfiledocs/run-rails/using-fastapi-server/run-guardrails-server.mdxnemoguardrails/server/api.pytests/server/test_health.py
Pouyanpi
left a comment
There was a problem hiding this comment.
Thanks, @tgasser-nv, for adding this!
One thing I noticed: this works well for a server-only image, but the same image also supports commands like nemoguardrails chat (see docs):
docker run --rm -it \
--name guardrails-chat \
-e OPENAI_API_KEY="$OPENAI_API_KEY" \
nemoguardrails \
chat --config=/config --verboseSince chat doesn’t start an HTTP server, the health check eventually marks the container as unhealthy, even though the CLI is working:
docker inspect \
--format='{{.State.Health.Status}}' \
guardrails-chatInitially it reports starting; if chat remains open long enough, it becomes unhealthy.
You can inspect individual probe attempts immediately:
docker inspect \
--format='{{range .State.Health.Log}}
{{println .End .ExitCode .Output}}{{end}}' \
guardrails-chatbecause chat has no http listener, the probe should eventually fail with a connection refused error and the status should become: unhealthy
Could we skip the HTTP probe when the container isn’t running in server mode? Another option would be to document --no-healthcheck for non-server commands. Or whatever you think is a better option would be great.
Also urllib.request.urlopen() uses HTTP_PROXY/ http_proxy. It might be worth disabling proxies for this localhost request so a healthy server isn’t marked unhealthy when NO_PROXY isn’t set.
It would be nice to cover both cases in the Docker workflow as well.
I added instructions in the docs to use
I bypassed proxies in the health checking code. |
Description
Adds
/v1/healthand/healthzendpoints for use by Kubernetes / Docker / clients to check the service is running. It's a shallow check, and doesn't check for downstream dependencies like the main-model or Guardrail models. It also doesn't check if a default Guardrails config was configured at server start-time, or if any are cached in the serverllm_rails_instancesvariable.Related Issue(s)
Test Plan
Pre-commit
Unit-test
Integration tests
Local server health checks
Server
Client
Docker health checks
Server
Client checking health endpoints inside Docker container
Docker inspect health
Docs build check
AI Assistance
Checklist
Summary by CodeRabbit
New Features
/v1/healthand/healthz.Documentation