diff --git a/instrumentation/influxdb-2.4/javaagent/build.gradle.kts b/instrumentation/influxdb-2.4/javaagent/build.gradle.kts index d6a74c455b43..ab93583d3e03 100644 --- a/instrumentation/influxdb-2.4/javaagent/build.gradle.kts +++ b/instrumentation/influxdb-2.4/javaagent/build.gradle.kts @@ -17,6 +17,8 @@ dependencies { compileOnly("com.google.auto.value:auto-value-annotations") annotationProcessor("com.google.auto.value:auto-value") + testInstrumentation(project(":instrumentation:okhttp:okhttp-3.0:javaagent")) + // we use methods that weren't present before 2.14 in tests testLibrary("org.influxdb:influxdb-java:2.14") } @@ -44,3 +46,9 @@ tasks { } } } + +tasks.withType().configureEach { + // we disable the okhttp instrumentation, so we don't need to assert on the okhttp spans + // from the okhttp instrumentation we need OkHttp3IgnoredTypesConfigurer to fix context leaks + jvmArgs("-Dotel.instrumentation.okhttp.enabled=false") +} diff --git a/instrumentation/influxdb-2.4/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/influxdb/v2_4/InfluxDbAttributesGetter.java b/instrumentation/influxdb-2.4/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/influxdb/v2_4/InfluxDbAttributesGetter.java index a2d209f1b435..c73bf348583b 100644 --- a/instrumentation/influxdb-2.4/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/influxdb/v2_4/InfluxDbAttributesGetter.java +++ b/instrumentation/influxdb-2.4/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/influxdb/v2_4/InfluxDbAttributesGetter.java @@ -19,11 +19,10 @@ public String getStatement(InfluxDbRequest request) { @Nullable @Override public String getOperation(InfluxDbRequest request) { - if (request.getSqlStatementInfo() != null) { - String operation = request.getSqlStatementInfo().getOperation(); - return operation == null ? request.getSql() : operation; + if (request.getOperation() != null) { + return request.getOperation(); } - return null; + return request.getSqlStatementInfo().getOperation(); } @Override diff --git a/instrumentation/influxdb-2.4/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/influxdb/v2_4/InfluxDbConstants.java b/instrumentation/influxdb-2.4/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/influxdb/v2_4/InfluxDbConstants.java deleted file mode 100644 index 06cbb2832da5..000000000000 --- a/instrumentation/influxdb-2.4/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/influxdb/v2_4/InfluxDbConstants.java +++ /dev/null @@ -1,18 +0,0 @@ -/* - * Copyright The OpenTelemetry Authors - * SPDX-License-Identifier: Apache-2.0 - */ - -package io.opentelemetry.javaagent.instrumentation.influxdb.v2_4; - -final class InfluxDbConstants { - - private InfluxDbConstants() {} - - public static final String CREATE_DATABASE_STATEMENT_NEW = "CREATE DATABASE \"%s\""; - - /** In influxDB 0.x version, it uses below statement format to create a database. */ - public static final String CREATE_DATABASE_STATEMENT_OLD = "CREATE DATABASE IF NOT EXISTS \"%s\""; - - public static final String DELETE_DATABASE_STATEMENT = "DROP DATABASE \"%s\""; -} diff --git a/instrumentation/influxdb-2.4/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/influxdb/v2_4/InfluxDbImplInstrumentation.java b/instrumentation/influxdb-2.4/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/influxdb/v2_4/InfluxDbImplInstrumentation.java index 2633e1d82433..92456cc181da 100644 --- a/instrumentation/influxdb-2.4/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/influxdb/v2_4/InfluxDbImplInstrumentation.java +++ b/instrumentation/influxdb-2.4/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/influxdb/v2_4/InfluxDbImplInstrumentation.java @@ -6,9 +6,6 @@ package io.opentelemetry.javaagent.instrumentation.influxdb.v2_4; import static io.opentelemetry.javaagent.bootstrap.Java8BytecodeBridge.currentContext; -import static io.opentelemetry.javaagent.instrumentation.influxdb.v2_4.InfluxDbConstants.CREATE_DATABASE_STATEMENT_NEW; -import static io.opentelemetry.javaagent.instrumentation.influxdb.v2_4.InfluxDbConstants.CREATE_DATABASE_STATEMENT_OLD; -import static io.opentelemetry.javaagent.instrumentation.influxdb.v2_4.InfluxDbConstants.DELETE_DATABASE_STATEMENT; import static io.opentelemetry.javaagent.instrumentation.influxdb.v2_4.InfluxDbSingletons.instrumenter; import static net.bytebuddy.matcher.ElementMatchers.isEnum; import static net.bytebuddy.matcher.ElementMatchers.isMethod; @@ -94,7 +91,7 @@ public static void onEnter( HttpUrl httpUrl = retrofit.baseUrl(); influxDbRequest = InfluxDbRequest.create( - httpUrl.host(), httpUrl.port(), query.getDatabase(), query.getCommand()); + httpUrl.host(), httpUrl.port(), query.getDatabase(), null, query.getCommand()); if (!instrumenter().shouldStart(parentContext, influxDbRequest)) { return; @@ -142,7 +139,6 @@ public static class InfluxDbModifyAdvice { @Advice.OnMethodEnter(suppress = Throwable.class) public static void onEnter( - @Advice.This InfluxDBImpl influxDbImpl, @Advice.Origin("#m") String methodName, @Advice.Argument(0) Object arg0, @Advice.FieldValue(value = "retrofit") Retrofit retrofit, @@ -168,17 +164,17 @@ public static void onEnter( // write data by UDP protocol, in this way, can't get database name. : arg0 instanceof Integer ? "" : String.valueOf(arg0); - String sql = methodName; + String operation; if ("createDatabase".equals(methodName)) { - sql = - influxDbImpl.version().startsWith("0.") - ? String.format(CREATE_DATABASE_STATEMENT_OLD, database) - : String.format(CREATE_DATABASE_STATEMENT_NEW, database); + operation = "CREATE DATABASE"; } else if ("deleteDatabase".equals(methodName)) { - sql = String.format(DELETE_DATABASE_STATEMENT, database); + operation = "DROP DATABASE"; + } else { + operation = "WRITE"; } - influxDbRequest = InfluxDbRequest.create(httpUrl.host(), httpUrl.port(), database, sql); + influxDbRequest = + InfluxDbRequest.create(httpUrl.host(), httpUrl.port(), database, operation, null); if (!instrumenter().shouldStart(parentContext, influxDbRequest)) { return; diff --git a/instrumentation/influxdb-2.4/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/influxdb/v2_4/InfluxDbRequest.java b/instrumentation/influxdb-2.4/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/influxdb/v2_4/InfluxDbRequest.java index 265b911d7102..23816208cecc 100644 --- a/instrumentation/influxdb-2.4/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/influxdb/v2_4/InfluxDbRequest.java +++ b/instrumentation/influxdb-2.4/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/influxdb/v2_4/InfluxDbRequest.java @@ -9,6 +9,7 @@ import io.opentelemetry.instrumentation.api.incubator.semconv.db.SqlStatementInfo; import io.opentelemetry.instrumentation.api.incubator.semconv.db.SqlStatementSanitizer; import io.opentelemetry.javaagent.bootstrap.internal.CommonConfig; +import javax.annotation.Nullable; @AutoValue public abstract class InfluxDbRequest { @@ -16,8 +17,9 @@ public abstract class InfluxDbRequest { private static final SqlStatementSanitizer sanitizer = SqlStatementSanitizer.create(CommonConfig.get().isStatementSanitizationEnabled()); - public static InfluxDbRequest create(String host, Integer port, String dbName, String sql) { - return new AutoValue_InfluxDbRequest(host, port, dbName, sql, sanitizer.sanitize(sql)); + public static InfluxDbRequest create( + String host, Integer port, String dbName, String operation, String sql) { + return new AutoValue_InfluxDbRequest(host, port, dbName, operation, sanitizer.sanitize(sql)); } public abstract String getHost(); @@ -26,7 +28,8 @@ public static InfluxDbRequest create(String host, Integer port, String dbName, S public abstract String getDbName(); - public abstract String getSql(); + @Nullable + public abstract String getOperation(); public abstract SqlStatementInfo getSqlStatementInfo(); } diff --git a/instrumentation/influxdb-2.4/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/influxdb/v2_4/InfluxDbClientTest.java b/instrumentation/influxdb-2.4/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/influxdb/v2_4/InfluxDbClientTest.java index d239606bfd37..40d5300fe5d8 100644 --- a/instrumentation/influxdb-2.4/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/influxdb/v2_4/InfluxDbClientTest.java +++ b/instrumentation/influxdb-2.4/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/influxdb/v2_4/InfluxDbClientTest.java @@ -5,8 +5,6 @@ package io.opentelemetry.javaagent.instrumentation.influxdb.v2_4; -import static io.opentelemetry.javaagent.instrumentation.influxdb.v2_4.InfluxDbConstants.CREATE_DATABASE_STATEMENT_NEW; -import static io.opentelemetry.javaagent.instrumentation.influxdb.v2_4.InfluxDbConstants.DELETE_DATABASE_STATEMENT; import static io.opentelemetry.sdk.testing.assertj.OpenTelemetryAssertions.equalTo; import static java.util.Arrays.asList; import static org.assertj.core.api.Assertions.assertThat; @@ -108,16 +106,13 @@ void testQueryAndModifyWithOneArgument() { span.hasName("CREATE DATABASE " + dbName) .hasKind(SpanKind.CLIENT) .hasAttributesSatisfying( - attributeAssertions( - String.format(CREATE_DATABASE_STATEMENT_NEW, dbName), - "CREATE DATABASE", - dbName))), + attributeAssertions(null, "CREATE DATABASE", dbName))), trace -> trace.hasSpansSatisfyingExactly( span -> - span.hasName("write " + dbName) + span.hasName("WRITE " + dbName) .hasKind(SpanKind.CLIENT) - .hasAttributesSatisfying(attributeAssertions("write", "write", dbName))), + .hasAttributesSatisfying(attributeAssertions(null, "WRITE", dbName))), trace -> trace.hasSpansSatisfyingExactly( span -> @@ -131,10 +126,7 @@ void testQueryAndModifyWithOneArgument() { span.hasName("DROP DATABASE " + dbName) .hasKind(SpanKind.CLIENT) .hasAttributesSatisfying( - attributeAssertions( - String.format(DELETE_DATABASE_STATEMENT, dbName), - "DROP DATABASE", - dbName)))); + attributeAssertions(null, "DROP DATABASE", dbName)))); } @Test @@ -279,10 +271,10 @@ void testWriteWithFourArguments() { trace -> trace.hasSpansSatisfyingExactly( span -> - span.hasName("write " + databaseName) + span.hasName("WRITE " + databaseName) .hasKind(SpanKind.CLIENT) .hasAttributesSatisfying( - attributeAssertions("write", "write", databaseName)))); + attributeAssertions(null, "WRITE", databaseName)))); } @Test @@ -297,10 +289,10 @@ void testWriteWithFiveArguments() { trace -> trace.hasSpansSatisfyingExactly( span -> - span.hasName("write " + databaseName) + span.hasName("WRITE " + databaseName) .hasKind(SpanKind.CLIENT) .hasAttributesSatisfying( - attributeAssertions("write", "write", databaseName)))); + attributeAssertions(null, "WRITE", databaseName)))); } @Test @@ -316,19 +308,24 @@ void testWriteWithUdp() { trace -> trace.hasSpansSatisfyingExactly( span -> - span.hasName("write") + span.hasName("WRITE") .hasKind(SpanKind.CLIENT) - .hasAttributesSatisfying(attributeAssertions("write", "write", null)))); + .hasAttributesSatisfying(attributeAssertions(null, "WRITE", null)))); } private static List attributeAssertions( String statement, String operation, String databaseName) { - return asList( - equalTo(DbIncubatingAttributes.DB_SYSTEM, "influxdb"), - equalTo(DbIncubatingAttributes.DB_NAME, databaseName), - equalTo(ServerAttributes.SERVER_ADDRESS, host), - equalTo(ServerAttributes.SERVER_PORT, port), - equalTo(DbIncubatingAttributes.DB_STATEMENT, statement), - equalTo(DbIncubatingAttributes.DB_OPERATION, operation)); + List result = new ArrayList<>(); + result.addAll( + asList( + equalTo(DbIncubatingAttributes.DB_SYSTEM, "influxdb"), + equalTo(DbIncubatingAttributes.DB_NAME, databaseName), + equalTo(ServerAttributes.SERVER_ADDRESS, host), + equalTo(ServerAttributes.SERVER_PORT, port), + equalTo(DbIncubatingAttributes.DB_OPERATION, operation))); + if (statement != null) { + result.add(equalTo(DbIncubatingAttributes.DB_STATEMENT, statement)); + } + return result; } } diff --git a/instrumentation/influxdb-2.4/javaagent/src/test24/java/io/opentelemetry/javaagent/instrumentation/influxdb/v2_4/InfluxDbClient24Test.java b/instrumentation/influxdb-2.4/javaagent/src/test24/java/io/opentelemetry/javaagent/instrumentation/influxdb/v2_4/InfluxDbClient24Test.java index faed6afea662..3cd33152551c 100644 --- a/instrumentation/influxdb-2.4/javaagent/src/test24/java/io/opentelemetry/javaagent/instrumentation/influxdb/v2_4/InfluxDbClient24Test.java +++ b/instrumentation/influxdb-2.4/javaagent/src/test24/java/io/opentelemetry/javaagent/instrumentation/influxdb/v2_4/InfluxDbClient24Test.java @@ -5,8 +5,6 @@ package io.opentelemetry.javaagent.instrumentation.influxdb.v2_4; -import static io.opentelemetry.javaagent.instrumentation.influxdb.v2_4.InfluxDbConstants.CREATE_DATABASE_STATEMENT_NEW; -import static io.opentelemetry.javaagent.instrumentation.influxdb.v2_4.InfluxDbConstants.DELETE_DATABASE_STATEMENT; import static io.opentelemetry.sdk.testing.assertj.OpenTelemetryAssertions.equalTo; import static java.util.Arrays.asList; import static org.assertj.core.api.Assertions.assertThat; @@ -17,6 +15,7 @@ import io.opentelemetry.sdk.testing.assertj.AttributeAssertion; import io.opentelemetry.semconv.ServerAttributes; import io.opentelemetry.semconv.incubating.DbIncubatingAttributes; +import java.util.ArrayList; import java.util.List; import java.util.concurrent.TimeUnit; import org.influxdb.InfluxDB; @@ -101,16 +100,13 @@ void testQueryAndModifyWithOneArgument() { span.hasName("CREATE DATABASE " + dbName) .hasKind(SpanKind.CLIENT) .hasAttributesSatisfying( - attributeAssertions( - String.format(CREATE_DATABASE_STATEMENT_NEW, dbName), - "CREATE DATABASE", - dbName))), + attributeAssertions(null, "CREATE DATABASE", dbName))), trace -> trace.hasSpansSatisfyingExactly( span -> - span.hasName("write " + dbName) + span.hasName("WRITE " + dbName) .hasKind(SpanKind.CLIENT) - .hasAttributesSatisfying(attributeAssertions("write", "write", dbName))), + .hasAttributesSatisfying(attributeAssertions(null, "WRITE", dbName))), trace -> trace.hasSpansSatisfyingExactly( span -> @@ -124,10 +120,7 @@ void testQueryAndModifyWithOneArgument() { span.hasName("DROP DATABASE " + dbName) .hasKind(SpanKind.CLIENT) .hasAttributesSatisfying( - attributeAssertions( - String.format(DELETE_DATABASE_STATEMENT, dbName), - "DROP DATABASE", - dbName)))); + attributeAssertions(null, "DROP DATABASE", dbName)))); } @Test @@ -150,12 +143,17 @@ void testQueryWithTwoArguments() { private static List attributeAssertions( String statement, String operation, String databaseName) { - return asList( - equalTo(DbIncubatingAttributes.DB_SYSTEM, "influxdb"), - equalTo(DbIncubatingAttributes.DB_NAME, databaseName), - equalTo(ServerAttributes.SERVER_ADDRESS, host), - equalTo(ServerAttributes.SERVER_PORT, port), - equalTo(DbIncubatingAttributes.DB_STATEMENT, statement), - equalTo(DbIncubatingAttributes.DB_OPERATION, operation)); + List result = new ArrayList<>(); + result.addAll( + asList( + equalTo(DbIncubatingAttributes.DB_SYSTEM, "influxdb"), + equalTo(DbIncubatingAttributes.DB_NAME, databaseName), + equalTo(ServerAttributes.SERVER_ADDRESS, host), + equalTo(ServerAttributes.SERVER_PORT, port), + equalTo(DbIncubatingAttributes.DB_OPERATION, operation))); + if (statement != null) { + result.add(equalTo(DbIncubatingAttributes.DB_STATEMENT, statement)); + } + return result; } }