Merge pull request #955 from lioolli/master

 Moved spring mvc servlet profiling feature to spring plugin

agent/pom.xml

@@ -50,6 +50,17 @@
             <artifactId>spring-context</artifactId>
             <scope>test</scope>
         </dependency>
+        <dependency>
+            <groupId>org.springframework</groupId>
+            <artifactId>spring-webmvc</artifactId>
+            <scope>test</scope>
+        </dependency>
+        <dependency>
+            <groupId>javax.servlet</groupId>
+            <artifactId>javax.servlet-api</artifactId>
+            <scope>test</scope>
+        </dependency>
+
         <dependency>
             <groupId>com.google.code.gson</groupId>
             <artifactId>gson</artifactId>

agent/src/test/java/com/navercorp/pinpoint/plugin/spring/web/SpringWebMvcIT.java

@@ -0,0 +1,69 @@
+/**
+ * Copyright 2014 NAVER Corp.
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ * 
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ * 
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package com.navercorp.pinpoint.plugin.spring.web;
+
+import java.lang.reflect.Method;
+
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletResponse;
+
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.springframework.mock.web.MockHttpServletRequest;
+import org.springframework.mock.web.MockHttpServletResponse;
+import org.springframework.mock.web.MockServletConfig;
+import org.springframework.web.servlet.DispatcherServlet;
+import org.springframework.web.servlet.FrameworkServlet;
+
+import com.navercorp.pinpoint.bootstrap.plugin.test.Expectations;
+import com.navercorp.pinpoint.bootstrap.plugin.test.PluginTestVerifier;
+import com.navercorp.pinpoint.bootstrap.plugin.test.PluginTestVerifierHolder;
+import com.navercorp.pinpoint.test.plugin.Dependency;
+import com.navercorp.pinpoint.test.plugin.PinpointPluginTestSuite;
+
+/**
+ * @author Jongho Moon
+ *
+ */
+@RunWith(PinpointPluginTestSuite.class)
+@Dependency({"org.springframework:spring-webmvc:[3.0.7.RELEASE],[3.1.4.RELEASE],[3.2.14.RELEASE],[4.0.9.RELEASE],[4.1.7.RELEASE],[4.2.0.RELEASE,)", "org.springframework:spring-test", "javax.servlet:javax.servlet-api:3.0.1"})
+public class SpringWebMvcIT {
+    private static final String SPRING_MVC = "SPRING_MVC";
+
+    @Test
+    public void testRequest() throws Exception {
+        MockServletConfig config = new MockServletConfig();
+        MockHttpServletRequest req = new MockHttpServletRequest();
+        MockHttpServletResponse res = new MockHttpServletResponse();
+
+        config.addInitParameter("contextConfigLocation", "classpath:spring-web-test.xml");
+        req.setMethod("GET");
+        req.setRequestURI("/");
+        req.setRemoteAddr("1.2.3.4");
+
+        DispatcherServlet servlet = new DispatcherServlet();
+        servlet.init(config);
+
+        servlet.service(req, res);
+
+        Method method = FrameworkServlet.class.getDeclaredMethod("doGet", HttpServletRequest.class, HttpServletResponse.class);
+
+        PluginTestVerifier verifier = PluginTestVerifierHolder.getInstance();
+        verifier.printCache();
+
+        verifier.verifyTrace(Expectations.event(SPRING_MVC, method));
+        verifier.verifyTraceCount(0);
+    }
+}

agent/src/test/resources/spring-web-test.xml

@@ -0,0 +1,12 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<beans xmlns="http://www.springframework.org/schema/beans" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+       xmlns:mvc="http://www.springframework.org/schema/mvc"
+       xmlns:beans="http://www.springframework.org/schema/beans"
+       xmlns:context="http://www.springframework.org/schema/context"
+       xsi:schemaLocation="http://www.springframework.org/schema/mvc http://www.springframework.org/schema/mvc/spring-mvc-3.0.xsd
+		http://www.springframework.org/schema/beans http://www.springframework.org/schema/beans/spring-beans.xsd
+		http://www.springframework.org/schema/context http://www.springframework.org/schema/context/spring-context.xsd">
+
+    <!-- Enables the Spring MVC @Controller programming model -->
+    <mvc:annotation-driven/>
+</beans>

bootstrap-core/src/main/java/com/navercorp/pinpoint/bootstrap/config/ProfilerConfig.java

@@ -108,11 +108,6 @@ public static ProfilerConfig load(String pinpointConfigFileName) throws IOExcept
     private int callStackMaxDepth = 512;
 
     private int jdbcSqlCacheSize = 1024;
-    private int jdbcMaxSqlBindValueSize = 1024;
-    private boolean jdbcProfile = true;
-
-    private boolean jdbcProfileDbcp = true;
-    private boolean jdbcProfileDbcpConnectionClose = false;
 
     private boolean tomcatHidePinpointHeader = true;
     private Filter<String> tomcatExcludeUrlFilter = new SkipFilter<String>();
@@ -274,18 +269,10 @@ public boolean isProfileEnable() {
         return profileEnable;
     }
 
-    public boolean isJdbcProfile() {
-        return jdbcProfile;
-    }
-
     public int getJdbcSqlCacheSize() {
         return jdbcSqlCacheSize;
     }
 
-    public int getJdbcMaxSqlBindValueSize() {
-        return jdbcMaxSqlBindValueSize;
-    }
-
     public boolean isSamplingEnable() {
         return samplingEnable;
     }
@@ -311,14 +298,6 @@ public long getAgentInfoSendRetryInterval() {
         return agentInfoSendRetryInterval;
     }
 
-    public boolean isJdbcProfileDbcp() {
-        return jdbcProfileDbcp;
-    }
-
-    public boolean isJdbcProfileDbcpConnectionClose() {
-        return jdbcProfileDbcpConnectionClose;
-    }
-
     public boolean isTomcatHidePinpointHeader() {
         return tomcatHidePinpointHeader;
     }
@@ -559,14 +538,7 @@ void readPropertyValues() {
         }
 
         // JDBC
-        this.jdbcProfile = readBoolean("profiler.jdbc", true);
-
         this.jdbcSqlCacheSize = readInt("profiler.jdbc.sqlcachesize", 1024);
-        this.jdbcMaxSqlBindValueSize = readInt("profiler.jdbc.maxsqlbindvaluesize", 1024);
-
-        this.jdbcProfileDbcp = readBoolean("profiler.jdbc.dbcp", true);
-        this.jdbcProfileDbcpConnectionClose = readBoolean("profiler.jdbc.dbcp.connectionclose", false);
-
 
         this.tomcatHidePinpointHeader = readBoolean("profiler.tomcat.hidepinpointheader", true);
         final String tomcatExcludeURL = readString("profiler.tomcat.excludeurl", "");
@@ -794,14 +766,6 @@ public String toString() {
         builder.append(callStackMaxDepth);
         builder.append(", jdbcSqlCacheSize=");
         builder.append(jdbcSqlCacheSize);
-        builder.append(", jdbcMaxSqlBindValueSize=");
-        builder.append(jdbcMaxSqlBindValueSize);
-        builder.append(", jdbcProfile=");
-        builder.append(jdbcProfile);
-        builder.append(", jdbcProfileDbcp=");
-        builder.append(jdbcProfileDbcp);
-        builder.append(", jdbcProfileDbcpConnectionClose=");
-        builder.append(jdbcProfileDbcpConnectionClose);
         builder.append(", tomcatHidePinpointHeader=");
         builder.append(tomcatHidePinpointHeader);
         builder.append(", tomcatExcludeUrlFilter=");

commons/src/main/java/com/navercorp/pinpoint/common/trace/ServiceType.java

@@ -272,7 +272,7 @@ public boolean equals(Object obj) {
 
     // Spring framework
     public static final ServiceType SPRING = of(5050, "SPRING", NORMAL_SCHEMA);
-    public static final ServiceType SPRING_MVC = of(5051, "SPRING_MVC", "SPRING", NORMAL_SCHEMA);
+//    public static final ServiceType SPRING_MVC = of(5051, "SPRING_MVC", "SPRING", NORMAL_SCHEMA);
     // FIXME replaced with IBATIS_SPRING (5501) under IBatis Plugin - kept for backwards compatibility
     public static final ServiceType SPRING_ORM_IBATIS = of(5061, "SPRING_ORM_IBATIS", "SPRING", NORMAL_SCHEMA);
     // FIXME need to define how to handle spring related codes

plugins/dbcp/src/main/java/com/navercorp/pinpoint/plugin/commons/dbcp/CommonsDbcpPlugin.java

@@ -36,22 +36,34 @@ public class CommonsDbcpPlugin implements ProfilerPlugin {
 
     @Override
     public void setup(ProfilerPluginSetupContext context) {
-        context.addClassFileTransformer("org.apache.commons.dbcp.BasicDataSource", new PinpointClassFileTransformer() {
+        addBasicDataSourceTransformer(context);
+
+        boolean profileClose = context.getConfig().readBoolean("profiler.jdbc.dbcp.connectionclose", false);
+
+        if (profileClose) {
+            addPoolGuardConnectionWrapperTransformer(context);
+        }
+    }
+
+    private void addPoolGuardConnectionWrapperTransformer(ProfilerPluginSetupContext context) {
+        context.addClassFileTransformer("org.apache.commons.dbcp.PoolingDataSource$PoolGuardConnectionWrapper", new PinpointClassFileTransformer() {
 
             @Override
             public byte[] transform(ProfilerPluginInstrumentContext pluginContext, ClassLoader loader, String className, Class<?> classBeingRedefined, ProtectionDomain protectionDomain, byte[] classfileBuffer) throws InstrumentException {
                 InstrumentClass target = pluginContext.getInstrumentClass(loader, className, classfileBuffer);
-                target.addInterceptor("com.navercorp.pinpoint.plugin.commons.dbcp.interceptor.DataSourceGetConnectionInterceptor");
+                target.addInterceptor("com.navercorp.pinpoint.plugin.commons.dbcp.interceptor.DataSourceCloseInterceptor");
                 return target.toBytecode();
             }
         });
-
-        context.addClassFileTransformer("org.apache.commons.dbcp.PoolingDataSource$PoolGuardConnectionWrapper", new PinpointClassFileTransformer() {
+    }
+
+    private void addBasicDataSourceTransformer(ProfilerPluginSetupContext context) {
+        context.addClassFileTransformer("org.apache.commons.dbcp.BasicDataSource", new PinpointClassFileTransformer() {
 
             @Override
             public byte[] transform(ProfilerPluginInstrumentContext pluginContext, ClassLoader loader, String className, Class<?> classBeingRedefined, ProtectionDomain protectionDomain, byte[] classfileBuffer) throws InstrumentException {
                 InstrumentClass target = pluginContext.getInstrumentClass(loader, className, classfileBuffer);
-                target.addInterceptor("com.navercorp.pinpoint.plugin.commons.dbcp.interceptor.DataSourceCloseInterceptor");
+                target.addInterceptor("com.navercorp.pinpoint.plugin.commons.dbcp.interceptor.DataSourceGetConnectionInterceptor");
                 return target.toBytecode();
             }
         });

plugins/pom.xml

@@ -30,7 +30,7 @@
         <module>arcus</module>
         <module>google-httpclient</module>
         <module>jetty</module>
-        <module>spring-beans</module>
+        <module>spring</module>
         <module>spring-boot</module>
         <module>ibatis</module>
         <module>mybatis</module>
@@ -129,7 +129,7 @@
         </dependency>
         <dependency>
             <groupId>com.navercorp.pinpoint</groupId>
-            <artifactId>pinpoint-spring-beans-plugin</artifactId>
+            <artifactId>pinpoint-spring-plugin</artifactId>
             <version>${project.version}</version>
         </dependency>
         <dependency>

plugins/spring-beans/pom.xml → plugins/spring/pom.xml

@@ -8,8 +8,8 @@
         <version>1.5.0-SNAPSHOT</version>
     </parent>
 
-    <artifactId>pinpoint-spring-beans-plugin</artifactId>
-    <name>pinpoint-spring-beans-plugin</name>
+    <artifactId>pinpoint-spring-plugin</artifactId>
+    <name>pinpoint-spring-plugin</name>
     <packaging>jar</packaging>
 
     <dependencies>

plugins/spring/src/main/java/com/navercorp/pinpoint/plugin/spring/web/SpringWebMvcPlugin.java

@@ -0,0 +1,54 @@
+/**
+ * Copyright 2014 NAVER Corp.
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ * 
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ * 
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package com.navercorp.pinpoint.plugin.spring.web;
+
+import static com.navercorp.pinpoint.common.trace.HistogramSchema.*;
+
+import java.security.ProtectionDomain;
+
+import com.navercorp.pinpoint.bootstrap.instrument.InstrumentClass;
+import com.navercorp.pinpoint.bootstrap.instrument.InstrumentException;
+import com.navercorp.pinpoint.bootstrap.interceptor.BasicMethodInterceptor;
+import com.navercorp.pinpoint.bootstrap.plugin.ProfilerPlugin;
+import com.navercorp.pinpoint.bootstrap.plugin.ProfilerPluginInstrumentContext;
+import com.navercorp.pinpoint.bootstrap.plugin.ProfilerPluginSetupContext;
+import com.navercorp.pinpoint.bootstrap.plugin.transformer.PinpointClassFileTransformer;
+import com.navercorp.pinpoint.common.trace.ServiceType;
+
+/**
+ * @author Jongho Moon
+ *
+ */
+public class SpringWebMvcPlugin implements ProfilerPlugin {
+    public static final ServiceType SPRING_MVC = ServiceType.of(5051, "SPRING_MVC", "SPRING", NORMAL_SCHEMA);
+
+    @Override
+    public void setup(ProfilerPluginSetupContext context) {
+        context.addClassFileTransformer("org.springframework.web.servlet.FrameworkServlet", new PinpointClassFileTransformer() {
+
+            @Override
+            public byte[] transform(ProfilerPluginInstrumentContext instrumentContext, ClassLoader loader, String className, Class<?> classBeingRedefined, ProtectionDomain protectionDomain, byte[] classfileBuffer) throws InstrumentException {
+                InstrumentClass target = instrumentContext.getInstrumentClass(loader, className, classfileBuffer);
+
+                target.getDeclaredMethod("doGet", "javax.servlet.http.HttpServletRequest", "javax.servlet.http.HttpServletResponse").addInterceptor(BasicMethodInterceptor.class.getName(), SPRING_MVC);
+                target.getDeclaredMethod("doPost", "javax.servlet.http.HttpServletRequest", "javax.servlet.http.HttpServletResponse").addInterceptor(BasicMethodInterceptor.class.getName(), SPRING_MVC);
+
+                return target.toBytecode();
+            }
+        });
+
+    }
+
+}

plugins/spring/src/main/java/com/navercorp/pinpoint/plugin/spring/web/SpringWebMvcTraceMetadataProvider.java

@@ -0,0 +1,34 @@
+/**
+ * Copyright 2014 NAVER Corp.
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ * 
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ * 
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package com.navercorp.pinpoint.plugin.spring.web;
+
+import com.navercorp.pinpoint.common.trace.TraceMetadataProvider;
+import com.navercorp.pinpoint.common.trace.TraceMetadataSetupContext;
+
+/**
+ * @author Jongho Moon
+ *
+ */
+public class SpringWebMvcTraceMetadataProvider implements TraceMetadataProvider {
+
+    /* (non-Javadoc)
+     * @see com.navercorp.pinpoint.common.trace.TraceMetadataProvider#setup(com.navercorp.pinpoint.common.trace.TraceMetadataSetupContext)
+     */
+    @Override
+    public void setup(TraceMetadataSetupContext context) {
+        context.addServiceType(SpringWebMvcPlugin.SPRING_MVC);
+    }
+
+}

plugins/spring-beans/src/main/resources/META-INF/services/com.navercorp.pinpoint.bootstrap.plugin.ProfilerPlugin → plugins/spring/src/main/resources/META-INF/services/com.navercorp.pinpoint.bootstrap.plugin.ProfilerPlugin

@@ -1 +1,2 @@
 com.navercorp.pinpoint.plugin.spring.beans.SpringBeansPlugin
+com.navercorp.pinpoint.plugin.spring.web.SpringWebMvcPlugin

plugins/spring-beans/src/main/resources/META-INF/services/com.navercorp.pinpoint.common.trace.TraceMetadataProvider → plugins/spring/src/main/resources/META-INF/services/com.navercorp.pinpoint.common.trace.TraceMetadataProvider

@@ -1 +1,2 @@
 com.navercorp.pinpoint.plugin.spring.beans.SpringBeansTraceMetadataProvider
+com.navercorp.pinpoint.plugin.spring.web.SpringWebMvcTraceMetadataProvider

profiler/src/main/java/com/navercorp/pinpoint/profiler/ClassFileTransformerDispatcher.java

@@ -181,8 +181,6 @@ private ModifierRegistry createModifierRegistry(List<DefaultProfilerPluginContex
 
         modifierRepository.addMethodModifier();
 
-        modifierRepository.addTomcatModifier();
-
         // rpc
         modifierRepository.addConnectorModifier();
 

profiler/src/main/java/com/navercorp/pinpoint/profiler/modifier/DefaultModifierRegistry.java

@@ -27,12 +27,9 @@
 import com.navercorp.pinpoint.bootstrap.instrument.matcher.Matcher;
 import com.navercorp.pinpoint.bootstrap.instrument.matcher.MultiClassNameMatcher;
 import com.navercorp.pinpoint.profiler.modifier.connector.asynchttpclient.AsyncHttpClientModifier;
-import com.navercorp.pinpoint.profiler.modifier.db.dbcp.DBCPBasicDataSourceModifier;
-import com.navercorp.pinpoint.profiler.modifier.db.dbcp.DBCPPoolGuardConnectionWrapperModifier;
 import com.navercorp.pinpoint.profiler.modifier.log.log4j.LoggingEventOfLog4jModifier;
 import com.navercorp.pinpoint.profiler.modifier.log.logback.LoggingEventOfLogbackModifier;
 import com.navercorp.pinpoint.profiler.modifier.method.MethodModifier;
-import com.navercorp.pinpoint.profiler.modifier.servlet.SpringFrameworkServletModifier;
 import com.navercorp.pinpoint.profiler.util.JavaAssistUtils;
 
 /**
@@ -100,23 +97,6 @@ public void addConnectorModifier() {
         addModifier(new AsyncHttpClientModifier(byteCodeInstrumentor, agent));
     }
 
-    public void addTomcatModifier() {
-        SpringFrameworkServletModifier springServletModifier = new SpringFrameworkServletModifier(byteCodeInstrumentor, agent);
-        addModifier(springServletModifier);
-    }
-
-    private void addDbcpDriver() {
-
-        // TODO Cubrid doesn't have connection impl too. Check it out.
-        AbstractModifier dbcpBasicDataSourceModifier = new DBCPBasicDataSourceModifier(byteCodeInstrumentor, agent);
-        addModifier(dbcpBasicDataSourceModifier);
-
-        if (profilerConfig.isJdbcProfileDbcpConnectionClose()) {
-            AbstractModifier dbcpPoolModifier = new DBCPPoolGuardConnectionWrapperModifier(byteCodeInstrumentor, agent);
-            addModifier(dbcpPoolModifier);
-        }
-    }
-
     public void addLog4jModifier() {
         if (profilerConfig.isLog4jLoggingTransactionInfo()) {
             addModifier(new LoggingEventOfLog4jModifier(byteCodeInstrumentor, agent));