Skip to content

Commit fd994a6

Browse files
authored
Code review sweep (run 25195526141) (open-telemetry#18468)
Co-authored-by: otelbot <197425009+otelbot@users.noreply.github.com>
1 parent 5349bca commit fd994a6

11 files changed

Lines changed: 35 additions & 44 deletions

File tree

instrumentation/lettuce/lettuce-4.0/metadata.yaml

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,14 +8,17 @@ semantic_conventions:
88
library_link: https://github.com/redis/lettuce
99
configurations:
1010
- name: otel.instrumentation.lettuce.connection-telemetry.enabled
11+
declarative_name: java.lettuce.connection_telemetry.enabled
1112
description: Enables connection telemetry spans for Redis connections.
1213
type: boolean
1314
default: false
1415
- name: otel.instrumentation.lettuce.experimental-span-attributes
16+
declarative_name: java.lettuce.experimental_span_attributes/development
1517
description: Enables experimental span attribute `lettuce.command.cancelled`.
1618
type: boolean
1719
default: false
1820
- name: otel.instrumentation.common.peer-service-mapping
21+
declarative_name: java.common.peer_service_mapping
1922
description: Used to specify a mapping from host names or IP addresses to peer services.
2023
type: map
2124
default: ""

instrumentation/lettuce/lettuce-5.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/lettuce/v5_0/LettuceAsyncCommandsInstrumentation.java

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -52,10 +52,10 @@ public AdviceScope(Context context, Scope scope) {
5252
public void end(
5353
@Nullable Throwable throwable,
5454
RedisCommand<?, ?, ?> command,
55-
AsyncCommand<?, ?, ?> asyncCommand) {
55+
@Nullable AsyncCommand<?, ?, ?> asyncCommand) {
5656
scope.close();
5757

58-
if (throwable != null) {
58+
if (throwable != null || asyncCommand == null) {
5959
instrumenter().end(context, command, null, throwable);
6060
return;
6161
}
@@ -70,6 +70,7 @@ public void end(
7070
}
7171

7272
@Advice.OnMethodEnter(suppress = Throwable.class, inline = false)
73+
@Nullable
7374
public static AdviceScope onEnter(@Advice.Argument(0) RedisCommand<?, ?, ?> command) {
7475

7576
Context parentContext = currentContext();

instrumentation/lettuce/lettuce-5.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/lettuce/v5_0/LettuceClientInstrumentation.java

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -56,11 +56,13 @@ public AdviceScope(Context context, Scope scope) {
5656
}
5757

5858
public void end(
59-
@Nullable Throwable throwable, RedisURI redisUri, ConnectionFuture<?> connectionFuture) {
59+
@Nullable Throwable throwable,
60+
RedisURI redisUri,
61+
@Nullable ConnectionFuture<?> connectionFuture) {
6062

6163
scope.close();
6264

63-
if (throwable != null) {
65+
if (throwable != null || connectionFuture == null) {
6466
connectInstrumenter().end(context, redisUri, null, throwable);
6567
return;
6668
}
@@ -69,6 +71,7 @@ public void end(
6971
}
7072

7173
@Advice.OnMethodEnter(suppress = Throwable.class, inline = false)
74+
@Nullable
7275
public static AdviceScope onEnter(@Advice.Argument(1) RedisURI redisUri) {
7376
Context parentContext = currentContext();
7477
if (!connectInstrumenter().shouldStart(parentContext, redisUri)) {

instrumentation/lettuce/lettuce-5.0/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/lettuce/v5_0/AbstractLettuceClientTest.java

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -36,10 +36,10 @@ abstract class AbstractLettuceClientTest {
3636
protected static final ClientOptions CLIENT_OPTIONS =
3737
ClientOptions.builder().autoReconnect(false).build();
3838

39-
static final DockerImageName containerImage = DockerImageName.parse("redis:6.2.3-alpine");
39+
static final DockerImageName CONTAINER_IMAGE = DockerImageName.parse("redis:6.2.3-alpine");
4040

4141
protected static final GenericContainer<?> redisServer =
42-
new GenericContainer<>(containerImage)
42+
new GenericContainer<>(CONTAINER_IMAGE)
4343
.withExposedPorts(6379)
4444
.withLogConsumer(new Slf4jLogConsumer(logger))
4545
.waitingFor(Wait.forLogMessage(".*Ready to accept connections.*", 1));
@@ -58,7 +58,7 @@ abstract class AbstractLettuceClientTest {
5858

5959
protected static StatefulRedisConnection<String, String> newContainerConnection() {
6060
GenericContainer<?> server =
61-
new GenericContainer<>(containerImage)
61+
new GenericContainer<>(CONTAINER_IMAGE)
6262
.withExposedPorts(6379)
6363
.withLogConsumer(new Slf4jLogConsumer(logger))
6464
.waitingFor(Wait.forLogMessage(".*Ready to accept connections.*", 1));

instrumentation/lettuce/lettuce-5.0/metadata.yaml

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,20 +8,24 @@ semantic_conventions:
88
library_link: https://github.com/redis/lettuce
99
configurations:
1010
- name: otel.instrumentation.lettuce.connection-telemetry.enabled
11+
declarative_name: java.lettuce.connection_telemetry.enabled
1112
description: Enables connection telemetry spans for Redis connections.
1213
type: boolean
1314
default: false
1415
- name: otel.instrumentation.lettuce.experimental-span-attributes
16+
declarative_name: java.lettuce.experimental_span_attributes/development
1517
description: >
1618
Enables experimental span attributes `lettuce.command.cancelled` and
1719
`lettuce.command.results.count`.
1820
type: boolean
1921
default: false
2022
- name: otel.instrumentation.common.db.query-sanitization.enabled
23+
declarative_name: java.common.db.query_sanitization.enabled
2124
description: Enables query sanitization for database queries.
2225
type: boolean
2326
default: true
2427
- name: otel.instrumentation.common.peer-service-mapping
28+
declarative_name: java.common.peer_service_mapping
2529
description: Used to specify a mapping from host names or IP addresses to peer services.
2630
type: map
2731
default: ""

instrumentation/lettuce/lettuce-5.1/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/lettuce/v5_1/LettuceSyncClientAuthTest.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,12 +6,12 @@
66
package io.opentelemetry.javaagent.instrumentation.lettuce.v5_1;
77

88
import io.lettuce.core.RedisClient;
9-
import io.opentelemetry.instrumentation.lettuce.v5_1.AbstractLettuceSyncClientTest;
9+
import io.opentelemetry.instrumentation.lettuce.v5_1.AbstractLettuceSyncClientAuthTest;
1010
import io.opentelemetry.instrumentation.testing.junit.AgentInstrumentationExtension;
1111
import io.opentelemetry.instrumentation.testing.junit.InstrumentationExtension;
1212
import org.junit.jupiter.api.extension.RegisterExtension;
1313

14-
class LettuceSyncClientAuthTest extends AbstractLettuceSyncClientTest {
14+
class LettuceSyncClientAuthTest extends AbstractLettuceSyncClientAuthTest {
1515
@RegisterExtension
1616
static final InstrumentationExtension testing = AgentInstrumentationExtension.create();
1717

instrumentation/lettuce/lettuce-5.1/library/src/main/java/io/opentelemetry/instrumentation/lettuce/v5_1/LettuceTelemetryBuilder.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,7 @@ public LettuceTelemetry build() {
7070
if (response != null && response.getErrorMessage() != null) {
7171
spanStatusBuilder.setStatus(StatusCode.ERROR);
7272
} else {
73-
SpanStatusExtractor.<LettuceRequest, LettuceResponse>getDefault()
73+
SpanStatusExtractor.getDefault()
7474
.extract(spanStatusBuilder, request, response, error);
7575
}
7676
})

instrumentation/lettuce/lettuce-5.1/testing/src/main/java/io/opentelemetry/instrumentation/lettuce/v5_1/AbstractLettuceAsyncClientTest.java

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,6 @@
4646
import java.util.function.BiFunction;
4747
import java.util.function.Consumer;
4848
import java.util.function.Function;
49-
import org.junit.jupiter.api.AfterAll;
5049
import org.junit.jupiter.api.BeforeAll;
5150
import org.junit.jupiter.api.Test;
5251

@@ -66,6 +65,7 @@ public abstract class AbstractLettuceAsyncClientTest extends AbstractLettuceClie
6665
@BeforeAll
6766
void setUp() throws UnknownHostException {
6867
redisServer.start();
68+
cleanup.deferAfterAll(redisServer::stop);
6969
host = redisServer.getHost();
7070
ip = InetAddress.getByName(host).getHostAddress();
7171
port = redisServer.getMappedPort(6379);
@@ -75,9 +75,11 @@ void setUp() throws UnknownHostException {
7575
dbUriNonExistent = "redis://" + host + ":" + incorrectPort + "/" + DB_INDEX;
7676

7777
redisClient = createClient(embeddedDbUri);
78+
cleanup.deferAfterAll(redisClient::shutdown);
7879
redisClient.setOptions(LettuceTestUtil.CLIENT_OPTIONS);
7980

8081
connection = redisClient.connect();
82+
cleanup.deferAfterAll(connection);
8183
asyncCommands = connection.async();
8284
RedisCommands<String, String> syncCommands = connection.sync();
8385

@@ -88,13 +90,6 @@ void setUp() throws UnknownHostException {
8890
testing().clearData();
8991
}
9092

91-
@AfterAll
92-
static void cleanUp() {
93-
connection.close();
94-
redisClient.shutdown();
95-
redisServer.stop();
96-
}
97-
9893
boolean testCallback() {
9994
return true;
10095
}

instrumentation/lettuce/lettuce-5.1/testing/src/main/java/io/opentelemetry/instrumentation/lettuce/v5_1/AbstractLettuceReactiveClientTest.java

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,6 @@
2929
import java.net.UnknownHostException;
3030
import java.util.concurrent.CompletableFuture;
3131
import java.util.function.Consumer;
32-
import org.junit.jupiter.api.AfterAll;
3332
import org.junit.jupiter.api.BeforeAll;
3433
import org.junit.jupiter.api.Test;
3534

@@ -41,15 +40,18 @@ public abstract class AbstractLettuceReactiveClientTest extends AbstractLettuceC
4140
@BeforeAll
4241
void setUp() throws UnknownHostException {
4342
redisServer.start();
43+
cleanup.deferAfterAll(redisServer::stop);
4444

4545
host = redisServer.getHost();
4646
ip = InetAddress.getByName(host).getHostAddress();
4747
port = redisServer.getMappedPort(6379);
4848
embeddedDbUri = "redis://" + host + ":" + port + "/" + DB_INDEX;
4949
redisClient = createClient(embeddedDbUri);
50+
cleanup.deferAfterAll(redisClient::shutdown);
5051
redisClient.setOptions(LettuceTestUtil.CLIENT_OPTIONS);
5152

5253
connection = redisClient.connect();
54+
cleanup.deferAfterAll(connection);
5355
reactiveCommands = connection.reactive();
5456
RedisCommands<String, String> syncCommands = connection.sync();
5557

@@ -60,13 +62,6 @@ void setUp() throws UnknownHostException {
6062
testing().clearData();
6163
}
6264

63-
@AfterAll
64-
static void cleanUp() {
65-
connection.close();
66-
redisClient.shutdown();
67-
redisServer.stop();
68-
}
69-
7065
@Test
7166
void testSetCommandWithSubscribeOnDefinedConsumer() throws Exception {
7267
CompletableFuture<String> future = new CompletableFuture<>();

instrumentation/lettuce/lettuce-5.1/testing/src/main/java/io/opentelemetry/instrumentation/lettuce/v5_1/AbstractLettuceSyncClientAuthTest.java

Lines changed: 5 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,6 @@
3030
import java.lang.reflect.Method;
3131
import java.net.InetAddress;
3232
import java.net.UnknownHostException;
33-
import org.junit.jupiter.api.AfterAll;
3433
import org.junit.jupiter.api.BeforeAll;
3534
import org.junit.jupiter.api.Test;
3635

@@ -41,25 +40,21 @@ public abstract class AbstractLettuceSyncClientAuthTest extends AbstractLettuceC
4140
void setUp() throws UnknownHostException {
4241
redisServer = redisServer.withCommand("redis-server", "--requirepass password");
4342
redisServer.start();
43+
// Set back so other tests don't fail due to NOAUTH error.
44+
cleanup.deferAfterAll(
45+
() -> redisServer = redisServer.withCommand("redis-server", "--requirepass \"\""));
46+
cleanup.deferAfterAll(redisServer::stop);
4447

4548
host = redisServer.getHost();
4649
ip = InetAddress.getByName(host).getHostAddress();
4750
port = redisServer.getMappedPort(6379);
4851
embeddedDbUri = "redis://" + host + ":" + port + "/" + DB_INDEX;
4952

5053
redisClient = createClient(embeddedDbUri);
54+
cleanup.deferAfterAll(redisClient::shutdown);
5155
redisClient.setOptions(LettuceTestUtil.CLIENT_OPTIONS);
5256
}
5357

54-
@AfterAll
55-
static void cleanUp() {
56-
redisClient.shutdown();
57-
redisServer.stop();
58-
59-
// Set back so other tests don't fail due to NOAUTH error
60-
redisServer = redisServer.withCommand("redis-server", "--requirepass \"\"");
61-
}
62-
6358
@Test
6459
void testAuthCommand() throws ReflectiveOperationException {
6560
Class<?> commandsClass = RedisCommands.class;

0 commit comments

Comments
 (0)