feat(health,version): add health and version endpoints#360
Closed
DurgaPrasad-54 wants to merge 3 commits intoPSMRI:mainfrom
Closed
feat(health,version): add health and version endpoints#360DurgaPrasad-54 wants to merge 3 commits intoPSMRI:mainfrom
DurgaPrasad-54 wants to merge 3 commits intoPSMRI:mainfrom
Conversation
Contributor
📝 WalkthroughWalkthroughAdds build-time git metadata generation, a /health REST endpoint with a concurrent multi-component HealthService (MySQL and Redis), refactors /version to return JSON from git.properties, and updates auth skip rules to allow unauthenticated access to /health and /version. Changes
Sequence Diagram(s)sequenceDiagram
actor Client
participant HC as HealthController
participant HS as HealthService
participant DS as DataSource (MySQL)
participant RT as RedisTemplate
Client->>HC: GET /health
HC->>HS: checkHealth()
par MySQL Health Check
HS->>DS: Validate connection / queries / metrics
DS-->>HS: Status & metrics
and Redis Health Check
HS->>RT: PING
RT-->>HS: PONG / error
end
HS->>HS: Aggregate component statuses → overall status
HS-->>HC: health map
HC->>HC: Map status → HTTP code (DOWN→503 else 200)
HC-->>Client: ResponseEntity with health data
sequenceDiagram
actor Client
participant VC as VersionController
participant Props as git.properties (file)
Client->>VC: GET /version
VC->>VC: loadGitProperties()
VC->>Props: Read from build output
Props-->>VC: git metadata
VC->>VC: Build response map (timestamp, version, branch, commitHash)
VC-->>Client: 200 OK + JSON map
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related issues
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
🧪 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 |
Contributor
Author
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
![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: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/main/java/com/iemr/common/utils/JwtUserIdValidationFilter.java (1)
239-256:⚠️ Potential issue | 🟡 MinorDuplicate
/user/refreshTokencheck.Line 254 (
path.startsWith(contextPath + "/user/refreshToken")) duplicates the existing check on Line 243 (path.equals(contextPath + "/user/refreshToken")). Note also the subtle inconsistency: one usesequalsand the other usesstartsWith. Remove the duplicate and settle on one form.Proposed fix
|| path.startsWith(contextPath + "/user/validateSecurityQuestionAndAnswer") || path.startsWith(contextPath + "/user/logOutUserFromConcurrentSession") - || path.startsWith(contextPath + "/user/refreshToken") || path.equals(contextPath + "/health") || path.equals(contextPath + "/version");🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/iemr/common/utils/JwtUserIdValidationFilter.java` around lines 239 - 256, In shouldSkipAuthentication(String path, String contextPath) remove the duplicated refreshToken check: consolidate the two conditions into a single check (choose either path.equals(contextPath + "/user/refreshToken") if you want exact match or path.startsWith(contextPath + "/user/refreshToken") if prefixes should be allowed) and delete the other occurrence so only one refreshToken check remains; update the list of OR'ed conditions accordingly.
🧹 Nitpick comments (3)
src/main/java/com/iemr/common/service/health/HealthService.java (2)
491-509: Consider using Java records for the inner data classes.Since the project targets Java 17 (per
pom.xml), these inner classes are good candidates for records, which are more concise and provideequals/hashCode/toStringfor free.Proposed refactor
- private static class AdvancedCheckResult { - final boolean isDegraded; - - AdvancedCheckResult(boolean isDegraded) { - this.isDegraded = isDegraded; - } - } - - private static class HealthCheckResult { - final boolean isHealthy; - final String error; - final boolean isDegraded; - - HealthCheckResult(boolean isHealthy, String error, boolean isDegraded) { - this.isHealthy = isHealthy; - this.error = error; - this.isDegraded = isDegraded; - } - } + private record AdvancedCheckResult(boolean isDegraded) {} + + private record HealthCheckResult(boolean isHealthy, String error, boolean isDegraded) {}Note: with records, field access changes from
result.isHealthytoresult.isHealthy(), so all call sites would need updating.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/iemr/common/service/health/HealthService.java` around lines 491 - 509, Replace the two inner classes AdvancedCheckResult and HealthCheckResult with Java records to reduce boilerplate and get generated equals/hashCode/toString; i.e., convert AdvancedCheckResult(boolean isDegraded) and HealthCheckResult(boolean isHealthy, String error, boolean isDegraded) into record declarations, then update all call sites to use the record accessor methods (isDegraded() and isHealthy()/error()/isDegraded()) instead of field access.
373-391:hasLockWaitsquery may miss states depending on MySQL version.The lock wait states checked (
'Waiting for table metadata lock','Waiting for row lock','Waiting for lock') are MySQL-version-dependent. For MySQL 8.0+, consider also queryingperformance_schema.data_lock_waitsfor more reliable lock-wait detection, if theperformance_schemais enabled.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/iemr/common/service/health/HealthService.java` around lines 373 - 391, The hasLockWaits method currently checks specific PROCESSLIST states which are MySQL-version dependent; update hasLockWaits to first attempt a more reliable check against performance_schema.data_lock_waits when performance_schema is available (e.g., run a lightweight EXISTS or try a SELECT COUNT(*) FROM performance_schema.data_lock_waits) and return true if any rows present, and fall back to the existing INFORMATION_SCHEMA.PROCESSLIST query if performance_schema is not present or the query fails; make these checks inside hasLockWaits (preserving the 2s timeout and exception handling) and reference the same method name (hasLockWaits) and PreparedStatement/ResultSet usage patterns to integrate the new query path cleanly.src/main/java/com/iemr/common/controller/version/VersionController.java (1)
68-77: Silent fallback whengit.propertiesis missing.When
getResourceAsStream("git.properties")returnsnull, the method returns an emptyPropertiesobject with no log message. This makes it hard to diagnose why the/versionendpoint returns all "unknown" values. Consider logging a warning.Proposed fix
private Properties loadGitProperties() throws IOException { Properties properties = new Properties(); try (InputStream input = getClass().getClassLoader() .getResourceAsStream("git.properties")) { if (input != null) { properties.load(input); + } else { + logger.warn("git.properties not found on classpath"); } } return properties; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/iemr/common/controller/version/VersionController.java` around lines 68 - 77, The loadGitProperties() method currently returns an empty Properties when getResourceAsStream("git.properties") is null without any diagnostics; update VersionController.loadGitProperties() to log a warning when the resource stream is null (e.g., using the class logger instance) so callers know the properties file was not found, and keep returning the empty Properties; specifically locate the loadGitProperties() method and add a logger.warn/ warning message when input == null and include the resource name ("git.properties") and context (VersionController) in the message.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@pom.xml`:
- Around line 554-558: Remove the duplicate maven-jar-plugin declaration: delete
the <plugin> block that specifies groupId org.apache.maven.plugins and
artifactId maven-jar-plugin with version 3.0.2 so the project uses the single
canonical maven-jar-plugin (inherited from the Spring Boot parent) instead of
having a confusing outdated duplicate.
In `@src/main/java/com/iemr/common/service/health/HealthService.java`:
- Around line 393-417: The hasDeadlocks method currently treats any presence of
"LATEST DETECTED DEADLOCK" as a current deadlock, causing permanent false
positives; change it to detect recent deadlocks instead by replacing the SHOW
ENGINE INNODB STATUS parse with a delta check against the Innodb_deadlocks
global counter: add logic in hasDeadlocks to query "SHOW GLOBAL STATUS LIKE
'Innodb_deadlocks'" (or use connection.createStatement()/PreparedStatement),
read the numeric value, compare it to a stored previousDeadlockCount field
(update the field after check), and only return true when the counter has
increased since the last check; keep the existing deadlockCheckDisabled behavior
and error handling (e.g., on SQL error codes 1142/1227 set deadlockCheckDisabled
and log via logger).
- Around line 116-164: checkHealth() can throw RejectedExecutionException when
executorService has been shut down; guard the executor submit calls by either
checking executorService.isShutdown()/isTerminated() before calling
executorService.submit(...) or wrap the submit(...) calls in a try/catch for
RejectedExecutionException and fall back to running performHealthCheck("MySQL",
mysqlStatus, this::checkMySQLHealthSync) and performHealthCheck("Redis",
redisStatus, this::checkRedisHealthSync) synchronously (or populate statuses as
"UNKNOWN"/"UNAVAILABLE") so the method still returns a proper response; update
references to mysqlFuture/redisFuture handling to account for the synchronous
fallback path.
---
Outside diff comments:
In `@src/main/java/com/iemr/common/utils/JwtUserIdValidationFilter.java`:
- Around line 239-256: In shouldSkipAuthentication(String path, String
contextPath) remove the duplicated refreshToken check: consolidate the two
conditions into a single check (choose either path.equals(contextPath +
"/user/refreshToken") if you want exact match or path.startsWith(contextPath +
"/user/refreshToken") if prefixes should be allowed) and delete the other
occurrence so only one refreshToken check remains; update the list of OR'ed
conditions accordingly.
---
Nitpick comments:
In `@src/main/java/com/iemr/common/controller/version/VersionController.java`:
- Around line 68-77: The loadGitProperties() method currently returns an empty
Properties when getResourceAsStream("git.properties") is null without any
diagnostics; update VersionController.loadGitProperties() to log a warning when
the resource stream is null (e.g., using the class logger instance) so callers
know the properties file was not found, and keep returning the empty Properties;
specifically locate the loadGitProperties() method and add a logger.warn/
warning message when input == null and include the resource name
("git.properties") and context (VersionController) in the message.
In `@src/main/java/com/iemr/common/service/health/HealthService.java`:
- Around line 491-509: Replace the two inner classes AdvancedCheckResult and
HealthCheckResult with Java records to reduce boilerplate and get generated
equals/hashCode/toString; i.e., convert AdvancedCheckResult(boolean isDegraded)
and HealthCheckResult(boolean isHealthy, String error, boolean isDegraded) into
record declarations, then update all call sites to use the record accessor
methods (isDegraded() and isHealthy()/error()/isDegraded()) instead of field
access.
- Around line 373-391: The hasLockWaits method currently checks specific
PROCESSLIST states which are MySQL-version dependent; update hasLockWaits to
first attempt a more reliable check against performance_schema.data_lock_waits
when performance_schema is available (e.g., run a lightweight EXISTS or try a
SELECT COUNT(*) FROM performance_schema.data_lock_waits) and return true if any
rows present, and fall back to the existing INFORMATION_SCHEMA.PROCESSLIST query
if performance_schema is not present or the query fails; make these checks
inside hasLockWaits (preserving the 2s timeout and exception handling) and
reference the same method name (hasLockWaits) and PreparedStatement/ResultSet
usage patterns to integrate the new query path cleanly.
src/main/java/com/iemr/common/service/health/HealthService.java
Outdated
Show resolved
Hide resolved
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
|
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
src/main/java/com/iemr/common/service/health/HealthService.java (4)
87-87:ADVANCED_HEALTH_CHECKS_ENABLEDis a compile-time constant — consider making it a configurable property.There's no way to toggle advanced checks off at runtime (e.g., via a feature flag or
application.properties) without recompiling. Deploying to restricted DB environments (e.g., read-only replicas, strict privilege grants) whereINFORMATION_SCHEMA.PROCESSLISTaccess is denied would require a code change.♻️ Proposed change
- private static final boolean ADVANCED_HEALTH_CHECKS_ENABLED = true; + `@Value`("${health.advanced-checks.enabled:true}") + private boolean advancedHealthChecksEnabled;Then reference
advancedHealthChecksEnabledinperformAdvancedMySQLChecksWithThrottle().🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/iemr/common/service/health/HealthService.java` at line 87, Replace the compile-time constant ADVANCED_HEALTH_CHECKS_ENABLED with a configurable property (e.g., a boolean field advancedHealthChecksEnabled injected from application.properties or a Spring `@Value/`@ConfigurationProperties), wire its value during bean construction or component initialization, and update performAdvancedMySQLChecksWithThrottle() to reference this new advancedHealthChecksEnabled field instead of the static constant so the feature can be toggled at runtime via configuration.
93-93:Executors.newFixedThreadPool(2)uses an unboundedLinkedBlockingQueue.Under a flood of health check requests (e.g., from an aggressive load balancer probe loop), submitted tasks accumulate without bound. Prefer a
ThreadPoolExecutorwith a bounded queue and a caller-runs or abort policy to prevent unbounded memory growth. Using daemon threads also avoids blocking JVM exit as a secondary benefit of the@PreDestroyshutdown.♻️ Proposed change
- this.executorService = Executors.newFixedThreadPool(2); + this.executorService = new ThreadPoolExecutor( + 2, 2, + 0L, TimeUnit.MILLISECONDS, + new java.util.concurrent.ArrayBlockingQueue<>(10), + r -> { + Thread t = new Thread(r, "health-check-worker"); + t.setDaemon(true); + return t; + }, + new ThreadPoolExecutor.CallerRunsPolicy() + );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/iemr/common/service/health/HealthService.java` at line 93, Replace the unbounded fixed thread pool with a bounded ThreadPoolExecutor in HealthService: instead of Executors.newFixedThreadPool(2) initialize executorService with new ThreadPoolExecutor(corePoolSize=2, maximumPoolSize=2, keepAliveTime, TimeUnit, a bounded BlockingQueue like ArrayBlockingQueue with a small capacity, and a RejectedExecutionHandler such as new ThreadPoolExecutor.CallerRunsPolicy() (or AbortPolicy if you prefer failing fast). Also supply a ThreadFactory that creates daemon threads so they don't block JVM shutdown and keep the existing `@PreDestroy` shutdown logic to gracefully terminate the executorService.
113-161: Unauthenticated/healthendpoint exposes detailed internal DB diagnostics.Per the AI summary, the JWT filter is updated to bypass authentication for
/health. The response produced by this service includes component-level details such as MySQL slow query counts, active lock waits, connection pool utilization percentages, and error messages from the DB layer. These fields can help an external attacker fingerprint infrastructure, infer load patterns, or identify degraded states to time an attack.Consider one of the following mitigations:
- Return only a minimal
{"status": "UP"}/{"status": "DOWN"}payload for unauthenticated requests, with full component details gated behind a role (e.g.,ACTUATORorADMIN).- Restrict access to
/healthto an internal network or a management port rather than the public API port.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/iemr/common/service/health/HealthService.java` around lines 113 - 161, checkHealth currently returns detailed DB component diagnostics to unauthenticated callers; change HealthService.checkHealth to detect caller privileges (e.g., via SecurityContextHolder or an injected Authentication/Authorization service) and if the request is unauthenticated or lacks ADMIN/ACTUATOR role, return a minimal response containing only timestamp and STATUS_KEY ("UP"/"DOWN") without populating mysqlStatus or redisStatus. Keep the existing concurrent health checks (performHealthCheck, ensurePopulated, computeOverallStatus) for privileged calls only—i.e., run them and include full components only when the caller has the required role; otherwise skip calling performHealthCheck/ensurePopulated and set overallStatus based on a lightweight internal probe or default to "UP"/"DOWN".
365-383: Consider usingperformance_schema.data_lock_waitsfor more reliable InnoDB lock detection.The current approach using
INFORMATION_SCHEMA.PROCESSLISTstate strings works, but MySQL 8.0 officially recommends using Performance Schema tables instead. The'Waiting for row lock'state in the current code is correct for MySQL 8.0, but parsing PROCESSLIST.STATE is inherently fragile across MySQL versions.The official alternative is
performance_schema.data_lock_waits, which directly exposes InnoDB lock wait relationships and aligns with MySQL's recommended best practice for MySQL 8.0+.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/iemr/common/service/health/HealthService.java` around lines 365 - 383, The hasLockWaits method should stop relying on fragile PROCESSLIST state strings and instead query Performance Schema's data_lock_waits for reliable InnoDB lock detection: update the implementation in hasLockWaits(Connection) to run a short-timeout prepared query against performance_schema.data_lock_waits (joining to appropriate thread/process tables or filtering by OWNER/THREAD_ID as needed) and return true if any rows indicate lock waits; retain the existing exception handling and timeout behavior (e.g., stmt.setQueryTimeout(2)) and ensure the query filters to the current user/session similarly to the original logic.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/main/java/com/iemr/common/service/health/HealthService.java`:
- Around line 425-442: The method checkPoolMetricsViaJMX currently always logs
"Pool exhaustion check disabled: HikariCP metrics unavailable" after iterating
MBeans which is misleading when MBeans were present but none exceeded threshold;
update checkPoolMetricsViaJMX to distinguish three outcomes by introducing flags
(e.g., boolean mbeansFound and boolean exhaustionDetected): set mbeansFound=true
when queryMBeans returns any, set exhaustionDetected=true if
evaluatePoolMetrics(...) returns true and immediately return true, keep the
catch block logging only for JMX access errors, and after the loop log either a
healthy message (e.g., "All HikariCP pools below exhaustion threshold") when
mbeansFound && !exhaustionDetected or the unavailable message when !mbeansFound,
referencing checkPoolMetricsViaJMX and evaluatePoolMetrics and use logger.debug
for these distinct cases.
---
Duplicate comments:
In `@src/main/java/com/iemr/common/service/health/HealthService.java`:
- Around line 120-123: The submit calls for mysqlFuture and redisFuture can
throw RejectedExecutionException if executorService has been shut down (e.g.,
after shutdown()/@PreDestroy) when checkHealth() runs; wrap the
executorService.submit(...) calls in a try/catch for RejectedExecutionException
inside checkHealth(), and on catch either (a) run performHealthCheck("MySQL",
...) / performHealthCheck("Redis", ...) synchronously as a fallback or (b) mark
the corresponding HealthStatus (mysqlStatus/redisStatus) as DOWN with an
explanatory message; ensure you reference executorService.submit,
performHealthCheck, mysqlFuture, redisFuture, checkHealth(),
shutdown()/@PreDestroy in the change so we handle the shutdown race safely.
---
Nitpick comments:
In `@src/main/java/com/iemr/common/service/health/HealthService.java`:
- Line 87: Replace the compile-time constant ADVANCED_HEALTH_CHECKS_ENABLED with
a configurable property (e.g., a boolean field advancedHealthChecksEnabled
injected from application.properties or a Spring
`@Value/`@ConfigurationProperties), wire its value during bean construction or
component initialization, and update performAdvancedMySQLChecksWithThrottle() to
reference this new advancedHealthChecksEnabled field instead of the static
constant so the feature can be toggled at runtime via configuration.
- Line 93: Replace the unbounded fixed thread pool with a bounded
ThreadPoolExecutor in HealthService: instead of Executors.newFixedThreadPool(2)
initialize executorService with new ThreadPoolExecutor(corePoolSize=2,
maximumPoolSize=2, keepAliveTime, TimeUnit, a bounded BlockingQueue like
ArrayBlockingQueue with a small capacity, and a RejectedExecutionHandler such as
new ThreadPoolExecutor.CallerRunsPolicy() (or AbortPolicy if you prefer failing
fast). Also supply a ThreadFactory that creates daemon threads so they don't
block JVM shutdown and keep the existing `@PreDestroy` shutdown logic to
gracefully terminate the executorService.
- Around line 113-161: checkHealth currently returns detailed DB component
diagnostics to unauthenticated callers; change HealthService.checkHealth to
detect caller privileges (e.g., via SecurityContextHolder or an injected
Authentication/Authorization service) and if the request is unauthenticated or
lacks ADMIN/ACTUATOR role, return a minimal response containing only timestamp
and STATUS_KEY ("UP"/"DOWN") without populating mysqlStatus or redisStatus. Keep
the existing concurrent health checks (performHealthCheck, ensurePopulated,
computeOverallStatus) for privileged calls only—i.e., run them and include full
components only when the caller has the required role; otherwise skip calling
performHealthCheck/ensurePopulated and set overallStatus based on a lightweight
internal probe or default to "UP"/"DOWN".
- Around line 365-383: The hasLockWaits method should stop relying on fragile
PROCESSLIST state strings and instead query Performance Schema's data_lock_waits
for reliable InnoDB lock detection: update the implementation in
hasLockWaits(Connection) to run a short-timeout prepared query against
performance_schema.data_lock_waits (joining to appropriate thread/process tables
or filtering by OWNER/THREAD_ID as needed) and return true if any rows indicate
lock waits; retain the existing exception handling and timeout behavior (e.g.,
stmt.setQueryTimeout(2)) and ensure the query filters to the current
user/session similarly to the original logic.
Comment on lines
+425
to
+442
| private boolean checkPoolMetricsViaJMX() { | ||
| try { | ||
| MBeanServer mBeanServer = ManagementFactory.getPlatformMBeanServer(); | ||
| ObjectName objectName = new ObjectName("com.zaxxer.hikari:type=Pool (*)"); | ||
| var mBeans = mBeanServer.queryMBeans(objectName, null); | ||
|
|
||
| for (var mBean : mBeans) { | ||
| if (evaluatePoolMetrics(mBeanServer, mBean.getObjectName())) { | ||
| return true; | ||
| } | ||
| } | ||
| } catch (Exception e) { | ||
| logger.debug("Could not access HikariCP pool metrics via JMX"); | ||
| } | ||
|
|
||
| logger.debug("Pool exhaustion check disabled: HikariCP metrics unavailable"); | ||
| return false; | ||
| } |
Contributor
There was a problem hiding this comment.
Misleading debug log when pool MBeans exist but utilization is below threshold.
Line 440 logs "Pool exhaustion check disabled: HikariCP metrics unavailable" after the loop completes, but this path is also reached when MBeans were found and evaluated — just none exceeded the 80% threshold. The log incorrectly signals that the check was skipped/unavailable when it actually ran and found healthy pools.
🐛 Proposed fix
private boolean checkPoolMetricsViaJMX() {
try {
MBeanServer mBeanServer = ManagementFactory.getPlatformMBeanServer();
ObjectName objectName = new ObjectName("com.zaxxer.hikari:type=Pool (*)");
var mBeans = mBeanServer.queryMBeans(objectName, null);
+ if (mBeans.isEmpty()) {
+ logger.debug("Pool exhaustion check skipped: no HikariCP MBeans registered (enable registerMbeans=true)");
+ return false;
+ }
+
for (var mBean : mBeans) {
if (evaluatePoolMetrics(mBeanServer, mBean.getObjectName())) {
return true;
}
}
} catch (Exception e) {
logger.debug("Could not access HikariCP pool metrics via JMX");
}
- logger.debug("Pool exhaustion check disabled: HikariCP metrics unavailable");
return false;
}📝 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
| private boolean checkPoolMetricsViaJMX() { | |
| try { | |
| MBeanServer mBeanServer = ManagementFactory.getPlatformMBeanServer(); | |
| ObjectName objectName = new ObjectName("com.zaxxer.hikari:type=Pool (*)"); | |
| var mBeans = mBeanServer.queryMBeans(objectName, null); | |
| for (var mBean : mBeans) { | |
| if (evaluatePoolMetrics(mBeanServer, mBean.getObjectName())) { | |
| return true; | |
| } | |
| } | |
| } catch (Exception e) { | |
| logger.debug("Could not access HikariCP pool metrics via JMX"); | |
| } | |
| logger.debug("Pool exhaustion check disabled: HikariCP metrics unavailable"); | |
| return false; | |
| } | |
| private boolean checkPoolMetricsViaJMX() { | |
| try { | |
| MBeanServer mBeanServer = ManagementFactory.getPlatformMBeanServer(); | |
| ObjectName objectName = new ObjectName("com.zaxxer.hikari:type=Pool (*)"); | |
| var mBeans = mBeanServer.queryMBeans(objectName, null); | |
| if (mBeans.isEmpty()) { | |
| logger.debug("Pool exhaustion check skipped: no HikariCP MBeans registered (enable registerMbeans=true)"); | |
| return false; | |
| } | |
| for (var mBean : mBeans) { | |
| if (evaluatePoolMetrics(mBeanServer, mBean.getObjectName())) { | |
| return true; | |
| } | |
| } | |
| } catch (Exception e) { | |
| logger.debug("Could not access HikariCP pool metrics via JMX"); | |
| } | |
| return false; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/java/com/iemr/common/service/health/HealthService.java` around lines
425 - 442, The method checkPoolMetricsViaJMX currently always logs "Pool
exhaustion check disabled: HikariCP metrics unavailable" after iterating MBeans
which is misleading when MBeans were present but none exceeded threshold; update
checkPoolMetricsViaJMX to distinguish three outcomes by introducing flags (e.g.,
boolean mbeansFound and boolean exhaustionDetected): set mbeansFound=true when
queryMBeans returns any, set exhaustionDetected=true if evaluatePoolMetrics(...)
returns true and immediately return true, keep the catch block logging only for
JMX access errors, and after the loop log either a healthy message (e.g., "All
HikariCP pools below exhaustion threshold") when mbeansFound &&
!exhaustionDetected or the unavailable message when !mbeansFound, referencing
checkPoolMetricsViaJMX and evaluatePoolMetrics and use logger.debug for these
distinct cases.
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.
📋 Description
JIRA ID:
This PR implements /health and /version endpoints in the Common-API as part of Issue PSMRI/AMRIT#102
What’s changed
/healthendpoint/versionendpointSummary by CodeRabbit
New Features
Improvements
Chores