From 7e912d01be5c09345dffa0e9dd48eb707fc034a6 Mon Sep 17 00:00:00 2001 From: aderm Date: Wed, 23 Oct 2019 22:44:43 +0800 Subject: [PATCH 01/13] Add postgresql agent sql query param show --- ...reparedStatementSetterInstrumentation.java | 32 +++++++++++++++++++ 1 file changed, 32 insertions(+) create mode 100644 apm-sniffer/apm-sdk-plugin/postgresql-8.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/jdbc/postgresql/define/PgPreparedStatementSetterInstrumentation.java diff --git a/apm-sniffer/apm-sdk-plugin/postgresql-8.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/jdbc/postgresql/define/PgPreparedStatementSetterInstrumentation.java b/apm-sniffer/apm-sdk-plugin/postgresql-8.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/jdbc/postgresql/define/PgPreparedStatementSetterInstrumentation.java new file mode 100644 index 000000000000..972d07a695bf --- /dev/null +++ b/apm-sniffer/apm-sdk-plugin/postgresql-8.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/jdbc/postgresql/define/PgPreparedStatementSetterInstrumentation.java @@ -0,0 +1,32 @@ +/* + * 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.jdbc.postgresql.define; + +import org.apache.skywalking.apm.agent.core.plugin.interceptor.InstanceMethodsInterceptPoint; +import org.apache.skywalking.apm.plugin.jdbc.PSSetterDefinitionOfJDBCInstrumentation; + +public class PgPreparedStatementSetterInstrumentation extends PgPreparedStatementInstrumentation { + + @Override + public final InstanceMethodsInterceptPoint[] getInstanceMethodsInterceptPoints() { + return new InstanceMethodsInterceptPoint[] { + new PSSetterDefinitionOfJDBCInstrumentation(false) + }; + } + +} From f50ed5d6694349be215f11c49586e69168f85a8d Mon Sep 17 00:00:00 2001 From: aderm Date: Wed, 23 Oct 2019 22:46:52 +0800 Subject: [PATCH 02/13] Add postgresql agent sql query param show --- .../apm/agent/core/conf/Config.java | 15 ++++++++++ ...SetterDefinitionOfJDBCInstrumentation.java | 3 +- .../jdbc/define/StatementEnhanceInfos.java | 30 +++++-------------- ...redStatementExecuteMethodsInterceptor.java | 9 +++++- .../src/main/resources/skywalking-plugin.def | 1 + 5 files changed, 34 insertions(+), 24 deletions(-) diff --git a/apm-sniffer/apm-agent-core/src/main/java/org/apache/skywalking/apm/agent/core/conf/Config.java b/apm-sniffer/apm-agent-core/src/main/java/org/apache/skywalking/apm/agent/core/conf/Config.java index c8e04593ce9c..26f0022473fd 100755 --- a/apm-sniffer/apm-agent-core/src/main/java/org/apache/skywalking/apm/agent/core/conf/Config.java +++ b/apm-sniffer/apm-agent-core/src/main/java/org/apache/skywalking/apm/agent/core/conf/Config.java @@ -273,6 +273,21 @@ public static class MySQL { public static int SQL_PARAMETERS_MAX_LENGTH = 512; } + public static class POSTGRESQL { + /** + * If set to true, the parameters of the sql (typically {@link java.sql.PreparedStatement}) would be + * collected. + */ + public static boolean TRACE_SQL_PARAMETERS = false; + /** + * For the sake of performance, SkyWalking won't save the entire parameters string into the tag, but only + * the first {@code SQL_PARAMETERS_MAX_LENGTH} characters. + * + * Set a negative number to save the complete parameter string to the tag. + */ + public static int SQL_PARAMETERS_MAX_LENGTH = 512; + } + public static class SolrJ { /** * If true, trace all the query parameters(include deleteByIds and deleteByQuery) in Solr query request, diff --git a/apm-sniffer/apm-sdk-plugin/jdbc-commons/src/main/java/org/apache/skywalking/apm/plugin/jdbc/PSSetterDefinitionOfJDBCInstrumentation.java b/apm-sniffer/apm-sdk-plugin/jdbc-commons/src/main/java/org/apache/skywalking/apm/plugin/jdbc/PSSetterDefinitionOfJDBCInstrumentation.java index fb7a4c060b07..1ab3284e87ef 100644 --- a/apm-sniffer/apm-sdk-plugin/jdbc-commons/src/main/java/org/apache/skywalking/apm/plugin/jdbc/PSSetterDefinitionOfJDBCInstrumentation.java +++ b/apm-sniffer/apm-sdk-plugin/jdbc-commons/src/main/java/org/apache/skywalking/apm/plugin/jdbc/PSSetterDefinitionOfJDBCInstrumentation.java @@ -21,6 +21,7 @@ import net.bytebuddy.description.method.MethodDescription; import net.bytebuddy.matcher.ElementMatcher; import org.apache.skywalking.apm.agent.core.conf.Config; +import org.apache.skywalking.apm.agent.core.conf.Config.Plugin.POSTGRESQL; import org.apache.skywalking.apm.agent.core.plugin.interceptor.InstanceMethodsInterceptPoint; import org.apache.skywalking.apm.plugin.jdbc.define.Constants; @@ -45,7 +46,7 @@ public PSSetterDefinitionOfJDBCInstrumentation(boolean ignorable) { public ElementMatcher getMethodsMatcher() { ElementMatcher.Junction matcher = none(); - if (Config.Plugin.MySQL.TRACE_SQL_PARAMETERS) { + if (Config.Plugin.MySQL.TRACE_SQL_PARAMETERS || POSTGRESQL.TRACE_SQL_PARAMETERS) { final Set setters = ignorable ? PS_IGNORABLE_SETTERS : PS_SETTERS; for (String setter : setters) { matcher = matcher.or(named(setter)); diff --git a/apm-sniffer/apm-sdk-plugin/jdbc-commons/src/main/java/org/apache/skywalking/apm/plugin/jdbc/define/StatementEnhanceInfos.java b/apm-sniffer/apm-sdk-plugin/jdbc-commons/src/main/java/org/apache/skywalking/apm/plugin/jdbc/define/StatementEnhanceInfos.java index 3ec22a4692c2..cb6707c2147d 100644 --- a/apm-sniffer/apm-sdk-plugin/jdbc-commons/src/main/java/org/apache/skywalking/apm/plugin/jdbc/define/StatementEnhanceInfos.java +++ b/apm-sniffer/apm-sdk-plugin/jdbc-commons/src/main/java/org/apache/skywalking/apm/plugin/jdbc/define/StatementEnhanceInfos.java @@ -19,6 +19,8 @@ package org.apache.skywalking.apm.plugin.jdbc.define; +import java.util.ArrayList; +import java.util.List; import org.apache.skywalking.apm.plugin.jdbc.trace.ConnectionInfo; import java.util.Arrays; @@ -33,8 +35,7 @@ public class StatementEnhanceInfos { private ConnectionInfo connectionInfo; private String statementName; private String sql; - private Object[] parameters; - private int maxIndex = 0; + private List parameters = new ArrayList(); public StatementEnhanceInfos(ConnectionInfo connectionInfo, String sql, String statementName) { this.connectionInfo = connectionInfo; @@ -55,29 +56,14 @@ public String getStatementName() { } public void setParameter(int index, final Object parameter) { - maxIndex = maxIndex > index ? maxIndex : index; - index--; // start from 1 - if (parameters == null) { - final int initialSize = Math.max(20, maxIndex); - parameters = new Object[initialSize]; - Arrays.fill(parameters, null); + // start from 1 + index--; + if (index >= 0) { + parameters.set(index, parameter); } - int length = parameters.length; - if (index >= length) { - int newSize = Math.max(index + 1, length * 2); - Object[] newParameters = new Object[newSize]; - System.arraycopy(parameters, 0, newParameters, 0, length); - Arrays.fill(newParameters, length, newSize, null); - parameters = newParameters; - } - parameters[index] = parameter; } - public Object[] getParameters() { + public List getParameters() { return parameters; } - - public int getMaxIndex() { - return maxIndex; - } } diff --git a/apm-sniffer/apm-sdk-plugin/postgresql-8.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/jdbc/postgresql/PreparedStatementExecuteMethodsInterceptor.java b/apm-sniffer/apm-sdk-plugin/postgresql-8.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/jdbc/postgresql/PreparedStatementExecuteMethodsInterceptor.java index 6ad648c24253..7b30e41b9cc4 100644 --- a/apm-sniffer/apm-sdk-plugin/postgresql-8.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/jdbc/postgresql/PreparedStatementExecuteMethodsInterceptor.java +++ b/apm-sniffer/apm-sdk-plugin/postgresql-8.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/jdbc/postgresql/PreparedStatementExecuteMethodsInterceptor.java @@ -19,7 +19,9 @@ package org.apache.skywalking.apm.plugin.jdbc.postgresql; +import com.google.common.base.Joiner; import java.lang.reflect.Method; +import java.util.List; import org.apache.skywalking.apm.agent.core.context.ContextManager; import org.apache.skywalking.apm.agent.core.context.tag.Tags; import org.apache.skywalking.apm.agent.core.context.trace.AbstractSpan; @@ -46,8 +48,13 @@ public final void beforeMethod(EnhancedInstance objInst, Method method, Object[] Tags.DB_TYPE.set(span, "sql"); Tags.DB_INSTANCE.set(span, connectInfo.getDatabaseName()); Tags.DB_STATEMENT.set(span, cacheObject.getSql()); - span.setComponent(connectInfo.getComponent()); + List parameters = cacheObject.getParameters(); + if (!parameters.isEmpty()) { + Tags.DB_BIND_VARIABLES.set(span, Joiner.on(",").join(parameters)); + } + + span.setComponent(connectInfo.getComponent()); SpanLayer.asDB(span); } diff --git a/apm-sniffer/apm-sdk-plugin/postgresql-8.x-plugin/src/main/resources/skywalking-plugin.def b/apm-sniffer/apm-sdk-plugin/postgresql-8.x-plugin/src/main/resources/skywalking-plugin.def index c1e3e0de6f27..e744f204fcfe 100644 --- a/apm-sniffer/apm-sdk-plugin/postgresql-8.x-plugin/src/main/resources/skywalking-plugin.def +++ b/apm-sniffer/apm-sdk-plugin/postgresql-8.x-plugin/src/main/resources/skywalking-plugin.def @@ -22,3 +22,4 @@ postgresql-8.x=org.apache.skywalking.apm.plugin.jdbc.postgresql.define.AbstractJ postgresql-8.x=org.apache.skywalking.apm.plugin.jdbc.postgresql.define.PgCallableStatementInstrumentation postgresql-8.x=org.apache.skywalking.apm.plugin.jdbc.postgresql.define.PgPreparedStatementInstrumentation postgresql-8.x=org.apache.skywalking.apm.plugin.jdbc.postgresql.define.PgStatementInstrumentation +postgresql-8.x=org.apache.skywalking.apm.plugin.jdbc.postgresql.define.PgPreparedStatementSetterInstrumentation From 807fc7e0cb39f022cafb38545f2ffd9af99ce571 Mon Sep 17 00:00:00 2001 From: aderm Date: Wed, 23 Oct 2019 22:58:54 +0800 Subject: [PATCH 03/13] delete unused import line --- .../apm/plugin/jdbc/define/StatementEnhanceInfos.java | 2 -- 1 file changed, 2 deletions(-) diff --git a/apm-sniffer/apm-sdk-plugin/jdbc-commons/src/main/java/org/apache/skywalking/apm/plugin/jdbc/define/StatementEnhanceInfos.java b/apm-sniffer/apm-sdk-plugin/jdbc-commons/src/main/java/org/apache/skywalking/apm/plugin/jdbc/define/StatementEnhanceInfos.java index cb6707c2147d..a9983c72cb5c 100644 --- a/apm-sniffer/apm-sdk-plugin/jdbc-commons/src/main/java/org/apache/skywalking/apm/plugin/jdbc/define/StatementEnhanceInfos.java +++ b/apm-sniffer/apm-sdk-plugin/jdbc-commons/src/main/java/org/apache/skywalking/apm/plugin/jdbc/define/StatementEnhanceInfos.java @@ -23,8 +23,6 @@ import java.util.List; import org.apache.skywalking.apm.plugin.jdbc.trace.ConnectionInfo; -import java.util.Arrays; - /** * {@link StatementEnhanceInfos} contain the {@link ConnectionInfo} and * sql for trace mysql. From fe2795a904a789bf640d5ecbc8a5d7ec1ba7e561 Mon Sep 17 00:00:00 2001 From: aderm Date: Wed, 23 Oct 2019 23:28:37 +0800 Subject: [PATCH 04/13] remove param count limit --- ...PreparedStatementExecuteMethodsInterceptor.java | 14 +++++++------- 1 file changed, 7 insertions(+), 7 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 732816c07d08..73a405c5866b 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 @@ -18,6 +18,7 @@ package org.apache.skywalking.apm.plugin.jdbc.mysql; +import java.util.List; import org.apache.skywalking.apm.agent.core.conf.Config; import org.apache.skywalking.apm.agent.core.context.ContextManager; import org.apache.skywalking.apm.agent.core.context.tag.Tags; @@ -56,10 +57,9 @@ public final void beforeMethod(EnhancedInstance objInst, Method method, Object[] span.setComponent(connectInfo.getComponent()); if (Config.Plugin.MySQL.TRACE_SQL_PARAMETERS) { - final Object[] parameters = cacheObject.getParameters(); - if (parameters != null && parameters.length > 0) { - int maxIndex = cacheObject.getMaxIndex(); - String parameterString = buildParameterString(parameters, maxIndex); + List parameters = cacheObject.getParameters(); + if (parameters != null && parameters.size() > 0) { + String parameterString = buildParameterString(parameters); int sqlParametersMaxLength = Config.Plugin.MySQL.SQL_PARAMETERS_MAX_LENGTH; if (sqlParametersMaxLength > 0 && parameterString.length() > sqlParametersMaxLength) { parameterString = parameterString.substring(0, sqlParametersMaxLength) + "..."; @@ -95,11 +95,11 @@ private String buildOperationName(ConnectionInfo connectionInfo, String methodNa return connectionInfo.getDBType() + "/JDBI/" + statementName + "/" + methodName; } - private String buildParameterString(Object[] parameters, int maxIndex) { + private String buildParameterString(List parameters) { String parameterString = "["; boolean first = true; - for (int i = 0; i < maxIndex; i++) { - Object parameter = parameters[i]; + for (int i = 0; i < parameters.size(); i++) { + Object parameter = parameters.get(i); if (!first) { parameterString += ","; } From cb057ab18d47f0aaa7d0db1a730e732f51818663 Mon Sep 17 00:00:00 2001 From: aderm Date: Sun, 27 Oct 2019 01:18:45 +0800 Subject: [PATCH 05/13] 1.change treeMap to store param;2.add param cnt limit;3.add param length limit; --- .../apm/agent/core/conf/Config.java | 8 ++- ...SetterDefinitionOfJDBCInstrumentation.java | 3 +- .../jdbc/define/StatementEnhanceInfos.java | 50 +++++++++++++++--- .../postgresql-8.x-plugin/pom.xml | 6 +++ .../CreateCallableStatementInterceptor.java | 0 .../CreatePreparedStatementInterceptor.java | 0 .../CreateStatementInterceptor.java | 0 ...reStatementWithStringArrayInterceptor.java | 0 ...redStatementExecuteMethodsInterceptor.java | 14 +++-- .../StatementExecuteMethodsInterceptor.java | 0 .../apm/plugin/jdbc/postgresql/Variables.java | 0 ...AbstractJdbc2StatementInstrumentation.java | 0 .../define/ConnectionInstrumentation.java | 0 .../jdbc/postgresql/define/Constants.java | 1 - .../define/DriverInstrumentation.java | 0 .../Jdbc3ConnectionInstrumentation.java | 0 .../Jdbc4ConnectionInstrumentation.java | 0 .../PgCallableStatementInstrumentation.java | 0 .../PgPreparedStatementInstrumentation.java | 0 ...reparedStatementSetterInstrumentation.java | 3 ++ .../define/PgStatementInstrumentation.java | 0 .../jdbc/postgresql/util/ParameterUtil.java | 51 +++++++++++++++++++ .../src/main/resources/skywalking-plugin.def | 0 23 files changed, 121 insertions(+), 15 deletions(-) mode change 100644 => 100755 apm-sniffer/apm-sdk-plugin/postgresql-8.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/jdbc/postgresql/CreateCallableStatementInterceptor.java mode change 100644 => 100755 apm-sniffer/apm-sdk-plugin/postgresql-8.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/jdbc/postgresql/CreatePreparedStatementInterceptor.java mode change 100644 => 100755 apm-sniffer/apm-sdk-plugin/postgresql-8.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/jdbc/postgresql/CreateStatementInterceptor.java mode change 100644 => 100755 apm-sniffer/apm-sdk-plugin/postgresql-8.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/jdbc/postgresql/JDBCPrepareStatementWithStringArrayInterceptor.java mode change 100644 => 100755 apm-sniffer/apm-sdk-plugin/postgresql-8.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/jdbc/postgresql/PreparedStatementExecuteMethodsInterceptor.java mode change 100644 => 100755 apm-sniffer/apm-sdk-plugin/postgresql-8.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/jdbc/postgresql/StatementExecuteMethodsInterceptor.java mode change 100644 => 100755 apm-sniffer/apm-sdk-plugin/postgresql-8.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/jdbc/postgresql/Variables.java mode change 100644 => 100755 apm-sniffer/apm-sdk-plugin/postgresql-8.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/jdbc/postgresql/define/AbstractJdbc2StatementInstrumentation.java mode change 100644 => 100755 apm-sniffer/apm-sdk-plugin/postgresql-8.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/jdbc/postgresql/define/ConnectionInstrumentation.java mode change 100644 => 100755 apm-sniffer/apm-sdk-plugin/postgresql-8.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/jdbc/postgresql/define/Constants.java mode change 100644 => 100755 apm-sniffer/apm-sdk-plugin/postgresql-8.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/jdbc/postgresql/define/DriverInstrumentation.java mode change 100644 => 100755 apm-sniffer/apm-sdk-plugin/postgresql-8.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/jdbc/postgresql/define/Jdbc3ConnectionInstrumentation.java mode change 100644 => 100755 apm-sniffer/apm-sdk-plugin/postgresql-8.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/jdbc/postgresql/define/Jdbc4ConnectionInstrumentation.java mode change 100644 => 100755 apm-sniffer/apm-sdk-plugin/postgresql-8.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/jdbc/postgresql/define/PgCallableStatementInstrumentation.java mode change 100644 => 100755 apm-sniffer/apm-sdk-plugin/postgresql-8.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/jdbc/postgresql/define/PgPreparedStatementInstrumentation.java mode change 100644 => 100755 apm-sniffer/apm-sdk-plugin/postgresql-8.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/jdbc/postgresql/define/PgStatementInstrumentation.java create mode 100644 apm-sniffer/apm-sdk-plugin/postgresql-8.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/jdbc/postgresql/util/ParameterUtil.java mode change 100644 => 100755 apm-sniffer/apm-sdk-plugin/postgresql-8.x-plugin/src/main/resources/skywalking-plugin.def diff --git a/apm-sniffer/apm-agent-core/src/main/java/org/apache/skywalking/apm/agent/core/conf/Config.java b/apm-sniffer/apm-agent-core/src/main/java/org/apache/skywalking/apm/agent/core/conf/Config.java index 26f0022473fd..f8db6f30c56c 100755 --- a/apm-sniffer/apm-agent-core/src/main/java/org/apache/skywalking/apm/agent/core/conf/Config.java +++ b/apm-sniffer/apm-agent-core/src/main/java/org/apache/skywalking/apm/agent/core/conf/Config.java @@ -278,7 +278,13 @@ public static class POSTGRESQL { * If set to true, the parameters of the sql (typically {@link java.sql.PreparedStatement}) would be * collected. */ - public static boolean TRACE_SQL_PARAMETERS = false; + public static boolean TRACE_SQL_PARAMETERS = true; + + /** + * For the sake of performance, limit the max number of parameter + */ + public static int SQL_PARAMETERS_MAX_COUNT = 20; + /** * For the sake of performance, SkyWalking won't save the entire parameters string into the tag, but only * the first {@code SQL_PARAMETERS_MAX_LENGTH} characters. diff --git a/apm-sniffer/apm-sdk-plugin/jdbc-commons/src/main/java/org/apache/skywalking/apm/plugin/jdbc/PSSetterDefinitionOfJDBCInstrumentation.java b/apm-sniffer/apm-sdk-plugin/jdbc-commons/src/main/java/org/apache/skywalking/apm/plugin/jdbc/PSSetterDefinitionOfJDBCInstrumentation.java index 1ab3284e87ef..7415254bedbd 100644 --- a/apm-sniffer/apm-sdk-plugin/jdbc-commons/src/main/java/org/apache/skywalking/apm/plugin/jdbc/PSSetterDefinitionOfJDBCInstrumentation.java +++ b/apm-sniffer/apm-sdk-plugin/jdbc-commons/src/main/java/org/apache/skywalking/apm/plugin/jdbc/PSSetterDefinitionOfJDBCInstrumentation.java @@ -21,7 +21,6 @@ import net.bytebuddy.description.method.MethodDescription; import net.bytebuddy.matcher.ElementMatcher; import org.apache.skywalking.apm.agent.core.conf.Config; -import org.apache.skywalking.apm.agent.core.conf.Config.Plugin.POSTGRESQL; import org.apache.skywalking.apm.agent.core.plugin.interceptor.InstanceMethodsInterceptPoint; import org.apache.skywalking.apm.plugin.jdbc.define.Constants; @@ -46,7 +45,7 @@ public PSSetterDefinitionOfJDBCInstrumentation(boolean ignorable) { public ElementMatcher getMethodsMatcher() { ElementMatcher.Junction matcher = none(); - if (Config.Plugin.MySQL.TRACE_SQL_PARAMETERS || POSTGRESQL.TRACE_SQL_PARAMETERS) { + if (Config.Plugin.MySQL.TRACE_SQL_PARAMETERS || Config.Plugin.POSTGRESQL.TRACE_SQL_PARAMETERS) { final Set setters = ignorable ? PS_IGNORABLE_SETTERS : PS_SETTERS; for (String setter : setters) { matcher = matcher.or(named(setter)); diff --git a/apm-sniffer/apm-sdk-plugin/jdbc-commons/src/main/java/org/apache/skywalking/apm/plugin/jdbc/define/StatementEnhanceInfos.java b/apm-sniffer/apm-sdk-plugin/jdbc-commons/src/main/java/org/apache/skywalking/apm/plugin/jdbc/define/StatementEnhanceInfos.java index a9983c72cb5c..0189ae0da07d 100644 --- a/apm-sniffer/apm-sdk-plugin/jdbc-commons/src/main/java/org/apache/skywalking/apm/plugin/jdbc/define/StatementEnhanceInfos.java +++ b/apm-sniffer/apm-sdk-plugin/jdbc-commons/src/main/java/org/apache/skywalking/apm/plugin/jdbc/define/StatementEnhanceInfos.java @@ -20,7 +20,12 @@ package org.apache.skywalking.apm.plugin.jdbc.define; import java.util.ArrayList; +import java.util.Arrays; +import java.util.Iterator; import java.util.List; +import java.util.Map; +import java.util.Map.Entry; +import java.util.TreeMap; import org.apache.skywalking.apm.plugin.jdbc.trace.ConnectionInfo; /** @@ -33,7 +38,9 @@ public class StatementEnhanceInfos { private ConnectionInfo connectionInfo; private String statementName; private String sql; - private List parameters = new ArrayList(); + private Object[] parameters; + private Map parameterMap = new TreeMap(); + private int maxIndex = 0; public StatementEnhanceInfos(ConnectionInfo connectionInfo, String sql, String statementName) { this.connectionInfo = connectionInfo; @@ -54,14 +61,45 @@ public String getStatementName() { } public void setParameter(int index, final Object parameter) { - // start from 1 - index--; - if (index >= 0) { - parameters.set(index, parameter); + maxIndex = maxIndex > index ? maxIndex : index; + index--; // start from 1 + if (parameters == null) { + final int initialSize = Math.max(20, maxIndex); + parameters = new Object[initialSize]; + Arrays.fill(parameters, null); } + int length = parameters.length; + if (index >= length) { + int newSize = Math.max(index + 1, length * 2); + Object[] newParameters = new Object[newSize]; + System.arraycopy(parameters, 0, newParameters, 0, length); + Arrays.fill(newParameters, length, newSize, null); + parameters = newParameters; + } + parameters[index] = parameter; + parameterMap.put(index, parameter); } - public List getParameters() { + public Object[] getParameters() { + return parameters; + } + + public int getMaxIndex() { + return maxIndex; + } + + public Map getParameterMap() { + return parameterMap; + } + + public List getSortParameters() { + List parameters = new ArrayList(); + if (parameterMap.size() > 0) { + Iterator> iterator = parameterMap.entrySet().iterator(); + while (iterator.hasNext()) { + parameters.add(iterator.next().getValue()); + } + } return parameters; } } diff --git a/apm-sniffer/apm-sdk-plugin/postgresql-8.x-plugin/pom.xml b/apm-sniffer/apm-sdk-plugin/postgresql-8.x-plugin/pom.xml index 11c781b425ab..7c7452909d22 100755 --- a/apm-sniffer/apm-sdk-plugin/postgresql-8.x-plugin/pom.xml +++ b/apm-sniffer/apm-sdk-plugin/postgresql-8.x-plugin/pom.xml @@ -33,6 +33,7 @@ UTF-8 42.0.0 + 20.0 @@ -48,6 +49,11 @@ ${project.version} provided + + com.google.guava + guava + ${guava.version} + diff --git a/apm-sniffer/apm-sdk-plugin/postgresql-8.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/jdbc/postgresql/CreateCallableStatementInterceptor.java b/apm-sniffer/apm-sdk-plugin/postgresql-8.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/jdbc/postgresql/CreateCallableStatementInterceptor.java old mode 100644 new mode 100755 diff --git a/apm-sniffer/apm-sdk-plugin/postgresql-8.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/jdbc/postgresql/CreatePreparedStatementInterceptor.java b/apm-sniffer/apm-sdk-plugin/postgresql-8.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/jdbc/postgresql/CreatePreparedStatementInterceptor.java old mode 100644 new mode 100755 diff --git a/apm-sniffer/apm-sdk-plugin/postgresql-8.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/jdbc/postgresql/CreateStatementInterceptor.java b/apm-sniffer/apm-sdk-plugin/postgresql-8.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/jdbc/postgresql/CreateStatementInterceptor.java old mode 100644 new mode 100755 diff --git a/apm-sniffer/apm-sdk-plugin/postgresql-8.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/jdbc/postgresql/JDBCPrepareStatementWithStringArrayInterceptor.java b/apm-sniffer/apm-sdk-plugin/postgresql-8.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/jdbc/postgresql/JDBCPrepareStatementWithStringArrayInterceptor.java old mode 100644 new mode 100755 diff --git a/apm-sniffer/apm-sdk-plugin/postgresql-8.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/jdbc/postgresql/PreparedStatementExecuteMethodsInterceptor.java b/apm-sniffer/apm-sdk-plugin/postgresql-8.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/jdbc/postgresql/PreparedStatementExecuteMethodsInterceptor.java old mode 100644 new mode 100755 index 7b30e41b9cc4..2ccffcd96502 --- a/apm-sniffer/apm-sdk-plugin/postgresql-8.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/jdbc/postgresql/PreparedStatementExecuteMethodsInterceptor.java +++ b/apm-sniffer/apm-sdk-plugin/postgresql-8.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/jdbc/postgresql/PreparedStatementExecuteMethodsInterceptor.java @@ -19,9 +19,10 @@ package org.apache.skywalking.apm.plugin.jdbc.postgresql; -import com.google.common.base.Joiner; import java.lang.reflect.Method; import java.util.List; +import java.util.Map; +import org.apache.skywalking.apm.agent.core.conf.Config.Plugin.POSTGRESQL; import org.apache.skywalking.apm.agent.core.context.ContextManager; import org.apache.skywalking.apm.agent.core.context.tag.Tags; import org.apache.skywalking.apm.agent.core.context.trace.AbstractSpan; @@ -30,6 +31,7 @@ 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.define.StatementEnhanceInfos; +import org.apache.skywalking.apm.plugin.jdbc.postgresql.util.ParameterUtil; import org.apache.skywalking.apm.plugin.jdbc.trace.ConnectionInfo; /** @@ -48,13 +50,15 @@ public final void beforeMethod(EnhancedInstance objInst, Method method, Object[] Tags.DB_TYPE.set(span, "sql"); Tags.DB_INSTANCE.set(span, connectInfo.getDatabaseName()); Tags.DB_STATEMENT.set(span, cacheObject.getSql()); + span.setComponent(connectInfo.getComponent()); - List parameters = cacheObject.getParameters(); - if (!parameters.isEmpty()) { - Tags.DB_BIND_VARIABLES.set(span, Joiner.on(",").join(parameters)); + Map parameterMap = cacheObject.getParameterMap(); + if (parameterMap.size() > 0) { + List parameters = cacheObject.getSortParameters(); + Tags.DB_BIND_VARIABLES.set(span, ParameterUtil + .format(parameters, POSTGRESQL.SQL_PARAMETERS_MAX_COUNT, POSTGRESQL.SQL_PARAMETERS_MAX_LENGTH)); } - span.setComponent(connectInfo.getComponent()); SpanLayer.asDB(span); } diff --git a/apm-sniffer/apm-sdk-plugin/postgresql-8.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/jdbc/postgresql/StatementExecuteMethodsInterceptor.java b/apm-sniffer/apm-sdk-plugin/postgresql-8.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/jdbc/postgresql/StatementExecuteMethodsInterceptor.java old mode 100644 new mode 100755 diff --git a/apm-sniffer/apm-sdk-plugin/postgresql-8.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/jdbc/postgresql/Variables.java b/apm-sniffer/apm-sdk-plugin/postgresql-8.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/jdbc/postgresql/Variables.java old mode 100644 new mode 100755 diff --git a/apm-sniffer/apm-sdk-plugin/postgresql-8.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/jdbc/postgresql/define/AbstractJdbc2StatementInstrumentation.java b/apm-sniffer/apm-sdk-plugin/postgresql-8.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/jdbc/postgresql/define/AbstractJdbc2StatementInstrumentation.java old mode 100644 new mode 100755 diff --git a/apm-sniffer/apm-sdk-plugin/postgresql-8.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/jdbc/postgresql/define/ConnectionInstrumentation.java b/apm-sniffer/apm-sdk-plugin/postgresql-8.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/jdbc/postgresql/define/ConnectionInstrumentation.java old mode 100644 new mode 100755 diff --git a/apm-sniffer/apm-sdk-plugin/postgresql-8.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/jdbc/postgresql/define/Constants.java b/apm-sniffer/apm-sdk-plugin/postgresql-8.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/jdbc/postgresql/define/Constants.java old mode 100644 new mode 100755 index 662d12c1bfb4..cda27df745f2 --- a/apm-sniffer/apm-sdk-plugin/postgresql-8.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/jdbc/postgresql/define/Constants.java +++ b/apm-sniffer/apm-sdk-plugin/postgresql-8.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/jdbc/postgresql/define/Constants.java @@ -28,5 +28,4 @@ public class Constants { public static final String CREATE_STATEMENT_INTERCEPTOR_CLASS = "org.apache.skywalking.apm.plugin.jdbc.postgresql.CreateStatementInterceptor"; public static final String CREATE_PREPARED_STATEMENT_INTERCEPTOR_CLASS = "org.apache.skywalking.apm.plugin.jdbc.postgresql.CreatePreparedStatementInterceptor"; public static final String CREATE_CALLABLE_STATEMENT_INTERCEPTOR_CLASS = "org.apache.skywalking.apm.plugin.jdbc.postgresql.CreateCallableStatementInterceptor"; - } diff --git a/apm-sniffer/apm-sdk-plugin/postgresql-8.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/jdbc/postgresql/define/DriverInstrumentation.java b/apm-sniffer/apm-sdk-plugin/postgresql-8.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/jdbc/postgresql/define/DriverInstrumentation.java old mode 100644 new mode 100755 diff --git a/apm-sniffer/apm-sdk-plugin/postgresql-8.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/jdbc/postgresql/define/Jdbc3ConnectionInstrumentation.java b/apm-sniffer/apm-sdk-plugin/postgresql-8.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/jdbc/postgresql/define/Jdbc3ConnectionInstrumentation.java old mode 100644 new mode 100755 diff --git a/apm-sniffer/apm-sdk-plugin/postgresql-8.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/jdbc/postgresql/define/Jdbc4ConnectionInstrumentation.java b/apm-sniffer/apm-sdk-plugin/postgresql-8.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/jdbc/postgresql/define/Jdbc4ConnectionInstrumentation.java old mode 100644 new mode 100755 diff --git a/apm-sniffer/apm-sdk-plugin/postgresql-8.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/jdbc/postgresql/define/PgCallableStatementInstrumentation.java b/apm-sniffer/apm-sdk-plugin/postgresql-8.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/jdbc/postgresql/define/PgCallableStatementInstrumentation.java old mode 100644 new mode 100755 diff --git a/apm-sniffer/apm-sdk-plugin/postgresql-8.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/jdbc/postgresql/define/PgPreparedStatementInstrumentation.java b/apm-sniffer/apm-sdk-plugin/postgresql-8.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/jdbc/postgresql/define/PgPreparedStatementInstrumentation.java old mode 100644 new mode 100755 diff --git a/apm-sniffer/apm-sdk-plugin/postgresql-8.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/jdbc/postgresql/define/PgPreparedStatementSetterInstrumentation.java b/apm-sniffer/apm-sdk-plugin/postgresql-8.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/jdbc/postgresql/define/PgPreparedStatementSetterInstrumentation.java index 972d07a695bf..3893b43d09ce 100644 --- a/apm-sniffer/apm-sdk-plugin/postgresql-8.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/jdbc/postgresql/define/PgPreparedStatementSetterInstrumentation.java +++ b/apm-sniffer/apm-sdk-plugin/postgresql-8.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/jdbc/postgresql/define/PgPreparedStatementSetterInstrumentation.java @@ -20,6 +20,9 @@ import org.apache.skywalking.apm.agent.core.plugin.interceptor.InstanceMethodsInterceptPoint; import org.apache.skywalking.apm.plugin.jdbc.PSSetterDefinitionOfJDBCInstrumentation; +/** + * @author aderm + */ public class PgPreparedStatementSetterInstrumentation extends PgPreparedStatementInstrumentation { @Override diff --git a/apm-sniffer/apm-sdk-plugin/postgresql-8.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/jdbc/postgresql/define/PgStatementInstrumentation.java b/apm-sniffer/apm-sdk-plugin/postgresql-8.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/jdbc/postgresql/define/PgStatementInstrumentation.java old mode 100644 new mode 100755 diff --git a/apm-sniffer/apm-sdk-plugin/postgresql-8.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/jdbc/postgresql/util/ParameterUtil.java b/apm-sniffer/apm-sdk-plugin/postgresql-8.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/jdbc/postgresql/util/ParameterUtil.java new file mode 100644 index 000000000000..22c5b85f48e6 --- /dev/null +++ b/apm-sniffer/apm-sdk-plugin/postgresql-8.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/jdbc/postgresql/util/ParameterUtil.java @@ -0,0 +1,51 @@ +/* + * 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.jdbc.postgresql.util; + +import com.google.common.base.Joiner; +import java.util.Arrays; +import java.util.List; + +/** + * @author aderm + */ +public class ParameterUtil { + + public static String format(Object[] params, int limitCnt, int limitLength) { + return format(Arrays.asList(params), limitCnt, limitLength); + } + + public static String format(List params, int limitCnt, int limitLength) { + boolean overLoad = false; + int actualCnt = params.size(); + if (params.size() > limitCnt) { + overLoad = true; + actualCnt = limitCnt; + } + + String paramStr = Joiner.on(",").join(params.subList(0, actualCnt)); + if (paramStr.length() > limitLength) { + overLoad = true; + } + + if (overLoad) { + return "[" + paramStr + "..." + "]"; + } + return "[" + paramStr + "]"; + } +} diff --git a/apm-sniffer/apm-sdk-plugin/postgresql-8.x-plugin/src/main/resources/skywalking-plugin.def b/apm-sniffer/apm-sdk-plugin/postgresql-8.x-plugin/src/main/resources/skywalking-plugin.def old mode 100644 new mode 100755 From 36a5adc5d483f4f1fb45b5ae75a417b0af57ecd7 Mon Sep 17 00:00:00 2001 From: aderm Date: Sun, 27 Oct 2019 01:33:09 +0800 Subject: [PATCH 06/13] reverse mysql param. --- ...PreparedStatementExecuteMethodsInterceptor.java | 14 +++++++------- 1 file changed, 7 insertions(+), 7 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 73a405c5866b..732816c07d08 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 @@ -18,7 +18,6 @@ package org.apache.skywalking.apm.plugin.jdbc.mysql; -import java.util.List; import org.apache.skywalking.apm.agent.core.conf.Config; import org.apache.skywalking.apm.agent.core.context.ContextManager; import org.apache.skywalking.apm.agent.core.context.tag.Tags; @@ -57,9 +56,10 @@ public final void beforeMethod(EnhancedInstance objInst, Method method, Object[] span.setComponent(connectInfo.getComponent()); if (Config.Plugin.MySQL.TRACE_SQL_PARAMETERS) { - List parameters = cacheObject.getParameters(); - if (parameters != null && parameters.size() > 0) { - String parameterString = buildParameterString(parameters); + final Object[] parameters = cacheObject.getParameters(); + if (parameters != null && parameters.length > 0) { + int maxIndex = cacheObject.getMaxIndex(); + String parameterString = buildParameterString(parameters, maxIndex); int sqlParametersMaxLength = Config.Plugin.MySQL.SQL_PARAMETERS_MAX_LENGTH; if (sqlParametersMaxLength > 0 && parameterString.length() > sqlParametersMaxLength) { parameterString = parameterString.substring(0, sqlParametersMaxLength) + "..."; @@ -95,11 +95,11 @@ private String buildOperationName(ConnectionInfo connectionInfo, String methodNa return connectionInfo.getDBType() + "/JDBI/" + statementName + "/" + methodName; } - private String buildParameterString(List parameters) { + private String buildParameterString(Object[] parameters, int maxIndex) { String parameterString = "["; boolean first = true; - for (int i = 0; i < parameters.size(); i++) { - Object parameter = parameters.get(i); + for (int i = 0; i < maxIndex; i++) { + Object parameter = parameters[i]; if (!first) { parameterString += ","; } From 8f88d6041f2aeedce36b423851dd1efe0c488c17 Mon Sep 17 00:00:00 2001 From: aderm Date: Sun, 27 Oct 2019 01:42:14 +0800 Subject: [PATCH 07/13] add limit length. --- .../apm/plugin/jdbc/postgresql/util/ParameterUtil.java | 1 + 1 file changed, 1 insertion(+) diff --git a/apm-sniffer/apm-sdk-plugin/postgresql-8.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/jdbc/postgresql/util/ParameterUtil.java b/apm-sniffer/apm-sdk-plugin/postgresql-8.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/jdbc/postgresql/util/ParameterUtil.java index 22c5b85f48e6..dc2839cf24ac 100644 --- a/apm-sniffer/apm-sdk-plugin/postgresql-8.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/jdbc/postgresql/util/ParameterUtil.java +++ b/apm-sniffer/apm-sdk-plugin/postgresql-8.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/jdbc/postgresql/util/ParameterUtil.java @@ -41,6 +41,7 @@ public static String format(List params, int limitCnt, int limitLength) String paramStr = Joiner.on(",").join(params.subList(0, actualCnt)); if (paramStr.length() > limitLength) { overLoad = true; + paramStr = paramStr.substring(0, limitLength); } if (overLoad) { From 770fa9b1d6d5c0a0d5b489cb463f25b8ec94a46f Mon Sep 17 00:00:00 2001 From: aderm Date: Sun, 27 Oct 2019 08:44:45 +0800 Subject: [PATCH 08/13] add test pg expect data db.bind_vars --- .../postgresql-above9.4.1207-scenario/config/expectedData.yaml | 1 + 1 file changed, 1 insertion(+) diff --git a/test/plugin/scenarios/postgresql-above9.4.1207-scenario/config/expectedData.yaml b/test/plugin/scenarios/postgresql-above9.4.1207-scenario/config/expectedData.yaml index efa343ecf18d..af073c15261a 100644 --- a/test/plugin/scenarios/postgresql-above9.4.1207-scenario/config/expectedData.yaml +++ b/test/plugin/scenarios/postgresql-above9.4.1207-scenario/config/expectedData.yaml @@ -53,6 +53,7 @@ segmentItems: - {key: "db.type", value: "sql"} - {key: "db.instance", value: "postgres"} - {key: "db.statement", value: "INSERT INTO test_007(id, value) VALUES(?,?)"} + - {key: "db.bind_vars", value: "[1,1]"} startTime: nq 0 endTime: nq 0 isError: false From 1c0bc80ed4743030b417740ec21f122ac693c20d Mon Sep 17 00:00:00 2001 From: aderm Date: Sun, 27 Oct 2019 21:19:48 +0800 Subject: [PATCH 09/13] 1.change parameter storage.2.update doc. --- .../apm/agent/core/conf/Config.java | 7 +-- .../jdbc/define/StatementEnhanceInfos.java | 19 +------ .../postgresql-8.x-plugin/pom.xml | 6 --- ...redStatementExecuteMethodsInterceptor.java | 40 ++++++++++---- .../jdbc/postgresql/util/ParameterUtil.java | 52 ------------------- .../setup/service-agent/java-agent/README.md | 2 + 6 files changed, 35 insertions(+), 91 deletions(-) delete mode 100644 apm-sniffer/apm-sdk-plugin/postgresql-8.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/jdbc/postgresql/util/ParameterUtil.java diff --git a/apm-sniffer/apm-agent-core/src/main/java/org/apache/skywalking/apm/agent/core/conf/Config.java b/apm-sniffer/apm-agent-core/src/main/java/org/apache/skywalking/apm/agent/core/conf/Config.java index f8db6f30c56c..18280dfa3160 100755 --- a/apm-sniffer/apm-agent-core/src/main/java/org/apache/skywalking/apm/agent/core/conf/Config.java +++ b/apm-sniffer/apm-agent-core/src/main/java/org/apache/skywalking/apm/agent/core/conf/Config.java @@ -278,12 +278,7 @@ public static class POSTGRESQL { * If set to true, the parameters of the sql (typically {@link java.sql.PreparedStatement}) would be * collected. */ - public static boolean TRACE_SQL_PARAMETERS = true; - - /** - * For the sake of performance, limit the max number of parameter - */ - public static int SQL_PARAMETERS_MAX_COUNT = 20; + public static boolean TRACE_SQL_PARAMETERS = false; /** * For the sake of performance, SkyWalking won't save the entire parameters string into the tag, but only diff --git a/apm-sniffer/apm-sdk-plugin/jdbc-commons/src/main/java/org/apache/skywalking/apm/plugin/jdbc/define/StatementEnhanceInfos.java b/apm-sniffer/apm-sdk-plugin/jdbc-commons/src/main/java/org/apache/skywalking/apm/plugin/jdbc/define/StatementEnhanceInfos.java index 0189ae0da07d..632f9703bd51 100644 --- a/apm-sniffer/apm-sdk-plugin/jdbc-commons/src/main/java/org/apache/skywalking/apm/plugin/jdbc/define/StatementEnhanceInfos.java +++ b/apm-sniffer/apm-sdk-plugin/jdbc-commons/src/main/java/org/apache/skywalking/apm/plugin/jdbc/define/StatementEnhanceInfos.java @@ -39,7 +39,6 @@ public class StatementEnhanceInfos { private String statementName; private String sql; private Object[] parameters; - private Map parameterMap = new TreeMap(); private int maxIndex = 0; public StatementEnhanceInfos(ConnectionInfo connectionInfo, String sql, String statementName) { @@ -64,7 +63,7 @@ public void setParameter(int index, final Object parameter) { maxIndex = maxIndex > index ? maxIndex : index; index--; // start from 1 if (parameters == null) { - final int initialSize = Math.max(20, maxIndex); + final int initialSize = Math.max(16, maxIndex); parameters = new Object[initialSize]; Arrays.fill(parameters, null); } @@ -77,7 +76,6 @@ public void setParameter(int index, final Object parameter) { parameters = newParameters; } parameters[index] = parameter; - parameterMap.put(index, parameter); } public Object[] getParameters() { @@ -87,19 +85,4 @@ public Object[] getParameters() { public int getMaxIndex() { return maxIndex; } - - public Map getParameterMap() { - return parameterMap; - } - - public List getSortParameters() { - List parameters = new ArrayList(); - if (parameterMap.size() > 0) { - Iterator> iterator = parameterMap.entrySet().iterator(); - while (iterator.hasNext()) { - parameters.add(iterator.next().getValue()); - } - } - return parameters; - } } diff --git a/apm-sniffer/apm-sdk-plugin/postgresql-8.x-plugin/pom.xml b/apm-sniffer/apm-sdk-plugin/postgresql-8.x-plugin/pom.xml index 7c7452909d22..11c781b425ab 100755 --- a/apm-sniffer/apm-sdk-plugin/postgresql-8.x-plugin/pom.xml +++ b/apm-sniffer/apm-sdk-plugin/postgresql-8.x-plugin/pom.xml @@ -33,7 +33,6 @@ UTF-8 42.0.0 - 20.0 @@ -49,11 +48,6 @@ ${project.version} provided - - com.google.guava - guava - ${guava.version} - diff --git a/apm-sniffer/apm-sdk-plugin/postgresql-8.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/jdbc/postgresql/PreparedStatementExecuteMethodsInterceptor.java b/apm-sniffer/apm-sdk-plugin/postgresql-8.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/jdbc/postgresql/PreparedStatementExecuteMethodsInterceptor.java index 2ccffcd96502..6686e6be0602 100755 --- a/apm-sniffer/apm-sdk-plugin/postgresql-8.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/jdbc/postgresql/PreparedStatementExecuteMethodsInterceptor.java +++ b/apm-sniffer/apm-sdk-plugin/postgresql-8.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/jdbc/postgresql/PreparedStatementExecuteMethodsInterceptor.java @@ -20,10 +20,9 @@ package org.apache.skywalking.apm.plugin.jdbc.postgresql; import java.lang.reflect.Method; -import java.util.List; -import java.util.Map; -import org.apache.skywalking.apm.agent.core.conf.Config.Plugin.POSTGRESQL; +import org.apache.skywalking.apm.agent.core.conf.Config; import org.apache.skywalking.apm.agent.core.context.ContextManager; +import org.apache.skywalking.apm.agent.core.context.tag.StringTag; import org.apache.skywalking.apm.agent.core.context.tag.Tags; import org.apache.skywalking.apm.agent.core.context.trace.AbstractSpan; import org.apache.skywalking.apm.agent.core.context.trace.SpanLayer; @@ -31,7 +30,6 @@ 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.define.StatementEnhanceInfos; -import org.apache.skywalking.apm.plugin.jdbc.postgresql.util.ParameterUtil; import org.apache.skywalking.apm.plugin.jdbc.trace.ConnectionInfo; /** @@ -40,6 +38,9 @@ * @author zhangxin */ public class PreparedStatementExecuteMethodsInterceptor implements InstanceMethodsAroundInterceptor { + + public static final StringTag SQL_PARAMETERS = new StringTag("db.sql.parameters"); + @Override public final void beforeMethod(EnhancedInstance objInst, Method method, Object[] allArguments, Class[] argumentsTypes, @@ -52,11 +53,17 @@ public final void beforeMethod(EnhancedInstance objInst, Method method, Object[] Tags.DB_STATEMENT.set(span, cacheObject.getSql()); span.setComponent(connectInfo.getComponent()); - Map parameterMap = cacheObject.getParameterMap(); - if (parameterMap.size() > 0) { - List parameters = cacheObject.getSortParameters(); - Tags.DB_BIND_VARIABLES.set(span, ParameterUtil - .format(parameters, POSTGRESQL.SQL_PARAMETERS_MAX_COUNT, POSTGRESQL.SQL_PARAMETERS_MAX_LENGTH)); + if (Config.Plugin.POSTGRESQL.TRACE_SQL_PARAMETERS) { + final Object[] parameters = cacheObject.getParameters(); + if (parameters != null && parameters.length > 0) { + int maxIndex = cacheObject.getMaxIndex(); + String parameterString = buildParameterString(parameters, maxIndex); + int sqlParametersMaxLength = Config.Plugin.MySQL.SQL_PARAMETERS_MAX_LENGTH; + if (sqlParametersMaxLength > 0 && parameterString.length() > sqlParametersMaxLength) { + parameterString = parameterString.substring(0, sqlParametersMaxLength) + "..." + "]"; + } + SQL_PARAMETERS.set(span, parameterString); + } } SpanLayer.asDB(span); @@ -84,4 +91,19 @@ public final Object afterMethod(EnhancedInstance objInst, Method method, Object[ private String buildOperationName(ConnectionInfo connectionInfo, String methodName, String statementName) { return connectionInfo.getDBType() + "/JDBI/" + statementName + "/" + methodName; } + + private String buildParameterString(Object[] parameters, int maxIndex) { + String parameterString = "["; + boolean first = true; + for (int i = 0; i < maxIndex; i++) { + Object parameter = parameters[i]; + if (!first) { + parameterString += ","; + } + parameterString += parameter; + first = false; + } + parameterString += "]"; + return parameterString; + } } diff --git a/apm-sniffer/apm-sdk-plugin/postgresql-8.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/jdbc/postgresql/util/ParameterUtil.java b/apm-sniffer/apm-sdk-plugin/postgresql-8.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/jdbc/postgresql/util/ParameterUtil.java deleted file mode 100644 index dc2839cf24ac..000000000000 --- a/apm-sniffer/apm-sdk-plugin/postgresql-8.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/jdbc/postgresql/util/ParameterUtil.java +++ /dev/null @@ -1,52 +0,0 @@ -/* - * 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.jdbc.postgresql.util; - -import com.google.common.base.Joiner; -import java.util.Arrays; -import java.util.List; - -/** - * @author aderm - */ -public class ParameterUtil { - - public static String format(Object[] params, int limitCnt, int limitLength) { - return format(Arrays.asList(params), limitCnt, limitLength); - } - - public static String format(List params, int limitCnt, int limitLength) { - boolean overLoad = false; - int actualCnt = params.size(); - if (params.size() > limitCnt) { - overLoad = true; - actualCnt = limitCnt; - } - - String paramStr = Joiner.on(",").join(params.subList(0, actualCnt)); - if (paramStr.length() > limitLength) { - overLoad = true; - paramStr = paramStr.substring(0, limitLength); - } - - if (overLoad) { - return "[" + paramStr + "..." + "]"; - } - return "[" + paramStr + "]"; - } -} diff --git a/docs/en/setup/service-agent/java-agent/README.md b/docs/en/setup/service-agent/java-agent/README.md index e5586140a738..979837a4792d 100755 --- a/docs/en/setup/service-agent/java-agent/README.md +++ b/docs/en/setup/service-agent/java-agent/README.md @@ -102,6 +102,8 @@ property key | Description | Default | `plugin.toolit.use_qualified_name_as_operation_name`|If true, the fully qualified method name will be used as the operation name instead of the given operation name, default is false.|`false`| `plugin.mysql.trace_sql_parameters`|If set to true, the parameters of the sql (typically `java.sql.PreparedStatement`) would be collected.|`false`| `plugin.mysql.sql_parameters_max_length`|If set to positive number, the `db.sql.parameters` would be truncated to this length, otherwise it would be completely saved, which may cause performance problem.|`512`| +`plugin.postgresql.trace_sql_parameters`|If set to true, the parameters of the sql (typically `java.sql.PreparedStatement`) would be collected.|`false`| +`plugin.postgresql.sql_parameters_max_length`|If set to positive number, the `db.sql.parameters` would be truncated to this length, otherwise it would be completely saved, which may cause performance problem.|`512`| `plugin.solrj.trace_statement`|If true, trace all the query parameters(include deleteByIds and deleteByQuery) in Solr query request, default is false.|`false`| `plugin.solrj.trace_ops_params`|If true, trace all the operation parameters in Solr request, default is false.|`false`| `plugin.light4j.trace_handler_chain`|If true, trace all middleware/business handlers that are part of the Light4J handler chain for a request.|false| From 7752e582c4ad93d2d6c9d50bae9ef0eea6d7632b Mon Sep 17 00:00:00 2001 From: aderm Date: Sun, 27 Oct 2019 21:35:36 +0800 Subject: [PATCH 10/13] remove unused import --- .../apm/plugin/jdbc/define/StatementEnhanceInfos.java | 6 ------ 1 file changed, 6 deletions(-) diff --git a/apm-sniffer/apm-sdk-plugin/jdbc-commons/src/main/java/org/apache/skywalking/apm/plugin/jdbc/define/StatementEnhanceInfos.java b/apm-sniffer/apm-sdk-plugin/jdbc-commons/src/main/java/org/apache/skywalking/apm/plugin/jdbc/define/StatementEnhanceInfos.java index 632f9703bd51..5c4a0a195f87 100644 --- a/apm-sniffer/apm-sdk-plugin/jdbc-commons/src/main/java/org/apache/skywalking/apm/plugin/jdbc/define/StatementEnhanceInfos.java +++ b/apm-sniffer/apm-sdk-plugin/jdbc-commons/src/main/java/org/apache/skywalking/apm/plugin/jdbc/define/StatementEnhanceInfos.java @@ -19,13 +19,7 @@ package org.apache.skywalking.apm.plugin.jdbc.define; -import java.util.ArrayList; import java.util.Arrays; -import java.util.Iterator; -import java.util.List; -import java.util.Map; -import java.util.Map.Entry; -import java.util.TreeMap; import org.apache.skywalking.apm.plugin.jdbc.trace.ConnectionInfo; /** From 2f5ca6a7a1012eaec2b804b2b36c0215c54c6a3b Mon Sep 17 00:00:00 2001 From: aderm Date: Sun, 27 Oct 2019 22:17:56 +0800 Subject: [PATCH 11/13] Add pg sql param test. --- .../scenarios/postgresql-above9.4.1207-scenario/bin/startup.sh | 2 +- .../postgresql-above9.4.1207-scenario/config/expectedData.yaml | 2 +- test/plugin/scenarios/postgresql-scenario/bin/startup.sh | 2 +- .../scenarios/postgresql-scenario/config/expectedData.yaml | 1 + 4 files changed, 4 insertions(+), 3 deletions(-) diff --git a/test/plugin/scenarios/postgresql-above9.4.1207-scenario/bin/startup.sh b/test/plugin/scenarios/postgresql-above9.4.1207-scenario/bin/startup.sh index b5f8537b0aec..ff5042d00303 100644 --- a/test/plugin/scenarios/postgresql-above9.4.1207-scenario/bin/startup.sh +++ b/test/plugin/scenarios/postgresql-above9.4.1207-scenario/bin/startup.sh @@ -17,4 +17,4 @@ home="$(cd "$(dirname $0)"; pwd)" -java -jar ${agent_opts} ${home}/../libs/postgresql-above9.4.1207-scenario.jar & +java -jar ${agent_opts} -Dskywalking.plugin.postgresql.trace_sql_parameters=true ${home}/../libs/postgresql-above9.4.1207-scenario.jar & diff --git a/test/plugin/scenarios/postgresql-above9.4.1207-scenario/config/expectedData.yaml b/test/plugin/scenarios/postgresql-above9.4.1207-scenario/config/expectedData.yaml index af073c15261a..bb5260be15ce 100644 --- a/test/plugin/scenarios/postgresql-above9.4.1207-scenario/config/expectedData.yaml +++ b/test/plugin/scenarios/postgresql-above9.4.1207-scenario/config/expectedData.yaml @@ -53,7 +53,7 @@ segmentItems: - {key: "db.type", value: "sql"} - {key: "db.instance", value: "postgres"} - {key: "db.statement", value: "INSERT INTO test_007(id, value) VALUES(?,?)"} - - {key: "db.bind_vars", value: "[1,1]"} + - {key: "db.sql.parameters", value: "[1,1]"} startTime: nq 0 endTime: nq 0 isError: false diff --git a/test/plugin/scenarios/postgresql-scenario/bin/startup.sh b/test/plugin/scenarios/postgresql-scenario/bin/startup.sh index 74ce435ded1e..9b42650683fa 100644 --- a/test/plugin/scenarios/postgresql-scenario/bin/startup.sh +++ b/test/plugin/scenarios/postgresql-scenario/bin/startup.sh @@ -17,4 +17,4 @@ home="$(cd "$(dirname $0)"; pwd)" -java -jar ${agent_opts} ${home}/../libs/postgresql-scenario.jar & +java -jar ${agent_opts} -Dskywalking.plugin.postgresql.trace_sql_parameters=true ${home}/../libs/postgresql-scenario.jar & diff --git a/test/plugin/scenarios/postgresql-scenario/config/expectedData.yaml b/test/plugin/scenarios/postgresql-scenario/config/expectedData.yaml index 9fa8c2339617..68d69b2db53c 100644 --- a/test/plugin/scenarios/postgresql-scenario/config/expectedData.yaml +++ b/test/plugin/scenarios/postgresql-scenario/config/expectedData.yaml @@ -54,6 +54,7 @@ segmentItems: - {key: "db.type", value: "sql"} - {key: "db.instance", value: "postgres"} - {key: "db.statement", value: "INSERT INTO test_007(id, value) VALUES(?,?)"} + - {key: "db.sql.parameters", value: "[1,1]"} startTime: nq 0 endTime: nq 0 isError: false From 090e59ff30f896b6b82f6af1c07fbec941314f16 Mon Sep 17 00:00:00 2001 From: aderm Date: Sun, 27 Oct 2019 22:35:11 +0800 Subject: [PATCH 12/13] fix -D error. --- .../scenarios/postgresql-above9.4.1207-scenario/bin/startup.sh | 2 +- test/plugin/scenarios/postgresql-scenario/bin/startup.sh | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/test/plugin/scenarios/postgresql-above9.4.1207-scenario/bin/startup.sh b/test/plugin/scenarios/postgresql-above9.4.1207-scenario/bin/startup.sh index ff5042d00303..8a7ef2d3e472 100644 --- a/test/plugin/scenarios/postgresql-above9.4.1207-scenario/bin/startup.sh +++ b/test/plugin/scenarios/postgresql-above9.4.1207-scenario/bin/startup.sh @@ -17,4 +17,4 @@ home="$(cd "$(dirname $0)"; pwd)" -java -jar ${agent_opts} -Dskywalking.plugin.postgresql.trace_sql_parameters=true ${home}/../libs/postgresql-above9.4.1207-scenario.jar & +java -jar -Dskywalking.plugin.postgresql.trace_sql_parameters=true ${agent_opts} ${home}/../libs/postgresql-above9.4.1207-scenario.jar & diff --git a/test/plugin/scenarios/postgresql-scenario/bin/startup.sh b/test/plugin/scenarios/postgresql-scenario/bin/startup.sh index 9b42650683fa..b8b096c4f70e 100644 --- a/test/plugin/scenarios/postgresql-scenario/bin/startup.sh +++ b/test/plugin/scenarios/postgresql-scenario/bin/startup.sh @@ -17,4 +17,4 @@ home="$(cd "$(dirname $0)"; pwd)" -java -jar ${agent_opts} -Dskywalking.plugin.postgresql.trace_sql_parameters=true ${home}/../libs/postgresql-scenario.jar & +java -jar -Dskywalking.plugin.postgresql.trace_sql_parameters=true ${agent_opts} ${home}/../libs/postgresql-scenario.jar & From 85737d7735a70d0e205e070dc76fa529145b42f4 Mon Sep 17 00:00:00 2001 From: aderm Date: Sun, 27 Oct 2019 22:45:53 +0800 Subject: [PATCH 13/13] remove old pg version test sql param. --- .../scenarios/postgresql-above9.4.1207-scenario/bin/startup.sh | 2 +- test/plugin/scenarios/postgresql-scenario/bin/startup.sh | 2 +- .../scenarios/postgresql-scenario/config/expectedData.yaml | 1 - 3 files changed, 2 insertions(+), 3 deletions(-) diff --git a/test/plugin/scenarios/postgresql-above9.4.1207-scenario/bin/startup.sh b/test/plugin/scenarios/postgresql-above9.4.1207-scenario/bin/startup.sh index 8a7ef2d3e472..ff5042d00303 100644 --- a/test/plugin/scenarios/postgresql-above9.4.1207-scenario/bin/startup.sh +++ b/test/plugin/scenarios/postgresql-above9.4.1207-scenario/bin/startup.sh @@ -17,4 +17,4 @@ home="$(cd "$(dirname $0)"; pwd)" -java -jar -Dskywalking.plugin.postgresql.trace_sql_parameters=true ${agent_opts} ${home}/../libs/postgresql-above9.4.1207-scenario.jar & +java -jar ${agent_opts} -Dskywalking.plugin.postgresql.trace_sql_parameters=true ${home}/../libs/postgresql-above9.4.1207-scenario.jar & diff --git a/test/plugin/scenarios/postgresql-scenario/bin/startup.sh b/test/plugin/scenarios/postgresql-scenario/bin/startup.sh index b8b096c4f70e..74ce435ded1e 100644 --- a/test/plugin/scenarios/postgresql-scenario/bin/startup.sh +++ b/test/plugin/scenarios/postgresql-scenario/bin/startup.sh @@ -17,4 +17,4 @@ home="$(cd "$(dirname $0)"; pwd)" -java -jar -Dskywalking.plugin.postgresql.trace_sql_parameters=true ${agent_opts} ${home}/../libs/postgresql-scenario.jar & +java -jar ${agent_opts} ${home}/../libs/postgresql-scenario.jar & diff --git a/test/plugin/scenarios/postgresql-scenario/config/expectedData.yaml b/test/plugin/scenarios/postgresql-scenario/config/expectedData.yaml index 68d69b2db53c..9fa8c2339617 100644 --- a/test/plugin/scenarios/postgresql-scenario/config/expectedData.yaml +++ b/test/plugin/scenarios/postgresql-scenario/config/expectedData.yaml @@ -54,7 +54,6 @@ segmentItems: - {key: "db.type", value: "sql"} - {key: "db.instance", value: "postgres"} - {key: "db.statement", value: "INSERT INTO test_007(id, value) VALUES(?,?)"} - - {key: "db.sql.parameters", value: "[1,1]"} startTime: nq 0 endTime: nq 0 isError: false