Skip to content
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

Provide a way to disable health checks with configuration #335

Open
xstefank opened this issue Jan 31, 2025 · 7 comments · May be fixed by #337
Open

Provide a way to disable health checks with configuration #335

xstefank opened this issue Jan 31, 2025 · 7 comments · May be fixed by #337
Milestone

Comments

@xstefank
Copy link
Member

xstefank commented Jan 31, 2025

Description

Provide a way to selectively disable individual health checks with MP Config values.

Proposal:

microprofile.health.check."check-name".enabled=true/false
microprofile.health.check."check-class".enabled=true/false

Use cases

Disable health check without the need to remove the implementation. This is useful when the same application deploys in different environments. We can disable checks that are not required or potentially cause problems without recompiling the application. This can also mean that we can offer avoiding having two "versions" of the same application for different environments.

Another use case is testing when we might not have dependent services available or the provided mocked instances don't offer health checks.

Seamless A/B testing when moving to the new version/implementation of a particular health check.

@xstefank xstefank added this to the 4.1 milestone Jan 31, 2025
@xstefank
Copy link
Member Author

xstefank commented Feb 4, 2025

@pgunapal I didn't know that MP Config doesn't support Map as of now, so the only possible way of implementing this is using arrays - https://download.eclipse.org/microprofile/microprofile-config-3.1/microprofile-config-spec-3.1.html#_array_converters. Or providing custom converter in the MP Health which I really don't want to do. So the disabling would rather look like this:

mp.health.disabled-checks=org.acme.HC1,org.acme.HC2

Is this OK with you? cc/ @fabiobrz @donbourne

@pgunapal
Copy link
Contributor

pgunapal commented Feb 4, 2025

@xstefank
Yeah, I agree to use arrays, as implementing our own custom converter, wouldn't be simple.
Is the MP Config name, supposed to be mp.health.disabled-checks or should it be mp.health.disabled.checks?

Is the org.acme.HC1.. is the provided Health Check name or the entire health check class name?

@xstefank
Copy link
Member Author

xstefank commented Feb 4, 2025

@pgunapal we can do both the classname and the check name as originally proposed. In SR we currently only use classnames.

@fabiobrz
Copy link

fabiobrz commented Feb 5, 2025

... I didn't know that MP Config doesn't support Map as of now, so the only possible way of implementing this is using arrays - https://download.eclipse.org/microprofile/microprofile-config-3.1/microprofile-config-spec-3.1.html#_array_converters. Or providing custom converter in the MP Health which I really don't want to do. So the disabling would rather look like this:

mp.health.disabled-checks=org.acme.HC1,org.acme.HC2

Is this OK with you? cc/ @fabiobrz ...

To me using arrays looks like the best option, thanks @xstefank - same question as @pgunapal regarding the property name, and the value too, since IIUC this would affect the spec implementation to cover both cases, right?

@xstefank
Copy link
Member Author

xstefank commented Feb 5, 2025

If no one objects, I would prefer to use both classname and health check name as possible values.

As for the name of the property, I'm ok with mp.health.disabled.checks. If everyone agrees, I can proceed with the implementation.

@xstefank
Copy link
Member Author

xstefank commented Feb 7, 2025

So one thing we all forgot about is that HealthCheck interface doesn't provide access to the name of the health check, which is only defined in the HealthCheckResponse once the HealthCheck is called. So we cannot get the name without calling the health checks. This means that we need to limit this feature only to the health check classnames. So:

mp.health.disabled.checks=org.acme.HC1,org.acme.HC2

but not:

mp.health.disabled.checks=my-health-check1,my-health-check2

@fabiobrz
Copy link

fabiobrz commented Feb 7, 2025

So one thing we all forgot about is that HealthCheck interface doesn't provide access to the name of the health check, which is only defined in the HealthCheckResponse once the HealthCheck is called. So we cannot get the name without calling the health checks. This means that we need to limit this feature only to the health check classnames. So:

mp.health.disabled.checks=org.acme.HC1,org.acme.HC2

but not:

mp.health.disabled.checks=my-health-check1,my-health-check2

+1, thanks @xstefank - this solves one question I had about #337

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants