support parameter collection for sqlserver (apache#7217)

Switch-vov · Jul 2, 2021 · 04bb667 · 04bb667
1 parent f32d3d0
commit 04bb667
Show file tree

Hide file tree

Showing 11 changed files with 155 additions and 5 deletions.
diff --git a/CHANGES.md b/CHANGES.md
@@ -23,6 +23,7 @@ Release Notes.
 * Add `Neo4j-4.x` plugin.
 * Correct `profile.duration` to `profile.max_duration` in the default `agent.config` file.
 * Fix the response time of gRPC.
+* Support parameter collection for SqlServer.
 * Add `ShardingSphere-5.0.0-beta` plugin.
 * Fix some method exception error.
 

diff --git a/...mssql-commons/src/main/java/org/apache/skywalking/apm/plugin/mssql/commons/Constants.java b/...mssql-commons/src/main/java/org/apache/skywalking/apm/plugin/mssql/commons/Constants.java
@@ -18,10 +18,14 @@
 
 package org.apache.skywalking.apm.plugin.mssql.commons;
 
+import org.apache.skywalking.apm.agent.core.context.tag.StringTag;
+
 public class Constants {
     public static final String CREATE_CALLABLE_STATEMENT_INTERCEPTOR = "org.apache.skywalking.apm.plugin.mssql.commons.CreateCallableStatementInterceptor";
     public static final String CREATE_PREPARED_STATEMENT_INTERCEPTOR = "org.apache.skywalking.apm.plugin.mssql.commons.CreatePreparedStatementInterceptor";
     public static final String CREATE_STATEMENT_INTERCEPTOR = "org.apache.skywalking.apm.plugin.mssql.commons.CreateStatementInterceptor";
     public static final String PREPARED_STATEMENT_EXECUTE_METHODS_INTERCEPTOR = "org.apache.skywalking.apm.plugin.mssql.commons.PreparedStatementExecuteMethodsInterceptor";
     public static final String STATEMENT_EXECUTE_METHODS_INTERCEPTOR = "org.apache.skywalking.apm.plugin.mssql.commons.StatementExecuteMethodsInterceptor";
+
+    public static final StringTag SQL_PARAMETERS = new StringTag("db.sql.parameters");
 }
diff --git a/...pache/skywalking/apm/plugin/mssql/commons/PreparedStatementExecuteMethodsInterceptor.java b/...pache/skywalking/apm/plugin/mssql/commons/PreparedStatementExecuteMethodsInterceptor.java
@@ -26,6 +26,8 @@
 import org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance.EnhancedInstance;
 import org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance.InstanceMethodsAroundInterceptor;
 import org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance.MethodInterceptResult;
+import org.apache.skywalking.apm.plugin.jdbc.JDBCPluginConfig;
+import org.apache.skywalking.apm.plugin.jdbc.PreparedStatementParameterBuilder;
 import org.apache.skywalking.apm.plugin.jdbc.SqlBodyUtil;
 import org.apache.skywalking.apm.plugin.jdbc.define.StatementEnhanceInfos;
 import org.apache.skywalking.apm.plugin.jdbc.trace.ConnectionInfo;
@@ -47,6 +49,13 @@ public final void beforeMethod(EnhancedInstance objInst, Method method, Object[]
             Tags.DB_INSTANCE.set(span, connectInfo.getDatabaseName());
             Tags.DB_STATEMENT.set(span, SqlBodyUtil.limitSqlBodySize(cacheObject.getSql()));
             span.setComponent(connectInfo.getComponent());
+            if (JDBCPluginConfig.Plugin.JDBC.TRACE_SQL_PARAMETERS) {
+                final Object[] parameters = cacheObject.getParameters();
+                if (parameters != null && parameters.length > 0) {
+                    int maxIndex = cacheObject.getMaxIndex();
+                    Constants.SQL_PARAMETERS.set(span, getParameterString(parameters, maxIndex));
+                }
+            }
             SpanLayer.asDB(span);
         }
     }
@@ -73,4 +82,11 @@ public final void handleMethodException(EnhancedInstance objInst, Method method,
     private String buildOperationName(ConnectionInfo connectionInfo, String methodName, String statementName) {
         return connectionInfo.getDBType() + "/JDBI/" + statementName + "/" + methodName;
     }
+
+    private String getParameterString(Object[] parameters, int maxIndex) {
+        return new PreparedStatementParameterBuilder()
+                .setParameters(parameters)
+                .setMaxIndex(maxIndex)
+                .build();
+    }
 }
diff --git a/...kywalking/apm/plugin/mssql/jdbc/define/PreparedStatementIgnoredSetterInstrumentation.java b/...kywalking/apm/plugin/mssql/jdbc/define/PreparedStatementIgnoredSetterInstrumentation.java
@@ -0,0 +1,33 @@
+/*
+ * 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.mssql.jdbc.define;
+
+import org.apache.skywalking.apm.agent.core.plugin.interceptor.InstanceMethodsInterceptPoint;
+import org.apache.skywalking.apm.plugin.jdbc.PSSetterDefinitionOfJDBCInstrumentation;
+
+public class PreparedStatementIgnoredSetterInstrumentation extends PreparedStatementInstrumentation {
+
+    @Override
+    public final InstanceMethodsInterceptPoint[] getInstanceMethodsInterceptPoints() {
+        return new InstanceMethodsInterceptPoint[] {
+            new PSSetterDefinitionOfJDBCInstrumentation(true)
+        };
+    }
+
+}
diff --git a/...e/skywalking/apm/plugin/mssql/jdbc/define/PreparedStatementNullSetterInstrumentation.java b/...e/skywalking/apm/plugin/mssql/jdbc/define/PreparedStatementNullSetterInstrumentation.java
@@ -0,0 +1,33 @@
+/*
+ * 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.mssql.jdbc.define;
+
+import org.apache.skywalking.apm.agent.core.plugin.interceptor.InstanceMethodsInterceptPoint;
+import org.apache.skywalking.apm.plugin.jdbc.JDBCPreparedStatementNullSetterInstanceMethodsInterceptPoint;
+
+public class PreparedStatementNullSetterInstrumentation extends PreparedStatementInstrumentation {
+
+    @Override
+    public final InstanceMethodsInterceptPoint[] getInstanceMethodsInterceptPoints() {
+        return new InstanceMethodsInterceptPoint[] {
+            new JDBCPreparedStatementNullSetterInstanceMethodsInterceptPoint()
+        };
+    }
+
+}
diff --git a/...pache/skywalking/apm/plugin/mssql/jdbc/define/PreparedStatementSetterInstrumentation.java b/...pache/skywalking/apm/plugin/mssql/jdbc/define/PreparedStatementSetterInstrumentation.java
@@ -0,0 +1,33 @@
+/*
+ * 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.mssql.jdbc.define;
+
+import org.apache.skywalking.apm.agent.core.plugin.interceptor.InstanceMethodsInterceptPoint;
+import org.apache.skywalking.apm.plugin.jdbc.PSSetterDefinitionOfJDBCInstrumentation;
+
+public class PreparedStatementSetterInstrumentation extends PreparedStatementInstrumentation {
+
+    @Override
+    public final InstanceMethodsInterceptPoint[] getInstanceMethodsInterceptPoints() {
+        return new InstanceMethodsInterceptPoint[] {
+            new PSSetterDefinitionOfJDBCInstrumentation(false)
+        };
+    }
+
+}
diff --git a/apm-sniffer/apm-sdk-plugin/mssql-jdbc-plugin/src/main/resources/skywalking-plugin.def b/apm-sniffer/apm-sdk-plugin/mssql-jdbc-plugin/src/main/resources/skywalking-plugin.def
@@ -17,4 +17,7 @@
 mssql-jdbc=org.apache.skywalking.apm.plugin.mssql.jdbc.define.DriverInstrumentation
 mssql-jdbc=org.apache.skywalking.apm.plugin.mssql.jdbc.define.ConnectionInstrumentation
 mssql-jdbc=org.apache.skywalking.apm.plugin.mssql.jdbc.define.PreparedStatementInstrumentation
-mssql-jdbc=org.apache.skywalking.apm.plugin.mssql.jdbc.define.StatementInstrumentation
+mssql-jdbc=org.apache.skywalking.apm.plugin.mssql.jdbc.define.StatementInstrumentation
+mssql-jdbc=org.apache.skywalking.apm.plugin.mssql.jdbc.define.PreparedStatementSetterInstrumentation
+mssql-jdbc=org.apache.skywalking.apm.plugin.mssql.jdbc.define.PreparedStatementNullSetterInstrumentation
+mssql-jdbc=org.apache.skywalking.apm.plugin.mssql.jdbc.define.PreparedStatementIgnoredSetterInstrumentation
diff --git a/test/plugin/scenarios/mssql-jdbc-scenario/bin/startup.sh b/test/plugin/scenarios/mssql-jdbc-scenario/bin/startup.sh
@@ -18,4 +18,4 @@
 
 home="$(cd "$(dirname $0)"; pwd)"
 
-java -jar ${agent_opts} ${home}/../libs/mssql-jdbc-scenario.jar &
+java -jar ${agent_opts}  -Dskywalking.plugin.jdbc.trace_sql_parameters=true ${home}/../libs/mssql-jdbc-scenario.jar &
diff --git a/test/plugin/scenarios/mssql-jdbc-scenario/config/expectedData.yaml b/test/plugin/scenarios/mssql-jdbc-scenario/config/expectedData.yaml
@@ -46,6 +46,7 @@ segmentItems:
               - {key: db.type, value: sql}
               - {key: db.instance, value: tempdb}
               - {key: db.statement, value: 'INSERT INTO test_007(id, value) VALUES(?,?)'}
+              - {key: db.sql.parameters, value: '[1,1]'}
             logs: []
             startTime: nq 0
             endTime: nq 0
@@ -55,10 +56,28 @@ segmentItems:
             componentId: 104
             peer: mssql-server:1433
             skipAnalysis: 'false'
-          - operationName: Mssql/JDBI/Statement/execute
+          - operationName: Mssql/JDBI/PreparedStatement/execute
             operationId: eq 0
             parentSpanId: 0
             spanId: 3
+            tags:
+              - {key: db.type, value: sql}
+              - {key: db.instance, value: tempdb}
+              - {key: db.statement, value: 'SELECT id, value FROM test_007 WHERE id=?'}
+              - {key: db.sql.parameters, value: '[1]'}
+            logs: []
+            startTime: nq 0
+            endTime: nq 0
+            isError: false
+            spanLayer: Database
+            spanType: Exit
+            componentId: 104
+            peer: mssql-server:1433
+            skipAnalysis: 'false'
+          - operationName: Mssql/JDBI/Statement/execute
+            operationId: eq 0
+            parentSpanId: 0
+            spanId: 4
             tags:
               - {key: db.type, value: sql}
               - {key: db.instance, value: tempdb}
@@ -75,7 +94,7 @@ segmentItems:
           - operationName: Mssql/JDBI/Connection/close
             operationId: eq 0
             parentSpanId: 0
-            spanId: 4
+            spanId: 5
             tags:
               - {key: db.type, value: sql}
               - {key: db.instance, value: tempdb}

diff --git a/...sql-jdbc-scenario/src/main/java/org/apache/skywalking/apm/testcase/mssql/SQLExecutor.java b/...sql-jdbc-scenario/src/main/java/org/apache/skywalking/apm/testcase/mssql/SQLExecutor.java
@@ -50,7 +50,14 @@ public void insertData(String sql, String id, String value) throws SQLException
         preparedStatement.execute();
         preparedStatement.close();
     }
-
+
+    public void queryData(String sql, String id) throws SQLException {
+        PreparedStatement preparedStatement = connection.prepareStatement(sql);
+        preparedStatement.setString(1, id);
+        preparedStatement.execute();
+        preparedStatement.close();
+    }
+
     public void dropTable(String sql) throws SQLException {
         executeStatement(sql);
     }

diff --git a/...rio/src/main/java/org/apache/skywalking/apm/testcase/mssql/controller/CaseController.java b/...rio/src/main/java/org/apache/skywalking/apm/testcase/mssql/controller/CaseController.java
@@ -45,6 +45,7 @@ public String testcase() throws Exception {
         try (SQLExecutor sqlExecute = new SQLExecutor()) {
             sqlExecute.createTable(CREATE_TABLE_SQL);
             sqlExecute.insertData(INSERT_DATA_SQL, "1", "1");
+            sqlExecute.queryData(QUERY_DATA_SQL, "1");
             sqlExecute.dropTable(DROP_TABLE_SQL);
         } catch (Exception e) {
             LOGGER.error("Failed to execute sql.", e);