Remove requirement for the disk space health indicator's path to exist when the app starts

[Phoosha]

Problem

When the path referenced by management.health.diskspace.path does not exist before a Spring Boot application with the actuator autoconfiguration is started, there is no possibility to create the path during application startup. This is sort of a chicken & egg problem. If I want to create the path during startup, I need to get it from the configuration. But to access the configuration the creation of DiskSpaceHealthIndicatorProperties must not fail. However, this is the case when that path does not exist (or is not readable) before application startup.

Use Case

My application has a data directory and I want the autoconfigured DiskSpaceHealthIndicator to monitor that directory. Let's say the configuration is like that:

myapp.data.path=myapp/data/
management.health.diskspace.path=${myapp.data.path}

I do not want to tell every user (or developer) that he needs to create the myapp/data/ directory first. So the application will create the directory itself when necessary. However, once I add the second line above this mechanism stops working and application startup fails until the directories are created outside the scope of the Spring Boot application and before its startup (e.g. manually).

Proposed Solution

Do not check mutable aspects of the system environment when binding the configuration properties. That is, move the existence & readability assertions from DiskSpaceHealthIndicatorProperties to DiskSpaceHealthContributorAutoconfiguration. This allows in my use case to access the configuration and to create the directories during startup before the autoconfiguration runs.

I picked this solution because the behavior of DiskSpaceHealthContributorAutoconfiguration would remain largely unchanged as indicated by the test case I added. I did not move the assertions to DiskSpaceHealthIndicator because that would affect users that directly use it and that might for some reason rely on being able to create it with a nonexistent or unreadable path.

Please let me know if you would prefer a different approach.

[wilkinsona]

Thanks for the proposal. I like the idea of moving the checks out of the properties class as we generally try to limit those to being simple POJOs with little to no logic in them.

I'm wondering if we should consider changing the current behaviour or allowing it to be configured. Instead of failing startup when the path does not exist (and causing the problem that you have observed) we could continue and allow the health indicator to report DOWN instead.

Flagging for team attention so that we can discuss our options.

[Phoosha]

Yes, not failing startup at all would also be an acceptable option for my use case.

I am however somewhat concerned that the application might report as unhealthy right after startup without an obvious cause. In development nobody might be monitoring the application health. Then an issue where the indicator can't become healthy would only just be caught in a staging or production environment.

Maybe a warning could be logged instead? Or the behaviour could be made configurable?

[wilkinsona]

We discussed this today and we'd like to align DiskSpaceHealthIndicator with the other health indicators so that it does not cause a failure at startup. The means that the check for whether or not the file exists should be removed from the properties class. Instead, the indicator will response with down as the usable space of a non-existent path is 0. We'd also like to add a detail to the indicator's response that shows whether or not the configured path exists and not require users to reverse engineer this from the amount of usable space. @Phoosha Would you be interested in updating your proposal to this effect?

[Phoosha]

@wilkinsona Thanks for the feedback. I'll give it a shot and report back here.

[Phoosha]

@wilkinsona I've implemented the behavior you described:

The means that the check for whether or not the file exists should be removed from the properties class.

Check is removed.

Instead, the indicator will response with down as the usable space of a non-existent path is 0.

Responds with DOWN including the corner case where the threshold is zero.

We'd also like to add a detail to the indicator's response that shows whether or not the configured path exists and not require users to reverse engineer this from the amount of usable space.

Details added for exists & canRead/Write/Execute.

It might be interesting to incorporate canRead/Write/Execute as well into the health check outcome. But I've left this out.

Also I didn't test any of this on a non-linux system. Specifically, this means I didn't explicitly verify that getUsableSpace()/exists()/canRead()/... are consistent and meaningful on other operating systems.

[wilkinsona]

Thanks very much for making your first contribution to Spring Boot, @poosha. I polished the proposed changes a little bit in f238812. This left the changes looking like this. I decided not to add read, write and execute to the details as they're not related to the main change being made here. I also simplified the tests as there appeared to be some overlap. Lastly, I left the indicator reporting up if the usable space is 0 because the path does not exist and the threshold is 0. That's consistent with the current behaviour and warning message and avoids warning that the free space is below the threshold when both are, in fact, zero.