org.apache.tomcat.jdbc.pool.DataSource register with jmx

[ralscha]

1.2.0.BUILD-SNAPSHOT

Would be nice if the DataSource object registers itself with jmx (if jmx is enabled)
I tried to enable it with spring.datasource.jmxEnabled=true
But it looks like this property does nothing.

I can use the following code to register the DataSource in my application

@Autowired
public Startup(DataSource dataSource, MBeanServer server) throws MalformedObjectNameException, Exception {
  if (dataSource instanceof org.apache.tomcat.jdbc.pool.DataSource) {
    org.apache.tomcat.jdbc.pool.DataSource tomcatDS =(org.apache.tomcat.jdbc.pool.DataSource)dataSource;
    tomcatDS.preRegister(server, new ObjectName(ConnectionPool.POOL_JMX_TYPE_PREFIX+tomcatDS.getClass().getName()+",name=mypool"));
        }
    }

[dsyer]

This would work too:

    @Bean
    public ConnectionPool getJmxPool(DataSource dataSource) throws SQLException {
        return dataSource.createPool().getJmxPool();
    }

[ralscha]

This does work. But it has one problem when jmxEnabled is set to false: spring.datasource.jmxEnabled=false

The application throws this error, because getJmxPool() returns null

org.springframework.jmx.export.UnableToRegisterMBeanException: Unable to register MBean [jdbcPool] with key 'jdbcPool'; nested exception is java.lang.IllegalArgumentException: Candidate object must not be null
    at org.springframework.jmx.export.MBeanExporter.registerBeanNameOrInstance(MBeanExporter.java:613)
    at org.springframework.jmx.export.MBeanExporter.registerBeans(MBeanExporter.java:538)
    at org.springframework.jmx.export.MBeanExporter.afterSingletonsInstantiated(MBeanExporter.java:420)

[dsyer]

That looks like a completely unrelated problem (the pool properties are not initialized until the pool is used).

[dsyer]

Or maybe not. Maybe you just need to put a check in there so you don't create the @Bean if the property is false, e.g. @ConditionalOnExpression("${spring.datasource.jmxEnabled:true}").

[dsyer]

You could put that in the TomcatDataSourceConfiguration BTW (fill in the contributor's agreement if you haven't already and send a pull request).

[ralscha]

The @ConditionalOnExpression solution works great.

@Bean
@ConditionalOnExpression("${spring.datasource.jmxEnabled:true}")
public ConnectionPool jdbcPool(DataSource dataSource) throws SQLException {
  return dataSource.createPool().getJmxPool();
}

I was looking for the TomcatDataSourceConfiguration class but there is only one such class and it's located in a test class (TomcatDataSourceConfigurationTests).

[dsyer]

Sorry, that was me remembering Spring Boot 1.0.x. The only way to do it generically and safely now might be to add a @Configuration that is @ConditionalOnBean and @ConditionalOnClass referring to the Tomcat DataSource.

[philwebb]

It turns out registering the JMX information by default can cause some issues if you use Spring's Test Context Framework. I've hit this trying to upgrade Sagan. By default, tests with @ContextConfiguration keep contexts open, so a second test case causes the following:

Caused by: javax.management.InstanceAlreadyExistsException: org.apache.tomcat.jdbc.pool.jmx:name=dataSourceMBean,type=ConnectionPool
    at com.sun.jmx.mbeanserver.Repository.addMBean(Repository.java:437)
    at com.sun.jmx.interceptor.DefaultMBeanServerInterceptor.registerWithRepository(DefaultMBeanServerInterceptor.java:1898)
    at com.sun.jmx.interceptor.DefaultMBeanServerInterceptor.registerDynamicMBean(DefaultMBeanServerInterceptor.java:966)
    at com.sun.jmx.interceptor.DefaultMBeanServerInterceptor.registerObject(DefaultMBeanServerInterceptor.java:900)
    at com.sun.jmx.interceptor.DefaultMBeanServerInterceptor.registerMBean(DefaultMBeanServerInterceptor.java:324)
    at com.sun.jmx.mbeanserver.JmxMBeanServer.registerMBean(JmxMBeanServer.java:522)
    at org.springframework.jmx.support.MBeanRegistrationSupport.doRegister(MBeanRegistrationSupport.java:195)
    at org.springframework.jmx.export.MBeanExporter.registerBeanInstance(MBeanExporter.java:658)
    at org.springframework.jmx.export.MBeanExporter.registerBeanNameOrInstance(MBeanExporter.java:603)
    ... 57 more

I can't find an easy way to fix this so I think we need to change spring.datasource.jmx-enabled to default to false and make this one opt-in.