Allow smallrye-health to return DOWN response compliant with rfc9457

[xstefank]

• Fixes Allow smallrye-health to return DOWN response compliant with rfc9457 #43125

[xstefank]

Asked for in https://stackoverflow.com/questions/78943916/how-to-customize-smallrye-health-response-content-type-to-application-problemjs/78964351#78964351, Quarkus 3.8.2

[github-actions]

🎊 PR Preview b8c5a62 has been successfully built and deployed to https://quarkus-pr-main-43209-preview.surge.sh/version/main/guides/

• Images of blog posts older than 3 months are not available.
• Newsletters older than 3 months are not available.

[jmartisk]

Nice!

[geoand]

Now that I think of it, this should be volatile

[xstefank]

why please?

[geoand]

The reason is that SmallRyeHealthHandlerBase.problemDetails is set by the main thread (although there is no strict contract for this, this is how it works in practice), but the field is read from an event loop thread (but potentially by any thread).

[jmartisk]

Just wondering how much of a potentially unnecessary performance cost it has given that it's only written once. This could be avoided by instead passing it to a constructor of the handler. But maybe the performance cost of it being volatile is negligible.

[geoand]

Obviously the best solution (for more than just performance) is to have final fields.

Given that this is not on the hot path, I wouldn't worry much about the (tiny) performance hit

[geoand]

If we want a solution that sets up the field in the constructor, I can have a look at some point

…

On Fri, Sep 13, 2024, 09:23 Jan Martiska ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In extensions/smallrye-health/runtime/src/main/java/io/quarkus/smallrye/health/runtime/SmallRyeHealthHandlerBase.java <#43209 (comment)>: > @@ -22,8 +31,14 @@ public abstract class SmallRyeHealthHandlerBase implements Handler<RoutingContext> { + static boolean problemDetails = false; Just wondering how much of a potentially unnecessary performance cost it has given that it's only written once. This could be avoided by instead passing it to a constructor of the handler. But maybe the performance cost of it being volatile is negligible. — Reply to this email directly, view it on GitHub <#43209 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABBMDP5QDI3NUSLCMUVR743ZWKAFXAVCNFSM6AAAAABOAI6PM6VHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDGMBSGIYTCMRSHE> . You are receiving this because you commented.Message ID: ***@***.***>

[xstefank]

@geoand I was thinking about it but this is a base class so we would need to go trough 7-8 subclasses to pass it in constructor. So I opted for a field.

[geoand]

Okay, makes sense. Let's just use a volatile

[quarkus-bot]

Status for workflow Quarkus Documentation CI

This is the status report for running Quarkus Documentation CI on commit 636939a.

✅ The latest workflow run for the pull request has completed successfully.

It should be safe to merge provided you have a look at the other checks in the summary.

Warning

There are other workflow runs running, you probably need to wait for their status before merging.

[quarkus-bot]

Status for workflow Quarkus CI

This is the status report for running Quarkus CI on commit 636939a.

✅ The latest workflow run for the pull request has completed successfully.

It should be safe to merge provided you have a look at the other checks in the summary.

You can consult the Develocity build scans.

[gsmet]

This is more a new feature than a fix. We could backport it at some point if there is a strong demand but I would rather have it released in 3.16 first.