Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -273,6 +273,22 @@ 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 = true;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Default value should be false due to possible performance issue

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How much impact on performance?


/**
* 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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,13 @@ public ElementMatcher<MethodDescription> getMethodsMatcher() {
}
}

if (Config.Plugin.PostgreSQL.TRACE_SQL_PARAMETERS) {
final Set<String> setters = ignorable ? PS_IGNORABLE_SETTERS : PS_SETTERS;
for (String setter : setters) {
matcher = matcher.or(named(setter));
}
}

Comment on lines +55 to +61
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Simply collapse this to line 48

        if (Config.Plugin.MySQL.TRACE_SQL_PARAMETERS || Config.Plugin.PostgreSQL.TRACE_SQL_PARAMETERS) {
            final Set<String> setters = ignorable ? PS_IGNORABLE_SETTERS : PS_SETTERS;
            for (String setter : setters) {
                matcher = matcher.or(named(setter));
             }
         }

return matcher;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,22 @@ public void setParameter(int index, final Object parameter) {
parameters[index] = parameter;
}

public String getFullSql() {
StringBuilder resultSql = new StringBuilder();
int index = 0;
int startPos = 0;
int findPos = 0;
while ((findPos = sql.indexOf("?", startPos)) > 0) {
resultSql.append(sql.substring(startPos, findPos));
resultSql.append("'");
resultSql.append(parameters[index++]);
Comment on lines +81 to +84
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's unsafe to parse the SQL using ?, are there possibilities when the sql includes ? itself in the value, not a parameter placeholder?

select * from news_table where author=? and title like '%?', I know it's a little tricky, just to remind the risk here

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My point is not to try to parse the SQL, store the parameters in another tag

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Parsing SQL is the next generation after ShardingShpere ( @tristaZero and @terrymanu ) provides the real SQL Parser. We don't do this by ourselves.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@wu-sheng @kezhenxu94 ACK,i will store the parameters in another tag.

resultSql.append("'");
startPos = findPos + 1;
}
resultSql.append(sql.substring(startPos));
return resultSql.toString();
}

public Object[] getParameters() {
return parameters;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ public final void beforeMethod(EnhancedInstance objInst, Method method, Object[]
AbstractSpan span = ContextManager.createExitSpan(buildOperationName(connectInfo, method.getName(), cacheObject.getStatementName()), connectInfo.getDatabasePeer());
Tags.DB_TYPE.set(span, "sql");
Tags.DB_INSTANCE.set(span, connectInfo.getDatabaseName());
Tags.DB_STATEMENT.set(span, cacheObject.getSql());
Tags.DB_STATEMENT.set(span, cacheObject.getFullSql());
span.setComponent(connectInfo.getComponent());

SpanLayer.asDB(span);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
/*
* 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;

/**
* @author aderm
*/
public class PgPreparedStatementSetterInstrumentation extends PgPreparedStatementInstrumentation {

@Override
public final InstanceMethodsInterceptPoint[] getInstanceMethodsInterceptPoints() {
return new InstanceMethodsInterceptPoint[] {
new PSSetterDefinitionOfJDBCInstrumentation(false)
};
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -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