✨ feat: Add Startup Probe to Healthcheck Middleware #3069
Conversation
kirankumar-grootan
requested review from
gaby,
sixcolors,
ReneWerner87 and
efectn
and removed request for
a team
|
Thanks for opening this pull request! 🎉 Please check out our contributing guidelines. If you need help or want to chat with us, join us on Discord https://gofiber.io/discord |
|
Warning Rate limit exceeded@kirankumar-grootan has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 19 minutes and 59 seconds before requesting another review. How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. WalkthroughThe recent changes introduce a new "Startup Probe" alongside the existing "Liveness" and "Readiness Probes" in the Fiber middleware. This probe helps check the startup state of HTTP applications using the Changes
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- middleware/healthcheck/config.go (1 hunks)
Additional context used
golangci-lint
middleware/healthcheck/config.go
27-27: File is not
gofumpt-ed with-extra(gofumpt)
Additional comments not posted (1)
middleware/healthcheck/config.go (1)
27-27: Addition ofDefaultStartupEndpointis consistent and appropriate.The new constant
DefaultStartupEndpointfollows the pattern of the existing constantsDefaultLivenessEndpointandDefaultReadinessEndpoint. This addition is appropriate and maintains consistency.Tools
golangci-lint
27-27: File is not
gofumpt-ed with-extra(gofumpt)
|
why did you close it? @kirankumar-grootan |
|
@kirankumar-grootan can you still add the default path for the startup probe and documentation so that people don't wonder why we didn't include them |
|
@ReneWerner87 I have added the default startup endpoint and updated docs and test accordingly. |
kirankumar-grootan
changed the title
kirankumar-grootan
changed the title
|
Will check it tomorrow, thx |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #3069 +/- ##
==========================================
+ Coverage 83.10% 83.12% +0.02%
==========================================
Files 115 115
Lines 8321 8321
==========================================
+ Hits 6915 6917 +2
+ Misses 1076 1075 -1
+ Partials 330 329 -1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
gaby
changed the title
|
@coderabbitai full review |
Actions performedFull review triggered. |
There was a problem hiding this comment.
Actionable comments posted: 2
Outside diff range, codebase verification and nitpick comments (11)
docs/middleware/healthcheck.md (2)
51-52: Ensure consistency in usage examples.Ensure that the usage example for the startup probe is consistent with the liveness and readiness probes.
- // Provide a minimal config for startup check + // Provide a minimal config for startup probe
68-73: Ensure consistency in customization examples.Ensure that the customization example for the startup probe is consistent with the liveness and readiness probes.
- // And it works the same for startup, just change the route + // And it works the same for the startup probe, just change the routemiddleware/healthcheck/healthcheck_test.go (9)
39-39: Ensure consistent naming in test functions.Ensure that the naming of the startup probe endpoint in test functions is consistent with the liveness and readiness probes.
- app.Get(DefaultStartupEndpoint, NewHealthChecker()) + app.Get(healthcheck.DefaultStartupEndpoint, NewHealthChecker())
43-46: Ensure consistent naming in test assertions.Ensure that the naming of the startup probe endpoint in test assertions is consistent with the liveness and readiness probes.
- shouldGiveOK(t, app, "/startupz") - shouldGiveNotFound(t, app, "/startupz/") + shouldGiveOK(t, app, healthcheck.DefaultStartupEndpoint) + shouldGiveNotFound(t, app, healthcheck.DefaultStartupEndpoint + "/")
58-58: Ensure consistent naming in test functions.Ensure that the naming of the startup probe endpoint in test functions is consistent with the liveness and readiness probes.
- app.Get(DefaultStartupEndpoint, NewHealthChecker()) + app.Get(healthcheck.DefaultStartupEndpoint, NewHealthChecker())
62-65: Ensure consistent naming in test assertions.Ensure that the naming of the startup probe endpoint in test assertions is consistent with the liveness and readiness probes.
- shouldGiveOK(t, app, "/startupz") - shouldGiveOK(t, app, "/startupz/") + shouldGiveOK(t, app, healthcheck.DefaultStartupEndpoint) + shouldGiveOK(t, app, healthcheck.DefaultStartupEndpoint + "/")
91-95: Ensure consistent naming in custom test functions.Ensure that the naming of the startup probe endpoint in custom test functions is consistent with the liveness and readiness probes.
- app.Get(DefaultStartupEndpoint, NewHealthChecker(Config{ + app.Get(healthcheck.DefaultStartupEndpoint, NewHealthChecker(Config{
113-114: Ensure consistent naming in custom test assertions.Ensure that the naming of the startup probe endpoint in custom test assertions is consistent with the liveness and readiness probes.
- shouldGiveStatus(t, app, "/startupz", fiber.StatusServiceUnavailable) + shouldGiveStatus(t, app, healthcheck.DefaultStartupEndpoint, fiber.StatusServiceUnavailable)
175-181: Ensure consistent naming in next test functions.Ensure that the naming of the startup probe endpoint in next test functions is consistent with the liveness and readiness probes.
- app.Get(DefaultStartupEndpoint, checker) - shouldGiveNotFound(t, app, "/startupz") + app.Get(healthcheck.DefaultStartupEndpoint, checker) + shouldGiveNotFound(t, app, healthcheck.DefaultStartupEndpoint)
189-189: Ensure consistent naming in benchmark test functions.Ensure that the naming of the startup probe endpoint in benchmark test functions is consistent with the liveness and readiness probes.
- app.Get(DefaultStartupEndpoint, NewHealthChecker()) + app.Get(healthcheck.DefaultStartupEndpoint, NewHealthChecker())
211-211: Ensure consistent naming in parallel benchmark test functions.Ensure that the naming of the startup probe endpoint in parallel benchmark test functions is consistent with the liveness and readiness probes.
- app.Get(DefaultStartupEndpoint, NewHealthChecker()) + app.Get(healthcheck.DefaultStartupEndpoint, NewHealthChecker())
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (3)
- docs/middleware/healthcheck.md (5 hunks)
- middleware/healthcheck/config.go (1 hunks)
- middleware/healthcheck/healthcheck_test.go (6 hunks)
Files skipped from review as they are similar to previous changes (1)
- middleware/healthcheck/config.go
There was a problem hiding this comment.
Actionable comments posted: 0
Outside diff range, codebase verification and nitpick comments (7)
middleware/healthcheck/config.go (1)
27-27: Add a comment for the new constant.To maintain consistency and clarity, consider adding a comment for the new
DefaultStartupEndpointconstant.+ // DefaultStartupEndpoint defines the default endpoint for the startup probe.docs/middleware/healthcheck.md (3)
7-7: Ensure consistency in the description.The description of the probes should consistently use Oxford commas for clarity.
- Liveness, readiness and startup probes middleware for [Fiber](https://github.com/gofiber/fiber) that provides three endpoints for checking the liveness, readiness and startup state of HTTP applications. + Liveness, readiness, and startup probes middleware for [Fiber](https://github.com/gofiber/fiber) that provides three endpoints for checking the liveness, readiness, and startup state of HTTP applications.
19-21: Clarify the behavior of the startup probe.The behavior description for the startup probe should differentiate it from the readiness probe.
- **Startup Probe**: Assesses if the application is ready to handle requests. + **Startup Probe**: Checks if the application has completed its startup sequence.
105-108: Improve the explanation of the probes.The explanation of the probes can be made clearer by separating the descriptions for each probe type.
- The readiness probe is typically used to indicate if the application is ready to start accepting traffic (e.g., all necessary components are initialized and dependent services are available) and the startup probe typically used to indicate if the application has completed its startup sequence and is ready to proceed with initialization and readiness checks + The readiness probe is typically used to indicate if the application is ready to start accepting traffic (e.g., all necessary components are initialized and dependent services are available). + The startup probe is typically used to indicate if the application has completed its startup sequence and is ready to proceed with initialization and readiness checks.middleware/healthcheck/healthcheck_test.go (3)
39-39: Ensure consistency in endpoint definitions.Consider using the
DefaultLivenessEndpointandDefaultReadinessEndpointconstants for consistency.- app.Get("/livez", NewHealthChecker()) - app.Get("/readyz", NewHealthChecker()) + app.Get(DefaultLivenessEndpoint, NewHealthChecker()) + app.Get(DefaultReadinessEndpoint, NewHealthChecker())
58-58: Ensure consistency in endpoint definitions.Consider using the
DefaultLivenessEndpointandDefaultReadinessEndpointconstants for consistency.- app.Get("/livez", NewHealthChecker()) - app.Get("/readyz", NewHealthChecker()) + app.Get(DefaultLivenessEndpoint, NewHealthChecker()) + app.Get(DefaultReadinessEndpoint, NewHealthChecker())
175-175: Ensure consistency in endpoint definitions.Consider using the
DefaultLivenessEndpointandDefaultReadinessEndpointconstants for consistency.- app.Get("/readyz", checker) - app.Get("/livez", checker) + app.Get(DefaultReadinessEndpoint, checker) + app.Get(DefaultLivenessEndpoint, checker)
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (3)
- docs/middleware/healthcheck.md (5 hunks)
- middleware/healthcheck/config.go (1 hunks)
- middleware/healthcheck/healthcheck_test.go (6 hunks)
Additional comments not posted (6)
docs/middleware/healthcheck.md (1)
51-52: Examples update looks good!The updated examples for the startup probe endpoint are clear and consistent with the other probes.
Also applies to: 68-69, 70-73
middleware/healthcheck/healthcheck_test.go (5)
43-43: Tests update looks good!The tests for the startup probe endpoint are well integrated and consistent with the other probes.
Also applies to: 46-46, 49-49
62-62: Tests update looks good!The tests for the startup probe endpoint are well integrated and consistent with the other probes.
Also applies to: 65-65, 68-68
91-95: Tests update looks good!The custom tests for the startup probe endpoint are well integrated and consistent with the other probes.
Also applies to: 113-114
181-181: Tests update looks good!The tests for the startup probe endpoint are well integrated and consistent with the other probes.
189-189: Benchmark tests update looks good!The benchmark tests for the startup probe endpoint are well integrated and consistent with the other probes.
Also applies to: 211-211
There was a problem hiding this comment.
Actionable comments posted: 0
Outside diff range, codebase verification and nitpick comments (1)
docs/middleware/healthcheck.md (1)
7-7: Improve readability by adding a comma.To improve readability, consider adding a comma after "readiness":
- Liveness, readiness and startup probes middleware for [Fiber](https://github.com/gofiber/fiber) that provides three endpoints for checking the liveness, readiness, and startup state of HTTP applications. + Liveness, readiness, and startup probes middleware for [Fiber](https://github.com/gofiber/fiber) that provides three endpoints for checking the liveness, readiness, and startup state of HTTP applications.
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- docs/middleware/healthcheck.md (5 hunks)
Additional comments not posted (4)
docs/middleware/healthcheck.md (4)
19-21: LGTM!The description of the startup probe is clear and aligns with its intended purpose.
51-52: LGTM!The code example for the startup check is consistent with the existing examples for liveness and readiness checks.
68-73: LGTM!The extended config example for the startup check is clear and consistent with the other probe examples.
105-108: LGTM!The updated comments for the Config struct are clear and provide the necessary information about the startup probe.
…ring for test cases
…ring for test cases
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- middleware/healthcheck/healthcheck_test.go (5 hunks)
Files skipped from review as they are similar to previous changes (1)
- middleware/healthcheck/healthcheck_test.go
gaby
changed the title
There was a problem hiding this comment.
Actionable comments posted: 1
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- docs/whats_new.md (3 hunks)
Additional context used
Markdownlint
docs/whats_new.md
323-323: null
Link fragments should be valid(MD051, link-fragments)
Additional comments not posted (1)
docs/whats_new.md (1)
475-520: Review of Healthcheck Middleware Code ExamplesThe updated documentation and code examples clearly delineate the changes from a combined setup to individual configurations for each probe type. This provides better modularity and allows users to easily customize each probe according to their specific requirements.
The code examples are correct and align well with the described functionality. It's beneficial for users to see both the before and after configurations to understand the improvements in flexibility and customization.
|
Congrats on merging your first pull request! 🎉 We here at Fiber are proud of you! If you need help or want to chat with us, join us on Discord https://gofiber.io/discord |
* added startup default probe endpoint * added test case * updated docs * updated test order * added test case * fixed go fmt and md lint * fixed go fmt and md lint * updated doc as per coderabbitai suggestions * changed healhtcheck route register to use default const instead of string for test cases * updated whats new with healthcheck content * updated whats new doc with coderabbitai sugg * updated migration guide
Description
Added default startup probe route for fiber v3
#3068