From e89942d4303059e5efcd717b4ed756670a25c3ca Mon Sep 17 00:00:00 2001 From: Mateusz Rzeszutek Date: Thu, 29 Oct 2020 15:20:33 +0100 Subject: [PATCH] Standardize DB query normalizer property names (#1509) * Standardize DB query normalizer property names And add normalizer configuration for Redis, Couchbase, Cassandra and Geode * apply code review comments --- .../v3_0/CassandraQueryNormalizer.java | 6 ++++ .../v4_0/CassandraQueryNormalizer.java | 6 ++++ .../v2_0/CouchbaseQueryNormalizer.java | 6 ++++ .../geode/GeodeQueryNormalizer.java | 8 +++-- .../instrumentation/jdbc/JDBCUtils.java | 8 ++--- .../jedis/v1_4/JedisClientTracer.java | 5 +++- .../jedis/v3_0/JedisClientTracer.java | 5 +++- .../v5_0/LettuceDatabaseClientTracer.java | 5 +++- .../lettuce/v5_1/OpenTelemetryTracing.java | 5 +++- .../redisson/RedissonClientTracer.java | 7 +++-- .../api/db/QueryNormalizationConfig.java | 30 +++++++++++++++++++ .../api/db/RedisCommandNormalizer.java | 14 +++++++-- .../api/db/RedisCommandNormalizerTest.groovy | 27 +++++++++++++++-- 13 files changed, 114 insertions(+), 18 deletions(-) create mode 100644 javaagent-api/src/main/java/io/opentelemetry/javaagent/instrumentation/api/db/QueryNormalizationConfig.java diff --git a/instrumentation/cassandra/cassandra-3.0/src/main/java/io/opentelemetry/javaagent/instrumentation/cassandra/v3_0/CassandraQueryNormalizer.java b/instrumentation/cassandra/cassandra-3.0/src/main/java/io/opentelemetry/javaagent/instrumentation/cassandra/v3_0/CassandraQueryNormalizer.java index c01efc5fb7..bc86f9860a 100644 --- a/instrumentation/cassandra/cassandra-3.0/src/main/java/io/opentelemetry/javaagent/instrumentation/cassandra/v3_0/CassandraQueryNormalizer.java +++ b/instrumentation/cassandra/cassandra-3.0/src/main/java/io/opentelemetry/javaagent/instrumentation/cassandra/v3_0/CassandraQueryNormalizer.java @@ -5,6 +5,8 @@ package io.opentelemetry.javaagent.instrumentation.cassandra.v3_0; +import static io.opentelemetry.javaagent.instrumentation.api.db.QueryNormalizationConfig.isQueryNormalizationEnabled; + import io.opentelemetry.javaagent.instrumentation.api.db.normalizer.ParseException; import io.opentelemetry.javaagent.instrumentation.api.db.normalizer.SqlNormalizer; import org.slf4j.Logger; @@ -12,8 +14,12 @@ import org.slf4j.LoggerFactory; public final class CassandraQueryNormalizer { private static final Logger log = LoggerFactory.getLogger(CassandraQueryNormalizer.class); + private static final boolean NORMALIZATION_ENABLED = isQueryNormalizationEnabled("cassandra"); public static String normalize(String query) { + if (!NORMALIZATION_ENABLED) { + return query; + } try { return SqlNormalizer.normalize(query); } catch (ParseException e) { diff --git a/instrumentation/cassandra/cassandra-4.0/src/main/java/io/opentelemetry/javaagent/instrumentation/cassandra/v4_0/CassandraQueryNormalizer.java b/instrumentation/cassandra/cassandra-4.0/src/main/java/io/opentelemetry/javaagent/instrumentation/cassandra/v4_0/CassandraQueryNormalizer.java index 6228728827..3c232149a9 100644 --- a/instrumentation/cassandra/cassandra-4.0/src/main/java/io/opentelemetry/javaagent/instrumentation/cassandra/v4_0/CassandraQueryNormalizer.java +++ b/instrumentation/cassandra/cassandra-4.0/src/main/java/io/opentelemetry/javaagent/instrumentation/cassandra/v4_0/CassandraQueryNormalizer.java @@ -5,6 +5,8 @@ package io.opentelemetry.javaagent.instrumentation.cassandra.v4_0; +import static io.opentelemetry.javaagent.instrumentation.api.db.QueryNormalizationConfig.isQueryNormalizationEnabled; + import io.opentelemetry.javaagent.instrumentation.api.db.normalizer.ParseException; import io.opentelemetry.javaagent.instrumentation.api.db.normalizer.SqlNormalizer; import org.slf4j.Logger; @@ -12,8 +14,12 @@ import org.slf4j.LoggerFactory; public final class CassandraQueryNormalizer { private static final Logger log = LoggerFactory.getLogger(CassandraQueryNormalizer.class); + private static final boolean NORMALIZATION_ENABLED = isQueryNormalizationEnabled("cassandra"); public static String normalize(String query) { + if (!NORMALIZATION_ENABLED) { + return query; + } try { return SqlNormalizer.normalize(query); } catch (ParseException e) { diff --git a/instrumentation/couchbase/couchbase-2.0/src/main/java/io/opentelemetry/javaagent/instrumentation/couchbase/v2_0/CouchbaseQueryNormalizer.java b/instrumentation/couchbase/couchbase-2.0/src/main/java/io/opentelemetry/javaagent/instrumentation/couchbase/v2_0/CouchbaseQueryNormalizer.java index 7700db79eb..c235886eda 100644 --- a/instrumentation/couchbase/couchbase-2.0/src/main/java/io/opentelemetry/javaagent/instrumentation/couchbase/v2_0/CouchbaseQueryNormalizer.java +++ b/instrumentation/couchbase/couchbase-2.0/src/main/java/io/opentelemetry/javaagent/instrumentation/couchbase/v2_0/CouchbaseQueryNormalizer.java @@ -5,6 +5,8 @@ package io.opentelemetry.javaagent.instrumentation.couchbase.v2_0; +import static io.opentelemetry.javaagent.instrumentation.api.db.QueryNormalizationConfig.isQueryNormalizationEnabled; + import io.opentelemetry.javaagent.instrumentation.api.db.normalizer.ParseException; import io.opentelemetry.javaagent.instrumentation.api.db.normalizer.SqlNormalizer; import java.lang.invoke.MethodHandle; @@ -15,6 +17,7 @@ import org.slf4j.LoggerFactory; public final class CouchbaseQueryNormalizer { private static final Logger log = LoggerFactory.getLogger(CouchbaseQueryNormalizer.class); + private static final boolean NORMALIZATION_ENABLED = isQueryNormalizationEnabled("couchbase"); private static final Class QUERY_CLASS; private static final Class STATEMENT_CLASS; @@ -118,6 +121,9 @@ public final class CouchbaseQueryNormalizer { } private static String normalizeString(String query) { + if (!NORMALIZATION_ENABLED || query == null) { + return query; + } try { return SqlNormalizer.normalize(query); } catch (ParseException e) { diff --git a/instrumentation/geode-1.4/src/main/java/io/opentelemetry/javaagent/instrumentation/geode/GeodeQueryNormalizer.java b/instrumentation/geode-1.4/src/main/java/io/opentelemetry/javaagent/instrumentation/geode/GeodeQueryNormalizer.java index 13e6f81cad..1f324645cf 100644 --- a/instrumentation/geode-1.4/src/main/java/io/opentelemetry/javaagent/instrumentation/geode/GeodeQueryNormalizer.java +++ b/instrumentation/geode-1.4/src/main/java/io/opentelemetry/javaagent/instrumentation/geode/GeodeQueryNormalizer.java @@ -5,6 +5,8 @@ package io.opentelemetry.javaagent.instrumentation.geode; +import static io.opentelemetry.javaagent.instrumentation.api.db.QueryNormalizationConfig.isQueryNormalizationEnabled; + import io.opentelemetry.javaagent.instrumentation.api.db.normalizer.ParseException; import io.opentelemetry.javaagent.instrumentation.api.db.normalizer.SqlNormalizer; import org.slf4j.Logger; @@ -12,10 +14,12 @@ import org.slf4j.LoggerFactory; public final class GeodeQueryNormalizer { private static final Logger log = LoggerFactory.getLogger(GeodeQueryNormalizer.class); + private static final boolean NORMALIZATION_ENABLED = + isQueryNormalizationEnabled("geode", "geode-client"); public static String normalize(String query) { - if (query == null) { - return null; + if (!NORMALIZATION_ENABLED || query == null) { + return query; } try { return SqlNormalizer.normalize(query); diff --git a/instrumentation/jdbc/src/main/java/io/opentelemetry/javaagent/instrumentation/jdbc/JDBCUtils.java b/instrumentation/jdbc/src/main/java/io/opentelemetry/javaagent/instrumentation/jdbc/JDBCUtils.java index 87ab24ea1c..5e96d8ac07 100644 --- a/instrumentation/jdbc/src/main/java/io/opentelemetry/javaagent/instrumentation/jdbc/JDBCUtils.java +++ b/instrumentation/jdbc/src/main/java/io/opentelemetry/javaagent/instrumentation/jdbc/JDBCUtils.java @@ -5,7 +5,8 @@ package io.opentelemetry.javaagent.instrumentation.jdbc; -import io.opentelemetry.instrumentation.api.config.Config; +import static io.opentelemetry.javaagent.instrumentation.api.db.QueryNormalizationConfig.isQueryNormalizationEnabled; + import io.opentelemetry.javaagent.instrumentation.api.db.normalizer.SqlNormalizer; import java.lang.reflect.Field; import java.sql.Connection; @@ -17,8 +18,7 @@ public abstract class JDBCUtils { private static final Logger log = LoggerFactory.getLogger(JDBCUtils.class); - private static final boolean SQL_NORMALIZER_ENABLED = - Config.get().getBooleanProperty("sql.normalizer.enabled", true); + private static final boolean NORMALIZATION_ENABLED = isQueryNormalizationEnabled("jdbc"); private static Field c3poField = null; @@ -71,7 +71,7 @@ public abstract class JDBCUtils { /** @return null if the sql could not be normalized for any reason */ public static String normalizeSql(String sql) { - if (!SQL_NORMALIZER_ENABLED) { + if (!NORMALIZATION_ENABLED) { return sql; } try { diff --git a/instrumentation/jedis/jedis-1.4/src/main/java/io/opentelemetry/javaagent/instrumentation/jedis/v1_4/JedisClientTracer.java b/instrumentation/jedis/jedis-1.4/src/main/java/io/opentelemetry/javaagent/instrumentation/jedis/v1_4/JedisClientTracer.java index b11c3f70a0..1c10bfa2ff 100644 --- a/instrumentation/jedis/jedis-1.4/src/main/java/io/opentelemetry/javaagent/instrumentation/jedis/v1_4/JedisClientTracer.java +++ b/instrumentation/jedis/jedis-1.4/src/main/java/io/opentelemetry/javaagent/instrumentation/jedis/v1_4/JedisClientTracer.java @@ -18,6 +18,9 @@ import redis.clients.jedis.Protocol.Command; public class JedisClientTracer extends DatabaseClientTracer { public static final JedisClientTracer TRACER = new JedisClientTracer(); + private final RedisCommandNormalizer commandNormalizer = + new RedisCommandNormalizer("jedis", "redis"); + @Override protected String spanName(Connection connection, CommandWithArgs query, String normalizedQuery) { return query.getStringCommand(); @@ -25,7 +28,7 @@ public class JedisClientTracer extends DatabaseClientTracer { public static final JedisClientTracer TRACER = new JedisClientTracer(); + private final RedisCommandNormalizer commandNormalizer = + new RedisCommandNormalizer("jedis", "redis"); + @Override protected String spanName(Connection connection, CommandWithArgs query, String normalizedQuery) { return query.getStringCommand(); @@ -27,7 +30,7 @@ public class JedisClientTracer extends DatabaseClientTracer> { public static final LettuceDatabaseClientTracer TRACER = new LettuceDatabaseClientTracer(); + private final RedisCommandNormalizer commandNormalizer = + new RedisCommandNormalizer("lettuce", "lettuce-5"); + @Override protected String spanName( RedisURI connection, RedisCommand query, String normalizedQuery) { @@ -29,6 +32,6 @@ public class LettuceDatabaseClientTracer redisCommand.getArgs() == null ? Collections.emptyList() : LettuceArgSplitter.splitArgs(redisCommand.getArgs().toCommandString()); - return RedisCommandNormalizer.normalize(command, args); + return commandNormalizer.normalize(command, args); } } diff --git a/instrumentation/lettuce/lettuce-5.1/src/main/java/io/opentelemetry/javaagent/instrumentation/lettuce/v5_1/OpenTelemetryTracing.java b/instrumentation/lettuce/lettuce-5.1/src/main/java/io/opentelemetry/javaagent/instrumentation/lettuce/v5_1/OpenTelemetryTracing.java index 9cc4dc3a23..e3ed404936 100644 --- a/instrumentation/lettuce/lettuce-5.1/src/main/java/io/opentelemetry/javaagent/instrumentation/lettuce/v5_1/OpenTelemetryTracing.java +++ b/instrumentation/lettuce/lettuce-5.1/src/main/java/io/opentelemetry/javaagent/instrumentation/lettuce/v5_1/OpenTelemetryTracing.java @@ -36,6 +36,9 @@ public enum OpenTelemetryTracing implements Tracing { public static final io.opentelemetry.trace.Tracer TRACER = OpenTelemetry.getGlobalTracer("io.opentelemetry.auto.lettuce-5.1"); + private static final RedisCommandNormalizer commandNormalizer = + new RedisCommandNormalizer("lettuce", "lettuce-5", "lettuce-5.1"); + @Override public TracerProvider getTracerProvider() { return OpenTelemetryTracerProvider.INSTANCE; @@ -250,7 +253,7 @@ public enum OpenTelemetryTracing implements Tracing { public synchronized void finish() { if (span != null) { if (name != null) { - String statement = RedisCommandNormalizer.normalize(name, splitArgs(args)); + String statement = commandNormalizer.normalize(name, splitArgs(args)); span.setAttribute(SemanticAttributes.DB_STATEMENT, statement); } span.end(); diff --git a/instrumentation/redisson-3.0/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/RedissonClientTracer.java b/instrumentation/redisson-3.0/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/RedissonClientTracer.java index 58cf97c7e0..31f70cad89 100644 --- a/instrumentation/redisson-3.0/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/RedissonClientTracer.java +++ b/instrumentation/redisson-3.0/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/RedissonClientTracer.java @@ -22,6 +22,9 @@ public class RedissonClientTracer extends DatabaseClientTracer command) { + private String normalizeSingleCommand(CommandData command) { Object[] commandParams = command.getParams(); List args = new ArrayList<>(commandParams.length + 1); if (command.getCommand().getSubName() != null) { @@ -86,7 +89,7 @@ public class RedissonClientTracer extends DatabaseClientTracer args) { + private final boolean normalizationEnabled; + + public RedisCommandNormalizer(String... instrumentationNames) { + this.normalizationEnabled = isQueryNormalizationEnabled(instrumentationNames); + } + + public String normalize(String command, List args) { + if (!normalizationEnabled) { + return KeepAllArgs.INSTANCE.normalize(command, args); + } return NORMALIZERS.getOrDefault(command.toUpperCase(), DEFAULT).normalize(command, args); } @@ -416,6 +426,4 @@ public final class RedisCommandNormalizer { } } } - - private RedisCommandNormalizer() {} } diff --git a/javaagent-api/src/test/groovy/io/opentelemetry/javaagent/instrumentation/api/db/RedisCommandNormalizerTest.groovy b/javaagent-api/src/test/groovy/io/opentelemetry/javaagent/instrumentation/api/db/RedisCommandNormalizerTest.groovy index b646597405..1e8219ec46 100644 --- a/javaagent-api/src/test/groovy/io/opentelemetry/javaagent/instrumentation/api/db/RedisCommandNormalizerTest.groovy +++ b/javaagent-api/src/test/groovy/io/opentelemetry/javaagent/instrumentation/api/db/RedisCommandNormalizerTest.groovy @@ -5,14 +5,35 @@ package io.opentelemetry.javaagent.instrumentation.api.db +import io.opentelemetry.instrumentation.test.utils.ConfigUtils import spock.lang.Specification import spock.lang.Unroll class RedisCommandNormalizerTest extends Specification { + def normalizer = new RedisCommandNormalizer("redis") + + def "should not normalize anything when turned off"() { + given: + def previousConfig = ConfigUtils.updateConfig({ + it.setProperty("otel.instrumentation.redis.query.normalizer.enabled", "false") + }) + + def normalizer = new RedisCommandNormalizer("redis-instrumentation", "redis") + + when: + def result = normalizer.normalize("AUTH", ["user", "password"]) + + then: + result == "AUTH user password" + + cleanup: + ConfigUtils.setConfig(previousConfig) + } + @Unroll def "should normalize #expected"() { when: - def normalised = RedisCommandNormalizer.normalize(command, args) + def normalised = normalizer.normalize(command, args) then: normalised == expected @@ -90,7 +111,7 @@ class RedisCommandNormalizerTest extends Specification { def args = ["arg1", "arg 2"] when: - def normalised = RedisCommandNormalizer.normalize(command, args) + def normalised = normalizer.normalize(command, args) then: normalised == command + " " + args.join(" ") @@ -140,7 +161,7 @@ class RedisCommandNormalizerTest extends Specification { def "should mask all arguments of an unknown command"() { when: - def normalised = RedisCommandNormalizer.normalize("NEWAUTH", ["password", "secret"]) + def normalised = normalizer.normalize("NEWAUTH", ["password", "secret"]) then: normalised == "NEWAUTH ? ?"