Hi,
I just looked into the relatively new LivenessStateChangedEvent
/ReadinessStateChangedEvent
stuff and found that the cause
field in LivenessStateChangedEvent
is never really used. It also seems to be a bit inconsistent to ReadinessStateChangedEvent
which doesn't have it. I was thinking of removing it directly, but stopped myself to discuss it first. I guess we have 3 options:
- Remove the cause field and change the APIs in
LivenessStateChangedEvent
to not take the additional parameter. That would align it withReadinessStateChangedEvent
, but would obviously change the APIs again. Since they were introduced only in 2.3, I guess that could be fine. - Expose the cause field in
LivenessStateChangedEvent
only and actually propagate it up to the respectiveHealthIndicator
. - Do the same as in option 2, but for
ReadinessStateChangedEvent
as well
Personally, I would prefer a harmonized solution between those two events, which leaves me with options 1 or 3. Both would mean an API change relatively late in the 2.3.x release process. The most information for the health indicator would be provided by option 3, so I'm choosing that one ;-)
Whatever decision is made, I'm happy to provide a PR for it.
Cheers, Christoph
Comment From: bclozel
I think I'd rather remove the cause altogether for now and rework the events to have richer types when we'll have feedback from the community or concrete use cases.
Maybe holding the actual exception would be useful in some cases? Or maybe we should let developers extend those events as a starting point?
The only downside to that approach is the LocalCacheVerifier
sample in our reference documentation - catching the exception and not using its information is a bit strange.
As for the health indicators, it doesn't seem that Kubernetes Probes will look at the response body, but actually just the HTTP response status. So I'm not sure this is useful. If the goal is to track and record state changes, maybe we should add an event listener that records Micrometer metrics.
I've marked this issue for team-attention.
Comment From: dreis2211
I guess https://github.com/spring-projects/spring-boot/commit/14a6c60b4f314d3d6e42aa4a44a4a1710d66a475 makes this issue obsolete.
Comment From: bclozel
Yes, this issue has been superseded by #20962 Thanks @dreis2211 !