Code review sweep (run 25201305534)

.github/agents/knowledge/gradle-conventions.md

@@ -199,7 +199,10 @@ usesService(gradle.sharedServices.registrations["testcontainersBuildService"].se
 ```
 
 Place the declaration in `withType<Test>().configureEach` when **all** test tasks in the
-module use Testcontainers. Otherwise place it on **only** the specific task(s) that do.
+module use Testcontainers **and** the module has multiple test tasks. When the module has
+only a single test task, put the declaration directly inside `tasks.test { ... }` — do not
+introduce a `withType<Test>().configureEach` block just for `usesService`. Otherwise place
+it on **only** the specific task(s) that use Testcontainers.
 
 **Do not over-apply.** Adding `usesService` to a task that does not use Testcontainers
 unnecessarily throttles it against the 2-slot concurrency limit. A module may have some
@@ -215,9 +218,10 @@ because the dependency may only be used by some suites in the module.
 
 ## Prefer `withType<Test>().configureEach` (ONLY when multiple test tasks exist)
 
-When a module has custom test tasks (e.g., `testStableSemconv`), system properties and JVM
-args that apply to **all** test tasks should be set once in a `withType<Test>().configureEach`
-block, not repeated on each individual task.
+When a module has custom test tasks (e.g., `testStableSemconv`), shared configuration
+(system properties, JVM args, `usesService` declarations, etc.) that applies to **all**
+test tasks should be set once in a `withType<Test>().configureEach` block, not repeated
+on each individual task.
 
 If a property or JVM arg is moved into `withType<Test>().configureEach`, remove any now-redundant
 copies from individual tasks unless a task intentionally overrides the shared value.

instrumentation/mybatis-3.2/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/mybatis/v3_2/SqlCommandUtil.java

@@ -12,16 +12,16 @@
 import org.apache.ibatis.binding.MapperMethod.SqlCommand;
 
 public class SqlCommandUtil {
-  private static final VirtualField<SqlCommand, ClassAndMethod> field =
+  private static final VirtualField<SqlCommand, ClassAndMethod> FIELD =
       VirtualField.find(SqlCommand.class, ClassAndMethod.class);
 
   public static void setClassAndMethod(SqlCommand command, Class<?> clazz, Method method) {
-    field.set(command, ClassAndMethod.create(clazz, method.getName()));
+    FIELD.set(command, ClassAndMethod.create(clazz, method.getName()));
   }
 
   @Nullable
   public static ClassAndMethod getClassAndMethod(SqlCommand command) {
-    return field.get(command);
+    return FIELD.get(command);
   }
 
   private SqlCommandUtil() {}

instrumentation/mybatis-3.2/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/mybatis/v3_2/TestMapper.java

@@ -7,7 +7,7 @@
 
 import org.apache.ibatis.annotations.Select;
 
-public interface TestMapper {
+interface TestMapper {
 
   @Select("SELECT 1")
   int select();

instrumentation/nats/nats-2.17/javaagent/build.gradle.kts

@@ -7,7 +7,6 @@ muzzle {
     group.set("io.nats")
     module.set("jnats")
     versions.set("[2.17.2,)")
-
     assertInverse.set(true)
   }
 }
@@ -20,7 +19,7 @@ dependencies {
 }
 
 tasks {
-  withType<Test>().configureEach {
+  test {
     usesService(gradle.sharedServices.registrations["testcontainersBuildService"].service)
     systemProperty("collectMetadata", otelProps.collectMetadata)
   }

instrumentation/nats/nats-2.17/library/build.gradle.kts

@@ -12,7 +12,7 @@ dependencies {
 }
 
 tasks {
-  withType<Test>().configureEach {
+  test {
     usesService(gradle.sharedServices.registrations["testcontainersBuildService"].service)
   }
 }

instrumentation/nats/nats-2.17/testing/src/main/java/io/opentelemetry/instrumentation/nats/v2_17/AbstractNatsPublishTest.java

@@ -13,7 +13,6 @@
 import io.nats.client.impl.NatsMessage;
 import io.opentelemetry.api.trace.SpanKind;
 import java.time.Duration;
-import org.junit.jupiter.api.AfterEach;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 
@@ -26,11 +25,7 @@ public abstract class AbstractNatsPublishTest extends AbstractNatsTest {
   void beforeEach() {
     clientId = connection.getServerInfo().getClientId();
     subscription = connection.subscribe("sub");
-  }
-
-  @AfterEach
-  void afterEach() throws InterruptedException {
-    subscription.drain(Duration.ofSeconds(10));
+    cleanup.deferCleanup(() -> subscription.drain(Duration.ofSeconds(10)));
   }
 
   @Test

instrumentation/nats/nats-2.17/testing/src/main/java/io/opentelemetry/instrumentation/nats/v2_17/AbstractNatsRequestTest.java

@@ -26,7 +26,6 @@
 import java.util.concurrent.CancellationException;
 import java.util.concurrent.CompletableFuture;
 import java.util.function.Consumer;
-import org.junit.jupiter.api.AfterEach;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 
@@ -40,11 +39,7 @@ public abstract class AbstractNatsRequestTest extends AbstractNatsTest {
   void beforeEach() {
     clientId = connection.getServerInfo().getClientId();
     subscription = connection.subscribe("sub");
-  }
-
-  @AfterEach
-  void afterEach() throws InterruptedException {
-    subscription.drain(Duration.ofSeconds(10));
+    cleanup.deferCleanup(() -> subscription.drain(Duration.ofSeconds(10)));
   }
 
   @Test
@@ -78,14 +73,13 @@ void testRequestBody() throws InterruptedException {
         connection
             .createDispatcher(m -> connection.publish(m.getReplyTo(), m.getData()))
             .subscribe("sub");
+    cleanup.deferCleanup(() -> connection.closeDispatcher(dispatcher));
 
     // when
     Message message =
         testing()
             .runWithSpan(
                 "parent", () -> connection.request("sub", new byte[] {0}, Duration.ofSeconds(10)));
-    connection.closeDispatcher(dispatcher);
-
     // then
     assertThat(message).isNotNull();
     assertPublishReceiveSpansSameTrace();
@@ -99,6 +93,7 @@ void testRequestHeadersBody() throws InterruptedException {
         connection
             .createDispatcher(m -> connection.publish(m.getReplyTo(), new Headers(), m.getData()))
             .subscribe("sub");
+    cleanup.deferCleanup(() -> connection.closeDispatcher(dispatcher));
 
     // when
     Message message =
@@ -108,8 +103,6 @@ void testRequestHeadersBody() throws InterruptedException {
                 () ->
                     connection.request(
                         "sub", new Headers(), new byte[] {0}, Duration.ofSeconds(10)));
-    connection.closeDispatcher(dispatcher);
-
     // then
     assertThat(message).isNotNull();
     assertPublishReceiveSpansSameTrace();
@@ -123,13 +116,12 @@ void testRequestMessage() throws InterruptedException {
         connection
             .createDispatcher(m -> connection.publish(m.getReplyTo(), m.getData()))
             .subscribe("sub");
+    cleanup.deferCleanup(() -> connection.closeDispatcher(dispatcher));
     NatsMessage message = NatsMessage.builder().subject("sub").data("x").build();
 
     // when
     Message response =
         testing().runWithSpan("parent", () -> connection.request(message, Duration.ofSeconds(10)));
-    connection.closeDispatcher(dispatcher);
-
     // then
     assertThat(response).isNotNull();
     assertPublishReceiveSpansSameTrace();
@@ -143,14 +135,13 @@ void testRequestMessageHeaders() throws InterruptedException {
         connection
             .createDispatcher(m -> connection.publish(m.getReplyTo(), new Headers(), m.getData()))
             .subscribe("sub");
+    cleanup.deferCleanup(() -> connection.closeDispatcher(dispatcher));
     NatsMessage message =
         NatsMessage.builder().subject("sub").headers(new Headers()).data("x").build();
 
     // when
     Message response =
         testing().runWithSpan("parent", () -> connection.request(message, Duration.ofSeconds(10)));
-    connection.closeDispatcher(dispatcher);
-
     // then
     assertThat(response).isNotNull();
     assertPublishReceiveSpansSameTrace();
@@ -164,12 +155,11 @@ void testRequestFutureBody() throws InterruptedException {
         connection
             .createDispatcher(m -> connection.publish(m.getReplyTo(), m.getData()))
             .subscribe("sub");
+    cleanup.deferCleanup(() -> connection.closeDispatcher(dispatcher));
 
     // when
     CompletableFuture<Message> message =
-        testing()
-            .runWithSpan("parent", () -> connection.request("sub", new byte[] {0}))
-            .whenComplete((m, e) -> connection.closeDispatcher(dispatcher));
+        testing().runWithSpan("parent", () -> connection.request("sub", new byte[] {0}));
 
     // then
     assertPublishReceiveSpansSameTrace();
@@ -184,12 +174,12 @@ void testRequestFutureHeadersBody() throws InterruptedException {
         connection
             .createDispatcher(m -> connection.publish(m.getReplyTo(), new Headers(), m.getData()))
             .subscribe("sub");
+    cleanup.deferCleanup(() -> connection.closeDispatcher(dispatcher));
 
     // when
     CompletableFuture<Message> message =
         testing()
-            .runWithSpan("parent", () -> connection.request("sub", new Headers(), new byte[] {0}))
-            .whenComplete((m, e) -> connection.closeDispatcher(dispatcher));
+            .runWithSpan("parent", () -> connection.request("sub", new Headers(), new byte[] {0}));
 
     // then
     assertPublishReceiveSpansSameTrace();
@@ -204,13 +194,12 @@ void testRequestFutureMessage() throws InterruptedException {
         connection
             .createDispatcher(m -> connection.publish(m.getReplyTo(), m.getData()))
             .subscribe("sub");
+    cleanup.deferCleanup(() -> connection.closeDispatcher(dispatcher));
     NatsMessage message = NatsMessage.builder().subject("sub").data("x").build();
 
     // when
     CompletableFuture<Message> response =
-        testing()
-            .runWithSpan("parent", () -> connection.request(message))
-            .whenComplete((m, e) -> connection.closeDispatcher(dispatcher));
+        testing().runWithSpan("parent", () -> connection.request(message));
 
     // then
     assertPublishReceiveSpansSameTrace();
@@ -225,14 +214,13 @@ void testRequestFutureMessageHeaders() throws InterruptedException {
         connection
             .createDispatcher(m -> connection.publish(m.getReplyTo(), new Headers(), m.getData()))
             .subscribe("sub");
+    cleanup.deferCleanup(() -> connection.closeDispatcher(dispatcher));
     NatsMessage message =
         NatsMessage.builder().subject("sub").headers(new Headers()).data("x").build();
 
     // when
     CompletableFuture<Message> response =
-        testing()
-            .runWithSpan("parent", () -> connection.request(message))
-            .whenComplete((m, e) -> connection.closeDispatcher(dispatcher));
+        testing().runWithSpan("parent", () -> connection.request(message));
 
     // then
     assertPublishReceiveSpansSameTrace();

instrumentation/netty/netty-3.8/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/v3_8/client/NettyHttpClientAttributesGetter.java

@@ -33,7 +33,7 @@ public String getUrlFull(NettyRequest requestAndChannel) {
         return getScheme(requestAndChannel) + "://" + hostHeader + target;
       }
       return uri.toString();
-    } catch (URISyntaxException e) {
+    } catch (URISyntaxException ignored) {
       return null;
     }
   }

instrumentation/netty/netty-3.8/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/netty/v3_8/client/Netty38ClientTest.java

@@ -50,7 +50,7 @@ void setUp() throws Exception {
     try {
       setConnectTimeout =
           AsyncHttpClientConfig.Builder.class.getMethod("setConnectTimeout", int.class);
-    } catch (NoSuchMethodException e) {
+    } catch (NoSuchMethodException ignored) {
       setConnectTimeout =
           AsyncHttpClientConfig.Builder.class.getMethod("setRequestTimeoutInMs", int.class);
     }
@@ -60,7 +60,7 @@ void setUp() throws Exception {
     try {
       setFollowRedirect =
           AsyncHttpClientConfig.Builder.class.getMethod("setFollowRedirect", boolean.class);
-    } catch (NoSuchMethodException e) {
+    } catch (NoSuchMethodException ignored) {
       setFollowRedirect =
           AsyncHttpClientConfig.Builder.class.getMethod("setFollowRedirects", boolean.class);
     }
@@ -69,7 +69,7 @@ void setUp() throws Exception {
     Method setMaxRedirects;
     try {
       setMaxRedirects = AsyncHttpClientConfig.Builder.class.getMethod("setMaxRedirects", int.class);
-    } catch (NoSuchMethodException e) {
+    } catch (NoSuchMethodException ignored) {
       setMaxRedirects =
           AsyncHttpClientConfig.Builder.class.getMethod("setMaximumNumberOfRedirects", int.class);
     }
@@ -80,7 +80,7 @@ void setUp() throws Exception {
       setAllowPoolingConnections =
           AsyncHttpClientConfig.Builder.class.getMethod(
               "setAllowPoolingConnections", boolean.class);
-    } catch (NoSuchMethodException e) {
+    } catch (NoSuchMethodException ignored) {
       setAllowPoolingConnections =
           AsyncHttpClientConfig.Builder.class.getMethod("setAllowPoolingConnection", boolean.class);
     }

instrumentation/netty/netty-4.0/metadata.yaml

@@ -10,54 +10,65 @@ semantic_conventions:
 library_link: https://netty.io/
 configurations:
   - name: otel.instrumentation.http.known-methods
+    declarative_name: java.common.http.known_methods
     description: >
       Configures the instrumentation to recognize an alternative set of HTTP request methods. All
       other methods will be treated as `_OTHER`.
     type: list
     default: "CONNECT,DELETE,GET,HEAD,OPTIONS,PATCH,POST,PUT,TRACE"
   - name: otel.instrumentation.http.client.capture-request-headers
+    declarative_name: general.http.client.request_captured_headers
     description: List of HTTP request headers to capture in HTTP client telemetry.
     type: list
     default: ""
   - name: otel.instrumentation.http.client.capture-response-headers
+    declarative_name: general.http.client.response_captured_headers
     description: List of HTTP response headers to capture in HTTP client telemetry.
     type: list
     default: ""
   - name: otel.instrumentation.common.peer-service-mapping
+    declarative_name: java.common.peer_service_mapping
     description: Used to specify a mapping from host names or IP addresses to peer services.
     type: map
     default: ""
   - name: otel.instrumentation.http.client.emit-experimental-telemetry
+    declarative_name: java.common.http.client.emit_experimental_telemetry/development
     description: >
       Enable the capture of experimental HTTP client telemetry. Adds the `http.request.body.size`
       and `http.response.body.size` attributes to spans, and records `http.client.request.size` and
       `http.client.response.size` metrics.
     type: boolean
     default: false
   - name: otel.instrumentation.http.server.capture-request-headers
+    declarative_name: general.http.server.request_captured_headers
     description: List of HTTP request headers to capture in HTTP server telemetry.
     type: list
     default: ""
   - name: otel.instrumentation.http.server.capture-response-headers
+    declarative_name: general.http.server.response_captured_headers
     description: List of HTTP response headers to capture in HTTP server telemetry.
     type: list
     default: ""
   - name: otel.instrumentation.http.server.emit-experimental-telemetry
+    declarative_name: java.common.http.server.emit_experimental_telemetry/development
     description: >
       Enable the capture of experimental HTTP server telemetry. Adds the `http.request.body.size`
       and `http.response.body.size` attributes to spans, and records `http.server.request.size` and
       `http.server.response.size` metrics.
     type: boolean
     default: false
   - name: otel.instrumentation.netty.connection-telemetry.enabled
+    declarative_name: java.netty.connection_telemetry.enabled
     description: Enable the creation of Connect and DNS spans.
     type: boolean
     default: false
   - name: otel.instrumentation.netty.ssl-telemetry.enabled
+    declarative_name: java.netty.ssl_telemetry.enabled
     description: Enable SSL telemetry.
     type: boolean
     default: false
   - name: otel.instrumentation.sanitization.url.experimental.sensitive-query-parameters
+    declarative_name: general.sanitization.url.sensitive_query_parameters/development
     description: List of URL query parameter names whose values are redacted in URL attributes. See https://opentelemetry.io/docs/specs/semconv/http/http-spans.
     type: list
     default: "AWSAccessKeyId,Signature,sig,X-Goog-Signature"