Added Health Check Module for Ktor#5498
Open
Subhanshu20135 wants to merge 6 commits intoktorio:mainfrom
Open
Conversation
Contributor
📝 WalkthroughWalkthroughAdds a new Ktor server plugin "HealthCheck" with config DSL for readiness and liveness endpoints, concurrent suspendable named checks, GET-only interception, JSON responses with per-check statuses and overall 200/503 mapping; includes README, build/settings, implementation, and tests. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@ktor-server/ktor-server-plugins/ktor-server-health-check/common/src/io/ktor/server/plugins/healthcheck/HealthCheck.kt`:
- Around line 75-80: The evaluateCheck function currently catches all Exceptions
and returns CheckStatus.DOWN with the raw cause.message which incorrectly
converts cancellations to DOWN and leaks backend error text; update
evaluateCheck to rethrow CancellationException (and other Cancellation-related
exceptions) instead of catching them, and when returning a DOWN result sanitize
the reason by using a generic message like "check failed" unless a configured
diagnostics flag (e.g., detailedDiagnosticsEnabled) is true, in which case
include the real cause message; reference the evaluateCheck function,
NamedCheck.check invocation, and CheckResult construction to locate where to
change the catch behavior and how to build the sanitized CheckResult.
- Around line 108-112: The escapeJson() implementation on String currently only
handles quotes, backslashes, and \n/\r/\t, leaving other control characters
(U+0000..U+001F) like \b and \f unescaped and producing invalid JSON; update
escapeJson() to iterate characters (or use a regex) and: map backspace and
formfeed to \"\\b\" and \"\\f\" respectively, keep the existing mappings for
'\\', '\"', '\\n', '\\r', '\\t', and for any other code point <= 0x1F produce a
\\u00XX hex escape; ensure the function returns the correctly escaped JSON
string for use in HealthCheck output.
In
`@ktor-server/ktor-server-plugins/ktor-server-health-check/common/src/io/ktor/server/plugins/healthcheck/HealthCheckConfig.kt`:
- Around line 27-77: This change introduced new public API types
(HealthCheckConfig, HealthCheckBuilder and the public HealthCheck plugin entry
point) but the module’s ABI signatures under the module's /api/ directory
weren’t updated; run the module's Gradle ABI update (./gradlew
:<module-name>:updateLegacyAbi) to regenerate the API signature files, verify
the new entries for HealthCheckConfig, HealthCheckBuilder and the HealthCheck
plugin are present in the /api/ output, and commit those generated signature
files so the legacy ABI checks include the new public DSL surface.
- Around line 39-53: Both readiness(path: String, block: HealthCheckBuilder.()
-> Unit) and liveness(path: String, block: HealthCheckBuilder.() -> Unit)
blindly append a new HealthCheckEndpoint to endpoints, allowing duplicate
normalized paths which later get ignored; change both methods to normalize the
path (use path.ensureLeadingSlash()), check endpoints.any { it.path ==
normalized } and require that it is false, e.g. require(!endpoints.any { it.path
== normalized }) { "Duplicate health-check endpoint configured: $normalized
(already registered)" }, then proceed to create and append the
HealthCheckEndpoint using HealthCheckBuilder().apply(block).checks.toList().
In
`@ktor-server/ktor-server-plugins/ktor-server-health-check/common/test/io/ktor/server/plugins/healthcheck/HealthCheckTest.kt`:
- Around line 112-126: Update the test function `only GET requests are handled`
to assert the explicit response for POST /health instead of only checking that a
substring is absent: after installing `HealthCheck` with `liveness("/health")`
call `client.post("/health")` and assert the concrete expected status code
(e.g., HttpStatusCode.MethodNotAllowed or the plugin's actual response) and the
exact response body text you expect for POST, replacing the current
contains/false check so the test fails on regressions that change POST behavior.
🪄 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: Pro
Run ID: 77c0e88c-5838-4055-bd82-c218b4c6f0a7
📒 Files selected for processing (6)
ktor-server/ktor-server-plugins/ktor-server-health-check/README.mdktor-server/ktor-server-plugins/ktor-server-health-check/build.gradle.ktsktor-server/ktor-server-plugins/ktor-server-health-check/common/src/io/ktor/server/plugins/healthcheck/HealthCheck.ktktor-server/ktor-server-plugins/ktor-server-health-check/common/src/io/ktor/server/plugins/healthcheck/HealthCheckConfig.ktktor-server/ktor-server-plugins/ktor-server-health-check/common/test/io/ktor/server/plugins/healthcheck/HealthCheckTest.ktsettings.gradle.kts
...lugins/ktor-server-health-check/common/src/io/ktor/server/plugins/healthcheck/HealthCheck.kt
Outdated
Show resolved
Hide resolved
...lugins/ktor-server-health-check/common/src/io/ktor/server/plugins/healthcheck/HealthCheck.kt
Outdated
Show resolved
Hide resolved
Comment on lines
+27
to
+77
| public class HealthCheckConfig { | ||
| internal val endpoints = mutableListOf<HealthCheckEndpoint>() | ||
|
|
||
| /** | ||
| * Configures a readiness endpoint at the given [path]. | ||
| * | ||
| * Readiness checks determine whether the application is ready to serve traffic. | ||
| * Failed readiness checks cause Kubernetes to stop routing requests to the pod. | ||
| * | ||
| * @param path the URL path for the readiness endpoint (e.g., "/ready") | ||
| * @param block builder for adding individual health checks | ||
| */ | ||
| public fun readiness(path: String, block: HealthCheckBuilder.() -> Unit) { | ||
| endpoints += HealthCheckEndpoint(path.ensureLeadingSlash(), HealthCheckBuilder().apply(block).checks.toList()) | ||
| } | ||
|
|
||
| /** | ||
| * Configures a liveness endpoint at the given [path]. | ||
| * | ||
| * Liveness checks determine whether the application is still running. | ||
| * Failed liveness checks cause Kubernetes to restart the container. | ||
| * | ||
| * @param path the URL path for the liveness endpoint (e.g., "/health") | ||
| * @param block builder for adding individual health checks | ||
| */ | ||
| public fun liveness(path: String, block: HealthCheckBuilder.() -> Unit) { | ||
| endpoints += HealthCheckEndpoint(path.ensureLeadingSlash(), HealthCheckBuilder().apply(block).checks.toList()) | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Builder for configuring individual health checks within a readiness or liveness endpoint. | ||
| */ | ||
| @KtorDsl | ||
| public class HealthCheckBuilder { | ||
| internal val checks = mutableListOf<NamedCheck>() | ||
|
|
||
| /** | ||
| * Adds a named health check. | ||
| * | ||
| * The [block] should return `true` if the component is healthy, `false` if unhealthy. | ||
| * If [block] throws an exception, the check is treated as unhealthy and the exception | ||
| * message is included in the JSON response. | ||
| * | ||
| * @param name a human-readable identifier for this check (e.g., "database", "redis") | ||
| * @param block a suspend function returning `true` for healthy, `false` for unhealthy | ||
| */ | ||
| public fun check(name: String, block: suspend () -> Boolean) { | ||
| checks += NamedCheck(name, block) | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
Add the module's /api/ signatures for this new public DSL.
This file introduces public API (HealthCheckConfig, HealthCheckBuilder), and HealthCheck.kt adds the public HealthCheck plugin entry point, but the PR doesn't include the new module's ABI dump under /api/. That leaves the public surface untracked and will usually fail the legacy ABI checks.
As per coding guidelines: "All public/protected API changes must be tracked in /api/ directories within modules using ABI validation" and "Run ./gradlew :module-name:updateLegacyAbi to update ABI signature files after public/protected API changes".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@ktor-server/ktor-server-plugins/ktor-server-health-check/common/src/io/ktor/server/plugins/healthcheck/HealthCheckConfig.kt`
around lines 27 - 77, This change introduced new public API types
(HealthCheckConfig, HealthCheckBuilder and the public HealthCheck plugin entry
point) but the module’s ABI signatures under the module's /api/ directory
weren’t updated; run the module's Gradle ABI update (./gradlew
:<module-name>:updateLegacyAbi) to regenerate the API signature files, verify
the new entries for HealthCheckConfig, HealthCheckBuilder and the HealthCheck
plugin are present in the /api/ output, and commit those generated signature
files so the legacy ABI checks include the new public DSL surface.
.../ktor-server-health-check/common/src/io/ktor/server/plugins/healthcheck/HealthCheckConfig.kt
Outdated
Show resolved
Hide resolved
...s/ktor-server-health-check/common/test/io/ktor/server/plugins/healthcheck/HealthCheckTest.kt
Show resolved
Hide resolved
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
ktor-server/ktor-server-plugins/ktor-server-health-check/common/src/io/ktor/server/plugins/healthcheck/HealthCheck.kt (1)
75-80:⚠️ Potential issue | 🟠 MajorDon't turn cancellation into
DOWN, and don't expose raw failure text.Line 79 catches
CancellationExceptionviaException, so shutdown/request cancellation is reported as a normal health failure. Line 80 also returns raw backend messages from a typically public probe endpoint. Rethrow cancellation and return a sanitized failure string instead.🔒 Proposed fix
private suspend fun evaluateCheck(namedCheck: NamedCheck): CheckResult = try { val healthy = namedCheck.check() CheckResult(namedCheck.name, if (healthy) CheckStatus.UP else CheckStatus.DOWN) + } catch (cause: CancellationException) { + throw cause } catch (cause: Exception) { - CheckResult(namedCheck.name, CheckStatus.DOWN, cause.message) + CheckResult(namedCheck.name, CheckStatus.DOWN, "Health check failed") }Based on learnings: "In Ktor,
io.ktor.utils.io.CancellationExceptionis a typealias forkotlinx.coroutines.CancellationException, which is a supertype ofJobCancellationException. Therefore, checkinge !is io.ktor.utils.io.CancellationExceptionis sufficient to exclude all coroutine cancellation exceptions."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ktor-server/ktor-server-plugins/ktor-server-health-check/common/src/io/ktor/server/plugins/healthcheck/HealthCheck.kt` around lines 75 - 80, In evaluateCheck, avoid turning coroutine cancellations into DOWN and stop leaking raw error text: change the catch in evaluateCheck(namedCheck: NamedCheck) to rethrow cancellation exceptions (check for io.ktor.utils.io.CancellationException or kotlinx.coroutines.CancellationException) and for other exceptions return CheckResult(namedCheck.name, CheckStatus.DOWN, "<unavailable>" or a fixed sanitized message) instead of cause.message; keep references to NamedCheck, CheckResult and CheckStatus so the handler location is clear.
🧹 Nitpick comments (1)
ktor-server/ktor-server-plugins/ktor-server-health-check/common/src/io/ktor/server/plugins/healthcheck/HealthCheckConfig.kt (1)
96-97: Reject blank health-check names when registering a check.
check("", ...)currently succeeds and produces ambiguous"name"fields in the JSON payload and logs. Validatename.isNotBlank()before storing it.🛡️ Proposed fix
public fun check(name: String, block: suspend () -> Boolean) { + require(name.isNotBlank()) { + "Health check name must not be blank (received length=${name.length})" + } checks += NamedCheck(name, block) }As per coding guidelines: "Use
require(...)for argument validation" and "Make error messages actionable by including the problematic value and context".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ktor-server/ktor-server-plugins/ktor-server-health-check/common/src/io/ktor/server/plugins/healthcheck/HealthCheckConfig.kt` around lines 96 - 97, The check(name: String, block: suspend () -> Boolean) function currently allows blank names; update it to validate the argument with require(name.isNotBlank()) before adding NamedCheck to checks, and make the require message actionable by including the invalid value and context (e.g. "health check name must not be blank: '<value>'") so callers get a clear error; adjust any tests or callers if they relied on blank names.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@ktor-server/ktor-server-plugins/ktor-server-health-check/common/src/io/ktor/server/plugins/healthcheck/HealthCheckConfig.kt`:
- Around line 27-40: Remove the accidental nested duplicate class declaration
and the extra closing braces: delete the second "class HealthCheckConfig" block
and its duplicate "internal val endpoints =
mutableListOf<HealthCheckEndpoint>()" so there is a single HealthCheckConfig
class containing the existing methods (e.g., readiness endpoint configuration
methods). Ensure the remaining braces are balanced for the single
HealthCheckConfig declaration and that only one endpoints property exists.
---
Duplicate comments:
In
`@ktor-server/ktor-server-plugins/ktor-server-health-check/common/src/io/ktor/server/plugins/healthcheck/HealthCheck.kt`:
- Around line 75-80: In evaluateCheck, avoid turning coroutine cancellations
into DOWN and stop leaking raw error text: change the catch in
evaluateCheck(namedCheck: NamedCheck) to rethrow cancellation exceptions (check
for io.ktor.utils.io.CancellationException or
kotlinx.coroutines.CancellationException) and for other exceptions return
CheckResult(namedCheck.name, CheckStatus.DOWN, "<unavailable>" or a fixed
sanitized message) instead of cause.message; keep references to NamedCheck,
CheckResult and CheckStatus so the handler location is clear.
---
Nitpick comments:
In
`@ktor-server/ktor-server-plugins/ktor-server-health-check/common/src/io/ktor/server/plugins/healthcheck/HealthCheckConfig.kt`:
- Around line 96-97: The check(name: String, block: suspend () -> Boolean)
function currently allows blank names; update it to validate the argument with
require(name.isNotBlank()) before adding NamedCheck to checks, and make the
require message actionable by including the invalid value and context (e.g.
"health check name must not be blank: '<value>'") so callers get a clear error;
adjust any tests or callers if they relied on blank names.
🪄 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: Pro
Run ID: e5c99178-a091-42f3-bd4f-fe93f5b61359
📒 Files selected for processing (2)
ktor-server/ktor-server-plugins/ktor-server-health-check/common/src/io/ktor/server/plugins/healthcheck/HealthCheck.ktktor-server/ktor-server-plugins/ktor-server-health-check/common/src/io/ktor/server/plugins/healthcheck/HealthCheckConfig.kt
.../ktor-server-health-check/common/src/io/ktor/server/plugins/healthcheck/HealthCheckConfig.kt
Outdated
Show resolved
Hide resolved
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
ktor-server/ktor-server-plugins/ktor-server-health-check/common/test/io/ktor/server/plugins/healthcheck/HealthCheckTest.kt (1)
180-191: Consider adding a test for duplicate endpoint path rejection.The configuration validates against duplicate paths with
require(), but there's no test verifying this behavior throws an exception.✅ Proposed test
`@Test` fun `duplicate endpoint path throws exception`() = testApplication { val exception = assertFailsWith<IllegalArgumentException> { install(HealthCheck) { liveness("/health") { check("ok") { true } } readiness("/health") { check("db") { true } } } } assertContains(exception.message.orEmpty(), "/health") assertContains(exception.message.orEmpty(), "already configured") }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ktor-server/ktor-server-plugins/ktor-server-health-check/common/test/io/ktor/server/plugins/healthcheck/HealthCheckTest.kt` around lines 180 - 191, Add a unit test that verifies the HealthCheck configuration rejects duplicate paths by asserting an IllegalArgumentException is thrown when installing HealthCheck with the same path twice; in the test (e.g., name it `duplicate endpoint path throws exception`) call install(HealthCheck) and configure liveness("/health") { check("ok") { true } } and readiness("/health") { check("db") { true } } inside assertFailsWith<IllegalArgumentException> and assert that the exception.message contains "/health" and "already configured" to ensure the require() validation in the HealthCheck setup is exercised.ktor-server/ktor-server-plugins/ktor-server-health-check/common/src/io/ktor/server/plugins/healthcheck/HealthCheckConfig.kt (1)
83-85: Consider validating check names to prevent duplicates and empty names.The
check()method accepts any string as a name without validation. Empty/blank names or duplicate check names within the same endpoint would produce confusing health responses.🛡️ Proposed validation
public fun check(name: String, block: suspend () -> Boolean) { + require(name.isNotBlank()) { "Health check name must not be blank" } + require(checks.none { it.name == name }) { + "Health check '$name' is already defined in this endpoint" + } checks += NamedCheck(name, block) }As per coding guidelines: "Use
require(...)for argument validation" and "Make error messages actionable by including the problematic value and context".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ktor-server/ktor-server-plugins/ktor-server-health-check/common/src/io/ktor/server/plugins/healthcheck/HealthCheckConfig.kt` around lines 83 - 85, The check(name: String, block: suspend () -> Boolean) method in HealthCheckConfig accepts any name; add argument validation using require(...) to ensure name.isNotBlank() and throw a clear message including the provided name and context (e.g. "check name must not be blank: '<name>'"); also ensure uniqueness by checking the existing checks collection (checks) for an existing NamedCheck with the same name and require it not to exist, with an actionable message like "duplicate check name for HealthCheckConfig: '<name>'"; update the check(...) function to perform these two require(...) validations before adding NamedCheck(name, block).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@ktor-server/ktor-server-plugins/ktor-server-health-check/common/test/io/ktor/server/plugins/healthcheck/HealthCheckTest.kt`:
- Around line 25-31: The test contains a duplicate assertion asserting
"\"status\":\"UP\"" twice in the client.get("/health") response block; remove
the second assertContains("\"status\":\"UP\"") (the duplicate in the response
handling block in HealthCheckTest.kt) so the test only checks overall status
once and the check name once (leave the assertions for response.status, body
contains "\"status\":\"UP\"" and "\"name\":\"alive\"").
---
Nitpick comments:
In
`@ktor-server/ktor-server-plugins/ktor-server-health-check/common/src/io/ktor/server/plugins/healthcheck/HealthCheckConfig.kt`:
- Around line 83-85: The check(name: String, block: suspend () -> Boolean)
method in HealthCheckConfig accepts any name; add argument validation using
require(...) to ensure name.isNotBlank() and throw a clear message including the
provided name and context (e.g. "check name must not be blank: '<name>'"); also
ensure uniqueness by checking the existing checks collection (checks) for an
existing NamedCheck with the same name and require it not to exist, with an
actionable message like "duplicate check name for HealthCheckConfig: '<name>'";
update the check(...) function to perform these two require(...) validations
before adding NamedCheck(name, block).
In
`@ktor-server/ktor-server-plugins/ktor-server-health-check/common/test/io/ktor/server/plugins/healthcheck/HealthCheckTest.kt`:
- Around line 180-191: Add a unit test that verifies the HealthCheck
configuration rejects duplicate paths by asserting an IllegalArgumentException
is thrown when installing HealthCheck with the same path twice; in the test
(e.g., name it `duplicate endpoint path throws exception`) call
install(HealthCheck) and configure liveness("/health") { check("ok") { true } }
and readiness("/health") { check("db") { true } } inside
assertFailsWith<IllegalArgumentException> and assert that the exception.message
contains "/health" and "already configured" to ensure the require() validation
in the HealthCheck setup is exercised.
🪄 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: Pro
Run ID: 38f4eb74-6a29-498e-9fc9-589cd19fa299
📒 Files selected for processing (3)
ktor-server/ktor-server-plugins/ktor-server-health-check/common/src/io/ktor/server/plugins/healthcheck/HealthCheck.ktktor-server/ktor-server-plugins/ktor-server-health-check/common/src/io/ktor/server/plugins/healthcheck/HealthCheckConfig.ktktor-server/ktor-server-plugins/ktor-server-health-check/common/test/io/ktor/server/plugins/healthcheck/HealthCheckTest.kt
🚧 Files skipped from review as they are similar to previous changes (1)
- ktor-server/ktor-server-plugins/ktor-server-health-check/common/src/io/ktor/server/plugins/healthcheck/HealthCheck.kt
Comment on lines
+25
to
+31
| client.get("/health").let { response -> | ||
| assertEquals(HttpStatusCode.OK, response.status) | ||
| val body = response.bodyAsText() | ||
| assertContains(body, "\"status\":\"UP\"") | ||
| assertContains(body, "\"name\":\"alive\"") | ||
| assertContains(body, "\"status\":\"UP\"") | ||
| } |
Contributor
There was a problem hiding this comment.
Duplicate assertion on line 30.
Line 30 duplicates the exact same assertion as line 28 ("\"status\":\"UP\""). The second check likely intended to verify the individual check's status field rather than the overall status again.
💚 Proposed fix
client.get("/health").let { response ->
assertEquals(HttpStatusCode.OK, response.status)
val body = response.bodyAsText()
assertContains(body, "\"status\":\"UP\"")
assertContains(body, "\"name\":\"alive\"")
- assertContains(body, "\"status\":\"UP\"")
}The overall status check on line 28 already confirms UP status; the "name":"alive" assertion on line 29 confirms the check is present. No need to re-assert status.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| client.get("/health").let { response -> | |
| assertEquals(HttpStatusCode.OK, response.status) | |
| val body = response.bodyAsText() | |
| assertContains(body, "\"status\":\"UP\"") | |
| assertContains(body, "\"name\":\"alive\"") | |
| assertContains(body, "\"status\":\"UP\"") | |
| } | |
| client.get("/health").let { response -> | |
| assertEquals(HttpStatusCode.OK, response.status) | |
| val body = response.bodyAsText() | |
| assertContains(body, "\"status\":\"UP\"") | |
| assertContains(body, "\"name\":\"alive\"") | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@ktor-server/ktor-server-plugins/ktor-server-health-check/common/test/io/ktor/server/plugins/healthcheck/HealthCheckTest.kt`
around lines 25 - 31, The test contains a duplicate assertion asserting
"\"status\":\"UP\"" twice in the client.get("/health") response block; remove
the second assertContains("\"status\":\"UP\"") (the duplicate in the response
handling block in HealthCheckTest.kt) so the test only checks overall status
once and the check name once (leave the assertions for response.status, body
contains "\"status\":\"UP\"" and "\"name\":\"alive\"").
…n/src/io/ktor/server/plugins/healthcheck/HealthCheckConfig.kt Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
…n/src/io/ktor/server/plugins/healthcheck/HealthCheck.kt Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Subhanshu20135
force-pushed
the
feature/health-check
branch
from
c2a202b to
fe974f9
Compare
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
ktor-server/ktor-server-plugins/ktor-server-health-check/common/src/io/ktor/server/plugins/healthcheck/HealthCheckConfig.kt (1)
31-37: Validate blankpathand blank checknameduring configuration.
readiness/livenesscurrently allow""(normalized to/), andcheck()allows empty names. Validating both early makes misconfiguration obvious.♻️ Proposed fix
private fun addEndpoint(path: String, block: HealthCheckBuilder.() -> Unit) { + require(path.isNotBlank()) { + "Health check endpoint path must not be blank (received: '$path')" + } val normalizedPath = path.ensureLeadingSlash() require(endpoints.none { it.path == normalizedPath }) { "Health check endpoint path '$normalizedPath' is already configured" } endpoints += HealthCheckEndpoint(normalizedPath, HealthCheckBuilder().apply(block).checks.toList()) } @@ public fun check(name: String, block: suspend () -> Boolean) { + require(name.isNotBlank()) { + "Health check name must not be blank (received: '$name')" + } checks += NamedCheck(name, block) }As per coding guidelines: "Use
require(...)for argument validation" and "Make error messages actionable by including the problematic value and context".Also applies to: 83-85
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ktor-server/ktor-server-plugins/ktor-server-health-check/common/src/io/ktor/server/plugins/healthcheck/HealthCheckConfig.kt` around lines 31 - 37, Reject blank endpoint paths and blank check names early: in addEndpoint (and the other endpoint-adding overload around lines 83-85) add a require that the normalizedPath is not "/" (or that the original path is not blank) with an actionable message including the offending value; and in HealthCheckBuilder.check validate the provided name with require(name.isNotBlank()) and an error message that includes the empty name/context. Use require(...) for these argument checks and reference HealthCheckEndpoint/HealthCheckBuilder.addEndpoint and HealthCheckBuilder.check so the validation is added in the right methods.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@ktor-server/ktor-server-plugins/ktor-server-health-check/common/src/io/ktor/server/plugins/healthcheck/HealthCheckConfig.kt`:
- Around line 31-37: Reject blank endpoint paths and blank check names early: in
addEndpoint (and the other endpoint-adding overload around lines 83-85) add a
require that the normalizedPath is not "/" (or that the original path is not
blank) with an actionable message including the offending value; and in
HealthCheckBuilder.check validate the provided name with
require(name.isNotBlank()) and an error message that includes the empty
name/context. Use require(...) for these argument checks and reference
HealthCheckEndpoint/HealthCheckBuilder.addEndpoint and HealthCheckBuilder.check
so the validation is added in the right methods.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 5aaf6d61-f401-4a76-aa16-eba4d064e851
📒 Files selected for processing (6)
ktor-server/ktor-server-plugins/ktor-server-health-check/README.mdktor-server/ktor-server-plugins/ktor-server-health-check/build.gradle.ktsktor-server/ktor-server-plugins/ktor-server-health-check/common/src/io/ktor/server/plugins/healthcheck/HealthCheck.ktktor-server/ktor-server-plugins/ktor-server-health-check/common/src/io/ktor/server/plugins/healthcheck/HealthCheckConfig.ktktor-server/ktor-server-plugins/ktor-server-health-check/common/test/io/ktor/server/plugins/healthcheck/HealthCheckTest.ktsettings.gradle.kts
✅ Files skipped from review due to trivial changes (4)
- ktor-server/ktor-server-plugins/ktor-server-health-check/README.md
- ktor-server/ktor-server-plugins/ktor-server-health-check/build.gradle.kts
- ktor-server/ktor-server-plugins/ktor-server-health-check/common/test/io/ktor/server/plugins/healthcheck/HealthCheckTest.kt
- settings.gradle.kts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Add HealthCheck plugin for Kubernetes readiness and liveness probes
Summary
ktor-server-health-checkmodule providing a declarative DSL for configuring health check endpoints (readinessandliveness) with named, concurrent checksMotivation
Every production Ktor application deployed to Kubernetes needs health and readiness endpoints. Currently, developers write these manually every time. Competing frameworks (Spring Actuator, Micronaut Health, Quarkus SmallRye Health) all ship this out of the box. This plugin makes it a one-liner:
Design
ApplicationPluginwithonCallinterception -- matches configured paths before routing, so endpoints work even withoutRoutinginstalledcoroutineScope { async { ... } }, minimizing probe latency for I/O-bound checksStringBuilder, no serialization dependencycommonMain, works on JVM, Native, and JS targetsResponse Format
The plugin responds with
application/json:All healthy (HTTP 200)
{ "status": "UP", "checks": [ { "name": "database", "status": "UP" }, { "name": "redis", "status": "UP" } ] }One or more unhealthy (HTTP 503)
{ "status": "DOWN", "checks": [ { "name": "database", "status": "UP" }, { "name": "redis", "status": "DOWN", "error": "Connection refused" } ] }Files Changed
settings.gradle.ktsktor-server-health-checkmodulektor-server-health-check/build.gradle.ktsktorbuild.project.server-pluginHealthCheckConfig.kt@KtorDslconfig classes:HealthCheckConfig,HealthCheckBuilderHealthCheck.ktonCallinterception, check evaluation, JSON responseHealthCheckTest.ktREADME.md