javamelody / grails-melody-plugin

JavaMelody monitoring plugin for Grails, to monitor application performance
https://plugins.grails.org/plugin/grails-melody-plugin
Apache License 2.0
33 stars 31 forks source link

JDBC DataSource is not wrapped, SQL requests are not monitored #3

Closed evernat closed 11 years ago

evernat commented 11 years ago

Moved from https://code.google.com/p/javamelody/issues/detail?id=280

What steps will reproduce the problem?

  1. Create a grails app
  2. Add some domain class and scaffold controller
  3. Add the melody plugin
  4. Enter to your app in /monitoring

I expect to see the sql statistics after of the creation of some data.

Issue in MacOSX 10.8.2, java version "1.6.0_37", grails 2.2.0

evernat commented 11 years ago

It's the same issue as: http://jira.grails.org/browse/GPMELODY-9

which gives a workaround known to work:

import net.bull.javamelody.JdbcWrapper

class BootStrap {

    def dataSource

    def init = { servletContext ->
        dataSource.targetDataSource = JdbcWrapper.SINGLETON.createDataSourceProxy(dataSource.targetDataSource)
    }
    //...
}
evernat commented 11 years ago

Can someone submit a patch or a pull request?

sergiomichels commented 11 years ago

Grails 2.x works with multiple datasources, I will take a look, but I think that this will need a little but more to work.

sergiomichels commented 11 years ago

@evernat Is there some way to check if the datasource is with the correct proxy, or unwrap the proxy with Melody?

I ended up with a change in GrailsDataSourceBeanPostProcessor:

public static final String DATA_SOURCE_PREFIX = "dataSourceUnproxied"

Object postProcessAfterInitialization(Object bean, String beanName) {

  if(beanName.startsWith(DATA_SOURCE_PREFIX)) {
    return JdbcWrapper.SINGLETON.createDataSourceProxy(bean)
  }
  return bean
}
evernat commented 11 years ago

There is currently the following in src/groovy/grails/plugin/melody/GrailsDataSourceBeanPostProcessor:

    Object postProcessAfterInitialization(Object bean, String beanName) {
        if (bean instanceof DataSource && "dataSource" == beanName){
            return JdbcWrapper.SINGLETON.createDataSourceProxy(bean)
        }
        return bean
    }

If what you want is to make a proxy for every datasources going into this BeanPostProcessor, and not only for a datasource named "dataSource", it could be changed to:

    Object postProcessAfterInitialization(Object bean, String beanName) {
        if (bean instanceof DataSource) {
            return JdbcWrapper.SINGLETON.createDataSourceProxy(beanName, bean)
        }
        return bean
    }

And if the dataSource is already a javamelody proxy, it will not create a proxy of the proxy.

If beanName.startsWith("dataSourceUnproxied") works in general for Grails 2, I am ok with your code in the previous comment.

Otherwise, I don't know why you would want to know if a datasource is already a javamelody proxy or to unwrap it. But, if you really want to do that, you can probably write (java version):

        import java.lang.reflect.*;
        import net.bull.javamelody.*;

        if (Proxy.isProxyClass(proxy.getClass())) {
            InvocationHandler invocationHandler = Proxy.getInvocationHandler(proxy);
            if (invocationHandler instanceof DelegatingInvocationHandler) {
                invocationHandler = ((DelegatingInvocationHandler) invocationHandler).getDelegate();
                if (invocationHandler instanceof AbstractInvocationHandler) {
                    final Object proxiedObject = ((AbstractInvocationHandler<?>) invocationHandler)
                            .getProxiedObject();
                                        ...
                }
            }
        }
ilopmar commented 11 years ago

Hi,

I've updated the plugin to the last version 1.47 (my previous version was 1.13) and removed the workaround in my BootStrap.groovy file. Now it's not logging sql. If I add again the workaround in the BootStrap everything is working again.

Should this feature work without the workaround?

The workaround:

dataSource.targetDataSource = net.bull.javamelody.JdbcWrapper.SINGLETON.createDataSourceProxy(dataSource.targetDataSource)
sergiomichels commented 11 years ago

I will take a look on this. This should work without the BootStrap because the BeanPostProcessor is wrapping all beans that are instance of DataSource.

ilopmar commented 11 years ago

Ok, thank you very much. If you need that I test something, please do not hesitate and tell me. I'm glad with helping.

Regards, Iván.

sergiomichels commented 11 years ago

So, I tested here, and for my surprise, the datasource beans are not passing though the BeanPostProcessor. I will need to figure out why this is happening. Probably we will need to take the same approach that tomcat jdbc plugin does: https://github.com/burtbeckwith/grails-jdbc-pool/blob/master/JdbcPoolGrailsPlugin.groovy

sergiomichels commented 11 years ago

Even if I create a PostBeanProcessor in the application the dataSource didn't pass through it, so the solution is to change it in the doWithApplicationContext closure:

def doWithApplicationContext = { ctx ->
  def beans = ctx.getBeansOfType(DataSource)
  beans.each { beanName, bean ->
    if(bean?.hasProperty("targetDataSource")) {
      bean.targetDataSource = JdbcWrapper.SINGLETON.createDataSourceProxy(bean.targetDataSource)
    }
  }
}

I will do a new pull request tonight.

evernat commented 11 years ago

pull request #15 is now merged, thanks

I have published version 1.47.1 of the plugin with it.