Influxdb client: don't fill db.statement for create/drop database and write operations (#11557)

This commit is contained in:
Lauri Tulmin 2024-06-23 13:58:59 +03:00 committed by GitHub
parent 4652156fa1
commit 559fc99f46
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
7 changed files with 64 additions and 81 deletions

View File

@ -17,6 +17,8 @@ dependencies {
compileOnly("com.google.auto.value:auto-value-annotations") compileOnly("com.google.auto.value:auto-value-annotations")
annotationProcessor("com.google.auto.value:auto-value") 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 // we use methods that weren't present before 2.14 in tests
testLibrary("org.influxdb:influxdb-java:2.14") testLibrary("org.influxdb:influxdb-java:2.14")
} }
@ -44,3 +46,9 @@ tasks {
} }
} }
} }
tasks.withType<Test>().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")
}

View File

@ -19,11 +19,10 @@ final class InfluxDbAttributesGetter implements DbClientAttributesGetter<InfluxD
@Nullable @Nullable
@Override @Override
public String getOperation(InfluxDbRequest request) { public String getOperation(InfluxDbRequest request) {
if (request.getSqlStatementInfo() != null) { if (request.getOperation() != null) {
String operation = request.getSqlStatementInfo().getOperation(); return request.getOperation();
return operation == null ? request.getSql() : operation;
} }
return null; return request.getSqlStatementInfo().getOperation();
} }
@Override @Override

View File

@ -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\"";
}

View File

@ -6,9 +6,6 @@
package io.opentelemetry.javaagent.instrumentation.influxdb.v2_4; package io.opentelemetry.javaagent.instrumentation.influxdb.v2_4;
import static io.opentelemetry.javaagent.bootstrap.Java8BytecodeBridge.currentContext; 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 io.opentelemetry.javaagent.instrumentation.influxdb.v2_4.InfluxDbSingletons.instrumenter;
import static net.bytebuddy.matcher.ElementMatchers.isEnum; import static net.bytebuddy.matcher.ElementMatchers.isEnum;
import static net.bytebuddy.matcher.ElementMatchers.isMethod; import static net.bytebuddy.matcher.ElementMatchers.isMethod;
@ -94,7 +91,7 @@ public class InfluxDbImplInstrumentation implements TypeInstrumentation {
HttpUrl httpUrl = retrofit.baseUrl(); HttpUrl httpUrl = retrofit.baseUrl();
influxDbRequest = influxDbRequest =
InfluxDbRequest.create( InfluxDbRequest.create(
httpUrl.host(), httpUrl.port(), query.getDatabase(), query.getCommand()); httpUrl.host(), httpUrl.port(), query.getDatabase(), null, query.getCommand());
if (!instrumenter().shouldStart(parentContext, influxDbRequest)) { if (!instrumenter().shouldStart(parentContext, influxDbRequest)) {
return; return;
@ -142,7 +139,6 @@ public class InfluxDbImplInstrumentation implements TypeInstrumentation {
@Advice.OnMethodEnter(suppress = Throwable.class) @Advice.OnMethodEnter(suppress = Throwable.class)
public static void onEnter( public static void onEnter(
@Advice.This InfluxDBImpl influxDbImpl,
@Advice.Origin("#m") String methodName, @Advice.Origin("#m") String methodName,
@Advice.Argument(0) Object arg0, @Advice.Argument(0) Object arg0,
@Advice.FieldValue(value = "retrofit") Retrofit retrofit, @Advice.FieldValue(value = "retrofit") Retrofit retrofit,
@ -168,17 +164,17 @@ public class InfluxDbImplInstrumentation implements TypeInstrumentation {
// write data by UDP protocol, in this way, can't get database name. // write data by UDP protocol, in this way, can't get database name.
: arg0 instanceof Integer ? "" : String.valueOf(arg0); : arg0 instanceof Integer ? "" : String.valueOf(arg0);
String sql = methodName; String operation;
if ("createDatabase".equals(methodName)) { if ("createDatabase".equals(methodName)) {
sql = operation = "CREATE DATABASE";
influxDbImpl.version().startsWith("0.")
? String.format(CREATE_DATABASE_STATEMENT_OLD, database)
: String.format(CREATE_DATABASE_STATEMENT_NEW, database);
} else if ("deleteDatabase".equals(methodName)) { } 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)) { if (!instrumenter().shouldStart(parentContext, influxDbRequest)) {
return; return;

View File

@ -9,6 +9,7 @@ import com.google.auto.value.AutoValue;
import io.opentelemetry.instrumentation.api.incubator.semconv.db.SqlStatementInfo; import io.opentelemetry.instrumentation.api.incubator.semconv.db.SqlStatementInfo;
import io.opentelemetry.instrumentation.api.incubator.semconv.db.SqlStatementSanitizer; import io.opentelemetry.instrumentation.api.incubator.semconv.db.SqlStatementSanitizer;
import io.opentelemetry.javaagent.bootstrap.internal.CommonConfig; import io.opentelemetry.javaagent.bootstrap.internal.CommonConfig;
import javax.annotation.Nullable;
@AutoValue @AutoValue
public abstract class InfluxDbRequest { public abstract class InfluxDbRequest {
@ -16,8 +17,9 @@ public abstract class InfluxDbRequest {
private static final SqlStatementSanitizer sanitizer = private static final SqlStatementSanitizer sanitizer =
SqlStatementSanitizer.create(CommonConfig.get().isStatementSanitizationEnabled()); SqlStatementSanitizer.create(CommonConfig.get().isStatementSanitizationEnabled());
public static InfluxDbRequest create(String host, Integer port, String dbName, String sql) { public static InfluxDbRequest create(
return new AutoValue_InfluxDbRequest(host, port, dbName, sql, sanitizer.sanitize(sql)); 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(); public abstract String getHost();
@ -26,7 +28,8 @@ public abstract class InfluxDbRequest {
public abstract String getDbName(); public abstract String getDbName();
public abstract String getSql(); @Nullable
public abstract String getOperation();
public abstract SqlStatementInfo getSqlStatementInfo(); public abstract SqlStatementInfo getSqlStatementInfo();
} }

View File

@ -5,8 +5,6 @@
package io.opentelemetry.javaagent.instrumentation.influxdb.v2_4; 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 io.opentelemetry.sdk.testing.assertj.OpenTelemetryAssertions.equalTo;
import static java.util.Arrays.asList; import static java.util.Arrays.asList;
import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThat;
@ -108,16 +106,13 @@ class InfluxDbClientTest {
span.hasName("CREATE DATABASE " + dbName) span.hasName("CREATE DATABASE " + dbName)
.hasKind(SpanKind.CLIENT) .hasKind(SpanKind.CLIENT)
.hasAttributesSatisfying( .hasAttributesSatisfying(
attributeAssertions( attributeAssertions(null, "CREATE DATABASE", dbName))),
String.format(CREATE_DATABASE_STATEMENT_NEW, dbName),
"CREATE DATABASE",
dbName))),
trace -> trace ->
trace.hasSpansSatisfyingExactly( trace.hasSpansSatisfyingExactly(
span -> span ->
span.hasName("write " + dbName) span.hasName("WRITE " + dbName)
.hasKind(SpanKind.CLIENT) .hasKind(SpanKind.CLIENT)
.hasAttributesSatisfying(attributeAssertions("write", "write", dbName))), .hasAttributesSatisfying(attributeAssertions(null, "WRITE", dbName))),
trace -> trace ->
trace.hasSpansSatisfyingExactly( trace.hasSpansSatisfyingExactly(
span -> span ->
@ -131,10 +126,7 @@ class InfluxDbClientTest {
span.hasName("DROP DATABASE " + dbName) span.hasName("DROP DATABASE " + dbName)
.hasKind(SpanKind.CLIENT) .hasKind(SpanKind.CLIENT)
.hasAttributesSatisfying( .hasAttributesSatisfying(
attributeAssertions( attributeAssertions(null, "DROP DATABASE", dbName))));
String.format(DELETE_DATABASE_STATEMENT, dbName),
"DROP DATABASE",
dbName))));
} }
@Test @Test
@ -279,10 +271,10 @@ class InfluxDbClientTest {
trace -> trace ->
trace.hasSpansSatisfyingExactly( trace.hasSpansSatisfyingExactly(
span -> span ->
span.hasName("write " + databaseName) span.hasName("WRITE " + databaseName)
.hasKind(SpanKind.CLIENT) .hasKind(SpanKind.CLIENT)
.hasAttributesSatisfying( .hasAttributesSatisfying(
attributeAssertions("write", "write", databaseName)))); attributeAssertions(null, "WRITE", databaseName))));
} }
@Test @Test
@ -297,10 +289,10 @@ class InfluxDbClientTest {
trace -> trace ->
trace.hasSpansSatisfyingExactly( trace.hasSpansSatisfyingExactly(
span -> span ->
span.hasName("write " + databaseName) span.hasName("WRITE " + databaseName)
.hasKind(SpanKind.CLIENT) .hasKind(SpanKind.CLIENT)
.hasAttributesSatisfying( .hasAttributesSatisfying(
attributeAssertions("write", "write", databaseName)))); attributeAssertions(null, "WRITE", databaseName))));
} }
@Test @Test
@ -316,19 +308,24 @@ class InfluxDbClientTest {
trace -> trace ->
trace.hasSpansSatisfyingExactly( trace.hasSpansSatisfyingExactly(
span -> span ->
span.hasName("write") span.hasName("WRITE")
.hasKind(SpanKind.CLIENT) .hasKind(SpanKind.CLIENT)
.hasAttributesSatisfying(attributeAssertions("write", "write", null)))); .hasAttributesSatisfying(attributeAssertions(null, "WRITE", null))));
} }
private static List<AttributeAssertion> attributeAssertions( private static List<AttributeAssertion> attributeAssertions(
String statement, String operation, String databaseName) { String statement, String operation, String databaseName) {
return asList( List<AttributeAssertion> result = new ArrayList<>();
equalTo(DbIncubatingAttributes.DB_SYSTEM, "influxdb"), result.addAll(
equalTo(DbIncubatingAttributes.DB_NAME, databaseName), asList(
equalTo(ServerAttributes.SERVER_ADDRESS, host), equalTo(DbIncubatingAttributes.DB_SYSTEM, "influxdb"),
equalTo(ServerAttributes.SERVER_PORT, port), equalTo(DbIncubatingAttributes.DB_NAME, databaseName),
equalTo(DbIncubatingAttributes.DB_STATEMENT, statement), equalTo(ServerAttributes.SERVER_ADDRESS, host),
equalTo(DbIncubatingAttributes.DB_OPERATION, operation)); equalTo(ServerAttributes.SERVER_PORT, port),
equalTo(DbIncubatingAttributes.DB_OPERATION, operation)));
if (statement != null) {
result.add(equalTo(DbIncubatingAttributes.DB_STATEMENT, statement));
}
return result;
} }
} }

View File

@ -5,8 +5,6 @@
package io.opentelemetry.javaagent.instrumentation.influxdb.v2_4; 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 io.opentelemetry.sdk.testing.assertj.OpenTelemetryAssertions.equalTo;
import static java.util.Arrays.asList; import static java.util.Arrays.asList;
import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThat;
@ -17,6 +15,7 @@ import io.opentelemetry.instrumentation.testing.junit.InstrumentationExtension;
import io.opentelemetry.sdk.testing.assertj.AttributeAssertion; import io.opentelemetry.sdk.testing.assertj.AttributeAssertion;
import io.opentelemetry.semconv.ServerAttributes; import io.opentelemetry.semconv.ServerAttributes;
import io.opentelemetry.semconv.incubating.DbIncubatingAttributes; import io.opentelemetry.semconv.incubating.DbIncubatingAttributes;
import java.util.ArrayList;
import java.util.List; import java.util.List;
import java.util.concurrent.TimeUnit; import java.util.concurrent.TimeUnit;
import org.influxdb.InfluxDB; import org.influxdb.InfluxDB;
@ -101,16 +100,13 @@ class InfluxDbClient24Test {
span.hasName("CREATE DATABASE " + dbName) span.hasName("CREATE DATABASE " + dbName)
.hasKind(SpanKind.CLIENT) .hasKind(SpanKind.CLIENT)
.hasAttributesSatisfying( .hasAttributesSatisfying(
attributeAssertions( attributeAssertions(null, "CREATE DATABASE", dbName))),
String.format(CREATE_DATABASE_STATEMENT_NEW, dbName),
"CREATE DATABASE",
dbName))),
trace -> trace ->
trace.hasSpansSatisfyingExactly( trace.hasSpansSatisfyingExactly(
span -> span ->
span.hasName("write " + dbName) span.hasName("WRITE " + dbName)
.hasKind(SpanKind.CLIENT) .hasKind(SpanKind.CLIENT)
.hasAttributesSatisfying(attributeAssertions("write", "write", dbName))), .hasAttributesSatisfying(attributeAssertions(null, "WRITE", dbName))),
trace -> trace ->
trace.hasSpansSatisfyingExactly( trace.hasSpansSatisfyingExactly(
span -> span ->
@ -124,10 +120,7 @@ class InfluxDbClient24Test {
span.hasName("DROP DATABASE " + dbName) span.hasName("DROP DATABASE " + dbName)
.hasKind(SpanKind.CLIENT) .hasKind(SpanKind.CLIENT)
.hasAttributesSatisfying( .hasAttributesSatisfying(
attributeAssertions( attributeAssertions(null, "DROP DATABASE", dbName))));
String.format(DELETE_DATABASE_STATEMENT, dbName),
"DROP DATABASE",
dbName))));
} }
@Test @Test
@ -150,12 +143,17 @@ class InfluxDbClient24Test {
private static List<AttributeAssertion> attributeAssertions( private static List<AttributeAssertion> attributeAssertions(
String statement, String operation, String databaseName) { String statement, String operation, String databaseName) {
return asList( List<AttributeAssertion> result = new ArrayList<>();
equalTo(DbIncubatingAttributes.DB_SYSTEM, "influxdb"), result.addAll(
equalTo(DbIncubatingAttributes.DB_NAME, databaseName), asList(
equalTo(ServerAttributes.SERVER_ADDRESS, host), equalTo(DbIncubatingAttributes.DB_SYSTEM, "influxdb"),
equalTo(ServerAttributes.SERVER_PORT, port), equalTo(DbIncubatingAttributes.DB_NAME, databaseName),
equalTo(DbIncubatingAttributes.DB_STATEMENT, statement), equalTo(ServerAttributes.SERVER_ADDRESS, host),
equalTo(DbIncubatingAttributes.DB_OPERATION, operation)); equalTo(ServerAttributes.SERVER_PORT, port),
equalTo(DbIncubatingAttributes.DB_OPERATION, operation)));
if (statement != null) {
result.add(equalTo(DbIncubatingAttributes.DB_STATEMENT, statement));
}
return result;
} }
} }