feat: add detailed component health checks for MySQL, Redis, and MongoDB #114
Conversation
- Enhanced /health endpoint to show per-component status - Added type, host, port, database, version, and response time details - Returns error messages and types when components are down - Added MongoDB dependency and health check support
…to contributor/suraj
📝 WalkthroughWalkthroughAdded MongoDB support and extended the health endpoint to perform component-level checks (MySQL, Redis, MongoDB) with timestamps and versions; introduced Spring Data MongoDB dependency and config; and added a GitHub Actions step to fetch the PR base branch for commit-lint comparisons. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant HealthService
participant MySQL
participant Redis
participant MongoDB
Client->>HealthService: GET /health
HealthService->>MySQL: perform MySQL health check (query/version)
MySQL-->>HealthService: status, version, responseTime
HealthService->>Redis: perform Redis ping/version
Redis-->>HealthService: status, version, responseTime
HealthService->>MongoDB: perform MongoDB ping/version
MongoDB-->>HealthService: status, version, responseTime
HealthService->>HealthService: aggregate component statuses, compute overall status, timestamp
HealthService-->>Client: JSON health payload (timestamp, status, components...)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 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
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 |
Suraj-kumar00
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @src/main/java/com/iemr/admin/service/health/HealthService.java:
- Around line 230-232: The ping check in HealthService uses
pingResult.getDouble("ok") which can throw ClassCastException if "ok" is an
Integer; change the check to safely handle numeric types by retrieving "ok" as a
Number (e.g., pingResult.get("ok", Number.class) or cast to Number) and compare
its doubleValue() to 1.0 (or compare number.intValue() == 1) after null-checking
pingResult; update the condition around mongoTemplate.getDb().runCommand(...) to
use this safe numeric comparison.
🧹 Nitpick comments (3)
src/main/resources/application.properties (1)
78-81: Consider adding MongoDB authentication configuration.The MongoDB configuration lacks authentication credentials (
spring.data.mongodb.username,spring.data.mongodb.password,spring.data.mongodb.authentication-database). While this may be acceptable for thetestprofile (line 10), ensure that environment-specific property files for production include proper authentication and consider TLS settings (spring.data.mongodb.ssl.enabled).src/main/java/com/iemr/admin/service/health/HealthService.java (2)
180-184: Consider adding a timeout for Redis operations.Unlike the MySQL health check which has explicit timeouts, the Redis ping operation doesn't have a timeout configured. If Redis is unresponsive, this could block the health check thread indefinitely. Consider configuring
spring.redis.timeoutin application properties or wrapping the operation with a timeout.
305-349: Consider extracting common URL parsing logic to reduce duplication.
extractHostandextractPortshare identical logic for prefix removal andhostPortextraction (lines 311-316 and 336-341). Consider extracting this to a helper method for maintainability.♻️ Suggested refactor
+ private String extractHostPort(String jdbcUrl) { + if (jdbcUrl == null || "unknown".equals(jdbcUrl)) { + return null; + } + try { + String withoutPrefix = jdbcUrl.replaceFirst("jdbc:mysql://", ""); + int slashIndex = withoutPrefix.indexOf('/'); + return slashIndex > 0 ? withoutPrefix.substring(0, slashIndex) : withoutPrefix; + } catch (Exception e) { + return null; + } + } + private String extractHost(String jdbcUrl) { - if (jdbcUrl == null || "unknown".equals(jdbcUrl)) { - return "unknown"; - } - try { - String withoutPrefix = jdbcUrl.replaceFirst("jdbc:mysql://", ""); - int slashIndex = withoutPrefix.indexOf('/'); - String hostPort = slashIndex > 0 - ? withoutPrefix.substring(0, slashIndex) - : withoutPrefix; + String hostPort = extractHostPort(jdbcUrl); + if (hostPort == null) return "unknown"; + try { int colonIndex = hostPort.indexOf(':'); return colonIndex > 0 ? hostPort.substring(0, colonIndex) : hostPort; } catch (Exception e) { - logger.debug("Could not extract host from URL: {}", e.getMessage()); + return "unknown"; } - return "unknown"; }
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.github/workflows/commit-lint.ymlpom.xmlsrc/main/java/com/iemr/admin/service/health/HealthService.javasrc/main/resources/application.properties
🔇 Additional comments (6)
.github/workflows/commit-lint.yml (1)
20-22: LGTM! Proper base branch fetching for commitlint.This step correctly fetches the base branch to ensure
commitlint --fromcan access the base SHA. Thefetch-depth: 0in the checkout step combined with this fetch ensures the full commit history is available for comparison.pom.xml (1)
203-206: LGTM!The MongoDB starter dependency is correctly added without an explicit version, allowing Spring Boot's dependency management (via
spring-boot-starter-parent:3.2.2) to resolve a compatible version.src/main/java/com/iemr/admin/service/health/HealthService.java (4)
54-77: LGTM! Good use of optional autowiring and configuration defaults.The
@Autowired(required = false)for bothRedisTemplateandMongoTemplatecorrectly handles scenarios where these datastores are not configured. The@Valueannotations with sensible defaults ensure the service doesn't fail if properties are missing.
78-114: LGTM! Clean health aggregation logic.The method correctly aggregates component health statuses and marks overall status as
DOWNif any component fails. The null checks forredisTemplateandmongoTemplateproperly handle optional datastores.
116-168: LGTM! Robust MySQL health check with proper resource management.Good implementation with:
- Try-with-resources for automatic connection and statement cleanup
- Reasonable timeouts (2s for connection validation, 3s for query)
- Comprehensive error details including exception type
- Connection validation before executing the health check query
263-299: LGTM! Defensive version retrieval with graceful fallback.All three version retrieval methods properly catch exceptions and log at
debuglevel (appropriate since version info is supplementary). Returningnullon failure is correctly handled by the callers with null checks before adding to details.
|
|
Hi @vanitha1822 and @vishwab1 |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @src/main/java/com/iemr/admin/service/health/HealthService.java:
- Around line 171-178: The MongoDB health check may NPE by unboxing a null from
pingResult.getDouble("ok") and uses fragile equality; update the lambda in
performHealthCheck to first verify pingResult is non-null and contains the "ok"
key, retrieve the value as a Number (e.g., Number okVal = (Number)
pingResult.get("ok")) or via getDouble with null-check, ensure okVal != null,
then use Double.compare(okVal.doubleValue(), 1.0) == 0 or a tolerance (e.g., >=
0.999) to decide success; if the key is missing or the value indicates failure
return new HealthCheckResult(false, null, "Ping returned unexpected or missing
ok value") and keep getMongoDBVersion() and HealthCheckResult usage unchanged.
🧹 Nitpick comments (1)
src/main/java/com/iemr/admin/service/health/HealthService.java (1)
190-215: Response time not recorded for failed health checks.
responseTimeMsis only added when the check succeeds. Recording response time for failures would help diagnose timeout vs immediate failure scenarios.Proposed improvement
try { HealthCheckResult result = checker.get(); long responseTime = System.currentTimeMillis() - startTime; + details.put("responseTimeMs", responseTime); if (result.isHealthy) { logger.debug("{} health check: UP ({}ms)", componentName, responseTime); status.put("status", "UP"); - details.put("responseTimeMs", responseTime); if (result.version != null) { details.put("version", result.version); } } else { logger.warn("{} health check: {}", componentName, result.error); status.put("status", "DOWN"); details.put("error", result.error); } status.put("details", details); return status; } catch (Exception e) { + long responseTime = System.currentTimeMillis() - startTime; logger.error("{} health check failed: {}", componentName, e.getMessage()); status.put("status", "DOWN"); details.put("error", e.getMessage()); details.put("errorType", e.getClass().getSimpleName()); + details.put("responseTimeMs", responseTime); status.put("details", details); return status; }
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/main/java/com/iemr/admin/service/health/HealthService.java
🔇 Additional comments (3)
src/main/java/com/iemr/admin/service/health/HealthService.java (3)
316-329: LGTM!Clean internal result holder. If targeting Java 16+, this could be simplified to a record, but the current implementation is perfectly fine.
79-115: Well-structured health aggregation.Good use of
LinkedHashMapfor predictable JSON ordering, conditional component checks based on template availability, and clean aggregation of overall status. The timestamp usingInstant.now().toString()produces ISO-8601 format which is appropriate for APIs.
260-276: No issue: URL parsing is appropriately MySQL-specific.The application consistently uses
com.mysql.cj.jdbc.Driveracross all environment configurations (admin_ci.properties, admin_docker.properties, admin_example.properties). TheextractHost,extractPort, andextractDatabaseNamemethods are correctly hardcoded for thejdbc:mysql://format that matches the actual JDBC driver in use. The specificity is intentional and appropriate for this codebase's design.Likely an incorrect or invalid review comment.
| return performHealthCheck("MongoDB", details, () -> { | ||
| Document pingResult = mongoTemplate.getDb().runCommand(new Document("ping", 1)); | ||
| if (pingResult != null && pingResult.getDouble("ok") == 1.0) { | ||
| String version = getMongoDBVersion(); | ||
| return new HealthCheckResult(true, version, null); | ||
| } | ||
| return new HealthCheckResult(false, null, "Ping returned unexpected response"); | ||
| }); |
There was a problem hiding this comment.
Potential NullPointerException on MongoDB health check.
Document.getDouble("ok") returns a Double wrapper that can be null if the key doesn't exist. Unboxing a null Double for comparison with 1.0 will throw NPE. Additionally, floating-point equality comparisons are fragile.
Proposed fix
return performHealthCheck("MongoDB", details, () -> {
Document pingResult = mongoTemplate.getDb().runCommand(new Document("ping", 1));
- if (pingResult != null && pingResult.getDouble("ok") == 1.0) {
+ if (pingResult != null) {
+ Double okValue = pingResult.getDouble("ok");
+ if (okValue != null && okValue >= 1.0) {
String version = getMongoDBVersion();
return new HealthCheckResult(true, version, null);
+ }
}
return new HealthCheckResult(false, null, "Ping returned unexpected response");
});📝 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.
| return performHealthCheck("MongoDB", details, () -> { | |
| Document pingResult = mongoTemplate.getDb().runCommand(new Document("ping", 1)); | |
| if (pingResult != null && pingResult.getDouble("ok") == 1.0) { | |
| String version = getMongoDBVersion(); | |
| return new HealthCheckResult(true, version, null); | |
| } | |
| return new HealthCheckResult(false, null, "Ping returned unexpected response"); | |
| }); | |
| return performHealthCheck("MongoDB", details, () -> { | |
| Document pingResult = mongoTemplate.getDb().runCommand(new Document("ping", 1)); | |
| if (pingResult != null) { | |
| Double okValue = pingResult.getDouble("ok"); | |
| if (okValue != null && okValue >= 1.0) { | |
| String version = getMongoDBVersion(); | |
| return new HealthCheckResult(true, version, null); | |
| } | |
| } | |
| return new HealthCheckResult(false, null, "Ping returned unexpected response"); | |
| }); |
🤖 Prompt for AI Agents
In @src/main/java/com/iemr/admin/service/health/HealthService.java around lines
171 - 178, The MongoDB health check may NPE by unboxing a null from
pingResult.getDouble("ok") and uses fragile equality; update the lambda in
performHealthCheck to first verify pingResult is non-null and contains the "ok"
key, retrieve the value as a Number (e.g., Number okVal = (Number)
pingResult.get("ok")) or via getDouble with null-check, ensure okVal != null,
then use Double.compare(okVal.doubleValue(), 1.0) == 0 or a tolerance (e.g., >=
0.999) to decide success; if the key is missing or the value indicates failure
return new HealthCheckResult(false, null, "Ping returned unexpected or missing
ok value") and keep getMongoDBVersion() and HealthCheckResult usage unchanged.
| biological-screening-device-url=http://localhost:8096/ezdx-hub-connect-srv | ||
|
|
||
| ## MongoDB Configuration | ||
| spring.data.mongodb.host=mongodb-container |
There was a problem hiding this comment.
These don't belong here. It's environment related variables.
| submodules: true | ||
| fetch-depth: 0 | ||
|
|
||
| - name: Fetch base branch |
|
Admin-API doesn't talk to Mongo directly. This is a dependency for FHIR-API. @Suraj-kumar00 can you also show the final JSON output? |
2 participants
📋 Description
JIRA ID:
Please provide a summary of the change and the motivation behind it. Include relevant context and details.
✅ Type of Change
ℹ️ Additional Information
Please describe how the changes were tested, and include any relevant screenshots, logs, or other information that provides additional context.
Summary by CodeRabbit
New Features
Chores
✏️ Tip: You can customize this high-level summary in your review settings.