Questions/Problems regarding commons-dbcp2 metrics

[jjank]

Hi everybody,

first of all thanks for working on micrometer - it is a true lifesaver 😃

I'm having trouble getting metrics out of apache commons dbcp2 data sources. I've set up a minimal working example to illustrate my problems.

Problem description

Disclaimer - I am terribly unfamiliar with JMX so it is very much possible, that this is the root cause of the problem.

I've configured the org.apache.commons.dbcp2.BasicDataSource with the following JMX-name: org.apache.commons.pool2:name=dbcp,type=GenericObjectPool.

After a little bit of digging through the source code of BasicDataSource this is necessary as JXM will not be enabled otherwise (this seems strange to me because the "vanilla" GenericObjectPool has a different behaviour).

If my understanding of CommonsObjectPool2Metrics is correct this name pattern is necessary in order for Micrometer to register this pool ?

When running the application (or the test in my example repo) I encounter the following erros in the log:

SEVERE: can not set name tag
javax.management.AttributeNotFoundException: No such attribute: FactoryType
	at java.management/com.sun.jmx.mbeanserver.PerInterface.getAttribute(PerInterface.java:81)
	at java.management/com.sun.jmx.mbeanserver.MBeanSupport.getAttribute(MBeanSupport.java:206)
	at java.management/com.sun.jmx.interceptor.DefaultMBeanServerInterceptor.getAttribute(DefaultMBeanServerInterceptor.java:641)
	at java.management/com.sun.jmx.mbeanserver.JmxMBeanServer.getAttribute(JmxMBeanServer.java:678)
	at io.micrometer.core.instrument.binder.commonspool2.CommonsObjectPool2Metrics.nameTag(CommonsObjectPool2Metrics.java:141)
	at io.micrometer.core.instrument.binder.commonspool2.CommonsObjectPool2Metrics.lambda$registerNotificationListener$1(CommonsObjectPool2Metrics.java:193)
	at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1128)
	at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:628)
	at java.base/java.lang.Thread.run(Thread.java:834)

May 07, 2021 1:25:15 PM io.micrometer.core.instrument.binder.commonspool2.CommonsObjectPool2Metrics lambda$registerNotificationListener$1
SEVERE: can not set name tag
javax.management.InstanceNotFoundException: org.apache.commons.pool2:name=dbcp,type=GenericObjectPool,connectionpool=connections,connection=0
	at java.management/com.sun.jmx.interceptor.DefaultMBeanServerInterceptor.getMBean(DefaultMBeanServerInterceptor.java:1083)
	at java.management/com.sun.jmx.interceptor.DefaultMBeanServerInterceptor.getAttribute(DefaultMBeanServerInterceptor.java:637)
	at java.management/com.sun.jmx.mbeanserver.JmxMBeanServer.getAttribute(JmxMBeanServer.java:678)
	at io.micrometer.core.instrument.binder.commonspool2.CommonsObjectPool2Metrics.nameTag(CommonsObjectPool2Metrics.java:141)
	at io.micrometer.core.instrument.binder.commonspool2.CommonsObjectPool2Metrics.lambda$registerNotificationListener$1(CommonsObjectPool2Metrics.java:193)

It seems that:

• The FactoryType is not set by dbcp2
• dbcp registeres every connection with jmx but the connection does not seem to have a name property

So my questions are:

• Have I misconfigured the JMX-Name?
• More generaly: Would it be valuable to implement a dedicated MeterBinder for commons dbcp2 that will handle the (JMX)-specific more explicitly? Or should CommonsObjectPool2Metrics be more graceful/resillient/configurable?

Thanks a lot in advance 👍

[shakuzen]

We still have #1710 open for specifically supporting metrics of Apache DBCP2. It was suggested in #1712 that the CommonsObjectPool2Metrics could be used to get some metrics from DBCP2, but I don't know if anyone actually tested that. We could be more defensive about the FactoryType not being present, though.

[jjank]

With the configuration I described you actually get metrics from DBCP2 - I should have been more clear about that. You "just" will see a lot of errors in the log 😬

CommonsObjectPool2Metrics seems to be a good basis for supporting DBCP2. I think there are a couple of valid approaches for fully supporting DBCP2. One of them might be to extract the JMX-querying code and let CommonsObjectPool2Metrics and some DBCP2Metrics delegate to that. Each with their specific JMX-queries.

Maybe the DBCP2 devs will be able to provide some guidance too?

[fzyzcjy]

Hi, I am seeing this bug in my code today with latest versions. Is there any updates? Thanks!

[jonatan-ivanov]

Latest Micrometer and latest DBCP2?
Could you please check that the object that the log entry is complaining about is available through JMX (jconsole, JMC, visualvm, etc.)?

[fzyzcjy]

@jonatan-ivanov Hi thanks for the reply! I will check that

[jjank]

Hi there !

I noticed that as of version 2.10.0 the underlying Connection pool (i.e. the org.apache.commons.pool2.impl.GenericObjectPool) is exposed publicly: apache/commons-dbcp@54ef9d3 in BasicDataSource.java. (In a previous discussion this was the main motivation for using JMX in CommonsObjectPool2Metrics)

This seems to be a deliberate choice - though I could not find an explanation/motivation in the respective mailing lists

This would make it quite easy to instrument because GenericObjectPool provides methods for all common metrics.

There's a couple of ways how to go forward:

1. "Just" implement a MeterBinder for BasicDataSource and ignore CommonsObjectPool2Metrics. To me this does not seem a great choice because:
   • it would require a dependency (albeit optional) on commons-dpcp2 in micrometer
   • a lot of duplication of code that exists in CommonsObjectPool2Metrics which leads to
2. "Enhance" CommonsObjectPool2Metrics to also gather metrics directly from a (user-passed) reference of GenericObjectPool in addition to querying jmx (Not clear what the Api would look like for users. It might lead to a lot of constructors/overloads so the details are tbd in that case)

I'd be willing to contribute since instrumenting dbcp would greatly benefit the app I'm currently maintaining (I have 0 experience with the Observation api though). But I'm not sure If you - in the long term - would rather like to see libraries provide instrumentation themselves and thus reduce the maintainance burden on micrometer. On the other hand recent PRs auch as #4037 have put in a lot of effort in improving 3rd-party libs instrumentation 🤔

Thoughts @jonatan-ivanov ?

[raymondchen625]

I ran into the same "missing the FactoryType attribute" issue when I was using CommonsObjectPool2Metrics to collect metrics.
The GenericObjectPool and GenericObjectPoolMXBean indeed have the FactoryType property. That might be the reason why CommonsObjectPool2Metrics expects it. But the BasicDataSource doesn't implement/extend GenericObjectPool, hence no FactoryType property. It only uses GenericObjectPool internally.
It appears to me the purpose of getting the FactoryType is just to add it to the tags. Not sure why we want to mandate this. If we stop getting the FactoryType attribute and adding it to the tags, the problem will be solved.

[jjank]

It appears to me the purpose of getting the FactoryType is just to add it to the tags. Not sure why we want to mandate this.

I can think of one reason: If you happen to have more than one object pool in your application for different purposes (some libraries use the pool under the hood) it is hard if not impossible to distinguish between different pools in the metrics.

[raymondchen625]

it is hard if not impossible to distinguish between different pools in the metrics

The name tag can be used to distinguish different pools. Actually, the non-configurable name tag might also cause naming conflict issue.

[shakuzen]

#5316 was merged, which should address the FactoryType issue. What's the remaining issue for getting metrics on BasicDataSource? I understand now things go through JMX and that needs to be configured on the BasicDataSource. Are things not working when doing that with the latest versions of Micrometer? I agree an option to directly bind metrics for a GenericObjectPool instance might be nice rather than having to go through JMX, but I'm trying to understand if the instrumentation via JMX works for BasicDataSource now or not.