Skip to content

Commit 122e296

Browse files
otelbot[bot]trask
otelbot[bot]
and
trask
authored
Code review sweep (run 25067608608) (#18378)
Co-authored-by: otelbot <197425009+otelbot@users.noreply.github.com> Co-authored-by: Trask Stalnaker <trask.stalnaker@gmail.com>
1 parent 7dfb733 commit 122e296

13 files changed

Lines changed: 94 additions & 85 deletions

File tree

‎instrumentation/rediscala-1.8/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/rediscala/v1_8/OnCompleteHandler.java‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,19 +8,21 @@
88
import static io.opentelemetry.javaagent.instrumentation.rediscala.v1_8.RediscalaSingletons.instrumenter;
99

1010
import io.opentelemetry.context.Context;
11+
import javax.annotation.Nullable;
1112
import redis.RedisCommand;
1213
import scala.runtime.AbstractFunction1;
1314
import scala.util.Try;
1415

15-
public class OnCompleteHandler extends AbstractFunction1<Try<Object>, Void> {
16+
class OnCompleteHandler extends AbstractFunction1<Try<Object>, Void> {
1617
private final Context context;
1718
private final RedisCommand<?, ?> request;
1819

19-
public OnCompleteHandler(Context context, RedisCommand<?, ?> request) {
20+
OnCompleteHandler(Context context, RedisCommand<?, ?> request) {
2021
this.context = context;
2122
this.request = request;
2223
}
2324

25+
@Nullable
2426
@Override
2527
public Void apply(Try<Object> result) {
2628
Throwable error = null;

‎instrumentation/redisson/redisson-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/CompletableFutureWrapper.java‎

Lines changed: 16 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -8,10 +8,23 @@
88
import io.opentelemetry.context.Context;
99
import io.opentelemetry.context.Scope;
1010
import java.util.concurrent.CompletableFuture;
11+
import javax.annotation.Nullable;
1112

1213
public class CompletableFutureWrapper<T> extends CompletableFuture<T> implements PromiseWrapper<T> {
13-
private static final Class<?> batchPromiseClass = getBatchPromiseClass();
14-
private volatile EndOperationListener<T> endOperationListener;
14+
@Nullable private static final Class<?> BATCH_PROMISE_CLASS = getBatchPromiseClass();
15+
16+
@Nullable
17+
private static Class<?> getBatchPromiseClass() {
18+
try {
19+
// using Class.forName because this class is not available in the redisson version we compile
20+
// against
21+
return Class.forName("org.redisson.command.BatchPromise");
22+
} catch (ClassNotFoundException ignored) {
23+
return null;
24+
}
25+
}
26+
27+
@Nullable private volatile EndOperationListener<T> endOperationListener;
1528

1629
private CompletableFutureWrapper(CompletableFuture<T> delegate) {
1730
Context context = Context.current();
@@ -37,7 +50,7 @@ private CompletableFutureWrapper(CompletableFuture<T> delegate) {
3750
*/
3851
public static <T> CompletableFuture<T> wrap(CompletableFuture<T> delegate) {
3952
if (delegate instanceof CompletableFutureWrapper
40-
|| (batchPromiseClass != null && batchPromiseClass.isInstance(delegate))) {
53+
|| (BATCH_PROMISE_CLASS != null && BATCH_PROMISE_CLASS.isInstance(delegate))) {
4154
return delegate;
4255
}
4356

@@ -48,14 +61,4 @@ public static <T> CompletableFuture<T> wrap(CompletableFuture<T> delegate) {
4861
public void setEndOperationListener(EndOperationListener<T> endOperationListener) {
4962
this.endOperationListener = endOperationListener;
5063
}
51-
52-
private static Class<?> getBatchPromiseClass() {
53-
try {
54-
// using Class.forName because this class is not available in the redisson version we compile
55-
// against
56-
return Class.forName("org.redisson.command.BatchPromise");
57-
} catch (ClassNotFoundException ignored) {
58-
return null;
59-
}
60-
}
6164
}

‎instrumentation/redisson/redisson-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/EndOperationListener.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
import io.opentelemetry.context.Context;
99
import io.opentelemetry.instrumentation.api.instrumenter.Instrumenter;
1010
import java.util.function.BiConsumer;
11+
import javax.annotation.Nullable;
1112

1213
public class EndOperationListener<T> implements BiConsumer<T, Throwable> {
1314
private final Instrumenter<RedissonRequest, Void> instrumenter;
@@ -22,7 +23,7 @@ public EndOperationListener(
2223
}
2324

2425
@Override
25-
public void accept(T t, Throwable error) {
26+
public void accept(@Nullable T unused, @Nullable Throwable error) {
2627
instrumenter.end(context, request, null, error);
2728
}
2829
}

‎instrumentation/redisson/redisson-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/RedissonRequest.java‎

Lines changed: 31 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,37 @@ public abstract class RedissonRequest {
3636
RedisCommandSanitizer.create(
3737
DbConfig.isQuerySanitizationEnabled(GlobalOpenTelemetry.get(), "redisson"));
3838

39+
@Nullable
40+
private static final MethodHandle COMMAND_DATA_GET_PROMISE =
41+
findGetPromiseMethod(CommandData.class);
42+
43+
@Nullable
44+
private static final MethodHandle COMMANDS_DATA_GET_PROMISE =
45+
findGetPromiseMethod(CommandsData.class);
46+
47+
@Nullable
48+
private static MethodHandle findGetPromiseMethod(Class<?> commandClass) {
49+
MethodHandles.Lookup lookup = MethodHandles.publicLookup();
50+
try {
51+
Class<?> promiseClass =
52+
Class.forName(
53+
"org.redisson.misc.RPromise", false, RedissonRequest.class.getClassLoader());
54+
// try versions older than 3.16.8
55+
return lookup.findVirtual(commandClass, "getPromise", MethodType.methodType(promiseClass));
56+
} catch (NoSuchMethodException | ClassNotFoundException ignored) {
57+
// in 3.16.8 CommandsData#getPromise() and CommandData#getPromise() return type was changed in
58+
// a backwards-incompatible way from RPromise to CompletableFuture
59+
try {
60+
return lookup.findVirtual(
61+
commandClass, "getPromise", MethodType.methodType(CompletableFuture.class));
62+
} catch (NoSuchMethodException | IllegalAccessException ignore) {
63+
return null;
64+
}
65+
} catch (IllegalAccessException ignored) {
66+
return null;
67+
}
68+
}
69+
3970
public static RedissonRequest create(InetSocketAddress address, Object command) {
4071
return new AutoValue_RedissonRequest(address, command);
4172
}
@@ -142,31 +173,4 @@ private CompletionStage<?> getPromise() {
142173
}
143174
return null;
144175
}
145-
146-
private static final MethodHandle COMMAND_DATA_GET_PROMISE =
147-
findGetPromiseMethod(CommandData.class);
148-
private static final MethodHandle COMMANDS_DATA_GET_PROMISE =
149-
findGetPromiseMethod(CommandsData.class);
150-
151-
private static MethodHandle findGetPromiseMethod(Class<?> commandClass) {
152-
MethodHandles.Lookup lookup = MethodHandles.publicLookup();
153-
try {
154-
Class<?> promiseClass =
155-
Class.forName(
156-
"org.redisson.misc.RPromise", false, RedissonRequest.class.getClassLoader());
157-
// try versions older than 3.16.8
158-
return lookup.findVirtual(commandClass, "getPromise", MethodType.methodType(promiseClass));
159-
} catch (NoSuchMethodException | ClassNotFoundException ignored) {
160-
// in 3.16.8 CommandsData#getPromise() and CommandData#getPromise() return type was changed in
161-
// a backwards-incompatible way from RPromise to CompletableFuture
162-
try {
163-
return lookup.findVirtual(
164-
commandClass, "getPromise", MethodType.methodType(CompletableFuture.class));
165-
} catch (NoSuchMethodException | IllegalAccessException ignore) {
166-
return null;
167-
}
168-
} catch (IllegalAccessException ignored) {
169-
return null;
170-
}
171-
}
172176
}

‎instrumentation/redisson/redisson-common/testing/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/AbstractRedissonAsyncClientTest.java‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -136,7 +136,7 @@ void futureSet() throws ExecutionException, InterruptedException, TimeoutExcepti
136136
.hasAttributesSatisfyingExactly(
137137
equalTo(NETWORK_TYPE, emitOldDatabaseSemconv() ? IPV4 : null),
138138
equalTo(NETWORK_PEER_ADDRESS, ip),
139-
equalTo(NETWORK_PEER_PORT, (long) port),
139+
equalTo(NETWORK_PEER_PORT, port),
140140
equalTo(maybeStable(DB_SYSTEM), REDIS),
141141
equalTo(maybeStable(DB_STATEMENT), "SET foo ?"),
142142
equalTo(maybeStable(DB_OPERATION), "SET"))));
@@ -169,7 +169,7 @@ void futureWhenComplete() throws ExecutionException, InterruptedException, Timeo
169169
.hasAttributesSatisfyingExactly(
170170
equalTo(NETWORK_TYPE, emitOldDatabaseSemconv() ? IPV4 : null),
171171
equalTo(NETWORK_PEER_ADDRESS, ip),
172-
equalTo(NETWORK_PEER_PORT, (long) port),
172+
equalTo(NETWORK_PEER_PORT, port),
173173
equalTo(maybeStable(DB_SYSTEM), REDIS),
174174
equalTo(maybeStable(DB_STATEMENT), "SADD set1 ?"),
175175
equalTo(maybeStable(DB_OPERATION), "SADD"))
@@ -242,7 +242,7 @@ void atomicBatchCommand() throws ExecutionException, InterruptedException, Timeo
242242
.hasAttributesSatisfyingExactly(
243243
equalTo(NETWORK_TYPE, emitOldDatabaseSemconv() ? IPV4 : null),
244244
equalTo(NETWORK_PEER_ADDRESS, ip),
245-
equalTo(NETWORK_PEER_PORT, (long) port),
245+
equalTo(NETWORK_PEER_PORT, port),
246246
equalTo(maybeStable(DB_SYSTEM), REDIS),
247247
equalTo(maybeStable(DB_STATEMENT), "MULTI;SET batch1 ?"))
248248
.hasParent(trace.getSpan(0)),
@@ -252,7 +252,7 @@ void atomicBatchCommand() throws ExecutionException, InterruptedException, Timeo
252252
.hasAttributesSatisfyingExactly(
253253
equalTo(NETWORK_TYPE, emitOldDatabaseSemconv() ? IPV4 : null),
254254
equalTo(NETWORK_PEER_ADDRESS, ip),
255-
equalTo(NETWORK_PEER_PORT, (long) port),
255+
equalTo(NETWORK_PEER_PORT, port),
256256
equalTo(maybeStable(DB_SYSTEM), REDIS),
257257
equalTo(maybeStable(DB_STATEMENT), "SET batch2 ?"),
258258
equalTo(maybeStable(DB_OPERATION), "SET"))
@@ -263,7 +263,7 @@ void atomicBatchCommand() throws ExecutionException, InterruptedException, Timeo
263263
.hasAttributesSatisfyingExactly(
264264
equalTo(NETWORK_TYPE, emitOldDatabaseSemconv() ? IPV4 : null),
265265
equalTo(NETWORK_PEER_ADDRESS, ip),
266-
equalTo(NETWORK_PEER_PORT, (long) port),
266+
equalTo(NETWORK_PEER_PORT, port),
267267
equalTo(maybeStable(DB_SYSTEM), REDIS),
268268
equalTo(maybeStable(DB_STATEMENT), "EXEC"),
269269
equalTo(maybeStable(DB_OPERATION), "EXEC"))

‎instrumentation/redisson/redisson-common/testing/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/AbstractRedissonClientTest.java‎

Lines changed: 16 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -150,7 +150,7 @@ void testDurationMetric() {
150150
.hasAttributesSatisfyingExactly(
151151
equalTo(NETWORK_TYPE, emitOldDatabaseSemconv() ? IPV4 : null),
152152
equalTo(NETWORK_PEER_ADDRESS, ip),
153-
equalTo(NETWORK_PEER_PORT, (long) port),
153+
equalTo(NETWORK_PEER_PORT, port),
154154
equalTo(maybeStable(DB_SYSTEM), REDIS),
155155
equalTo(maybeStable(DB_STATEMENT), "SET foo ?"),
156156
equalTo(maybeStable(DB_OPERATION), "SET")));
@@ -180,7 +180,7 @@ void stringCommand() {
180180
.hasAttributesSatisfyingExactly(
181181
equalTo(NETWORK_TYPE, emitOldDatabaseSemconv() ? IPV4 : null),
182182
equalTo(NETWORK_PEER_ADDRESS, ip),
183-
equalTo(NETWORK_PEER_PORT, (long) port),
183+
equalTo(NETWORK_PEER_PORT, port),
184184
equalTo(maybeStable(DB_SYSTEM), REDIS),
185185
equalTo(maybeStable(DB_STATEMENT), "SET foo ?"),
186186
equalTo(maybeStable(DB_OPERATION), "SET"))),
@@ -192,7 +192,7 @@ void stringCommand() {
192192
.hasAttributesSatisfyingExactly(
193193
equalTo(NETWORK_TYPE, emitOldDatabaseSemconv() ? IPV4 : null),
194194
equalTo(NETWORK_PEER_ADDRESS, ip),
195-
equalTo(NETWORK_PEER_PORT, (long) port),
195+
equalTo(NETWORK_PEER_PORT, port),
196196
equalTo(maybeStable(DB_SYSTEM), REDIS),
197197
equalTo(maybeStable(DB_STATEMENT), "GET foo"),
198198
equalTo(maybeStable(DB_OPERATION), "GET"))));
@@ -217,7 +217,7 @@ void batchCommand() throws ReflectiveOperationException {
217217
.hasAttributesSatisfyingExactly(
218218
equalTo(NETWORK_TYPE, emitOldDatabaseSemconv() ? IPV4 : null),
219219
equalTo(NETWORK_PEER_ADDRESS, ip),
220-
equalTo(NETWORK_PEER_PORT, (long) port),
220+
equalTo(NETWORK_PEER_PORT, port),
221221
equalTo(maybeStable(DB_SYSTEM), REDIS),
222222
equalTo(maybeStable(DB_STATEMENT), "SET batch1 ?;SET batch2 ?"))));
223223
}
@@ -256,7 +256,7 @@ void atomicBatchCommand() {
256256
.hasAttributesSatisfyingExactly(
257257
equalTo(NETWORK_TYPE, emitOldDatabaseSemconv() ? IPV4 : null),
258258
equalTo(NETWORK_PEER_ADDRESS, ip),
259-
equalTo(NETWORK_PEER_PORT, (long) port),
259+
equalTo(NETWORK_PEER_PORT, port),
260260
equalTo(maybeStable(DB_SYSTEM), REDIS),
261261
equalTo(maybeStable(DB_STATEMENT), "MULTI;SET batch1 ?"))
262262
.hasParent(trace.getSpan(0)),
@@ -266,7 +266,7 @@ void atomicBatchCommand() {
266266
.hasAttributesSatisfyingExactly(
267267
equalTo(NETWORK_TYPE, emitOldDatabaseSemconv() ? IPV4 : null),
268268
equalTo(NETWORK_PEER_ADDRESS, ip),
269-
equalTo(NETWORK_PEER_PORT, (long) port),
269+
equalTo(NETWORK_PEER_PORT, port),
270270
equalTo(maybeStable(DB_SYSTEM), REDIS),
271271
equalTo(maybeStable(DB_STATEMENT), "SET batch2 ?"),
272272
equalTo(maybeStable(DB_OPERATION), "SET"))
@@ -277,7 +277,7 @@ void atomicBatchCommand() {
277277
.hasAttributesSatisfyingExactly(
278278
equalTo(NETWORK_TYPE, emitOldDatabaseSemconv() ? IPV4 : null),
279279
equalTo(NETWORK_PEER_ADDRESS, ip),
280-
equalTo(NETWORK_PEER_PORT, (long) port),
280+
equalTo(NETWORK_PEER_PORT, port),
281281
equalTo(maybeStable(DB_SYSTEM), REDIS),
282282
equalTo(maybeStable(DB_STATEMENT), "EXEC"),
283283
equalTo(maybeStable(DB_OPERATION), "EXEC"))
@@ -299,7 +299,7 @@ void listCommand() {
299299
.hasAttributesSatisfyingExactly(
300300
equalTo(NETWORK_TYPE, emitOldDatabaseSemconv() ? IPV4 : null),
301301
equalTo(NETWORK_PEER_ADDRESS, ip),
302-
equalTo(NETWORK_PEER_PORT, (long) port),
302+
equalTo(NETWORK_PEER_PORT, port),
303303
equalTo(maybeStable(DB_SYSTEM), REDIS),
304304
equalTo(maybeStable(DB_STATEMENT), "RPUSH list1 ?"),
305305
equalTo(maybeStable(DB_OPERATION), "RPUSH"))
@@ -324,7 +324,7 @@ void hashCommand() {
324324
.hasAttributesSatisfyingExactly(
325325
equalTo(NETWORK_TYPE, emitOldDatabaseSemconv() ? IPV4 : null),
326326
equalTo(NETWORK_PEER_ADDRESS, ip),
327-
equalTo(NETWORK_PEER_PORT, (long) port),
327+
equalTo(NETWORK_PEER_PORT, port),
328328
equalTo(maybeStable(DB_SYSTEM), REDIS),
329329
equalTo(
330330
maybeStable(DB_STATEMENT),
@@ -338,7 +338,7 @@ void hashCommand() {
338338
.hasAttributesSatisfyingExactly(
339339
equalTo(NETWORK_TYPE, emitOldDatabaseSemconv() ? IPV4 : null),
340340
equalTo(NETWORK_PEER_ADDRESS, ip),
341-
equalTo(NETWORK_PEER_PORT, (long) port),
341+
equalTo(NETWORK_PEER_PORT, port),
342342
equalTo(maybeStable(DB_SYSTEM), REDIS),
343343
equalTo(maybeStable(DB_STATEMENT), "HGET map1 key1"),
344344
equalTo(maybeStable(DB_OPERATION), "HGET"))));
@@ -359,7 +359,7 @@ void setCommand() {
359359
.hasAttributesSatisfyingExactly(
360360
equalTo(NETWORK_TYPE, emitOldDatabaseSemconv() ? IPV4 : null),
361361
equalTo(NETWORK_PEER_ADDRESS, ip),
362-
equalTo(NETWORK_PEER_PORT, (long) port),
362+
equalTo(NETWORK_PEER_PORT, port),
363363
equalTo(maybeStable(DB_SYSTEM), REDIS),
364364
equalTo(maybeStable(DB_STATEMENT), "SADD set1 ?"),
365365
equalTo(maybeStable(DB_OPERATION), "SADD"))));
@@ -386,7 +386,7 @@ void sortedSetCommand() throws ReflectiveOperationException {
386386
.hasAttributesSatisfyingExactly(
387387
equalTo(NETWORK_TYPE, emitOldDatabaseSemconv() ? IPV4 : null),
388388
equalTo(NETWORK_PEER_ADDRESS, ip),
389-
equalTo(NETWORK_PEER_PORT, (long) port),
389+
equalTo(NETWORK_PEER_PORT, port),
390390
equalTo(maybeStable(DB_SYSTEM), REDIS),
391391
equalTo(maybeStable(DB_STATEMENT), "ZADD sort_set1 ? ? ? ? ? ?"),
392392
equalTo(maybeStable(DB_OPERATION), "ZADD"))));
@@ -412,7 +412,7 @@ void atomicLongCommand() {
412412
.hasAttributesSatisfyingExactly(
413413
equalTo(NETWORK_TYPE, emitOldDatabaseSemconv() ? IPV4 : null),
414414
equalTo(NETWORK_PEER_ADDRESS, ip),
415-
equalTo(NETWORK_PEER_PORT, (long) port),
415+
equalTo(NETWORK_PEER_PORT, port),
416416
equalTo(maybeStable(DB_SYSTEM), REDIS),
417417
equalTo(maybeStable(DB_STATEMENT), "INCR AtomicLong"),
418418
equalTo(maybeStable(DB_OPERATION), "INCR"))));
@@ -438,7 +438,7 @@ void lockCommand() {
438438
.hasAttributesSatisfyingExactly(
439439
equalTo(NETWORK_TYPE, emitOldDatabaseSemconv() ? IPV4 : null),
440440
equalTo(NETWORK_PEER_ADDRESS, ip),
441-
equalTo(NETWORK_PEER_PORT, (long) port),
441+
equalTo(NETWORK_PEER_PORT, port),
442442
equalTo(maybeStable(DB_SYSTEM), REDIS),
443443
equalTo(maybeStable(DB_OPERATION), "EVAL"),
444444
satisfies(maybeStable(DB_STATEMENT), val -> val.startsWith("EVAL")))));
@@ -451,7 +451,7 @@ void lockCommand() {
451451
.hasAttributesSatisfyingExactly(
452452
equalTo(NETWORK_TYPE, emitOldDatabaseSemconv() ? IPV4 : null),
453453
equalTo(NETWORK_PEER_ADDRESS, ip),
454-
equalTo(NETWORK_PEER_PORT, (long) port),
454+
equalTo(NETWORK_PEER_PORT, port),
455455
equalTo(maybeStable(DB_SYSTEM), REDIS),
456456
equalTo(maybeStable(DB_OPERATION), "EVAL"),
457457
satisfies(maybeStable(DB_STATEMENT), val -> val.startsWith("EVAL")))));
@@ -465,7 +465,7 @@ void lockCommand() {
465465
.hasAttributesSatisfyingExactly(
466466
equalTo(NETWORK_TYPE, emitOldDatabaseSemconv() ? IPV4 : null),
467467
equalTo(NETWORK_PEER_ADDRESS, ip),
468-
equalTo(NETWORK_PEER_PORT, (long) port),
468+
equalTo(NETWORK_PEER_PORT, port),
469469
equalTo(maybeStable(DB_SYSTEM), REDIS),
470470
equalTo(maybeStable(DB_OPERATION), "DEL"),
471471
satisfies(maybeStable(DB_STATEMENT), val -> val.startsWith("DEL")))));

0 commit comments

Comments
 (0)