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

Exclude Beans from the Health Check #14639

Closed
wants to merge 1,378 commits into from
Closed

Exclude Beans from the Health Check #14639

wants to merge 1,378 commits into from

Conversation

MahmoudAGawad
Copy link

@MahmoudAGawad MahmoudAGawad commented Sep 30, 2018

The Problem:
Currently, the user can configure whether to enable/disable the health check (by default /health). Moreover, the user can choose a class of bean type (Redis for instance) to disable even if the default /health is enabled. However, if the user has many beans of some type (Redis), some of them are not mandatory/necessary to be functional for the application to provide the service(s), the user cannot straightforwardly exclude them from the health check. In cases where these dependencies are down, it will affect the health checking result which has consequences. The user has options including rewriting his/her own health indicator, or manipulate the eligible candidate beans somehow.

Proposed Solution:
Adding @ExcludeFromHealthCheck annotation which flags the designated bean and excluding it from the health checking. It can also exclude a bean by its name (if the bean is in a dependency).

Case Study:
In our system, we have multiple redis clusters. One of them was for pushing notifications which wasn't critical and can be easily bypassed (or providing a fallback) if it's down. The work around was to override the RedisHealthIndicatorConfiguration to inject the beans marked with the custom annotation @ExcludeFromHealthCheck and do the filtering.

Pros:

  1. More control to the user.
  2. Reusable which requires no overhead on the user.

Cons:

  1. Added overhead during the context initialization.

wilkinsona and others added 30 commits September 7, 2018 11:09
* pr/14252:
  Polish "Add support for configuring missingQueuesFatal property"
  Add support for configuring missingQueuesFatal property
This commit updates the annotation processor to write metadata in a
consistent way. Groups, properties and hints are written and each item
is ordered alphabetically based on its name.

Also, deprecated items are written last.

Closes gh-14347
philwebb and others added 22 commits September 24, 2018 09:45
Update the auto-configuration annotation processor to generate
properties for `@ConditionalOnBean` and `@ConditionalOnSingleCandidate`.

See gh-13328
Update `OnBeanCondition` to be an `AutoConfigurationImportFilter` and
filter out classes early.

See gh-13328
Update the auto-configuration annotation processor to generate
properties for `@OnWebApplication`.

See gh-13328
Update `OnWebApplicationCondition` to be an
`AutoConfigurationImportFilter` and filter out classes early.

Closes gh-13328
Make `ResourceHandlerRegistrationCustomizer` a public top level class.

Closes gh-14587
Downgrade the `spring-boot-maven-plugin` maven version to 3.3.

Closes gh-14464
* pr/14600:
  Fix "Query Parameters" section name
While MongoDB 3.6.7 has been released, 3.6.5 is the latest version that's supported
by the version of Embedded Mongo that we're currently using.

Closes gh-14476
* gh-14631:
  Upgrade Java 11 CI image to 11-ea-28-jdk
@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Sep 30, 2018
izeye and others added 2 commits September 30, 2018 17:47
* pr/14637:
  Use Commons Logging for OnlyOnceLoggingDenyMeterFilter
@philwebb
Copy link
Member

philwebb commented Oct 1, 2018

@MahmoudAGawad It looks like something has gone wrong with your pull request and all the commits from master are being merged into 2.0.x. Could you please try again and just submit a single commit that can be applied to current master branch. You can do a force push to update this PR.

@philwebb
Copy link
Member

philwebb commented Oct 1, 2018

Related, we also have issue #14022 which might be relevant.

@MahmoudAGawad
Copy link
Author

Thank you @philwebb. The issue #14022 is indirectly relevant (since it supports api calls). I find this #4965 more relevant and provide exactly the wanted feature.

@philwebb philwebb removed the status: waiting-for-triage An issue we've not yet triaged label Oct 1, 2018
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 this pull request may close these issues.

None yet