Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

4.x SE HealthCheckResponse no longer supports a name #7827

Closed
tjquinno opened this issue Oct 18, 2023 · 3 comments · Fixed by #7994
Closed

4.x SE HealthCheckResponse no longer supports a name #7827

tjquinno opened this issue Oct 18, 2023 · 3 comments · Fixed by #7994
Assignees
Labels
Milestone

Comments

@tjquinno
Copy link
Member

Environment Details

  • Helidon Version: 4.x
  • Helidon SE or Helidon MP SE
  • JDK version:
  • OS:
  • Docker version (if applicable):

Problem Description

The HealthCheckResponse interface (and its record implementation HealthCheckResponseImpl) and the associated builder do not support a name.

The result is that developers cannot create a convenient lambda which returns a health check response that also has a name. Lambda-based responses appear, with details enabled in the output, like this:

{
  "status": "UP",
  "checks": [
    {
      "name": "Main$$Lambda/0x00000001320ac800",
      "status": "UP",
      "data": {
        "exampleHealthCheck": "looking-good"
      }
    },
...
}

Developers can, of course, write a class which implements HealthCheck and apply the name to the check (not the response) but that approach does not allow the convenience of using in-line lambdas.

Steps to reproduce

Add a health check response in-line something like this:

ObserveFeature observe = ObserveFeature.builder()
    .addObserver(HealthObserver.builder()
    .addCheck(() -> HealthCheckResponse.builder()
             .status(readyTime.get() != 0)
             .detail("time", readyTime.get())
             .build(), HealthCheckType.READINESS)

You get a name like the example above.

@tjquinno tjquinno added SE health 4.x Version 4.x labels Oct 18, 2023
@tomas-langer
Copy link
Member

The name defaults to the simple name of the class. To get a nice name, you can implement the HealthCheck interface, where you can provide custom name, path, and type of the Health check.

@tomas-langer
Copy link
Member

Maybe we could add another method to HealthObserver.builder(), such as:

addCheck(Supplier<HealthCheckresponse> healthCheck, HealthCheckType type, String name)

So the user code could be:

.addCheck(() -> HealthCheckResponse.builder()
             .status(readyTime.get() != 0)
             .detail("time", readyTime.get())
             .build(), 
             HealthCheckType.READINESS,
            "Nice name")

@tjquinno
Copy link
Member Author

To get a nice name, you can implement the HealthCheck interface, where you can provide custom name, path, and type of the Health check.

Yes, as noted in the issue description.

Maybe we could add another method to HealthObserver.builder()

Looks like a good solution, especially given that:

  • As you pointed out in a private DM, associating a name with a health response would potentially allow different responses to the same health check to have different names, which would be confusing at best.
  • This would be a backward-compatible change and we could add it later in an update to 4.0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Archived in project
3 participants