MP health 2.0 check with CDI producers fails

[sdaschner]

Defining a mp-health check as CDI producer fails at deployment.

The following, taken from the samples:

@ApplicationScoped
public class Checks {

    @Readiness
    @Produces
    HealthCheck databaseCheck() {
        return () -> HealthCheckResponse.named("random-check").up().build();
    }
}

... results in ...

[ERROR   ] SRVE0777E: Exception thrown by application class 'org.jboss.weld.manager.BeanManagerImpl.getReference:712'
org.jboss.weld.exceptions.IllegalArgumentException: WELD-001305: The given type class com.sebastian_daschner.openliberty_russia.Checks is not a type of the bean Producer Method [HealthCheck] with qualifiers [@Readiness @Any] declared as [[BackedAnnotatedMethod] @Readiness @Produces @ApplicationScoped com.sebastian_daschner.openliberty_russia.Checks.databaseCheck()]
  at org.jboss.weld.manager.BeanManagerImpl.getReference(BeanManagerImpl.java:712)
  at [internal classes]

Tested with Open Liberty 19.0.0.7/wlp-1.0.30.cl190720190623-0300 on OpenJ9, version 12.0.1+12 and feature mpHealth-2.0.

[NottyCode]

The example in the spec annotates the methods with @ApplicationScoped which you don’t have. I’m wondering if that makes a difference:

@ApplicationScoped
class MyChecks {

  @Produces
  @ApplicationScoped
  @Liveness
  HealthCheck check1() {
    return () -> HealthStatus.state(getMemUsage() < 0.9);
  }

  @Produces
  @ApplicationScoped
  @Readiness
  HealthCheck check2() {
    return () -> HealthStatus.state(getCpuUsage() < 0.9);
  }
}}

[sdaschner]

... no, that also doesn't work. From a CDI perspective, annotating both also doesn't make a difference scope-wise (also doesn't really make sense IMO).

[fmhwong]

Our implementation passed the TCK. This is one of the test https://github.com/eclipse/microprofile-health/blob/c505362dc6d9f02a9c46c88f5ff56b16cd90337e/tck/src/main/java/org/eclipse/microprofile/health/tck/deployment/SuccessfulReadiness.java.

@Emily-Jiang Can you help to see if this is a CDI problem?

[sdaschner]

@fmhwong Btw, the test verifies a CDI application-scoped bean, not a CDI producer...

[Azquelt]

Hi @fmhwong I think this is an issue with mpHealth caused by the line of code here: https://github.com/OpenLiberty/open-liberty/blob/integration/dev/com.ibm.ws.microprofile.health.2.0/src/com/ibm/ws/microprofile/health20/services/impl/HealthCheck20CDIBeanInvokerImpl.java#L134

At this point in the code you do this:

beanManager.getReference(bean, bean.getBeanClass(), beanManager.createCreationalContext(bean))

The bean here is the Bean<T> created for the producer method. Somewhat unintuitively, when you have a Bean for a producer method, bean.getBeanClass() returns the class which declares the producer method, and not the type which the producer method returns, see the Javadoc.

The second argument to BeanManager.getReference must be one of the bean types. In this case, you're going to call the the bean through the HealthCheck interface, so on that line, you can just do this, which will return you an instance of HealthCheck:

beanManager.getReference(bean, HealthCheck.class, beanManager.createCreationalContext(bean))

As a side note, as @sdaschner points out, the test you link doesn't test a Producer method and furthermore I can't see any tests for health check beans created through a producer method. We should add a FAT test for this to liberty and raise a spec issue to get one added to the TCK.

[fmhwong]

Thanks @Azquelt

[fmhwong]

@Azquelt @sdaschner Do you think this is a must fix before GA?

[sdaschner]

@fmhwong Good question, which depends how important we evaluate mpHealth in general (in my projects, I haven't seen too much use of it b/c of the current limitations in cloud-native environments). If folks are using mpHealth in a more complex way then yes, they would expect it to work properly, but at least, due to the error, they don't get false positives.

[jhanders34]

@Channyboy I see that you made the change in the Health 2.0 code. Is there a reason you didn't also make the same change in the corresponding file for Health 1.x? Does this problem not also exist there?

[pgunapal]

The HealthCheck procedures being defined as CDI Producers was added to the mpHealth-2.0 spec (https://github.com/eclipse/microprofile-health/blob/2.0-RC1/spec/src/main/asciidoc/java-api.adoc) and but was not part of the mpHealth-1.0 spec (https://github.com/eclipse/microprofile-health/blob/1.0/spec/src/main/asciidoc/java-api.adoc).

[pgunapal]

Created a follow-up issue to add a test case in our FATs: #8255

[Azquelt]

The HealthCheck procedures being defined as CDI Producers was added to the mpHealth-2.0 spec but was not part of the mpHealth-1.0 spec

This is not true.

The @Produces example was only added in the 2.0 spec but the 1.0 spec still says this:

Within CDI contexts, beans that implement HealthCheck and annotated with @Health are discovered automatically

Beans created by producer methods are still beans (though note that it's the producer method, not the class of the returned object, which would need to be annotated with @Health qualifier annotation).