Skip to content

Commit 53155ef

Browse files
authored
Code review sweep (run 25028314434) (open-telemetry#18360)
Co-authored-by: otelbot <197425009+otelbot@users.noreply.github.com>
1 parent 1825b84 commit 53155ef

10 files changed

Lines changed: 140 additions & 109 deletions

File tree

instrumentation/netty/netty-common-4.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/common/v4_0/FutureListenerWrappers.java

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ public class FutureListenerWrappers {
2929
private static final VirtualField<
3030
GenericFutureListener<? extends Future<?>>,
3131
WeakReference<GenericFutureListener<? extends Future<?>>>>
32-
wrapperVirtualField = VirtualField.find(GenericFutureListener.class, WeakReference.class);
32+
WRAPPER_VIRTUAL_FIELD = VirtualField.find(GenericFutureListener.class, WeakReference.class);
3333

3434
private static final ClassValue<Boolean> shouldWrap =
3535
new ClassValue<Boolean>() {
@@ -54,7 +54,7 @@ public static GenericFutureListener<?> wrap(
5454
// collected before we have a chance to make (and return) a strong reference to the wrapper
5555

5656
WeakReference<GenericFutureListener<? extends Future<?>>> resultReference =
57-
wrapperVirtualField.get(delegate);
57+
WRAPPER_VIRTUAL_FIELD.get(delegate);
5858

5959
if (resultReference != null) {
6060
GenericFutureListener<? extends Future<?>> wrapper = resultReference.get();
@@ -75,14 +75,14 @@ public static GenericFutureListener<?> wrap(
7575
} else {
7676
wrapper = new WrappedFutureListener(context, (GenericFutureListener<Future<?>>) delegate);
7777
}
78-
wrapperVirtualField.set(delegate, new WeakReference<>(wrapper));
78+
WRAPPER_VIRTUAL_FIELD.set(delegate, new WeakReference<>(wrapper));
7979
return wrapper;
8080
}
8181

8282
public static GenericFutureListener<? extends Future<?>> getWrapper(
8383
GenericFutureListener<? extends Future<?>> delegate) {
8484
WeakReference<GenericFutureListener<? extends Future<?>>> wrapperReference =
85-
wrapperVirtualField.get(delegate);
85+
WRAPPER_VIRTUAL_FIELD.get(delegate);
8686
if (wrapperReference == null) {
8787
return delegate;
8888
}

instrumentation/okhttp/okhttp-2.2/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/okhttp/v2_2/OkHttpClientInstrumentation.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ public void transform(TypeTransformer transformer) {
3131
@SuppressWarnings("unused")
3232
public static class ConstructorAdvice {
3333

34-
@Advice.OnMethodExit(inline = false)
34+
@Advice.OnMethodExit(suppress = Throwable.class, inline = false)
3535
public static void addTracingInterceptor(@Advice.This OkHttpClient client) {
3636
for (Interceptor interceptor : client.interceptors()) {
3737
if (interceptor instanceof TracingInterceptor) {

instrumentation/okhttp/okhttp-2.2/metadata.yaml

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,16 +5,19 @@ semantic_conventions:
55
- HTTP_CLIENT_METRICS
66
configurations:
77
- name: otel.instrumentation.http.known-methods
8+
declarative_name: java.common.http.known_methods
89
description: >
910
Configures the instrumentation to recognize an alternative set of HTTP request methods. All
1011
other methods will be treated as `_OTHER`.
1112
type: list
1213
default: "CONNECT,DELETE,GET,HEAD,OPTIONS,PATCH,POST,PUT,TRACE"
1314
- name: otel.instrumentation.http.client.capture-request-headers
15+
declarative_name: general.http.client.request_captured_headers
1416
description: List of HTTP request headers to capture in HTTP client telemetry.
1517
type: list
1618
default: ""
1719
- name: otel.instrumentation.http.client.capture-response-headers
20+
declarative_name: general.http.client.response_captured_headers
1821
description: List of HTTP response headers to capture in HTTP client telemetry.
1922
type: list
2023
default: ""
@@ -23,13 +26,15 @@ configurations:
2326
type: map
2427
default: ""
2528
- name: otel.instrumentation.http.client.emit-experimental-telemetry
29+
declarative_name: java.common.http.client.emit_experimental_telemetry/development
2630
description: >
2731
Enable the capture of experimental HTTP client telemetry. Adds the `http.request.body.size`
2832
and `http.response.body.size` attributes to spans, and records `http.client.request.size` and
2933
`http.client.response.size` metrics.
3034
type: boolean
3135
default: false
3236
- name: otel.instrumentation.sanitization.url.experimental.sensitive-query-parameters
37+
declarative_name: general.sanitization.url.sensitive_query_parameters/development
3338
description: List of URL query parameter names whose values are redacted in URL attributes. See https://opentelemetry.io/docs/specs/semconv/http/http-spans.
3439
type: list
3540
default: "AWSAccessKeyId,Signature,sig,X-Goog-Signature"

instrumentation/okhttp/okhttp-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/okhttp/v3_0/OkHttp3DispatcherInstrumentation.java

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
import io.opentelemetry.javaagent.bootstrap.executors.PropagatedContext;
1818
import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation;
1919
import io.opentelemetry.javaagent.extension.instrumentation.TypeTransformer;
20+
import javax.annotation.Nullable;
2021
import net.bytebuddy.asm.Advice;
2122
import net.bytebuddy.description.type.TypeDescription;
2223
import net.bytebuddy.matcher.ElementMatcher;
@@ -39,6 +40,7 @@ public void transform(TypeTransformer transformer) {
3940
public static class AttachStateAdvice {
4041

4142
@Advice.OnMethodEnter(suppress = Throwable.class, inline = false)
43+
@Nullable
4244
public static PropagatedContext onEnter(@Advice.Argument(0) Runnable call) {
4345
Context context = Java8BytecodeBridge.currentContext();
4446
if (ExecutorAdviceHelper.shouldPropagateContext(context, call)) {
@@ -50,8 +52,8 @@ public static PropagatedContext onEnter(@Advice.Argument(0) Runnable call) {
5052
@Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class, inline = false)
5153
public static void onExit(
5254
@Advice.Argument(0) Runnable call,
53-
@Advice.Enter PropagatedContext propagatedContext,
54-
@Advice.Thrown Throwable throwable) {
55+
@Advice.Enter @Nullable PropagatedContext propagatedContext,
56+
@Advice.Thrown @Nullable Throwable throwable) {
5557
ExecutorAdviceHelper.cleanUpAfterSubmit(
5658
propagatedContext, throwable, PROPAGATED_CONTEXT, call);
5759
}

instrumentation/okhttp/okhttp-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/okhttp/v3_0/OkHttp3Instrumentation.java

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
import io.opentelemetry.javaagent.bootstrap.CallDepth;
1515
import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation;
1616
import io.opentelemetry.javaagent.extension.instrumentation.TypeTransformer;
17+
import javax.annotation.Nullable;
1718
import net.bytebuddy.asm.Advice;
1819
import net.bytebuddy.description.type.TypeDescription;
1920
import net.bytebuddy.matcher.ElementMatcher;
@@ -42,10 +43,10 @@ public static CallDepth trackCallDepth() {
4243

4344
@Advice.OnMethodExit(suppress = Throwable.class, inline = false)
4445
public static void addTracingInterceptor(
45-
@Advice.This OkHttpClient.Builder builder, @Advice.Enter CallDepth callDepth) {
46+
@Advice.This OkHttpClient.Builder builder, @Advice.Enter @Nullable CallDepth callDepth) {
4647
// No-args constructor is automatically called by constructors with args, but we only want to
4748
// run once from the constructor with args because that is where the dedupe needs to happen.
48-
if (callDepth.decrementAndGet() > 0) {
49+
if (callDepth == null || callDepth.decrementAndGet() > 0) {
4950
return;
5051
}
5152
if (!builder.interceptors().contains(contextInterceptor())) {

instrumentation/okhttp/okhttp-3.0/metadata.yaml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ configurations:
3535
type: boolean
3636
default: false
3737
- name: otel.instrumentation.sanitization.url.experimental.sensitive-query-parameters
38+
declarative_name: general.sanitization.url.sensitive_query_parameters/development
3839
description: List of URL query parameter names whose values are redacted in URL attributes. See https://opentelemetry.io/docs/specs/semconv/http/http-spans.
3940
type: list
4041
default: "AWSAccessKeyId,Signature,sig,X-Goog-Signature"

instrumentation/openai/openai-java-1.1/javaagent/build.gradle.kts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ dependencies {
2222
}
2323

2424
tasks {
25-
withType<Test>().configureEach {
25+
test {
2626
systemProperty("testLatestDeps", otelProps.testLatestDeps)
2727
// TODO run tests both with and without genai message capture
2828

instrumentation/openai/openai-java-1.1/library/src/main/java/io/opentelemetry/instrumentation/openai/v1_1/ChatCompletionEventsHelper.java

Lines changed: 31 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -271,10 +271,10 @@ private static String invokeStringHandle(@Nullable MethodHandle methodHandle, Ob
271271
}
272272

273273
private static class V1FunctionAccess implements FunctionAccess {
274-
@Nullable private static final MethodHandle idHandle;
275-
@Nullable private static final MethodHandle functionHandle;
276-
@Nullable private static final MethodHandle nameHandle;
277-
@Nullable private static final MethodHandle argumentsHandle;
274+
@Nullable private static final MethodHandle ID_HANDLE;
275+
@Nullable private static final MethodHandle FUNCTION_HANDLE;
276+
@Nullable private static final MethodHandle NAME_HANDLE;
277+
@Nullable private static final MethodHandle ARGUMENTS_HANDLE;
278278

279279
static {
280280
MethodHandle id;
@@ -304,10 +304,10 @@ private static class V1FunctionAccess implements FunctionAccess {
304304
name = null;
305305
arguments = null;
306306
}
307-
idHandle = id;
308-
functionHandle = function;
309-
nameHandle = name;
310-
argumentsHandle = arguments;
307+
ID_HANDLE = id;
308+
FUNCTION_HANDLE = function;
309+
NAME_HANDLE = name;
310+
ARGUMENTS_HANDLE = arguments;
311311
}
312312

313313
private final ChatCompletionMessageToolCall toolCall;
@@ -320,43 +320,43 @@ private static class V1FunctionAccess implements FunctionAccess {
320320

321321
@Nullable
322322
static FunctionAccess create(ChatCompletionMessageToolCall toolCall) {
323-
if (functionHandle == null) {
323+
if (FUNCTION_HANDLE == null) {
324324
return null;
325325
}
326326

327327
try {
328-
return new V1FunctionAccess(toolCall, functionHandle.invoke(toolCall));
328+
return new V1FunctionAccess(toolCall, FUNCTION_HANDLE.invoke(toolCall));
329329
} catch (Throwable ignored) {
330330
return null;
331331
}
332332
}
333333

334334
static boolean isAvailable() {
335-
return idHandle != null;
335+
return ID_HANDLE != null;
336336
}
337337

338338
@Override
339339
public String id() {
340-
return invokeStringHandle(idHandle, toolCall);
340+
return invokeStringHandle(ID_HANDLE, toolCall);
341341
}
342342

343343
@Override
344344
public String name() {
345-
return invokeStringHandle(nameHandle, function);
345+
return invokeStringHandle(NAME_HANDLE, function);
346346
}
347347

348348
@Override
349349
public String arguments() {
350-
return invokeStringHandle(argumentsHandle, function);
350+
return invokeStringHandle(ARGUMENTS_HANDLE, function);
351351
}
352352
}
353353

354354
private static class V3FunctionAccess implements FunctionAccess {
355-
@Nullable private static final MethodHandle functionToolCallHandle;
356-
@Nullable private static final MethodHandle idHandle;
357-
@Nullable private static final MethodHandle functionHandle;
358-
@Nullable private static final MethodHandle nameHandle;
359-
@Nullable private static final MethodHandle argumentsHandle;
355+
@Nullable private static final MethodHandle FUNCTION_TOOL_CALL_HANDLE;
356+
@Nullable private static final MethodHandle ID_HANDLE;
357+
@Nullable private static final MethodHandle FUNCTION_HANDLE;
358+
@Nullable private static final MethodHandle NAME_HANDLE;
359+
@Nullable private static final MethodHandle ARGUMENTS_HANDLE;
360360

361361
static {
362362
MethodHandle functionToolCall;
@@ -392,11 +392,11 @@ private static class V3FunctionAccess implements FunctionAccess {
392392
name = null;
393393
arguments = null;
394394
}
395-
functionToolCallHandle = functionToolCall;
396-
idHandle = id;
397-
functionHandle = function;
398-
nameHandle = name;
399-
argumentsHandle = arguments;
395+
FUNCTION_TOOL_CALL_HANDLE = functionToolCall;
396+
ID_HANDLE = id;
397+
FUNCTION_HANDLE = function;
398+
NAME_HANDLE = name;
399+
ARGUMENTS_HANDLE = arguments;
400400
}
401401

402402
private final Object functionToolCall;
@@ -409,40 +409,40 @@ private static class V3FunctionAccess implements FunctionAccess {
409409

410410
@Nullable
411411
static FunctionAccess create(ChatCompletionMessageToolCall toolCall) {
412-
if (functionToolCallHandle == null || functionHandle == null) {
412+
if (FUNCTION_TOOL_CALL_HANDLE == null || FUNCTION_HANDLE == null) {
413413
return null;
414414
}
415415

416416
try {
417417
@SuppressWarnings("unchecked") // casting MethodHandle.invoke result
418-
Optional<Object> optional = (Optional<Object>) functionToolCallHandle.invoke(toolCall);
418+
Optional<Object> optional = (Optional<Object>) FUNCTION_TOOL_CALL_HANDLE.invoke(toolCall);
419419
if (!optional.isPresent()) {
420420
return null;
421421
}
422422
Object functionToolCall = optional.get();
423-
return new V3FunctionAccess(functionToolCall, functionHandle.invoke(functionToolCall));
423+
return new V3FunctionAccess(functionToolCall, FUNCTION_HANDLE.invoke(functionToolCall));
424424
} catch (Throwable ignored) {
425425
return null;
426426
}
427427
}
428428

429429
static boolean isAvailable() {
430-
return idHandle != null;
430+
return ID_HANDLE != null;
431431
}
432432

433433
@Override
434434
public String id() {
435-
return invokeStringHandle(idHandle, functionToolCall);
435+
return invokeStringHandle(ID_HANDLE, functionToolCall);
436436
}
437437

438438
@Override
439439
public String name() {
440-
return invokeStringHandle(nameHandle, function);
440+
return invokeStringHandle(NAME_HANDLE, function);
441441
}
442442

443443
@Override
444444
public String arguments() {
445-
return invokeStringHandle(argumentsHandle, function);
445+
return invokeStringHandle(ARGUMENTS_HANDLE, function);
446446
}
447447
}
448448

0 commit comments

Comments
 (0)