smallrye-metrics icon indicating copy to clipboard operation
smallrye-metrics copied to clipboard

How is `mp.metrics.appName` supposed to work?

Open jmesnil opened this issue 5 years ago • 6 comments

MP Metrics provides a recommendation to support multiple applications However it also mention that this recommendation also applies to EAR with subdeployments.

It write that the subdeployments can use their META-INF/microprofile-config.properties properties file to specify an unique mp.metrics.appName property.

This does not work with smallrye-metrics. All injected metrics (regardless of the subdeployments) are registered from io.smallrye.metrics.setup.MetricCdiInjectionExtension. The code will end up in org.eclipse.microprofile.metrics.MetricID#MetricID(java.lang.String, org.eclipse.microprofile.metrics.Tag...) that will call ConfigProvider.getConfig() to load the Config object to read the configuration properties.

At this point, the Config will be fetched corresponding to the TCCL which is the EAR ClassLoader. All registered metrics in the EAR will use the same Config object and will not use their properties from their respective META-INF/microprofile-config.properties file.

Am I missing something on how this feature is supposed to work for EAP deployment with subdeployments?

jmesnil avatar Dec 02 '19 14:12 jmesnil

Sorry I was on paternity leave, hence the late response. Do you mean that in WildFly, all modules of an EAR share a common TCCL and a common Config instance? I thought every module had its own TCCL and therefore ConfigProvider.getConfig() would resolve to a separate Config instance scoped to that module. So each module will get its mp.metrics.appName property attached in the config's SubsystemDeploymentProcessor (as in my crude PoC in https://github.com/jmartisk/wildfly/commit/0dd89cf16a2ea885b0d1eacea17f42002b431afc) and later when metrics are being registered, the TCCL will be the TCCL of the particular module, so MetricID constructor will pick up the correct Config instance. Is that not so?

jmartisk avatar Dec 11 '19 07:12 jmartisk

From my quick experiments, it looks like each module has its own Config instance and its own TCCL, and the top-level EAR has got the same as well, but when metrics are being registered by MetricCdiInjectionExtension, the TCCL is set to the TCCL of the whole EAR, not the module. That seems to be the cause why all metrics see the same mp.metrics.appName property. Isn't that a bug, shouldn't the TCCL be the TCCL of the module when CDI extensions are called?

jmartisk avatar Dec 11 '19 07:12 jmartisk

Most importantly, congrats for the baby! :)

I debugged the loading and execution of the MetricCdiInjectionExtension extension in WildFly.

The class is loaded with the EAR module class loader. When its @Observes method are called from org.jboss.as.weld.WeldStartService#start the TCCL is set to the EAR module class loader.

That means that hen metrics are registered in io.smallrye.metrics.setup.MetricCdiInjectionExtension#registerMetrics, the TCCL is still set to the EAR module class loader. At this point, all metrics uses the same class loader (which is the EAR module class loader) regardless of the actual location of the metrics annotations (in the EAR itself or in subdeployments).

jmesnil avatar Dec 11 '19 09:12 jmesnil

Thanks! :)

Well yep, so the @Observes methods are registered with the TCCL of the top-level deployment, which I think is weird, because each sub-deployment has its own TCCL, so my expectation would be that they should be called with the TCCL of the sub-deployment. I don't know if this is somehow defined by any spec, but intuitively it sounds like a bug in WildFly to me.

Changing this (if it's really a bug) would fix our problem I suppose, because we would then pick up the correct Config which can contain mp.metrics.appName computed for the particular sub-deployment.

jmartisk avatar Dec 11 '19 10:12 jmartisk

The _app feature was removed with MP-Metrics Spec 3.0! Now, they say, its up to the implementation to support this feature. I cannot find it yet in SmallRye-Metrics :(. See: https://download.eclipse.org/microprofile/microprofile-metrics-3.0/microprofile-metrics-spec-3.0.pdf and: https://github.com/eclipse/microprofile-metrics/commit/6690db404a9f415f794ecc6363078bf2b4448a45#diff-0eb6f5c712741992073cacdae252851c7dfbdb8d774b4e618c3c259433e39284

svenhaag avatar Dec 16 '21 08:12 svenhaag

SR Metrics itself does not support application-name tags right now, it would be up to the app server (WildFly etc) to provide it, but we haven't figured out how exactly that should be done (SR would probably have to provide some facility to allow it, now that it was moved out of the MP API). Given the status of SmallRye Metrics being deprecated and replaced with Micrometer, I don't see that very likely. The workaround for now would be to simply make sure you always include an _app (or similar) tag manually in the application's code

jmartisk avatar Dec 16 '21 08:12 jmartisk