Skip to content

Commit 559fc99

Browse files
lauritlaurit
authored
Influxdb client: don't fill db.statement for create/drop database and write operations (#11557)
1 parent 4652156 commit 559fc99

File tree

7 files changed

+64
-81
lines changed

7 files changed

+64
-81
lines changed

‎instrumentation/influxdb-2.4/javaagent/build.gradle.kts

+8
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,8 @@ dependencies {
1717
compileOnly("com.google.auto.value:auto-value-annotations")
1818
annotationProcessor("com.google.auto.value:auto-value")
1919

20+
testInstrumentation(project(":instrumentation:okhttp:okhttp-3.0:javaagent"))
21+
2022
// we use methods that weren't present before 2.14 in tests
2123
testLibrary("org.influxdb:influxdb-java:2.14")
2224
}
@@ -44,3 +46,9 @@ tasks {
4446
}
4547
}
4648
}
49+
50+
tasks.withType<Test>().configureEach {
51+
// we disable the okhttp instrumentation, so we don't need to assert on the okhttp spans
52+
// from the okhttp instrumentation we need OkHttp3IgnoredTypesConfigurer to fix context leaks
53+
jvmArgs("-Dotel.instrumentation.okhttp.enabled=false")
54+
}

‎instrumentation/influxdb-2.4/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/influxdb/v2_4/InfluxDbAttributesGetter.java

+3-4
Original file line numberDiff line numberDiff line change
@@ -19,11 +19,10 @@ public String getStatement(InfluxDbRequest request) {
1919
@Nullable
2020
@Override
2121
public String getOperation(InfluxDbRequest request) {
22-
if (request.getSqlStatementInfo() != null) {
23-
String operation = request.getSqlStatementInfo().getOperation();
24-
return operation == null ? request.getSql() : operation;
22+
if (request.getOperation() != null) {
23+
return request.getOperation();
2524
}
26-
return null;
25+
return request.getSqlStatementInfo().getOperation();
2726
}
2827

2928
@Override

‎instrumentation/influxdb-2.4/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/influxdb/v2_4/InfluxDbConstants.java

-18
This file was deleted.

‎instrumentation/influxdb-2.4/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/influxdb/v2_4/InfluxDbImplInstrumentation.java

+8-12
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,6 @@
66
package io.opentelemetry.javaagent.instrumentation.influxdb.v2_4;
77

88
import static io.opentelemetry.javaagent.bootstrap.Java8BytecodeBridge.currentContext;
9-
import static io.opentelemetry.javaagent.instrumentation.influxdb.v2_4.InfluxDbConstants.CREATE_DATABASE_STATEMENT_NEW;
10-
import static io.opentelemetry.javaagent.instrumentation.influxdb.v2_4.InfluxDbConstants.CREATE_DATABASE_STATEMENT_OLD;
11-
import static io.opentelemetry.javaagent.instrumentation.influxdb.v2_4.InfluxDbConstants.DELETE_DATABASE_STATEMENT;
129
import static io.opentelemetry.javaagent.instrumentation.influxdb.v2_4.InfluxDbSingletons.instrumenter;
1310
import static net.bytebuddy.matcher.ElementMatchers.isEnum;
1411
import static net.bytebuddy.matcher.ElementMatchers.isMethod;
@@ -94,7 +91,7 @@ public static void onEnter(
9491
HttpUrl httpUrl = retrofit.baseUrl();
9592
influxDbRequest =
9693
InfluxDbRequest.create(
97-
httpUrl.host(), httpUrl.port(), query.getDatabase(), query.getCommand());
94+
httpUrl.host(), httpUrl.port(), query.getDatabase(), null, query.getCommand());
9895

9996
if (!instrumenter().shouldStart(parentContext, influxDbRequest)) {
10097
return;
@@ -142,7 +139,6 @@ public static class InfluxDbModifyAdvice {
142139

143140
@Advice.OnMethodEnter(suppress = Throwable.class)
144141
public static void onEnter(
145-
@Advice.This InfluxDBImpl influxDbImpl,
146142
@Advice.Origin("#m") String methodName,
147143
@Advice.Argument(0) Object arg0,
148144
@Advice.FieldValue(value = "retrofit") Retrofit retrofit,
@@ -168,17 +164,17 @@ public static void onEnter(
168164
// write data by UDP protocol, in this way, can't get database name.
169165
: arg0 instanceof Integer ? "" : String.valueOf(arg0);
170166

171-
String sql = methodName;
167+
String operation;
172168
if ("createDatabase".equals(methodName)) {
173-
sql =
174-
influxDbImpl.version().startsWith("0.")
175-
? String.format(CREATE_DATABASE_STATEMENT_OLD, database)
176-
: String.format(CREATE_DATABASE_STATEMENT_NEW, database);
169+
operation = "CREATE DATABASE";
177170
} else if ("deleteDatabase".equals(methodName)) {
178-
sql = String.format(DELETE_DATABASE_STATEMENT, database);
171+
operation = "DROP DATABASE";
172+
} else {
173+
operation = "WRITE";
179174
}
180175

181-
influxDbRequest = InfluxDbRequest.create(httpUrl.host(), httpUrl.port(), database, sql);
176+
influxDbRequest =
177+
InfluxDbRequest.create(httpUrl.host(), httpUrl.port(), database, operation, null);
182178

183179
if (!instrumenter().shouldStart(parentContext, influxDbRequest)) {
184180
return;

‎instrumentation/influxdb-2.4/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/influxdb/v2_4/InfluxDbRequest.java

+6-3
Original file line numberDiff line numberDiff line change
@@ -9,15 +9,17 @@
99
import io.opentelemetry.instrumentation.api.incubator.semconv.db.SqlStatementInfo;
1010
import io.opentelemetry.instrumentation.api.incubator.semconv.db.SqlStatementSanitizer;
1111
import io.opentelemetry.javaagent.bootstrap.internal.CommonConfig;
12+
import javax.annotation.Nullable;
1213

1314
@AutoValue
1415
public abstract class InfluxDbRequest {
1516

1617
private static final SqlStatementSanitizer sanitizer =
1718
SqlStatementSanitizer.create(CommonConfig.get().isStatementSanitizationEnabled());
1819

19-
public static InfluxDbRequest create(String host, Integer port, String dbName, String sql) {
20-
return new AutoValue_InfluxDbRequest(host, port, dbName, sql, sanitizer.sanitize(sql));
20+
public static InfluxDbRequest create(
21+
String host, Integer port, String dbName, String operation, String sql) {
22+
return new AutoValue_InfluxDbRequest(host, port, dbName, operation, sanitizer.sanitize(sql));
2123
}
2224

2325
public abstract String getHost();
@@ -26,7 +28,8 @@ public static InfluxDbRequest create(String host, Integer port, String dbName, S
2628

2729
public abstract String getDbName();
2830

29-
public abstract String getSql();
31+
@Nullable
32+
public abstract String getOperation();
3033

3134
public abstract SqlStatementInfo getSqlStatementInfo();
3235
}

‎instrumentation/influxdb-2.4/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/influxdb/v2_4/InfluxDbClientTest.java

+22-25
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,6 @@
55

66
package io.opentelemetry.javaagent.instrumentation.influxdb.v2_4;
77

8-
import static io.opentelemetry.javaagent.instrumentation.influxdb.v2_4.InfluxDbConstants.CREATE_DATABASE_STATEMENT_NEW;
9-
import static io.opentelemetry.javaagent.instrumentation.influxdb.v2_4.InfluxDbConstants.DELETE_DATABASE_STATEMENT;
108
import static io.opentelemetry.sdk.testing.assertj.OpenTelemetryAssertions.equalTo;
119
import static java.util.Arrays.asList;
1210
import static org.assertj.core.api.Assertions.assertThat;
@@ -108,16 +106,13 @@ void testQueryAndModifyWithOneArgument() {
108106
span.hasName("CREATE DATABASE " + dbName)
109107
.hasKind(SpanKind.CLIENT)
110108
.hasAttributesSatisfying(
111-
attributeAssertions(
112-
String.format(CREATE_DATABASE_STATEMENT_NEW, dbName),
113-
"CREATE DATABASE",
114-
dbName))),
109+
attributeAssertions(null, "CREATE DATABASE", dbName))),
115110
trace ->
116111
trace.hasSpansSatisfyingExactly(
117112
span ->
118-
span.hasName("write " + dbName)
113+
span.hasName("WRITE " + dbName)
119114
.hasKind(SpanKind.CLIENT)
120-
.hasAttributesSatisfying(attributeAssertions("write", "write", dbName))),
115+
.hasAttributesSatisfying(attributeAssertions(null, "WRITE", dbName))),
121116
trace ->
122117
trace.hasSpansSatisfyingExactly(
123118
span ->
@@ -131,10 +126,7 @@ void testQueryAndModifyWithOneArgument() {
131126
span.hasName("DROP DATABASE " + dbName)
132127
.hasKind(SpanKind.CLIENT)
133128
.hasAttributesSatisfying(
134-
attributeAssertions(
135-
String.format(DELETE_DATABASE_STATEMENT, dbName),
136-
"DROP DATABASE",
137-
dbName))));
129+
attributeAssertions(null, "DROP DATABASE", dbName))));
138130
}
139131

140132
@Test
@@ -279,10 +271,10 @@ void testWriteWithFourArguments() {
279271
trace ->
280272
trace.hasSpansSatisfyingExactly(
281273
span ->
282-
span.hasName("write " + databaseName)
274+
span.hasName("WRITE " + databaseName)
283275
.hasKind(SpanKind.CLIENT)
284276
.hasAttributesSatisfying(
285-
attributeAssertions("write", "write", databaseName))));
277+
attributeAssertions(null, "WRITE", databaseName))));
286278
}
287279

288280
@Test
@@ -297,10 +289,10 @@ void testWriteWithFiveArguments() {
297289
trace ->
298290
trace.hasSpansSatisfyingExactly(
299291
span ->
300-
span.hasName("write " + databaseName)
292+
span.hasName("WRITE " + databaseName)
301293
.hasKind(SpanKind.CLIENT)
302294
.hasAttributesSatisfying(
303-
attributeAssertions("write", "write", databaseName))));
295+
attributeAssertions(null, "WRITE", databaseName))));
304296
}
305297

306298
@Test
@@ -316,19 +308,24 @@ void testWriteWithUdp() {
316308
trace ->
317309
trace.hasSpansSatisfyingExactly(
318310
span ->
319-
span.hasName("write")
311+
span.hasName("WRITE")
320312
.hasKind(SpanKind.CLIENT)
321-
.hasAttributesSatisfying(attributeAssertions("write", "write", null))));
313+
.hasAttributesSatisfying(attributeAssertions(null, "WRITE", null))));
322314
}
323315

324316
private static List<AttributeAssertion> attributeAssertions(
325317
String statement, String operation, String databaseName) {
326-
return asList(
327-
equalTo(DbIncubatingAttributes.DB_SYSTEM, "influxdb"),
328-
equalTo(DbIncubatingAttributes.DB_NAME, databaseName),
329-
equalTo(ServerAttributes.SERVER_ADDRESS, host),
330-
equalTo(ServerAttributes.SERVER_PORT, port),
331-
equalTo(DbIncubatingAttributes.DB_STATEMENT, statement),
332-
equalTo(DbIncubatingAttributes.DB_OPERATION, operation));
318+
List<AttributeAssertion> result = new ArrayList<>();
319+
result.addAll(
320+
asList(
321+
equalTo(DbIncubatingAttributes.DB_SYSTEM, "influxdb"),
322+
equalTo(DbIncubatingAttributes.DB_NAME, databaseName),
323+
equalTo(ServerAttributes.SERVER_ADDRESS, host),
324+
equalTo(ServerAttributes.SERVER_PORT, port),
325+
equalTo(DbIncubatingAttributes.DB_OPERATION, operation)));
326+
if (statement != null) {
327+
result.add(equalTo(DbIncubatingAttributes.DB_STATEMENT, statement));
328+
}
329+
return result;
333330
}
334331
}

‎instrumentation/influxdb-2.4/javaagent/src/test24/java/io/opentelemetry/javaagent/instrumentation/influxdb/v2_4/InfluxDbClient24Test.java

+17-19
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,6 @@
55

66
package io.opentelemetry.javaagent.instrumentation.influxdb.v2_4;
77

8-
import static io.opentelemetry.javaagent.instrumentation.influxdb.v2_4.InfluxDbConstants.CREATE_DATABASE_STATEMENT_NEW;
9-
import static io.opentelemetry.javaagent.instrumentation.influxdb.v2_4.InfluxDbConstants.DELETE_DATABASE_STATEMENT;
108
import static io.opentelemetry.sdk.testing.assertj.OpenTelemetryAssertions.equalTo;
119
import static java.util.Arrays.asList;
1210
import static org.assertj.core.api.Assertions.assertThat;
@@ -17,6 +15,7 @@
1715
import io.opentelemetry.sdk.testing.assertj.AttributeAssertion;
1816
import io.opentelemetry.semconv.ServerAttributes;
1917
import io.opentelemetry.semconv.incubating.DbIncubatingAttributes;
18+
import java.util.ArrayList;
2019
import java.util.List;
2120
import java.util.concurrent.TimeUnit;
2221
import org.influxdb.InfluxDB;
@@ -101,16 +100,13 @@ void testQueryAndModifyWithOneArgument() {
101100
span.hasName("CREATE DATABASE " + dbName)
102101
.hasKind(SpanKind.CLIENT)
103102
.hasAttributesSatisfying(
104-
attributeAssertions(
105-
String.format(CREATE_DATABASE_STATEMENT_NEW, dbName),
106-
"CREATE DATABASE",
107-
dbName))),
103+
attributeAssertions(null, "CREATE DATABASE", dbName))),
108104
trace ->
109105
trace.hasSpansSatisfyingExactly(
110106
span ->
111-
span.hasName("write " + dbName)
107+
span.hasName("WRITE " + dbName)
112108
.hasKind(SpanKind.CLIENT)
113-
.hasAttributesSatisfying(attributeAssertions("write", "write", dbName))),
109+
.hasAttributesSatisfying(attributeAssertions(null, "WRITE", dbName))),
114110
trace ->
115111
trace.hasSpansSatisfyingExactly(
116112
span ->
@@ -124,10 +120,7 @@ void testQueryAndModifyWithOneArgument() {
124120
span.hasName("DROP DATABASE " + dbName)
125121
.hasKind(SpanKind.CLIENT)
126122
.hasAttributesSatisfying(
127-
attributeAssertions(
128-
String.format(DELETE_DATABASE_STATEMENT, dbName),
129-
"DROP DATABASE",
130-
dbName))));
123+
attributeAssertions(null, "DROP DATABASE", dbName))));
131124
}
132125

133126
@Test
@@ -150,12 +143,17 @@ void testQueryWithTwoArguments() {
150143

151144
private static List<AttributeAssertion> attributeAssertions(
152145
String statement, String operation, String databaseName) {
153-
return asList(
154-
equalTo(DbIncubatingAttributes.DB_SYSTEM, "influxdb"),
155-
equalTo(DbIncubatingAttributes.DB_NAME, databaseName),
156-
equalTo(ServerAttributes.SERVER_ADDRESS, host),
157-
equalTo(ServerAttributes.SERVER_PORT, port),
158-
equalTo(DbIncubatingAttributes.DB_STATEMENT, statement),
159-
equalTo(DbIncubatingAttributes.DB_OPERATION, operation));
146+
List<AttributeAssertion> result = new ArrayList<>();
147+
result.addAll(
148+
asList(
149+
equalTo(DbIncubatingAttributes.DB_SYSTEM, "influxdb"),
150+
equalTo(DbIncubatingAttributes.DB_NAME, databaseName),
151+
equalTo(ServerAttributes.SERVER_ADDRESS, host),
152+
equalTo(ServerAttributes.SERVER_PORT, port),
153+
equalTo(DbIncubatingAttributes.DB_OPERATION, operation)));
154+
if (statement != null) {
155+
result.add(equalTo(DbIncubatingAttributes.DB_STATEMENT, statement));
156+
}
157+
return result;
160158
}
161159
}

0 commit comments

Comments
 (0)