feat(health): adding a grace period to the checker#69
Conversation
There was a problem hiding this comment.
Pull Request Overview
This pull request enhances the health check system by introducing an error grace period and by separating readiness and liveness checks.
- Introduces error grace period in Checker to tolerate transient errors within a configurable time window.
- Implements separate readiness and liveness health check endpoints using gorilla/mux.
- Adds corresponding tests, options, and updates to App configuration.
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| options.go | Updated with separate readiness and liveness checkers and registration via mux. |
| health/checker_test.go | Added tests verifying error grace period behavior. |
| health/checker_options.go | Introduced the WithCheckerErrorGracePeriod option. |
| health/checker.go | Implemented logic for error grace period handling during health checks. |
| app.go | Extended App configuration with new health check fields for readiness and liveness. |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull Request Overview
This pull request adds an error grace period for health checks and separates readiness from liveness checks, while updating the API and tests accordingly.
- Introduces a configurable error grace period in the Checker struct.
- Separates readiness and liveness checks via distinct checkers and HTTP endpoints.
- Enhances test coverage for the new error grace period feature.
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| options.go | Separates readiness and liveness checkers and sets up distinct HTTP endpoints. |
| health/checker_test.go | Adds tests to verify the error grace period behavior. |
| health/checker_options.go | Introduces option to set the error grace period. |
| health/checker.go | Implements error grace period logic with a shared failure timestamp. |
| app.go | Imports and introduces new health check fields. |
| README.md | Updates documentation to mention the new endpoints for health checks. |
Comments suppressed due to low confidence (2)
options.go:245
- The liveness endpoint is defined as "/healthz", but README.md refers to it as "/livez". Consider aligning the endpoint naming to avoid confusion.
r.Handle("/healthz", livenessChecker.Handler()).Methods(http.MethodGet)
health/checker.go:111
- The use of a shared atomic value for firstFailInCycle across all checks may lead to unexpected behavior when multiple checks run concurrently. Please verify that a single shared timestamp is the intended design or consider using per-check timers.
if c.errorGracePeriod > 0 && now.Sub(firstFail) <= c.errorGracePeriod {
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull Request Overview
This PR introduces an error grace period for health checks and separates readiness and liveness checks with distinct endpoints.
- Added errorGracePeriod functionality with related options and fields in the Checker struct.
- Updated the health check system in options.go to create separate readiness and liveness checkers with endpoints registered via Gorilla Mux.
- Extended tests (checker_test.go) to cover the newly introduced grace period behavior.
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| options.go | Created separate readiness and liveness checkers and endpoints. |
| health/checker_test.go | Added new tests for error grace period in health checks. |
| health/checker_options.go | Introduced the WithCheckerErrorGracePeriod option. |
| health/checker.go | Integrated error grace period logic into the Checker.Check method. |
| README.md | Updated documentation to reflect new endpoints, though with minor mismatch. |
Comments suppressed due to low confidence (1)
README.md:136
- The documentation mentions the liveness endpoint as
/livez, but the code registers it as/healthz. Please update one of them to ensure consistent naming.
the health checks get setup on the routes `/readyz` and `/livez`; this allows for Kubernetes to check
There was a problem hiding this comment.
Pull Request Overview
This pull request enhances the health check system by introducing an error grace period feature and separating readiness and liveness checks.
- Added a new error grace period configuration via the WithCheckerErrorGracePeriod option.
- Implemented distinct readiness and liveness checkers with separate endpoints in the health check server setup.
- Updated the health check API documentation and tests to verify the new behavior.
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| options.go | Updated to create separate readiness and liveness checkers with a grace period. |
| health/checker_test.go | Added test cases for error grace period functionality and multiple async checks. |
| health/checker_options.go | Introduced a new option to set the error grace period for the Checker. |
| health/checker.go | Modified Check logic to incorporate error grace period handling. |
| README.md | Updated documentation with newly defined endpoints for health checks. |
Comments suppressed due to low confidence (1)
options.go:248
- [nitpick] The PR description references a '/healthz' endpoint for liveness checks, but the code uses '/livez'. Consider aligning the endpoint naming for consistency.
r.Handle("/livez", livenessChecker.Handler()).Methods(http.MethodGet)
There was a problem hiding this comment.
Pull Request Overview
This PR enhances the health check system by introducing separate endpoints for readiness and liveness probes and adding an error grace period to tolerate transient failures, along with improved test coverage.
- Adds separate readiness and liveness checkers with individual configurations in options.go
- Introduces the WithCheckerErrorGracePeriod option and corresponding tests in health/checker_test.go
- Updates documentation in README.md to reflect the new endpoints
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| options.go | Configures separate readiness and liveness health check routers with distinct grace periods |
| health/checker_test.go | Implements tests for the error grace period and multiple asynchronous checks |
| health/checker_options.go | Adds the WithCheckerErrorGracePeriod option |
| health/checker.go | Implements error grace period logic in the Checker |
| README.md | Updates documentation to include new health check endpoints for Kubernetes readiness/liveness |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull Request Overview
This PR enhances the health check system by introducing separate readiness and liveness probes along with an error grace period to avoid premature failure during transient errors. Key changes include:
- Establishing two distinct health checkers where liveness checks include a configurable grace period.
- Integrating the gorilla/mux router to serve the new
/readyzand/livezendpoints. - Expanding unit tests to cover error grace period scenarios.
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| options.go | Configures distinct readiness and liveness checkers with appropriate routes |
| health/checker_test.go | Adds tests for error grace period behavior and endpoint responses |
| health/checker_options.go | Introduces the new Option to configure the error grace period |
| health/checker.go | Implements error tolerance logic using the grace period mechanism |
| README.md | Updates documentation to describe the new Kubernetes-friendly endpoints |
|
I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. |
Describe your changes
This pull request introduces several enhancements and changes to the health check system, including support for readiness and liveness probes, an error grace period for health checks, and improved test coverage. These changes aim to make the health checks more robust and Kubernetes-friendly.
Health Check Enhancements:
/readyzand/livez, allowing Kubernetes to distinguish between readiness and liveness probes. (README.md, README.mdL136-R137)WithCheckerErrorGracePeriodoption and logic in theCheckerstruct to handle the grace period. (health/checker.go, [1] [2];health/checker_options.go, [3]Codebase Updates:
WithHealthCheckfunction to use separatereadinessCheckerandlivenessCheckerinstances, with different grace period configurations. (options.go, options.goL222-L238)gorilla/muxrouter to handle the new health check endpoints. (options.go, [1] [2]Test Improvements:
health/checker_test.go, health/checker_test.goR335-R412)health/checker_test.go, [1] [2]