Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Normalize sql statements to elide literal numbers and strings. #366

Merged
merged 9 commits into from
May 4, 2020
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,9 @@ public class Config {
private static final String DEFAULT_TRACE_EXECUTORS = "";
private static final String DEFAULT_TRACE_METHODS = null;

public static final String DISABLE_SQL_NORMALIZER = "disable.sql.normalizer";
public static final boolean DEFAULT_DISABLE_SQL_NORMALIZER = false;
johnbley marked this conversation as resolved.
Show resolved Hide resolved

@Getter private final String exporterJar;
@Getter private final String serviceName;
@Getter private final boolean traceEnabled;
Expand Down Expand Up @@ -133,6 +136,8 @@ public class Config {
@Getter private final boolean traceExecutorsAll;
@Getter private final List<String> traceExecutors;

@Getter private final boolean disableSqlNormalizer;

// Values from an optionally provided properties file
private static Properties propertiesFromConfigFile;

Expand Down Expand Up @@ -189,6 +194,9 @@ public class Config {

traceExecutors = getListSettingFromEnvironment(TRACE_EXECUTORS, DEFAULT_TRACE_EXECUTORS);

disableSqlNormalizer =
getBooleanSettingFromEnvironment(DISABLE_SQL_NORMALIZER, DEFAULT_DISABLE_SQL_NORMALIZER);

log.debug("New instance: {}", this);
}

Expand Down Expand Up @@ -243,6 +251,9 @@ private Config(final Properties properties, final Config parent) {
getPropertyBooleanValue(properties, TRACE_EXECUTORS_ALL, parent.traceExecutorsAll);
traceExecutors = getPropertyListValue(properties, TRACE_EXECUTORS, parent.traceExecutors);

disableSqlNormalizer =
getPropertyBooleanValue(properties, DISABLE_SQL_NORMALIZER, DEFAULT_DISABLE_SQL_NORMALIZER);

log.debug("New instance: {}", this);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -222,15 +222,31 @@ public static List<Reference.Mismatch> checkMatch(
return mismatches;
}

private static boolean matchesPrimitive(String longName, String shortName) {
// The two meta type systems in use here differ in their treatment of primitive type names....
return shortName.equals("I") && longName.equals(int.class.getName())
|| shortName.equals("C") && longName.equals(char.class.getName())
|| shortName.equals("Z") && longName.equals(boolean.class.getName())
|| shortName.equals("J") && longName.equals(long.class.getName())
|| shortName.equals("S") && longName.equals(short.class.getName())
|| shortName.equals("F") && longName.equals(float.class.getName())
|| shortName.equals("D") && longName.equals(double.class.getName())
|| shortName.equals("B") && longName.equals(byte.class.getName());
}

private static FieldDescription.InDefinedShape findField(
final Reference.Field fieldRef, final TypeDescription typeOnClasspath) {
for (final FieldDescription.InDefinedShape fieldType : typeOnClasspath.getDeclaredFields()) {
if (fieldType.getName().equals(fieldRef.getName())
&& fieldType
.getType()
.asErasure()
.getInternalName()
.equals(fieldRef.getType().getInternalName())) {
&& ((fieldType
.getType()
.asErasure()
.getInternalName()
.equals(fieldRef.getType().getInternalName()))
|| (fieldType.getType().asErasure().isPrimitive()
&& matchesPrimitive(
fieldType.getType().asErasure().getInternalName(),
fieldRef.getType().getInternalName())))) {
return fieldType;
}
}
Expand Down
1 change: 1 addition & 0 deletions gradle/spotless.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ apply plugin: 'com.diffplug.gradle.spotless'
spotless {
java {
licenseHeaderFile rootProject.file('gradle/enforcement/spotless.license.java'), '(package|import|public)'
targetExclude '**/generated/**'
}
groovy {
licenseHeaderFile rootProject.file('gradle/enforcement/spotless.license.java'), '(package|import|public)'
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
* limitations under the License.
*/
import io.opentelemetry.auto.instrumentation.api.Tags
import io.opentelemetry.auto.instrumentation.jdbc.JDBCUtils
import io.opentelemetry.auto.test.AgentTestRunner

import static io.opentelemetry.trace.Span.Kind.CLIENT
Expand Down Expand Up @@ -41,15 +42,15 @@ class SlickTest extends AgentTestRunner {
}
}
span(1) {
operationName SlickUtils.TestQuery()
operationName JDBCUtils.normalizeSql(SlickUtils.TestQuery())
spanKind CLIENT
childOf span(0)
errored false
tags {
"$Tags.DB_TYPE" "sql"
"$Tags.DB_INSTANCE" SlickUtils.Db()
"$Tags.DB_USER" SlickUtils.Username()
"$Tags.DB_STATEMENT" SlickUtils.TestQuery()
"$Tags.DB_STATEMENT" JDBCUtils.normalizeSql(SlickUtils.TestQuery())
"$Tags.DB_URL" "h2:mem:"
"span.origin.type" "org.h2.jdbc.JdbcPreparedStatement"
}
Expand Down
22 changes: 22 additions & 0 deletions instrumentation/jdbc/jdbc.gradle
Original file line number Diff line number Diff line change
@@ -1,12 +1,33 @@
plugins {
id 'com.intershop.gradle.javacc' version '4.0.0'
}

apply from: "${rootDir}/gradle/instrumentation.gradle"
apply plugin: 'org.unbroken-dome.test-sets'


muzzle {
pass {
coreJdk()
}
}

javacc {
configs {
template {
inputFile = file('src/main/javacc/SqlNormalizer.jj')
packageName = 'io.opentelemetry.auto.instrumentation.jdbc.normalizer'
}
}
}

tasks.withType(com.github.sherter.googlejavaformatgradleplugin.VerifyGoogleJavaFormat).configureEach {
exclude '**/jdbc/normalizer/*.java'
}
tasks.withType(Checkstyle).configureEach {
exclude '**/jdbc/normalizer/*.java'
}

testSets {
latestDepTest {
dirName = 'test'
Expand Down Expand Up @@ -34,3 +55,4 @@ dependencies {
latestDepTestCompile group: 'com.zaxxer', name: 'HikariCP', version: '+'
latestDepTestCompile group: 'com.mchange', name: 'c3p0', version: '+'
}

Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,14 @@ public ElementMatcher<TypeDescription> typeMatcher() {
public String[] helperClassNames() {
return new String[] {
packageName + ".JDBCMaps",
packageName + ".JDBCUtils",
packageName + ".normalizer.ParseException",
packageName + ".normalizer.SimpleCharStream",
packageName + ".normalizer.SqlNormalizer",
packageName + ".normalizer.SqlNormalizerConstants",
packageName + ".normalizer.SqlNormalizerTokenManager",
packageName + ".normalizer.Token",
packageName + ".normalizer.TokenMgrError",
};
}

Expand All @@ -65,7 +73,10 @@ public static class ConnectionPrepareAdvice {
@Advice.OnMethodExit(suppress = Throwable.class)
public static void addDBInfo(
@Advice.Argument(0) final String sql, @Advice.Return final PreparedStatement statement) {
JDBCMaps.preparedStatements.put(statement, sql);
final String normalizedSql = JDBCUtils.normalizeSql(sql);
if (normalizedSql != null) {
JDBCMaps.preparedStatements.put(statement, normalizedSql);
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@
package io.opentelemetry.auto.instrumentation.jdbc;

import io.opentelemetry.auto.bootstrap.ExceptionLogger;
import io.opentelemetry.auto.config.Config;
import io.opentelemetry.auto.instrumentation.jdbc.normalizer.SqlNormalizer;
import java.lang.reflect.Field;
import java.sql.Connection;
import java.sql.Statement;
Expand Down Expand Up @@ -69,4 +71,17 @@ public static Connection connectionFromStatement(final Statement statement) {
}
return connection;
}

/** @return null if the sql could not be normalized for any reason */
public static String normalizeSql(String sql) {
if (Config.get().isDisableSqlNormalizer()) {
return sql;
}
try {
return SqlNormalizer.normalize(sql);
} catch (Exception e) {
ExceptionLogger.LOGGER.debug("Could not normalize sql", e);
return null;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,16 @@ public ElementMatcher<TypeDescription> typeMatcher() {
@Override
public String[] helperClassNames() {
return new String[] {
packageName + ".JDBCMaps", packageName + ".JDBCUtils", packageName + ".JDBCDecorator",
packageName + ".JDBCMaps",
packageName + ".JDBCUtils",
packageName + ".JDBCDecorator",
packageName + ".normalizer.ParseException",
packageName + ".normalizer.SimpleCharStream",
packageName + ".normalizer.SqlNormalizer",
packageName + ".normalizer.SqlNormalizerConstants",
packageName + ".normalizer.SqlNormalizerTokenManager",
packageName + ".normalizer.Token",
packageName + ".normalizer.TokenMgrError",
};
}

Expand All @@ -80,16 +89,17 @@ public static SpanWithScope onEnter(
if (callDepth > 0) {
return null;
}
final String normalizedSql = JDBCUtils.normalizeSql(sql);

final Span span =
TRACER
.spanBuilder(DECORATE.spanNameOnStatement(sql))
.spanBuilder(DECORATE.spanNameOnStatement(normalizedSql))
.setSpanKind(CLIENT)
.setAttribute("span.origin.type", statement.getClass().getName())
.startSpan();
DECORATE.afterStart(span);
DECORATE.onConnection(span, connection);
DECORATE.onStatement(span, sql);
DECORATE.onStatement(span, normalizedSql);
return new SpanWithScope(span, currentContextWith(span));
}

Expand Down
Loading