oscerd commented on PR #1898: URL: https://github.com/apache/camel-spring-boot/pull/1898#issuecomment-5424645998
Thanks @luigidemasi — this was a good catch on P1, and I checked each point rather than taking them as read. Head is now `85b446273ae`. **[P1] Default health-exposure contract — agreed, change dropped.** You're right, and my framing of it as accidental placement was wrong. I verified it: the original CAMEL-18832 implementation (`22a6e0429b9`) already had the error block outside the exposure-level branch — the `full` check governs metadata filtering of the details map, not the error block. The commit message says "Aligned output to be similar to microprofile-health", and `camel-microprofile-health`'s `testExposureLevelDefault` does assert `error.stacktrace` at the default level. The docs agree: `default` "only include detailed information for health check that was DOWN". I checked git history for the route-detail half (CAMEL-20993) but not for the health half, which is exactly how I ended up reverting an intentional decision. The health change is removed from this PR, along with its test; the PR is now the route-detail fix only. The underlying exposure concern is real but belongs with CAMEL-24498, where the observability starter raises `camel.health.exposure-level` to `full` and `show-details` to `always` on an all-interfaces listener. At Spring Boot's own default (`show-details=never`) these details are not exposed at all. If the cross-runtime default should change, that needs a coordinated change with camel-microprofile rather than a unilateral one here — happy to open that if you and @davsclaus think it's worth it. **[P2] `error.message` overwrite — confirmed, filed separately.** Verified: `CamelHealthCheckIndicator.doHealthCheck` passes the same `Health.Builder` to `applyHealthDetail` for every result, and `error.message` is a flat top-level key, so `Map.put` semantics mean the last DOWN check wins. It pre-exists on main and is independent of this PR, so rather than widen this changeset I've filed [CAMEL-24512](https://issues.apache.org/jira/browse/CAMEL-24512) with your reproduction and the suggested fix (move it into the check-scoped `data` map, regression test with two DOWN results). **[P2] Asynchronous route-failure precondition — fixed.** Fair point. The tests did fail against the unfixed implementation when I ran them, but that was timing-dependent rather than guaranteed. A `@BeforeEach` now uses Awaitility (`atMost` 30s) to wait until the raw route properties actually hold a `Throwable` under `route.start.exception` before either view is built, and both tests additionally assert the benign properties are still serialized. Re-verified against the base: the detail test still fails there, so the regression coverage is intact. **[P3] Server-log guarantee — gone with the health change.** The comment making that claim was part of the reverted hunk, so it no longer exists. You were right that nothing guaranteed it. Full `core/camel-spring-boot` suite after the changes: 138 tests, 0 failures. I've left the conversations open for you to resolve. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
