Repository: pinpoint-apm/pinpoint
Issue: Fix that the profiler.jdbc.dbcp option is not working.
Labels: bug

Currently, there is a problem that does not apply when setting profiler.jdbc.dbcp in pinpoint.config. (always:true)
I will fix it. 


Issue comments:

Pull request review comments:

Patch that resolved the issue:
diff --git a/plugins/dbcp/src/main/java/com/navercorp/pinpoint/plugin/commons/dbcp/CommonsDbcpConfig.java b/plugins/dbcp/src/main/java/com/navercorp/pinpoint/plugin/commons/dbcp/CommonsDbcpConfig.java
new file mode 100644
index 000000000000..faec905908d3
--- /dev/null
+++ b/plugins/dbcp/src/main/java/com/navercorp/pinpoint/plugin/commons/dbcp/CommonsDbcpConfig.java
@@ -0,0 +1,45 @@
+/*
+ * Copyright 2016 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.commons.dbcp;
+
+import com.navercorp.pinpoint.bootstrap.config.ProfilerConfig;
+
+/**
+ * @author Taejin Koo
+ */
+public class CommonsDbcpConfig {
+
+    static final String DBCP_PLUGIN_ENABLE = "profiler.jdbc.dbcp";
+    static final String DBCP_PROFILE_CONNECTIONCLOSE_ENABLE = "profiler.jdbc.dbcp.connectionclose";
+
+    private final boolean pluginEnable;
+    private final boolean profileClose;
+
+    public CommonsDbcpConfig(ProfilerConfig config) {
+        pluginEnable = config.readBoolean(DBCP_PLUGIN_ENABLE, false);
+        profileClose = config.readBoolean(DBCP_PROFILE_CONNECTIONCLOSE_ENABLE, false);
+    }
+
+    public boolean isPluginEnable() {
+        return pluginEnable;
+    }
+
+    public boolean isProfileClose() {
+        return profileClose;
+    }
+
+}
diff --git a/plugins/dbcp/src/main/java/com/navercorp/pinpoint/plugin/commons/dbcp/CommonsDbcpPlugin.java b/plugins/dbcp/src/main/java/com/navercorp/pinpoint/plugin/commons/dbcp/CommonsDbcpPlugin.java
index cca71f8db740..34a30f6e38e9 100644
--- a/plugins/dbcp/src/main/java/com/navercorp/pinpoint/plugin/commons/dbcp/CommonsDbcpPlugin.java
+++ b/plugins/dbcp/src/main/java/com/navercorp/pinpoint/plugin/commons/dbcp/CommonsDbcpPlugin.java
@@ -15,35 +15,46 @@
  */
 package com.navercorp.pinpoint.plugin.commons.dbcp;
 
-import java.security.ProtectionDomain;
-
 import com.navercorp.pinpoint.bootstrap.instrument.InstrumentClass;
 import com.navercorp.pinpoint.bootstrap.instrument.InstrumentException;
 import com.navercorp.pinpoint.bootstrap.instrument.Instrumentor;
 import com.navercorp.pinpoint.bootstrap.instrument.transformer.TransformCallback;
 import com.navercorp.pinpoint.bootstrap.instrument.transformer.TransformTemplate;
 import com.navercorp.pinpoint.bootstrap.instrument.transformer.TransformTemplateAware;
+import com.navercorp.pinpoint.bootstrap.logging.PLogger;
+import com.navercorp.pinpoint.bootstrap.logging.PLoggerFactory;
 import com.navercorp.pinpoint.bootstrap.plugin.ProfilerPlugin;
 import com.navercorp.pinpoint.bootstrap.plugin.ProfilerPluginSetupContext;
 import com.navercorp.pinpoint.common.trace.ServiceType;
 import com.navercorp.pinpoint.common.trace.ServiceTypeFactory;
 
+import java.security.ProtectionDomain;
+
 /**
  * @author Jongho Moon
  */
 public class CommonsDbcpPlugin implements ProfilerPlugin, TransformTemplateAware {
+
+    private final PLogger logger = PLoggerFactory.getLogger(this.getClass());
+
+
     public static final ServiceType DBCP_SERVICE_TYPE = ServiceTypeFactory.of(6050, "DBCP");
     public static final String DBCP_SCOPE = "DBCP_SCOPE";
 
+    private  CommonsDbcpConfig config;
+
     private TransformTemplate transformTemplate;
 
     @Override
     public void setup(ProfilerPluginSetupContext context) {
+        config = new CommonsDbcpConfig(context.getConfig());
+        if (!config.isPluginEnable()) {
+            logger.info("Disable commons dbcp option. 'profiler.jdbc.dbcp=false'");
+            return;
+        }
+
         addBasicDataSourceTransformer();
-        
-        boolean profileClose = context.getConfig().readBoolean("profiler.jdbc.dbcp.connectionclose", false);
-        
-        if (profileClose) {
+        if (config.isProfileClose()) {
             addPoolGuardConnectionWrapperTransformer();
         }
     }


Test patch:
diff --git a/plugins/dbcp/src/test/java/com/navercorp/pinpoint/plugin/commons/dbcp/CommonsDbcpConfigTest.java b/plugins/dbcp/src/test/java/com/navercorp/pinpoint/plugin/commons/dbcp/CommonsDbcpConfigTest.java
new file mode 100644
index 000000000000..fb24d26bef44
--- /dev/null
+++ b/plugins/dbcp/src/test/java/com/navercorp/pinpoint/plugin/commons/dbcp/CommonsDbcpConfigTest.java
@@ -0,0 +1,73 @@
+/*
+ * Copyright 2016 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.commons.dbcp;
+
+import com.navercorp.pinpoint.bootstrap.config.DefaultProfilerConfig;
+import com.navercorp.pinpoint.bootstrap.config.ProfilerConfig;
+import org.junit.Assert;
+import org.junit.Test;
+
+import java.util.Properties;
+
+/**
+ * @author Taejin Koo
+ */
+public class CommonsDbcpConfigTest {
+
+    @Test
+    public void configTest1() throws Exception {
+        CommonsDbcpConfig commonsDbcpConfig = createCommonsDbcpConfig("false", "false");
+
+        Assert.assertFalse(commonsDbcpConfig.isPluginEnable());
+        Assert.assertFalse(commonsDbcpConfig.isProfileClose());
+    }
+
+    @Test
+    public void configTest2() throws Exception {
+        CommonsDbcpConfig commonsDbcpConfig = createCommonsDbcpConfig("false", "true");
+
+        Assert.assertFalse(commonsDbcpConfig.isPluginEnable());
+        Assert.assertTrue(commonsDbcpConfig.isProfileClose());
+    }
+
+    @Test
+    public void configTest3() throws Exception {
+        CommonsDbcpConfig commonsDbcpConfig = createCommonsDbcpConfig("true", "false");
+
+        Assert.assertTrue(commonsDbcpConfig.isPluginEnable());
+        Assert.assertFalse(commonsDbcpConfig.isProfileClose());
+    }
+
+    @Test
+    public void configTest4() throws Exception {
+        CommonsDbcpConfig commonsDbcpConfig = createCommonsDbcpConfig("true", "true");
+
+        Assert.assertTrue(commonsDbcpConfig.isPluginEnable());
+        Assert.assertTrue(commonsDbcpConfig.isProfileClose());
+    }
+
+    private CommonsDbcpConfig createCommonsDbcpConfig(String pluginEnable, String profileConnectionCloseEnable) {
+        Properties properties = new Properties();
+        properties.put(CommonsDbcpConfig.DBCP_PLUGIN_ENABLE, pluginEnable);
+        properties.put(CommonsDbcpConfig.DBCP_PROFILE_CONNECTIONCLOSE_ENABLE, profileConnectionCloseEnable);
+
+        ProfilerConfig profilerConfig = new DefaultProfilerConfig(properties);
+
+        return new CommonsDbcpConfig(profilerConfig);
+    }
+
+}