Ensure distinct health procedure names

[jmesnil]

The current API names the health check procedure in the Response they returns.

There is no assumption on these names in the spec or the API.
However the protocol document implies that the names are unique (a.k.a id):

The JSON response MUST contain the id entry specifying the name of the check, to support protocols that support external identifier (i.e. URI)

The specification should clarify this and the API should reflect it.
I propose the spec is amended to state that:

• The name of a health check MUST be unique for a given producer.

To enforce this, I propose we remove the name for the HealthStatus interface and adds a new annotation @Health(String name) that must be set on a HealthCheckProcedure to identify it.

During the deployment of the application, CDI can check:

• that a HealthCheckProcedure has a @Health annotation with an unique name. If the name is already taken, the provider will reject the procedure (and should fail the deployment).
• in the absence of a @Health annotation, the provider will assign an unique name to the procedure.

This would guarantee that there is no multiple procedures with the same name (making them ID) for a given deployment.

There is still another issue of having health check procedures with the same name in separate deployments. I'd argue that the provider COULD modify the name of the health check procedure (provided by the @Health annotation) to ensure uniqueness across deployments.
For example, an app server would prepend the deployment name (e.g. myapp.war) to the name of the health check procedure.

[heiko-braun]

@jmesnil

I've been reading that sentence twice, but cannot identify anything the part that makes you believe id's should be unique:

The JSON response MUST contain the id entry specifying the name of the check, to support protocols that support external identifier (i.e. URI)

That said, it makes sense for the id's in a single, composite response to be distinct from each other, but that's merely a concern for a human operator reasoning about failed checks, no?

But to be honest, i cannot remember who put this part into the doc and what it should express:
"to support protocols that support external identifier (i.e. URI)"

[jmesnil]

I may have misunderstood that sentence. As I understand it, it means that the "id" of the "check" must be unique in the JSON response to be identified.
E.g. with the JSON response below, we can not identify/link to myCheck health checks in it.:

{
  "outcome": "DOWN",
  "checks": [
    {
      "id": "myCheck",
      "result": "UP"
      },
    {
      "id": "myCheck",
      "result": "DOWN"
      },
    }
  ]
}

I think the sentence should be clarified as I'm not sure to understand what "support protocols that support external identifier (i.e. URI)" means or we should constrain the health check names to be unique.

[heiko-braun]

yes, it unclear. but I agree, making them distinct is probably an improvement.

[heiko-braun]

Currently we are using flat names for id's, but looking at #37 I believe actual namespaces for id's would be better suited. This would allow us to prevent name collisions and easily mix in system level health checks (the ones that come out-of-the-box) by reserving certain namespaces in the spec.

For example:

microprofile.host.disk or microprofile.jvm.heap could become reserved in the microprofile.* namespace

[antoinesd]

Interesting enhancement, but should probably discuss after #35 and 1.1

[Emily-Jiang]

I am not sure whether this is an improvement. What’s the use case? I will prefer to leave as it is as if we force the uniqueness some old apps will stop working for no apparent reason.

[antoinesd]

Discussion is continuing in #161