From 62f76881de990917f229e5121e9094f977599da3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Patrick=20Jiang=28=E7=99=BD=E6=B3=BD=29?= Date: Fri, 17 Jul 2020 17:36:02 +0800 Subject: [PATCH] Fix issue 4965 (#5109) --- ...redStatementExecuteMethodsInterceptor.java | 12 ++-- .../mysql-scenario/config/expectedData.yaml | 69 ++++++++++++++++++- .../apm/testcase/mysql/SQLExecutor.java | 21 +++++- .../mysql/controller/CaseController.java | 8 ++- 4 files changed, 101 insertions(+), 9 deletions(-) diff --git a/apm-sniffer/apm-sdk-plugin/mysql-common/src/main/java/org/apache/skywalking/apm/plugin/jdbc/mysql/PreparedStatementExecuteMethodsInterceptor.java b/apm-sniffer/apm-sdk-plugin/mysql-common/src/main/java/org/apache/skywalking/apm/plugin/jdbc/mysql/PreparedStatementExecuteMethodsInterceptor.java index fa0613af9..27f4c6b94 100644 --- a/apm-sniffer/apm-sdk-plugin/mysql-common/src/main/java/org/apache/skywalking/apm/plugin/jdbc/mysql/PreparedStatementExecuteMethodsInterceptor.java +++ b/apm-sniffer/apm-sdk-plugin/mysql-common/src/main/java/org/apache/skywalking/apm/plugin/jdbc/mysql/PreparedStatementExecuteMethodsInterceptor.java @@ -40,15 +40,15 @@ public class PreparedStatementExecuteMethodsInterceptor implements InstanceMetho public final void beforeMethod(EnhancedInstance objInst, Method method, Object[] allArguments, Class[] argumentsTypes, MethodInterceptResult result) { StatementEnhanceInfos cacheObject = (StatementEnhanceInfos) objInst.getSkyWalkingDynamicField(); - ConnectionInfo connectInfo = cacheObject.getConnectionInfo(); /** * For avoid NPE. In this particular case, Execute sql inside the {@link com.mysql.jdbc.ConnectionImpl} constructor, * before the interceptor sets the connectionInfo. - * + * When invoking prepareCall, cacheObject is null. Because it will determine procedures's parameter types by executing sql in mysql + * before the interceptor sets the statementEnhanceInfos. * @see JDBCDriverInterceptor#afterMethod(EnhancedInstance, Method, Object[], Class[], Object) */ - if (connectInfo != null) { - + if (cacheObject != null && cacheObject.getConnectionInfo() != null) { + ConnectionInfo connectInfo = cacheObject.getConnectionInfo(); AbstractSpan span = ContextManager.createExitSpan(buildOperationName(connectInfo, method.getName(), cacheObject .getStatementName()), connectInfo.getDatabasePeer()); Tags.DB_TYPE.set(span, "sql"); @@ -73,7 +73,7 @@ public class PreparedStatementExecuteMethodsInterceptor implements InstanceMetho public final Object afterMethod(EnhancedInstance objInst, Method method, Object[] allArguments, Class[] argumentsTypes, Object ret) { StatementEnhanceInfos cacheObject = (StatementEnhanceInfos) objInst.getSkyWalkingDynamicField(); - if (cacheObject.getConnectionInfo() != null) { + if (cacheObject != null && cacheObject.getConnectionInfo() != null) { ContextManager.stopSpan(); } return ret; @@ -83,7 +83,7 @@ public class PreparedStatementExecuteMethodsInterceptor implements InstanceMetho public final void handleMethodException(EnhancedInstance objInst, Method method, Object[] allArguments, Class[] argumentsTypes, Throwable t) { StatementEnhanceInfos cacheObject = (StatementEnhanceInfos) objInst.getSkyWalkingDynamicField(); - if (cacheObject.getConnectionInfo() != null) { + if (cacheObject != null && cacheObject.getConnectionInfo() != null) { ContextManager.activeSpan().errorOccurred().log(t); } } diff --git a/test/plugin/scenarios/mysql-scenario/config/expectedData.yaml b/test/plugin/scenarios/mysql-scenario/config/expectedData.yaml index 9313e7974..904004e9b 100644 --- a/test/plugin/scenarios/mysql-scenario/config/expectedData.yaml +++ b/test/plugin/scenarios/mysql-scenario/config/expectedData.yaml @@ -72,10 +72,77 @@ segmentItems: componentId: 33 peer: mysql-server:3306 skipAnalysis: 'false' - - operationName: Mysql/JDBI/Connection/close + - operationName: Mysql/JDBI/Statement/execute operationId: eq 0 parentSpanId: 0 spanId: 4 + tags: + - {key: db.type, value: sql} + - {key: db.instance, value: test} + - {key: db.statement, value: "create procedure testProcedure(IN id varchar(10)) \n begin \n select id; \n end"} + logs: [] + startTime: nq 0 + endTime: nq 0 + isError: false + spanLayer: Database + spanType: Exit + componentId: 33 + peer: mysql-server:3306 + skipAnalysis: 'false' + - operationName: Mysql/JDBI/Statement/executeQuery + operationId: eq 0 + parentSpanId: 0 + spanId: 5 + spanLayer: Database + startTime: nq 0 + endTime: nq 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: SHOW CREATE PROCEDURE `test`.`testProcedure`} + - operationName: Mysql/JDBI/CallableStatement/execute + operationId: eq 0 + parentSpanId: 0 + spanId: 6 + tags: + - {key: db.type, value: sql} + - {key: db.instance, value: test} + - {key: db.statement, value: "call testProcedure( ? )"} + logs: [] + startTime: nq 0 + endTime: nq 0 + isError: false + spanLayer: Database + spanType: Exit + componentId: 33 + peer: mysql-server:3306 + skipAnalysis: 'false' + - operationName: Mysql/JDBI/Statement/execute + operationId: eq 0 + parentSpanId: 0 + spanId: 7 + tags: + - {key: db.type, value: sql} + - {key: db.instance, value: test} + - {key: db.statement, value: "drop procedure testProcedure"} + logs: [] + startTime: nq 0 + endTime: nq 0 + isError: false + spanLayer: Database + spanType: Exit + componentId: 33 + peer: mysql-server:3306 + skipAnalysis: 'false' + - operationName: Mysql/JDBI/Connection/close + operationId: eq 0 + parentSpanId: 0 + spanId: 8 tags: - {key: db.type, value: sql} - {key: db.instance, value: test} diff --git a/test/plugin/scenarios/mysql-scenario/src/main/java/org/apache/skywalking/apm/testcase/mysql/SQLExecutor.java b/test/plugin/scenarios/mysql-scenario/src/main/java/org/apache/skywalking/apm/testcase/mysql/SQLExecutor.java index 77e020534..268ef97bf 100644 --- a/test/plugin/scenarios/mysql-scenario/src/main/java/org/apache/skywalking/apm/testcase/mysql/SQLExecutor.java +++ b/test/plugin/scenarios/mysql-scenario/src/main/java/org/apache/skywalking/apm/testcase/mysql/SQLExecutor.java @@ -51,11 +51,30 @@ public class SQLExecutor implements AutoCloseable { } public void dropTable(String sql) throws SQLException { + executeStatement(sql); + } + + public void createProcedure(String sql) throws SQLException { + executeStatement(sql); + } + + public void dropProcedure(String sql) throws SQLException { + executeStatement(sql); + } + + public void callProcedure(String sql, String id) throws SQLException { + PreparedStatement preparedStatement = connection.prepareCall(sql); + preparedStatement.setString(1, id); + preparedStatement.execute(); + preparedStatement.close(); + } + + public void executeStatement(String sql) throws SQLException { Statement preparedStatement = connection.createStatement(); preparedStatement.execute(sql); preparedStatement.close(); } - + public void closeConnection() throws SQLException { if (this.connection != null) { this.connection.close(); diff --git a/test/plugin/scenarios/mysql-scenario/src/main/java/org/apache/skywalking/apm/testcase/mysql/controller/CaseController.java b/test/plugin/scenarios/mysql-scenario/src/main/java/org/apache/skywalking/apm/testcase/mysql/controller/CaseController.java index f72b4efe6..415c65496 100644 --- a/test/plugin/scenarios/mysql-scenario/src/main/java/org/apache/skywalking/apm/testcase/mysql/controller/CaseController.java +++ b/test/plugin/scenarios/mysql-scenario/src/main/java/org/apache/skywalking/apm/testcase/mysql/controller/CaseController.java @@ -40,7 +40,10 @@ public class CaseController { private static final String QUERY_DATA_SQL = "SELECT id, value FROM test_007 WHERE id=?"; private static final String DELETE_DATA_SQL = "DELETE FROM test_007 WHERE id=?"; private static final String DROP_TABLE_SQL = "DROP table test_007"; - + private static final String CREATE_PROCEDURE_SQL = "create procedure testProcedure(IN id varchar(10)) \n begin \n select id; \n end"; + private static final String CALL_PROCEDURE_SQL = "call testProcedure( ? )"; + private static final String DROP_PROCEDURE_SQL = "drop procedure testProcedure"; + @RequestMapping("/mysql-scenario") @ResponseBody public String testcase() throws Exception { @@ -48,6 +51,9 @@ public class CaseController { sqlExecute.createTable(CREATE_TABLE_SQL); sqlExecute.insertData(INSERT_DATA_SQL, "1", "1"); sqlExecute.dropTable(DROP_TABLE_SQL); + sqlExecute.createProcedure(CREATE_PROCEDURE_SQL); + sqlExecute.callProcedure(CALL_PROCEDURE_SQL, "nihao"); + sqlExecute.dropProcedure(DROP_PROCEDURE_SQL); } catch (Exception e) { logger.error("Failed to execute sql.", e); throw e;