Rename and change the visibility of the logExceptionIfPresent method to protected so that it is possible to replace the default logic when necessary, increasing the possibility of reusing this abstract class.
Comment From: philwebb
Can provide some more details about how/why you want to replace the default logic?
Comment From: marcusvoltolim
Can provide some more details about how/why you want to replace the default logic?
Of course! Features such as metrics, sending alerts in Slack, WebHook or simply changing the log level from warn to error.
Comment From: philwebb
I feel like we may need to improve the way that we log health indicator errors rather than change the visibility of logExceptionIfPresent. For the use-cases you've described, it feels odd that only certain AbstractReactiveHealthIndicator subclasses would get the modified behavior. I think we should move the logging code to somewhere central so that it can be tweaked.
I'm also not convinced that we should be sending alerts as a side effect of calling the health indicator. Currently the indicators are invoked only when someone hits the /actuator/health endpoint. Sending alerts feels like something that should happen based on scheduled checks. There are also complexities around sending alerts as you probably only want to send them when a health indicator result changes. Generally speaking, I think the system that is invoking /actuator/health should be the thing responsible for sending alerts.
I'd like to repurpose this issue for relocating the logging code and making it easier to change. I'm not convinced that we should be adding scheduled health indicator invocations just yet.
Let's see what the rest of the team thinks.
Comment From: marcussatelis
I found what I need https://stackoverflow.com/questions/51190750/springboot-health-endpoints-overriden
Comment From: philwebb
I've opened a new issue (#43589) to look at centralizing the logging.