♻️ refactor: Migrate HealthChecker to v3

[luk3skyw4lker]

Description

Related to #2863

Changes Introduced

Migrating the healthchecker middleware to v3, using the proposition made by @ReneWerner87 in the above mentioned PR. Here is the benchmark and successful test results:

Type of Change

Please delete options that are not relevant.

• New feature (non-breaking change which adds functionality)
• Enhancement (improvement to existing features and functionality)
• Documentation update (changes to documentation)
• Performance improvement (non-breaking change which improves efficiency)
• Code consistency (non-breaking change which improves code reliability and robustness)

Checklist

Before you submit your pull request, please make sure you meet these requirements:

• Followed the inspiration of the Express.js framework for new functionalities, making them similar in usage.
• Conducted a self-review of the code and provided comments for complex or critical parts.
• Updated the documentation in the /docs/ directory for Fiber's documentation.
• Added or updated unit tests to validate the effectiveness of the changes or new features.
• Ensured that new and existing unit tests pass locally with the changes.
• Verified that any new dependencies are essential and have been agreed upon by the maintainers/community.
• Aimed for optimal performance with minimal allocations in the new code.
• Provided benchmarks for the new code to analyze and improve upon.

Summary by CodeRabbit

• New Features
  • Enhanced customization for health check middleware in Fiber apps, allowing for custom probes for liveness and readiness checks.
• Refactor
  • Simplified the configuration structure for health checks by consolidating probe functions.
  • Unified the approach for handling liveness and readiness checks.
• Documentation
  • Updated documentation to reflect new customization capabilities and configuration simplifications.
• Tests
  • Added new routes and updated tests to align with the refactored health check functionality.

[coderabbitai]

Walkthrough

The healthcheck middleware in Fiber has been updated to allow for more customization, enabling users to define probes for liveness and readiness checks with custom endpoints. The default configuration structure has been simplified, consolidating probe functions and enhancing flexibility.

Changes

Files	Summary
middleware/healthcheck/healthcheck_test.go	Added new routes /livez and /readyz with NewHealthChecker() in tests. Updated routing and configurations for health checks.
middleware/healthcheck/healthcheck.go	Merged LivenessProbe and ReadinessProbe into a single Probe field. Refactored health check handling.
middleware/healthcheck/config.go	Updated to allow more customization with probes for liveness and readiness.

🐇✨
In the land of code where the Fiber rabbits hop,
A new way to check if they're healthy or not.
With probes so sleek, and configs so neat,
They dance and they tweak, their checks now complete.
So here's to the devs, with their code so bright,
May your apps run smooth, and your load be light.
🌟🐰

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?

Share

• X
• Mastodon
• Reddit
• LinkedIn

---

Tips

Chat

There are 3 ways to chat with CodeRabbit:

• Review comments: Directly reply to a review comment made by CodeRabbit. Example:
  • I pushed a fix in commit <commit_id>.
  • Generate unit-tests for this file.
• Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
  • @coderabbitai generate unit tests for this file.
  • @coderabbitai modularize this function.
• PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
  • @coderabbitai generate interesting stats about this repository from git and render them as a table.
  • @coderabbitai show all the console.log statements in this repository.
  • @coderabbitai read src/utils.ts and generate unit tests.
  • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

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)

• @coderabbitai pause to pause the reviews on a PR.
• @coderabbitai resume to resume the paused reviews.
• @coderabbitai review to trigger a review. This is useful when automatic reviews are disabled for the repository.
• @coderabbitai resolve resolve all the CodeRabbit review comments.
• @coderabbitai help to get help.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Configration File (.coderabbit.yaml)

• You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
• The JSON schema for the configuration file is available here.
• If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/coderabbit-overrides.v2.json

CodeRabbit Discord Community

Join our Discord Community to get help, request features, and share feedback.

[coderabbitai]

Review Status

Actionable comments generated: 2

Configuration used: CodeRabbit UI

Commits

Files that changed from the base of the PR and between 4a1891b and 6701a93.

Files selected for processing (4)

• docs/api/middleware/healthcheck.md (2 hunks)
• middleware/healthcheck/config.go (1 hunks)
• middleware/healthcheck/healthcheck.go (2 hunks)
• middleware/healthcheck/healthcheck_test.go (7 hunks)

Additional comments: 3

middleware/healthcheck/healthcheck.go (1)

• 20-29: > 📝 NOTE

This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [10-27]

The implementation of NewHealthChecker function correctly follows the new design of using a single Probe function for health checks. However, there are a few points to consider:

• The check for c.Method() != fiber.MethodGet ensures that only GET requests are processed by the middleware, which aligns with the typical use case for health checks. However, this might limit flexibility for users who might want to use other HTTP methods for health checks. Consider making the allowed HTTP methods configurable or documenting this behavior clearly.
• The use of c.SendStatus directly sends HTTP status codes based on the probe result, which is straightforward and efficient. Ensure that this aligns with the users' expectations in terms of response body content for health check endpoints.
• It's good practice to include logging or metrics collection for health check failures to aid in debugging and monitoring the application's health status.

Overall, the changes are well-aligned with the objectives of simplifying and enhancing the middleware's functionality. Just ensure to consider the points mentioned for further refinement.

middleware/healthcheck/config.go (2)

• 19-19: The consolidation of LivenessProbe and ReadinessProbe into a single Probe field in the Config struct simplifies the configuration and usage of the healthcheck middleware. This change enhances flexibility by allowing users to define custom probes more easily. Ensure that the documentation and examples are updated to clearly demonstrate how to use the new Probe field for both liveness and readiness checks.
• 27-39: The implementation of defaultProbe as a function that always returns true is a sensible default behavior, indicating that the application is healthy by default. The defaultConfigV3 function correctly handles the case where no custom configuration is provided by the user, applying this default probe. Additionally, it ensures that a Probe is always set, preventing potential nil pointer dereferences. This approach maintains the simplicity and safety of the middleware's configuration. Good job on ensuring that the default behavior is both logical and user-friendly.

[luk3skyw4lker]

Seems like it wasn't only healthchecker that suffered from this:

[gaby]

Seems like it wasn't only healthchecker that suffered from this:

Thats fine, we can address that in #2864 I'm more concern with the tests failing

[coderabbitai]

Review Status

Actionable comments generated: 3

Configuration used: CodeRabbit UI

Commits

Files that changed from the base of the PR and between 6701a93 and 2aaa501.

Files selected for processing (1)

• middleware/healthcheck/healthcheck_test.go (8 hunks)

Additional comments: 4

middleware/healthcheck/healthcheck_test.go (4)

• 38-39: The addition of explicit routes /livez and /readyz with NewHealthChecker() in Test_HealthCheck_Strict_Routing_Default is a good practice. It ensures that the healthcheck middleware is tested under strict routing conditions, which is crucial for applications that require precise path matching. This change aligns with the PR's objective to enhance customization and simplify configuration.
• 53-54: Replacing generic middleware usage with specific GET routes for /livez and /readyz in Test_HealthCheck_Default is a significant improvement. It directly tests the middleware's response to these endpoints, ensuring that the healthcheck functionality is accessible and behaves as expected under default routing settings. This change contributes to the overall goal of simplifying the middleware's configuration and usage.
• 91-91: The assertions for StatusMethodNotAllowed in POST requests to /live and /ready endpoints are crucial for verifying that the middleware correctly restricts HTTP methods. This ensures that the healthcheck endpoints adhere to best practices by only responding to GET requests, aligning with the principle of using appropriate HTTP methods for specific actions. This test effectively validates the middleware's method filtering capabilities.

Also applies to: 96-96

• 112-118: In Test_HealthCheck_Custom_Nested, the tests for nested paths /probe/live and /probe/ready are well-structured and provide valuable coverage for applications that might structure their healthcheck endpoints within a nested path. This flexibility is a key aspect of the middleware's enhanced customization capabilities. Including tests for both successful and unsuccessful probes in nested paths ensures the middleware's behavior is predictable in various configurations.

[coderabbitai]

Review Status

Actionable comments generated: 0

Configuration used: CodeRabbit UI

Commits

Files that changed from the base of the PR and between 2aaa501 and 4308308.

Files selected for processing (1)

• middleware/healthcheck/healthcheck_test.go (8 hunks)

Files skipped from review as they are similar to previous changes (1)

• middleware/healthcheck/healthcheck_test.go

[gaby]

Performance Diff between Sequential / Parallel:

Benchmark_HealthCheck-4            	 6781260	       179.2 ns/op	       0 B/op	       0 allocs/op
Benchmark_HealthCheck_Parallel-4   	14886872	        85.98 ns/op	       0 B/op	       0 allocs/op

[ReneWerner87]

@ReneWerner87 @gaby I don't know how to explain this, but after this commit at the v2ToV3Merge branch, the performance of the middleware dropped in half:

Although the commit did not changed anything in the middleware itself, the performance dropped drastically, at least running locally. I'll push the changes to see what's the results

what was the performance in the v2 branch ?

its maybe related to my fix 5a0167a
for this real bug #2871 (comment)

[luk3skyw4lker]

@ReneWerner87 It was around 120 ns/op according to this comment: #2863 (comment)

But the performance before the commit and with the migration was around 110 ns/op according to this PR. I'll try and run it again with @gaby commits

[luk3skyw4lker]

goos: linux
goarch: amd64
pkg: github.com/gofiber/fiber/v3/middleware/healthcheck
cpu: Intel(R) Core(TM) i5-7300HQ CPU @ 2.50GHz
Benchmark_HealthCheck-4                  5853678               198.3 ns/op             0 B/op          0 allocs/op
Benchmark_HealthCheck-4                  5897978               199.9 ns/op             0 B/op          0 allocs/op
Benchmark_HealthCheck-4                  5854675               197.6 ns/op             0 B/op          0 allocs/op
Benchmark_HealthCheck-4                  5786534               197.8 ns/op             0 B/op          0 allocs/op
Benchmark_HealthCheck-4                  5776459               199.2 ns/op             0 B/op          0 allocs/op
Benchmark_HealthCheck_Parallel-4        17670471                58.34 ns/op            0 B/op          0 allocs/op
Benchmark_HealthCheck_Parallel-4        18804096                64.86 ns/op            0 B/op          0 allocs/op
Benchmark_HealthCheck_Parallel-4        19699030                63.55 ns/op            0 B/op          0 allocs/op
Benchmark_HealthCheck_Parallel-4        20749539                75.82 ns/op            0 B/op          0 allocs/op
Benchmark_HealthCheck_Parallel-4        17202957                74.76 ns/op            0 B/op          0 allocs/op
PASS
ok      github.com/gofiber/fiber/v3/middleware/healthcheck      16.499s

Looks the same as before to me, but since @gaby said it can be addressed in #2864 which also has other affected benchmarks

[gaby]

@luk3skyw4lker It's also 3-4x faster in the Parallel benchmarks which simulates real world concurrent users. Something about single sequential requests slowed down in V3

[luk3skyw4lker]

@gaby yep, noticed that too, I'm gonna be taking care of some code rabbit comments right now and the PR should be good to go, do you guys have any other ideas for changes/improvements?

[coderabbitai]

Review Status

Actionable comments generated: 3

Configuration used: CodeRabbit UI

Commits

Files that changed from the base of the PR and between 4308308 and 08904d1.

Files selected for processing (3)

• docs/api/middleware/healthcheck.md (2 hunks)
• middleware/healthcheck/config.go (2 hunks)
• middleware/healthcheck/healthcheck_test.go (7 hunks)

Files skipped from review as they are similar to previous changes (1)

• docs/api/middleware/healthcheck.md

Additional comments: 7

middleware/healthcheck/config.go (2)

• 29-29: The defaultProbe function provides a sensible default behavior by always returning true. This is a good practice for a health check middleware, ensuring that the service is considered healthy by default unless explicitly configured otherwise.
• 31-41: The defaultConfigV3 function correctly handles the case where no configuration is provided by the user, applying a default probe. This approach ensures backward compatibility and ease of use for developers integrating the health check middleware into their applications.

middleware/healthcheck/healthcheck_test.go (5)

• 38-39: Adding explicit routes for /livez and /readyz with NewHealthChecker() in the Test_HealthCheck_Strict_Routing_Default function is a good practice. It ensures that the health check endpoints are correctly set up and respond as expected under strict routing configurations.
• 53-54: The updates in Test_HealthCheck_Default to use NewHealthChecker() for /livez and /readyz endpoints are appropriate and reflect the middleware's intended usage. This ensures that the health check functionality works as expected with the framework's default routing behavior.
• 91-96: The tests for method restrictions (POST requests resulting in StatusMethodNotAllowed) are important for ensuring that the health check endpoints adhere to the intended HTTP method constraints. This helps in maintaining the security and integrity of the health check mechanism.
• 112-118: The Test_HealthCheck_Custom_Nested function correctly tests health check endpoints with nested paths, ensuring that the middleware functions correctly in more complex routing scenarios. This is crucial for applications that may organize health checks within a nested route structure.
• 153-163: The use of the Next function in Test_HealthCheck_Next to simulate a scenario where the middleware is skipped is well-implemented. This test ensures that the middleware behaves as expected when the Next function condition is met, providing flexibility in routing decisions.

[coderabbitai]

Review Status

Actionable comments generated: 0

Configuration used: CodeRabbit UI

Commits

Files that changed from the base of the PR and between 08904d1 and 7fc5469.

Files selected for processing (1)

• middleware/healthcheck/healthcheck_test.go (7 hunks)

Files skipped from review as they are similar to previous changes (1)

• middleware/healthcheck/healthcheck_test.go

[gaby]

LGTM