Skip to content

Commit 34d101b

Browse files
otelbot[bot]trask
andauthored
Code review sweep (run 24949954141) (#18310)
Co-authored-by: otelbot <197425009+otelbot@users.noreply.github.com> Co-authored-by: Trask Stalnaker <trask.stalnaker@gmail.com>
1 parent 113306c commit 34d101b

10 files changed

Lines changed: 36 additions & 68 deletions

File tree

instrumentation/cassandra/cassandra-4.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/cassandra/v4_0/CassandraRequest.java

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -9,16 +9,15 @@
99
import com.google.auto.value.AutoValue;
1010

1111
@AutoValue
12-
public abstract class CassandraRequest {
12+
abstract class CassandraRequest {
1313

14-
public static CassandraRequest create(
15-
Session session, String queryText, boolean parameterizedQuery) {
14+
static CassandraRequest create(Session session, String queryText, boolean parameterizedQuery) {
1615
return new AutoValue_CassandraRequest(session, queryText, parameterizedQuery);
1716
}
1817

19-
public abstract Session getSession();
18+
abstract Session getSession();
2019

21-
public abstract String getQueryText();
20+
abstract String getQueryText();
2221

23-
public abstract boolean isParameterizedQuery();
22+
abstract boolean isParameterizedQuery();
2423
}

instrumentation/cassandra/cassandra-4.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/cassandra/v4_0/CassandraSingletons.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@
1717
import io.opentelemetry.instrumentation.api.instrumenter.SpanKindExtractor;
1818

1919
@SuppressWarnings("deprecation") // using deprecated semconv
20-
public class CassandraSingletons {
20+
class CassandraSingletons {
2121
private static final String INSTRUMENTATION_NAME = "io.opentelemetry.cassandra-4.0";
2222

2323
// using ExecutionInfo because we can get that from ResultSet, AsyncResultSet and DriverException
@@ -42,7 +42,7 @@ public class CassandraSingletons {
4242
.buildInstrumenter(SpanKindExtractor.alwaysClient());
4343
}
4444

45-
public static Instrumenter<CassandraRequest, ExecutionInfo> instrumenter() {
45+
static Instrumenter<CassandraRequest, ExecutionInfo> instrumenter() {
4646
return instrumenter;
4747
}
4848

instrumentation/cassandra/cassandra-4.0/metadata.yaml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ semantic_conventions:
88
library_link: https://github.com/apache/cassandra-java-driver
99
configurations:
1010
- name: otel.instrumentation.common.db.query-sanitization.enabled
11+
declarative_name: java.common.db.query_sanitization.enabled
1112
description: Enables query sanitization for database queries.
1213
type: boolean
1314
default: true

instrumentation/cassandra/cassandra-4.4/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/cassandra/v4_4/SessionBuilderInstrumentation.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ public static class BuildAdvice {
3939
/**
4040
* Strategy: each time we build a connection to a Cassandra cluster, the
4141
* com.datastax.oss.driver.api.core.session.SessionBuilder.buildAsync() method is called. The
42-
* opentracing contribution is a simple wrapper, so we just have to wrap the new session.
42+
* OpenTelemetry instrumentation is a simple wrapper, so we just have to wrap the new session.
4343
*
4444
* @param stage The fresh CompletionStage to patch. This stage produces session which is
4545
* replaced with new session

instrumentation/cassandra/cassandra-4.4/library/src/main/java/io/opentelemetry/instrumentation/cassandra/v4_4/CassandraAttributesExtractor.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -186,7 +186,7 @@ private static Field getProxyAddressField() {
186186
Field field = SniEndPoint.class.getDeclaredField("proxyAddress");
187187
field.setAccessible(true);
188188
return field;
189-
} catch (Exception e) {
189+
} catch (Exception ignored) {
190190
return null;
191191
}
192192
}

instrumentation/cassandra/cassandra-common-4.0/testing/src/main/java/io/opentelemetry/cassandra/common/v4_0/AbstractCassandraTest.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -158,7 +158,7 @@ void syncTest(Parameter parameter) {
158158

159159
@ParameterizedTest(name = "{index}: {0}")
160160
@MethodSource("provideAsyncParameters")
161-
void asyncTest(Parameter parameter) throws Exception {
161+
void asyncTest(Parameter parameter) {
162162
CqlSession session = getSession(parameter.keyspace);
163163
cleanup.deferCleanup(session);
164164

@@ -170,7 +170,7 @@ void asyncTest(Parameter parameter) throws Exception {
170170
.executeAsync(parameter.queryText)
171171
.toCompletableFuture()
172172
.whenComplete((result, throwable) -> testing().runWithSpan("child", () -> {}))
173-
.get());
173+
.join());
174174

175175
testing()
176176
.waitAndAssertTraces(

instrumentation/clickhouse/clickhouse-client-v1-0.5/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/clickhouse/clientv1/v0_5/ClickHouseClientV1Test.java

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -290,15 +290,15 @@ void testExecuteAndWaitThrowsException() {
290290
}
291291

292292
@Test
293-
void testAsyncExecuteQuery() throws Exception {
293+
void testAsyncExecuteQuery() {
294294
CompletableFuture<ClickHouseResponse> response =
295295
client
296296
.read(server)
297297
.format(ClickHouseFormat.RowBinaryWithNamesAndTypes)
298298
.query("select * from " + tableName)
299299
.execute();
300300

301-
ClickHouseResponse result = response.get();
301+
ClickHouseResponse result = response.join();
302302
assertThat(result).isNotNull();
303303
result.close();
304304

@@ -326,13 +326,13 @@ void testAsyncExecuteQuery() throws Exception {
326326
}
327327

328328
@Test
329-
void testSendQuery() throws Exception {
329+
void testSendQuery() {
330330
testing.runWithSpan(
331331
"parent",
332332
() -> {
333333
CompletableFuture<List<ClickHouseResponseSummary>> future =
334334
ClickHouseClient.send(server, "select * from " + tableName + " limit 1");
335-
List<ClickHouseResponseSummary> results = future.get();
335+
List<ClickHouseResponseSummary> results = future.join();
336336
assertThat(results).hasSize(1);
337337
});
338338

@@ -364,7 +364,7 @@ void testSendQuery() throws Exception {
364364
}
365365

366366
@Test
367-
void testSendMultipleQueries() throws Exception {
367+
void testSendMultipleQueries() {
368368
testing.runWithSpan(
369369
"parent",
370370
() -> {
@@ -373,7 +373,7 @@ void testSendMultipleQueries() throws Exception {
373373
server,
374374
"insert into " + tableName + " values('1')('2')('3')",
375375
"select * from " + tableName + " limit 1");
376-
List<ClickHouseResponseSummary> results = future.get();
376+
List<ClickHouseResponseSummary> results = future.join();
377377
assertThat(results).hasSize(2);
378378
});
379379

@@ -508,7 +508,7 @@ void testParameterizedQueryInput() throws ClickHouseException {
508508
// that this syntax error isn't detected when running with the agent as it is also ignored when
509509
// running without the agent.
510510
@Test
511-
void testPlaceholderQueryInput() throws Exception {
511+
void testPlaceholderQueryInput() {
512512
ClickHouseRequest<?> request =
513513
client.read(server).format(ClickHouseFormat.RowBinaryWithNamesAndTypes);
514514
testing.runWithSpan(
@@ -520,7 +520,7 @@ void testPlaceholderQueryInput() throws Exception {
520520
.query("select * from " + tableName + " where s={s:String}")
521521
.settings(ImmutableMap.of("param_s", "" + Instant.now().getEpochSecond()))
522522
.execute()
523-
.get();
523+
.join();
524524
response.close();
525525
});
526526

instrumentation/clickhouse/clickhouse-client-v2-0.8/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/clickhouse/clientv2/v0_8/ClickHouseClientV2Test.java

Lines changed: 15 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -40,8 +40,6 @@
4040
import java.util.HashMap;
4141
import java.util.List;
4242
import java.util.Map;
43-
import java.util.concurrent.CompletableFuture;
44-
import org.junit.jupiter.api.AfterAll;
4543
import org.junit.jupiter.api.BeforeAll;
4644
import org.junit.jupiter.api.Test;
4745
import org.junit.jupiter.api.extension.RegisterExtension;
@@ -53,22 +51,23 @@ class ClickHouseClientV2Test {
5351
@RegisterExtension
5452
private static final InstrumentationExtension testing = AgentInstrumentationExtension.create();
5553

56-
@RegisterExtension final AutoCleanupExtension cleanup = AutoCleanupExtension.create();
54+
@RegisterExtension static final AutoCleanupExtension cleanup = AutoCleanupExtension.create();
5755

5856
private static final GenericContainer<?> clickhouseServer =
5957
new GenericContainer<>("clickhouse/clickhouse-server:24.4.2").withExposedPorts(8123);
6058

6159
private static final String databaseName = "default";
6260
private static final String tableName = "test_table";
61+
private static final String username = "default";
62+
private static final String password = "";
6363
private static int port;
6464
private static String host;
6565
private static Client client;
66-
private static final String username = "default";
67-
private static final String password = "";
6866

6967
@BeforeAll
7068
static void setup() throws Exception {
7169
clickhouseServer.start();
70+
cleanup.deferAfterAll(clickhouseServer::stop);
7271
port = clickhouseServer.getMappedPort(8123);
7372
host = clickhouseServer.getHost();
7473

@@ -80,26 +79,19 @@ static void setup() throws Exception {
8079
.setPassword(password)
8180
.setOption("compress", "false")
8281
.build();
82+
cleanup.deferAfterAll(client);
8383

8484
QueryResponse response =
8585
client
8686
.query("create table if not exists " + tableName + "(value String) engine=Memory")
87-
.get();
87+
.join();
8888
response.close();
8989

9090
// wait for CREATE operation and clear
9191
testing.waitForTraces(1);
9292
testing.clearData();
9393
}
9494

95-
@AfterAll
96-
static void cleanup() {
97-
if (client != null) {
98-
client.close();
99-
}
100-
clickhouseServer.stop();
101-
}
102-
10395
@Test
10496
void testConnectionStringWithoutDatabaseSpecifiedStillGeneratesSpans() throws Exception {
10597
Client client =
@@ -111,7 +103,7 @@ void testConnectionStringWithoutDatabaseSpecifiedStillGeneratesSpans() throws Ex
111103
.build();
112104
cleanup.deferCleanup(client);
113105

114-
QueryResponse response = client.query("select * from " + tableName).get();
106+
QueryResponse response = client.query("select * from " + tableName).join();
115107
response.close();
116108

117109
testing.waitAndAssertTraces(
@@ -153,10 +145,10 @@ void testQueryWithStringQuery() throws Exception {
153145
"parent",
154146
() -> {
155147
QueryResponse response =
156-
client.query("insert into " + tableName + " values('1')('2')('3')").get();
148+
client.query("insert into " + tableName + " values('1')('2')('3')").join();
157149
response.close();
158150

159-
response = client.query("select * from " + tableName).get();
151+
response = client.query("select * from " + tableName).join();
160152
response.close();
161153
});
162154

@@ -214,7 +206,7 @@ void testQueryWithStringQueryAndId() throws Exception {
214206
QuerySettings querySettings = new QuerySettings();
215207
querySettings.setQueryId("test_query_id");
216208

217-
QueryResponse response = client.query("select * from " + tableName, querySettings).get();
209+
QueryResponse response = client.query("select * from " + tableName, querySettings).join();
218210
response.close();
219211
});
220212

@@ -285,9 +277,8 @@ void testSendQuery() throws Exception {
285277
testing.runWithSpan(
286278
"parent",
287279
() -> {
288-
CompletableFuture<CommandResponse> future =
289-
client.execute("select * from " + tableName + " limit 1");
290-
CommandResponse results = future.get();
280+
CommandResponse results =
281+
client.execute("select * from " + tableName + " limit 1").join();
291282
assertThat(results.getReadRows()).isEqualTo(0);
292283
});
293284

@@ -360,10 +351,10 @@ void testSendQueryRecords() throws Exception {
360351
"parent",
361352
() -> {
362353
Records records =
363-
client.queryRecords("insert into " + tableName + " values('test_value')").get();
354+
client.queryRecords("insert into " + tableName + " values('test_value')").join();
364355
records.close();
365356

366-
records = client.queryRecords("select * from " + tableName + " limit 1").get();
357+
records = client.queryRecords("select * from " + tableName + " limit 1").join();
367358
records.close();
368359
assertThat(records.getReadRows()).isEqualTo(1);
369360
});
@@ -430,7 +421,7 @@ void testPlaceholderQuery() throws Exception {
430421
"select * from " + tableName + " where value={param_s: String}",
431422
queryParams,
432423
null)
433-
.get();
424+
.join();
434425
response.close();
435426
});
436427

instrumentation/couchbase/couchbase-2.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/couchbase/v2_0/CouchbaseInstrumentationModule.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ public List<String> injectedClassNames() {
4646

4747
@Override
4848
public ElementMatcher.Junction<ClassLoader> classLoaderMatcher() {
49-
// removed in 3.0.0
49+
// added in 2.0.0, removed in 3.0.0
5050
return hasClassesNamed("com.couchbase.client.java.CouchbaseAsyncBucket");
5151
}
5252
}

instrumentation/couchbase/couchbase-2.0/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/couchbase/v2_0/CouchbaseUtil.java

Lines changed: 0 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -5,39 +5,16 @@
55

66
package io.opentelemetry.javaagent.instrumentation.couchbase.v2_0;
77

8-
import static java.util.Collections.emptyList;
98
import static java.util.concurrent.TimeUnit.DAYS;
109
import static java.util.concurrent.TimeUnit.SECONDS;
1110

1211
import com.couchbase.client.core.metrics.DefaultLatencyMetricsCollectorConfig;
1312
import com.couchbase.client.core.metrics.DefaultMetricsCollectorConfig;
1413
import com.couchbase.client.java.cluster.BucketSettings;
1514
import com.couchbase.client.java.env.DefaultCouchbaseEnvironment;
16-
import io.opentelemetry.sdk.testing.assertj.AttributeAssertion;
17-
import java.util.List;
1815

1916
public class CouchbaseUtil {
2017

21-
public static List<AttributeAssertion> couchbaseAttributes() {
22-
return couchbaseKvAttributes();
23-
}
24-
25-
public static List<AttributeAssertion> couchbaseKvAttributes() {
26-
return emptyList();
27-
}
28-
29-
public static List<AttributeAssertion> couchbaseQueryAttributes() {
30-
return emptyList();
31-
}
32-
33-
public static List<AttributeAssertion> couchbaseClusterManagerAttributes() {
34-
return emptyList();
35-
}
36-
37-
public static List<AttributeAssertion> couchbaseN1qlAttributes() {
38-
return emptyList();
39-
}
40-
4118
public static DefaultCouchbaseEnvironment.Builder envBuilder(
4219
BucketSettings bucketSettings, int carrierDirectPort, int httpDirectPort) {
4320
// Couchbase seems to be really slow to start sometimes

0 commit comments

Comments
 (0)