Metrics are improperly unregistered when application is undeployed

[jmesnil]

The code in

smallrye-metrics/implementation/src/main/java/io/smallrye/metrics/MetricRegistries.java

Line 61 in de37ba4

@PreDestroy

is problement when smallrye-metrics is used in a application server that has a longer lifecycle than application deployment.

Since MetricRegistries is @ApplicationScoped, when the application deployment is undeployed, its cleanUp method is invoked and all 3 registries are cleared (without actually unregistering the metrics inside them btw).

Imagine that an application server that support redeployment is running:

• its base and vendor metrics are registered
• when the deployment is deployed, its application metrics are registered
• during redeployment (i.e. undeploy + deploy), the MetricsRegistries#cleanUp method is invoked
  => there are no longer any base and vendor metrics.

Proposed fix:

The `@ApplicationScoped' MetricRegistries should only clean up the application registry and leave the base and vendor ones untouched when it is destroyed.

In addition, I am not sure that the getBaseRegistry() and getVendorRegistry() should be @ApplicationScoped. By definition, their scope is wider than the application as they provides metrics for the whole base (JVM) and vendor (app server) whose lifecycle is longer than application deployment.

[jmesnil]

@pilhuhn Do you think that is something that can be fixed in smallrye-metrics or is it a spec concern too?

[mkouba]

In addition, I am not sure that the getBaseRegistry() and getVendorRegistry() should be @ApplicationScoped.

I think that the scope does not really matter here. If you removed the @ApplicationScoped then the only difference would be that getBaseRegistry()/getVendorRegistry() will be invoked for every injection point, i.e. no client proxy is created and the relevant MetricRegistry instance is not stored in app context. But since there are no disposers methods or anything it should not make any difference.

[jmesnil]

closed by #13