You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@skywalking.apache.org by GitBox <gi...@apache.org> on 2021/09/18 16:38:28 UTC

[GitHub] [skywalking-java] VictorZeng opened a new pull request #27: Add Alibaba Druid connection pool plugin.

VictorZeng opened a new pull request #27:
URL: https://github.com/apache/skywalking-java/pull/27


   <!--
       ⚠️ Please make sure to read this template first, pull requests that don't accord with this template
       maybe closed without notice.
       Texts surrounded by `<` and `>` are meant to be replaced by you, e.g. <framework name>, <issue number>.
       Put an `x` in the `[ ]` to mark the item as CHECKED. `[x]`
   -->
   
   <!-- ==== πŸ› Remove this line WHEN AND ONLY WHEN you're fixing a bug, follow the checklist πŸ‘‡ ====
   ### Fix <bug description or the bug issue number or bug issue link>
   - [ ] Add a unit test to verify that the fix works.
   - [ ] Explain briefly why the bug exists and how to fix it.
        ==== πŸ› Remove this line WHEN AND ONLY WHEN you're fixing a bug, follow the checklist πŸ‘† ==== -->
   
   <!-- ==== πŸ”Œ Remove this line WHEN AND ONLY WHEN you're adding a new plugin, follow the checklist πŸ‘‡ ====
        ==== πŸ”Œ Remove this line WHEN AND ONLY WHEN you're adding a new plugin, follow the checklist πŸ‘† ==== -->
   
   ### Add an agent plugin to support <framework name>
   - [x] Add a test case for the new plugin, refer to [the doc](https://github.com/apache/skywalking-java/blob/main/docs/en/setup/service-agent/java-agent/Plugin-test.md)
   - [x] Add a component id in [the component-libraries.yml](https://github.com/apache/skywalking/blob/master/oap-server/server-starter/src/main/resources/component-libraries.yml)
   - [ ] Add a logo in [the UI repo](https://github.com/apache/skywalking-rocketbot-ui/tree/master/src/views/components/topology/assets)
   
   <!-- ==== πŸ“ˆ Remove this line WHEN AND ONLY WHEN you're improving the performance, follow the checklist πŸ‘‡ ====
   ### Improve the performance of <class or module or ...>
   - [ ] Add a benchmark for the improvement, refer to [the existing ones](https://github.com/apache/skywalking-java/blob/main/apm-commons/apm-datacarrier/src/test/java/org/apache/skywalking/apm/commons/datacarrier/LinkedArrayBenchmark.java)
   - [ ] The benchmark result.
   ```text
   <Paste the benchmark results here>
   ```
   - [ ] Links/URLs to the theory proof or discussion articles/blogs. <links/URLs here>
        ==== πŸ“ˆ Remove this line WHEN AND ONLY WHEN you're improving the performance, follow the checklist πŸ‘† ==== -->
   
   <!-- ==== πŸ†• Remove this line WHEN AND ONLY WHEN you're adding a new feature, follow the checklist πŸ‘‡ ====
   ### <Feature description>
   - [ ] If this is non-trivial feature, paste the links/URLs to the design doc.
   - [ ] Update the documentation to include this new feature.
   - [ ] Tests(including UT, IT, E2E) are added to verify the new feature.
   - [ ] If it's UI related, attach the screenshots below.
        ==== πŸ†• Remove this line WHEN AND ONLY WHEN you're adding a new feature, follow the checklist πŸ‘† ==== -->
   
   
   - [x] If this pull request closes/resolves/fixes an existing issue, replace the issue number. Closes #<https://github.com/apache/skywalking/issues/7735>.
   - [x] Update the [`CHANGES` log](https://github.com/apache/skywalking-java/blob/main/CHANGES.md).
   
   - [x] Whether to compile and test locally, and provide screenshots.
   ![image](https://user-images.githubusercontent.com/9109915/133895946-d0be79d3-8664-4116-8cfb-0210096e0314.png)
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [skywalking-java] wu-sheng commented on pull request #27: Add Alibaba Druid connection pool plugin.

Posted by GitBox <gi...@apache.org>.
wu-sheng commented on pull request #27:
URL: https://github.com/apache/skywalking-java/pull/27#issuecomment-922412651


   Where is the dead link from? I remember we don't link to Alicloud usually.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [skywalking-java] VictorZeng commented on pull request #27: Add Alibaba Druid connection pool plugin.

Posted by GitBox <gi...@apache.org>.
VictorZeng commented on pull request #27:
URL: https://github.com/apache/skywalking-java/pull/27#issuecomment-922405820


   The DeadLink check failed, But I don't know why...
   
   ```
   FILE: ./docs/en/setup/service-agent/java-agent/Supported-list.md
   [βœ–] https://help.aliyun.com/document_detail/114448.html
   
   
   ERROR: 1 dead links found!
   85 links checked.
   [βœ–] https://help.aliyun.com/document_detail/114448.html β†’ Status: 0
   Error: Process completed with exit code 1.
   ```


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [skywalking-java] VictorZeng commented on a change in pull request #27: Add Alibaba Druid connection pool plugin.

Posted by GitBox <gi...@apache.org>.
VictorZeng commented on a change in pull request #27:
URL: https://github.com/apache/skywalking-java/pull/27#discussion_r711735453



##########
File path: test/plugin/scenarios/druid-1.x-scenario/config/expectedData.yaml
##########
@@ -0,0 +1,313 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you 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.
+segmentItems:
+- serviceName: druid-1.x-scenario
+  segmentSize: ge 0
+  segments:
+  - segmentId: not null
+    spans:
+    - operationName: Druid/Connection/getConnection
+      operationId: 0
+      parentSpanId: 1
+      spanId: 2
+      spanLayer: Unknown
+      startTime: gt 0
+      endTime: gt 0
+      componentId: 115
+      isError: false
+      spanType: Local
+      peer: ''
+      skipAnalysis: false
+    - operationName: Druid/Connection/getConnection
+      operationId: 0
+      parentSpanId: 0
+      spanId: 1
+      spanLayer: Unknown
+      startTime: gt 0
+      endTime: gt 0
+      componentId: 115
+      isError: false
+      spanType: Local
+      peer: ''
+      skipAnalysis: false
+    - operationName: Mysql/JDBI/Statement/execute
+      operationId: 0
+      parentSpanId: 0
+      spanId: 3
+      spanLayer: Database
+      startTime: gt 0
+      endTime: gt 0
+      componentId: 33
+      isError: false
+      spanType: Exit
+      peer: mysql-server:3306
+      skipAnalysis: false
+      tags:
+      - {key: db.type, value: sql}
+      - {key: db.instance, value: test}
+      - {key: db.statement, value: 'CREATE TABLE test_DRUID(id VARCHAR(1) PRIMARY
+          KEY, value VARCHAR(1) NOT NULL)'}
+    - operationName: Druid/Connection/close
+      operationId: 0
+      parentSpanId: 0
+      spanId: 4
+      spanLayer: Unknown
+      startTime: gt 0
+      endTime: gt 0
+      componentId: 115
+      isError: false
+      spanType: Local
+      peer: ''
+      skipAnalysis: false
+    - operationName: Druid/Connection/getConnection
+      operationId: 0
+      parentSpanId: 5
+      spanId: 6
+      spanLayer: Unknown
+      startTime: gt 0
+      endTime: gt 0
+      componentId: 115
+      isError: false
+      spanType: Local
+      peer: ''
+      skipAnalysis: false
+    - operationName: Druid/Connection/getConnection
+      operationId: 0
+      parentSpanId: 0
+      spanId: 5
+      spanLayer: Unknown
+      startTime: gt 0
+      endTime: gt 0
+      componentId: 115
+      isError: false
+      spanType: Local
+      peer: ''
+      skipAnalysis: false
+    - operationName: Mysql/JDBI/Statement/execute
+      operationId: 0
+      parentSpanId: 0
+      spanId: 7
+      spanLayer: Database
+      startTime: gt 0
+      endTime: gt 0
+      componentId: 33
+      isError: false
+      spanType: Exit
+      peer: mysql-server:3306
+      skipAnalysis: false
+      tags:
+      - {key: db.type, value: sql}
+      - {key: db.instance, value: test}
+      - {key: db.statement, value: 'INSERT INTO test_DRUID(id, value) VALUES(1,1)'}
+    - operationName: Druid/Connection/close
+      operationId: 0
+      parentSpanId: 0
+      spanId: 8
+      spanLayer: Unknown
+      startTime: gt 0
+      endTime: gt 0
+      componentId: 115
+      isError: false
+      spanType: Local
+      peer: ''
+      skipAnalysis: false
+    - operationName: Druid/Connection/getConnection
+      operationId: 0

Review comment:
       I will remove `operationId` and .




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [skywalking-java] wu-sheng commented on a change in pull request #27: Add Alibaba Druid connection pool plugin.

Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #27:
URL: https://github.com/apache/skywalking-java/pull/27#discussion_r711735928



##########
File path: apm-protocol/apm-network/src/main/java/org/apache/skywalking/apm/network/trace/component/ComponentsDefine.java
##########
@@ -207,4 +207,6 @@
   
     public static final OfficialComponent GUAVA_CACHE = new OfficialComponent(114, "GuavaCache");
 
+    public static final OfficialComponent DRUID = new OfficialComponent(115, "druid");

Review comment:
       `druid` should be renamed as `alibaba druid`. As I checked, Druid is an Apache software foundation trademark, see https://druid.apache.org/.
   
   I treat this as potential trademark conflict, we at least should keep it clear.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [skywalking-java] VictorZeng commented on pull request #27: Add Alibaba Druid connection pool plugin.

Posted by GitBox <gi...@apache.org>.
VictorZeng commented on pull request #27:
URL: https://github.com/apache/skywalking-java/pull/27#issuecomment-922416412


   > One question, according to all spans are local, without logic span, extra statistics are made, I feel this is better to be an optional plugin, considering resource cost. All Spring local annocation spans are optional too.
   
   I see that DBCP is not an optional plugin, so is Druid an official plugin?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [skywalking-java] VictorZeng commented on a change in pull request #27: Add Alibaba Druid connection pool plugin.

Posted by GitBox <gi...@apache.org>.
VictorZeng commented on a change in pull request #27:
URL: https://github.com/apache/skywalking-java/pull/27#discussion_r711746734



##########
File path: test/plugin/scenarios/druid-1.x-scenario/src/main/java/org/apache/skywalking/apm/testcase/druid/service/CaseService.java
##########
@@ -0,0 +1,69 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You 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 org.apache.skywalking.apm.testcase.druid.service;
+
+import com.alibaba.druid.pool.DruidDataSource;
+import org.apache.skywalking.apm.testcase.druid.MySQLConfig;
+import org.springframework.stereotype.Service;
+
+import java.sql.Connection;
+import java.sql.SQLException;
+import java.sql.Statement;
+
+@Service
+public class CaseService {
+
+    public static DruidDataSource DS;
+    private static final String CREATE_TABLE_SQL = "CREATE TABLE test_DRUID(id VARCHAR(1) PRIMARY KEY, value VARCHAR(1) NOT NULL)";
+    private static final String INSERT_DATA_SQL = "INSERT INTO test_DRUID(id, value) VALUES(1,1)";
+    private static final String QUERY_DATA_SQL = "SELECT id, value FROM test_DRUID WHERE id=1";
+    private static final String DELETE_DATA_SQL = "DELETE FROM test_DRUID WHERE id=1";
+    private static final String DROP_TABLE_SQL = "DROP table test_DRUID";
+
+    static {
+        DS = new DruidDataSource();
+        try {
+            DS.setUrl(MySQLConfig.getUrl());
+            DS.setUsername(MySQLConfig.getUserName());
+            DS.setPassword(MySQLConfig.getPassword());
+            DS.init();
+        } catch (Exception e) {
+            e.printStackTrace();
+        }
+    }
+
+    public void testCase() {
+        sqlExecutor(CREATE_TABLE_SQL);
+        sqlExecutor(INSERT_DATA_SQL);
+        sqlExecutor(QUERY_DATA_SQL);
+        sqlExecutor(DELETE_DATA_SQL);
+        sqlExecutor(DROP_TABLE_SQL);
+        DS.close();

Review comment:
       I noticed this, thanks.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [skywalking-java] wu-sheng commented on a change in pull request #27: Add Alibaba Druid connection pool plugin.

Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #27:
URL: https://github.com/apache/skywalking-java/pull/27#discussion_r711746532



##########
File path: test/plugin/scenarios/druid-1.x-scenario/src/main/java/org/apache/skywalking/apm/testcase/druid/service/CaseService.java
##########
@@ -0,0 +1,69 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You 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 org.apache.skywalking.apm.testcase.druid.service;
+
+import com.alibaba.druid.pool.DruidDataSource;
+import org.apache.skywalking.apm.testcase.druid.MySQLConfig;
+import org.springframework.stereotype.Service;
+
+import java.sql.Connection;
+import java.sql.SQLException;
+import java.sql.Statement;
+
+@Service
+public class CaseService {
+
+    public static DruidDataSource DS;
+    private static final String CREATE_TABLE_SQL = "CREATE TABLE test_DRUID(id VARCHAR(1) PRIMARY KEY, value VARCHAR(1) NOT NULL)";
+    private static final String INSERT_DATA_SQL = "INSERT INTO test_DRUID(id, value) VALUES(1,1)";
+    private static final String QUERY_DATA_SQL = "SELECT id, value FROM test_DRUID WHERE id=1";
+    private static final String DELETE_DATA_SQL = "DELETE FROM test_DRUID WHERE id=1";
+    private static final String DROP_TABLE_SQL = "DROP table test_DRUID";
+
+    static {
+        DS = new DruidDataSource();
+        try {
+            DS.setUrl(MySQLConfig.getUrl());
+            DS.setUsername(MySQLConfig.getUserName());
+            DS.setPassword(MySQLConfig.getPassword());
+            DS.init();
+        } catch (Exception e) {
+            e.printStackTrace();
+        }
+    }
+
+    public void testCase() {
+        sqlExecutor(CREATE_TABLE_SQL);
+        sqlExecutor(INSERT_DATA_SQL);
+        sqlExecutor(QUERY_DATA_SQL);
+        sqlExecutor(DELETE_DATA_SQL);
+        sqlExecutor(DROP_TABLE_SQL);
+        DS.close();

Review comment:
       I think as the application is going to be killed eventually, we should not close this manually?




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [skywalking-java] wu-sheng commented on pull request #27: Add Alibaba Druid connection pool plugin.

Posted by GitBox <gi...@apache.org>.
wu-sheng commented on pull request #27:
URL: https://github.com/apache/skywalking-java/pull/27#issuecomment-922390670


   >   Error:  /home/runner/work/skywalking-java/skywalking-java/test/plugin/scenarios/druid-1.x-scenario/src/main/java/org/apache/skywalking/apm/testcase/druid/service/CaseService.java:32:35: Name 'dataSource' must match pattern '(^[A-Z][A-Z0-9]*(_[A-Z0-9]+)*$)'. [StaticVariableName]
     Audit done.
     [INFO] There is 1 error reported by Checkstyle 8.38 with /home/runner/work/skywalking-java/skywalking-java/apm-checkstyle/checkStyle.xml ruleset.
     Error:  scenarios/druid-1.x-scenario/src/main/java/org/apache/skywalking/apm/testcase/druid/service/CaseService.java:[32,35] (naming) StaticVariableName: Name 'dataSource' must match pattern '(^[A-Z][A-Z0-9]*(_[A-Z0-9]+)*$)'.
   
   You have compiling issue.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [skywalking-java] VictorZeng commented on a change in pull request #27: Add Alibaba Druid connection pool plugin.

Posted by GitBox <gi...@apache.org>.
VictorZeng commented on a change in pull request #27:
URL: https://github.com/apache/skywalking-java/pull/27#discussion_r711737186



##########
File path: test/plugin/scenarios/druid-1.x-scenario/config/expectedData.yaml
##########
@@ -0,0 +1,313 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you 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.
+segmentItems:
+- serviceName: druid-1.x-scenario
+  segmentSize: ge 0
+  segments:
+  - segmentId: not null
+    spans:
+    - operationName: Druid/Connection/getConnection

Review comment:
       Because this method `getConnection` is overloaded in the class `DruidDataSource`, I will fix the issue.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [skywalking-java] VictorZeng commented on pull request #27: Add Alibaba Druid connection pool plugin.

Posted by GitBox <gi...@apache.org>.
VictorZeng commented on pull request #27:
URL: https://github.com/apache/skywalking-java/pull/27#issuecomment-922396839


   > > Error:  /home/runner/work/skywalking-java/skywalking-java/test/plugin/scenarios/druid-1.x-scenario/src/main/java/org/apache/skywalking/apm/testcase/druid/service/CaseService.java:32:35: Name 'dataSource' must match pattern '(^[A-Z][A-Z0-9]_(_[A-Z0-9]+)_$)'. [StaticVariableName]
   > > Audit done.
   > > [INFO] There is 1 error reported by Checkstyle 8.38 with /home/runner/work/skywalking-java/skywalking-java/apm-checkstyle/checkStyle.xml ruleset.
   > > Error:  scenarios/druid-1.x-scenario/src/main/java/org/apache/skywalking/apm/testcase/druid/service/CaseService.java:[32,35] (naming) StaticVariableName: Name 'dataSource' must match pattern '(^[A-Z][A-Z0-9]_(_[A-Z0-9]+)_$)'.
   > 
   > You have compiling issue.
   
   Thank U, I will fix it.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [skywalking-java] VictorZeng commented on pull request #27: Add Alibaba Druid connection pool plugin.

Posted by GitBox <gi...@apache.org>.
VictorZeng commented on pull request #27:
URL: https://github.com/apache/skywalking-java/pull/27#issuecomment-922416956


   > Where is the dead link from? I remember we don't link to Alicloud usually.
   
   in [Supported-list.md](https://github.com/apache/skywalking-java/blob/main/docs/en/setup/service-agent/java-agent/Supported-list.md)
   
     * [Aliyun ONS](https://help.aliyun.com/document_detail/114448.html) 1.x (OptionalΒΉ)
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [skywalking-java] wu-sheng commented on a change in pull request #27: Add Alibaba Druid connection pool plugin.

Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #27:
URL: https://github.com/apache/skywalking-java/pull/27#discussion_r711729052



##########
File path: test/plugin/scenarios/druid-1.x-scenario/config/expectedData.yaml
##########
@@ -0,0 +1,313 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you 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.
+segmentItems:
+- serviceName: druid-1.x-scenario
+  segmentSize: ge 0
+  segments:
+  - segmentId: not null
+    spans:
+    - operationName: Druid/Connection/getConnection
+      operationId: 0
+      parentSpanId: 1
+      spanId: 2
+      spanLayer: Unknown
+      startTime: gt 0
+      endTime: gt 0
+      componentId: 115
+      isError: false
+      spanType: Local
+      peer: ''
+      skipAnalysis: false
+    - operationName: Druid/Connection/getConnection
+      operationId: 0
+      parentSpanId: 0
+      spanId: 1
+      spanLayer: Unknown
+      startTime: gt 0
+      endTime: gt 0
+      componentId: 115
+      isError: false
+      spanType: Local
+      peer: ''
+      skipAnalysis: false
+    - operationName: Mysql/JDBI/Statement/execute
+      operationId: 0
+      parentSpanId: 0
+      spanId: 3
+      spanLayer: Database
+      startTime: gt 0
+      endTime: gt 0
+      componentId: 33
+      isError: false
+      spanType: Exit
+      peer: mysql-server:3306
+      skipAnalysis: false
+      tags:
+      - {key: db.type, value: sql}
+      - {key: db.instance, value: test}
+      - {key: db.statement, value: 'CREATE TABLE test_DRUID(id VARCHAR(1) PRIMARY
+          KEY, value VARCHAR(1) NOT NULL)'}
+    - operationName: Druid/Connection/close
+      operationId: 0
+      parentSpanId: 0
+      spanId: 4
+      spanLayer: Unknown
+      startTime: gt 0
+      endTime: gt 0
+      componentId: 115
+      isError: false
+      spanType: Local
+      peer: ''
+      skipAnalysis: false
+    - operationName: Druid/Connection/getConnection
+      operationId: 0
+      parentSpanId: 5
+      spanId: 6
+      spanLayer: Unknown
+      startTime: gt 0
+      endTime: gt 0
+      componentId: 115
+      isError: false
+      spanType: Local
+      peer: ''
+      skipAnalysis: false
+    - operationName: Druid/Connection/getConnection
+      operationId: 0
+      parentSpanId: 0
+      spanId: 5
+      spanLayer: Unknown
+      startTime: gt 0
+      endTime: gt 0
+      componentId: 115
+      isError: false
+      spanType: Local
+      peer: ''
+      skipAnalysis: false
+    - operationName: Mysql/JDBI/Statement/execute
+      operationId: 0
+      parentSpanId: 0
+      spanId: 7
+      spanLayer: Database
+      startTime: gt 0
+      endTime: gt 0
+      componentId: 33
+      isError: false
+      spanType: Exit
+      peer: mysql-server:3306
+      skipAnalysis: false
+      tags:
+      - {key: db.type, value: sql}
+      - {key: db.instance, value: test}
+      - {key: db.statement, value: 'INSERT INTO test_DRUID(id, value) VALUES(1,1)'}
+    - operationName: Druid/Connection/close
+      operationId: 0
+      parentSpanId: 0
+      spanId: 8
+      spanLayer: Unknown
+      startTime: gt 0
+      endTime: gt 0
+      componentId: 115
+      isError: false
+      spanType: Local
+      peer: ''
+      skipAnalysis: false
+    - operationName: Druid/Connection/getConnection
+      operationId: 0

Review comment:
       We don't have operationId for now.

##########
File path: test/plugin/scenarios/druid-1.x-scenario/config/expectedData.yaml
##########
@@ -0,0 +1,313 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you 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.
+segmentItems:
+- serviceName: druid-1.x-scenario
+  segmentSize: ge 0
+  segments:
+  - segmentId: not null
+    spans:
+    - operationName: Druid/Connection/getConnection

Review comment:
       Could you explain why there are so many `Druid/Connection/getConnection`s?




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [skywalking-java] VictorZeng commented on a change in pull request #27: Add Alibaba Druid connection pool plugin.

Posted by GitBox <gi...@apache.org>.
VictorZeng commented on a change in pull request #27:
URL: https://github.com/apache/skywalking-java/pull/27#discussion_r711746734



##########
File path: test/plugin/scenarios/druid-1.x-scenario/src/main/java/org/apache/skywalking/apm/testcase/druid/service/CaseService.java
##########
@@ -0,0 +1,69 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You 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 org.apache.skywalking.apm.testcase.druid.service;
+
+import com.alibaba.druid.pool.DruidDataSource;
+import org.apache.skywalking.apm.testcase.druid.MySQLConfig;
+import org.springframework.stereotype.Service;
+
+import java.sql.Connection;
+import java.sql.SQLException;
+import java.sql.Statement;
+
+@Service
+public class CaseService {
+
+    public static DruidDataSource DS;
+    private static final String CREATE_TABLE_SQL = "CREATE TABLE test_DRUID(id VARCHAR(1) PRIMARY KEY, value VARCHAR(1) NOT NULL)";
+    private static final String INSERT_DATA_SQL = "INSERT INTO test_DRUID(id, value) VALUES(1,1)";
+    private static final String QUERY_DATA_SQL = "SELECT id, value FROM test_DRUID WHERE id=1";
+    private static final String DELETE_DATA_SQL = "DELETE FROM test_DRUID WHERE id=1";
+    private static final String DROP_TABLE_SQL = "DROP table test_DRUID";
+
+    static {
+        DS = new DruidDataSource();
+        try {
+            DS.setUrl(MySQLConfig.getUrl());
+            DS.setUsername(MySQLConfig.getUserName());
+            DS.setPassword(MySQLConfig.getPassword());
+            DS.init();
+        } catch (Exception e) {
+            e.printStackTrace();
+        }
+    }
+
+    public void testCase() {
+        sqlExecutor(CREATE_TABLE_SQL);
+        sqlExecutor(INSERT_DATA_SQL);
+        sqlExecutor(QUERY_DATA_SQL);
+        sqlExecutor(DELETE_DATA_SQL);
+        sqlExecutor(DROP_TABLE_SQL);
+        DS.close();

Review comment:
       I noticed this, thanks




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [skywalking-java] wu-sheng merged pull request #27: Add Alibaba Druid connection pool plugin.

Posted by GitBox <gi...@apache.org>.
wu-sheng merged pull request #27:
URL: https://github.com/apache/skywalking-java/pull/27


   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [skywalking-java] VictorZeng commented on pull request #27: Add Alibaba Druid connection pool plugin.

Posted by GitBox <gi...@apache.org>.
VictorZeng commented on pull request #27:
URL: https://github.com/apache/skywalking-java/pull/27#issuecomment-922337514


   - [x] Whether to compile and test locally, and provide screenshots.
   ![image](https://user-images.githubusercontent.com/9109915/133895946-d0be79d3-8664-4116-8cfb-0210096e0314.png)


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [skywalking-java] wu-sheng commented on a change in pull request #27: Add Alibaba Druid connection pool plugin.

Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #27:
URL: https://github.com/apache/skywalking-java/pull/27#discussion_r711735928



##########
File path: apm-protocol/apm-network/src/main/java/org/apache/skywalking/apm/network/trace/component/ComponentsDefine.java
##########
@@ -207,4 +207,6 @@
   
     public static final OfficialComponent GUAVA_CACHE = new OfficialComponent(114, "GuavaCache");
 
+    public static final OfficialComponent DRUID = new OfficialComponent(115, "druid");

Review comment:
       `druid` should be renamed as `AlibabaDruid`. As I checked, Druid is an Apache software foundation trademark, see https://druid.apache.org/.
   
   I treat this as potential trademark conflict, we at least should keep it clear.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [skywalking-java] kezhenxu94 commented on a change in pull request #27: Add Alibaba Druid connection pool plugin.

Posted by GitBox <gi...@apache.org>.
kezhenxu94 commented on a change in pull request #27:
URL: https://github.com/apache/skywalking-java/pull/27#discussion_r711738096



##########
File path: apm-sniffer/apm-sdk-plugin/druid-1.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/druid/v1/define/DruidDataSourceInstrumentation.java
##########
@@ -0,0 +1,96 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You 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 org.apache.skywalking.apm.plugin.druid.v1.define;
+
+import net.bytebuddy.description.method.MethodDescription;
+import net.bytebuddy.matcher.ElementMatcher;
+import org.apache.skywalking.apm.agent.core.plugin.interceptor.ConstructorInterceptPoint;
+import org.apache.skywalking.apm.agent.core.plugin.interceptor.InstanceMethodsInterceptPoint;
+import org.apache.skywalking.apm.agent.core.plugin.interceptor.StaticMethodsInterceptPoint;
+import org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance.ClassInstanceMethodsEnhancePluginDefine;
+import org.apache.skywalking.apm.agent.core.plugin.match.ClassMatch;
+
+import static net.bytebuddy.matcher.ElementMatchers.named;
+import static net.bytebuddy.matcher.ElementMatchers.takesArguments;
+import static org.apache.skywalking.apm.agent.core.plugin.match.NameMatch.byName;
+
+/**
+ * Druid is a database connection pool from Alibaba inc.
+ * <p>
+ * DruidDataSource provides a "one stop shopping" solution for database connection pool solution

Review comment:
       ```suggestion
    * DruidDataSource provides a "one stop" solution for database connection pool solution
   ```




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [skywalking-java] VictorZeng commented on a change in pull request #27: Add Alibaba Druid connection pool plugin.

Posted by GitBox <gi...@apache.org>.
VictorZeng commented on a change in pull request #27:
URL: https://github.com/apache/skywalking-java/pull/27#discussion_r711737639



##########
File path: apm-protocol/apm-network/src/main/java/org/apache/skywalking/apm/network/trace/component/ComponentsDefine.java
##########
@@ -207,4 +207,6 @@
   
     public static final OfficialComponent GUAVA_CACHE = new OfficialComponent(114, "GuavaCache");
 
+    public static final OfficialComponent DRUID = new OfficialComponent(115, "druid");

Review comment:
       I think what you said makes sense, and I will make changes.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [skywalking-java] wu-sheng commented on a change in pull request #27: Add Alibaba Druid connection pool plugin.

Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #27:
URL: https://github.com/apache/skywalking-java/pull/27#discussion_r711729629



##########
File path: test/plugin/scenarios/druid-1.x-scenario/config/expectedData.yaml
##########
@@ -0,0 +1,313 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you 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.
+segmentItems:
+- serviceName: druid-1.x-scenario
+  segmentSize: ge 0
+  segments:
+  - segmentId: not null
+    spans:
+    - operationName: Druid/Connection/getConnection

Review comment:
       Could you share what is the case of this? This seems very strange to have many local spans, which also impact performance very seriously.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [skywalking-java] VictorZeng commented on a change in pull request #27: Add Alibaba Druid connection pool plugin.

Posted by GitBox <gi...@apache.org>.
VictorZeng commented on a change in pull request #27:
URL: https://github.com/apache/skywalking-java/pull/27#discussion_r711684718



##########
File path: test/plugin/scenarios/druid-1.x-scenario/config/expectedData.yaml
##########
@@ -0,0 +1,178 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you 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.
+segmentItems:
+- serviceName: druid-1.x-scenario
+  segmentSize: ge 0
+  segments:
+  - segmentId: not null
+    spans:
+    - {operationName: Druid/Connection/getConnection, operationId: 0, parentSpanId: 1,
+      spanId: 2, spanLayer: Unknown, startTime: gt 0, endTime: gt 0,
+      componentId: 115, isError: false, spanType: Local, peer: '', skipAnalysis: false}
+    - {operationName: Druid/Connection/getConnection, operationId: 0, parentSpanId: 0,
+      spanId: 1, spanLayer: Unknown, startTime: gt 0, endTime: gt 0,
+      componentId: 115, isError: false, spanType: Local, peer: '', skipAnalysis: false}

Review comment:
       OK, I will format the expectedData.yaml.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [skywalking-java] VictorZeng commented on a change in pull request #27: Add Alibaba Druid connection pool plugin.

Posted by GitBox <gi...@apache.org>.
VictorZeng commented on a change in pull request #27:
URL: https://github.com/apache/skywalking-java/pull/27#discussion_r711684905



##########
File path: test/plugin/scenarios/druid-1.x-scenario/support-version.list
##########
@@ -0,0 +1,22 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you 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.
+
+1.2.6

Review comment:
       OK, I will  keep one version.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [skywalking-java] wu-sheng commented on pull request #27: Add Alibaba Druid connection pool plugin.

Posted by GitBox <gi...@apache.org>.
wu-sheng commented on pull request #27:
URL: https://github.com/apache/skywalking-java/pull/27#issuecomment-922418459


   > > One question, according to all spans are local, without logic span, extra statistics are made, I feel this is better to be an optional plugin, considering resource cost. All Spring local annocation spans are optional too.
   > 
   > 
   > 
   > I see that DBCP is not an optional plugin, so is Druid an official plugin?
   
   OK, then it is fine to be in default for now.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [skywalking-java] wu-sheng commented on a change in pull request #27: Add Alibaba Druid connection pool plugin.

Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #27:
URL: https://github.com/apache/skywalking-java/pull/27#discussion_r711679233



##########
File path: test/plugin/scenarios/druid-1.x-scenario/config/expectedData.yaml
##########
@@ -0,0 +1,178 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you 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.
+segmentItems:
+- serviceName: druid-1.x-scenario
+  segmentSize: ge 0
+  segments:
+  - segmentId: not null
+    spans:
+    - {operationName: Druid/Connection/getConnection, operationId: 0, parentSpanId: 1,
+      spanId: 2, spanLayer: Unknown, startTime: gt 0, endTime: gt 0,
+      componentId: 115, isError: false, spanType: Local, peer: '', skipAnalysis: false}
+    - {operationName: Druid/Connection/getConnection, operationId: 0, parentSpanId: 0,
+      spanId: 1, spanLayer: Unknown, startTime: gt 0, endTime: gt 0,
+      componentId: 115, isError: false, spanType: Local, peer: '', skipAnalysis: false}

Review comment:
       Please format YAML. Don't make mutiple components sharing one line.

##########
File path: test/plugin/scenarios/druid-1.x-scenario/support-version.list
##########
@@ -0,0 +1,22 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you 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.
+
+1.2.6

Review comment:
       Please only keep one version for very mini version. We can't afford to test all fix versions.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [skywalking-java] wu-sheng commented on a change in pull request #27: Add Alibaba Druid connection pool plugin.

Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #27:
URL: https://github.com/apache/skywalking-java/pull/27#discussion_r711746430



##########
File path: test/plugin/scenarios/druid-1.x-scenario/src/main/java/org/apache/skywalking/apm/testcase/druid/service/CaseService.java
##########
@@ -0,0 +1,69 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You 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 org.apache.skywalking.apm.testcase.druid.service;
+
+import com.alibaba.druid.pool.DruidDataSource;
+import org.apache.skywalking.apm.testcase.druid.MySQLConfig;
+import org.springframework.stereotype.Service;
+
+import java.sql.Connection;
+import java.sql.SQLException;
+import java.sql.Statement;
+
+@Service
+public class CaseService {
+
+    public static DruidDataSource DS;
+    private static final String CREATE_TABLE_SQL = "CREATE TABLE test_DRUID(id VARCHAR(1) PRIMARY KEY, value VARCHAR(1) NOT NULL)";
+    private static final String INSERT_DATA_SQL = "INSERT INTO test_DRUID(id, value) VALUES(1,1)";
+    private static final String QUERY_DATA_SQL = "SELECT id, value FROM test_DRUID WHERE id=1";
+    private static final String DELETE_DATA_SQL = "DELETE FROM test_DRUID WHERE id=1";
+    private static final String DROP_TABLE_SQL = "DROP table test_DRUID";
+
+    static {
+        DS = new DruidDataSource();
+        try {
+            DS.setUrl(MySQLConfig.getUrl());
+            DS.setUsername(MySQLConfig.getUserName());
+            DS.setPassword(MySQLConfig.getPassword());
+            DS.init();
+        } catch (Exception e) {
+            e.printStackTrace();
+        }
+    }
+
+    public void testCase() {
+        sqlExecutor(CREATE_TABLE_SQL);
+        sqlExecutor(INSERT_DATA_SQL);
+        sqlExecutor(QUERY_DATA_SQL);
+        sqlExecutor(DELETE_DATA_SQL);
+        sqlExecutor(DROP_TABLE_SQL);
+        DS.close();

Review comment:
       Why `DS` created in static block, but close here? I think in some cases(retry), this method could execute repeatedly.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org