-
Notifications
You must be signed in to change notification settings - Fork 306
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
FISH-333 Add Payara HealthCheck Service checkers to MicroProfile Health Readiness Checks #4883
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the way the bridge or adaptor between Payara and MP health checks is done needs to change. Configuration changes to the newly introduced flag should register or un-register the adaptor in MP health so that the checks can be processed as before without the servlet knowing about any special extra thing that needs to be done.
...althcheck/src/main/java/fish/payara/microprofile/healthcheck/servlet/HealthCheckServlet.java
Outdated
Show resolved
Hide resolved
...althcheck/src/main/java/fish/payara/microprofile/healthcheck/servlet/HealthCheckServlet.java
Outdated
Show resolved
Hide resolved
...ofile/healthcheck/src/main/java/fish/payara/microprofile/healthcheck/HealthCheckService.java
Outdated
Show resolved
Hide resolved
...lthcheck-core/src/main/java/fish/payara/nucleus/healthcheck/preliminary/BaseHealthCheck.java
Outdated
Show resolved
Hide resolved
...lthcheck-core/src/main/java/fish/payara/nucleus/healthcheck/preliminary/BaseHealthCheck.java
Outdated
Show resolved
Hide resolved
...lthcheck-core/src/main/java/fish/payara/nucleus/healthcheck/preliminary/BaseHealthCheck.java
Show resolved
Hide resolved
...healthcheck/src/main/java/fish/payara/microprofile/healthcheck/checks/PayaraHealthCheck.java
Outdated
Show resolved
Hide resolved
…egister or un-register of Payara HealthCheck Service Checker into Microprofile Health
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think using the events is a good solution to link the two health check systems.
Only important comment is the one about NPE, I am pretty sure I have encountered NPEs in the past because options were null at some point before the whole thing is tuned on. So have a look again. Might no longer be the case or apply but in general I'd think the issue should be the same in isEnabled
.
Otherwise just some comments for consideration.
...healthcheck/src/main/java/fish/payara/microprofile/healthcheck/checks/PayaraHealthCheck.java
Outdated
Show resolved
Hide resolved
...healthcheck/src/main/java/fish/payara/microprofile/healthcheck/checks/PayaraHealthCheck.java
Show resolved
Hide resolved
...lthcheck-core/src/main/java/fish/payara/nucleus/healthcheck/HealthCheckExecutionOptions.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Susan an I discussed the wording and he will clarify that the flag refers to MicroProfile. No further comments.
…eHealth' to reflect more accurate description.
Jenkins test please |
Jenkins test please |
Merge pull request payara#4883 from MeroRai/FISH-333
Description
Added Payara HealthCheck Service Checkers to MicroProfile Health Readiness Checks. The
set-healthcheck-service-configuration
asadmin command has a new optionsadd-to-microprofile-health
, which can be used to add a HealthCheck Service Checkers to MicroProfile Health and display its result on the MicroProfile Health endpoint. Below is an example of what the/health/ready
endpoint returns:Testing
Testing Performed
Testing Environment
Zulu JDK 1.8_222 on Elementary OS 0.4.1 Loki with Maven 3.5.4
Documentation
https://docs.payara.fish/community/docs/5.2020.7/documentation/payara-server/health-check-service/asadmin-commands.html#set-healthcheck-service-configuration