Code review sweep (run 24976350016) (#18334)

Co-authored-by: otelbot <197425009+otelbot@users.noreply.github.com>
Co-authored-by: Trask Stalnaker <trask.stalnaker@gmail.com>

Author: otelbot[bot]

instrumentation/jdbc/testing/src/main/java/io/opentelemetry/instrumentation/jdbc/testing/AbstractJdbcInstrumentationTest.java

@@ -47,7 +47,6 @@
 import io.opentelemetry.sdk.testing.assertj.TraceAssert;
 import java.beans.PropertyVetoException;
 import java.io.Closeable;
-import java.io.IOException;
 import java.sql.CallableStatement;
 import java.sql.Connection;
 import java.sql.Driver;
@@ -69,7 +68,6 @@
 import org.assertj.core.api.ThrowingConsumer;
 import org.h2.jdbcx.JdbcDataSource;
 import org.hsqldb.jdbc.JDBCDriver;
-import org.junit.jupiter.api.AfterAll;
 import org.junit.jupiter.api.BeforeAll;
 import org.junit.jupiter.api.DisplayName;
 import org.junit.jupiter.api.Test;
@@ -129,32 +127,19 @@ static void setUp() {
     prepareConnectionPoolDatasources();
   }
 
-  @AfterAll
-  static void tearDown() {
-    cpDatasources
-        .values()
-        .forEach(
-            k ->
-                k.values()
-                    .forEach(
-                        dataSource -> {
-                          if (dataSource instanceof Closeable) {
-                            try {
-                              ((Closeable) dataSource).close();
-                            } catch (IOException ignored) {
-                              // ignore exceptions during close
-                            }
-                          }
-                        }));
-  }
-
   static void prepareConnectionPoolDatasources() {
     List<String> connectionPoolNames = asList("tomcat", "hikari", "c3p0");
     connectionPoolNames.forEach(
         cpName -> {
           Map<String, DataSource> dbDsMapping = new HashMap<>();
           jdbcUrls.forEach(
-              (dbType, jdbcUrl) -> dbDsMapping.put(dbType, createDs(cpName, dbType, jdbcUrl)));
+              (dbType, jdbcUrl) -> {
+                DataSource dataSource = createDs(cpName, dbType, jdbcUrl);
+                if (dataSource instanceof Closeable) {
+                  cleanup.deferAfterAll((Closeable) dataSource);
+                }
+                dbDsMapping.put(dbType, dataSource);
+              });
           cpDatasources.put(cpName, dbDsMapping);
         });
   }

instrumentation/jdbc/testing/src/main/java/io/opentelemetry/instrumentation/jdbc/testing/ProxyStatementFactory.java

@@ -14,7 +14,8 @@
 
 class ProxyStatementFactory {
 
-  static Statement proxyStatementWithCustomClassLoader(Statement statement) throws Exception {
+  static Statement proxyStatementWithCustomClassLoader(Statement statement)
+      throws ClassNotFoundException {
     TestClassLoader classLoader = new TestClassLoader(ProxyStatementFactory.class.getClassLoader());
     Class<?> testInterface = classLoader.loadClass(TestInterface.class.getName());
     if (testInterface.getClassLoader() != classLoader) {

instrumentation/jedis/jedis-1.4/testing/src/main/java/io/opentelemetry/javaagent/instrumentation/jedis/AbstractJedisTest.java

@@ -21,10 +21,9 @@
 import static org.assertj.core.api.Assertions.assertThat;
 
 import io.opentelemetry.api.trace.SpanKind;
+import io.opentelemetry.instrumentation.testing.internal.AutoCleanupExtension;
 import io.opentelemetry.instrumentation.testing.junit.AgentInstrumentationExtension;
 import io.opentelemetry.instrumentation.testing.junit.InstrumentationExtension;
-import java.io.IOException;
-import org.junit.jupiter.api.AfterAll;
 import org.junit.jupiter.api.BeforeAll;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
@@ -37,6 +36,9 @@ public abstract class AbstractJedisTest {
   @RegisterExtension
   private static final InstrumentationExtension testing = AgentInstrumentationExtension.create();
 
+  @RegisterExtension
+  private static final AutoCleanupExtension cleanup = AutoCleanupExtension.create();
+
   private static final GenericContainer<?> redisServer =
       new GenericContainer<>("redis:6.2.3-alpine").withExposedPorts(6379);
 
@@ -49,15 +51,11 @@ public abstract class AbstractJedisTest {
   @BeforeAll
   static void setup() {
     redisServer.start();
+    cleanup.deferAfterAll(redisServer::stop);
     host = redisServer.getHost();
     port = redisServer.getMappedPort(6379);
     jedis = new Jedis(host, port);
-  }
-
-  @AfterAll
-  static void cleanup() throws IOException {
-    jedis.disconnect();
-    redisServer.stop();
+    cleanup.deferAfterAll(jedis::disconnect);
   }
 
   @BeforeEach

instrumentation/jedis/jedis-3.0/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/jedis/v3_0/Jedis30ClientTest.java

@@ -27,12 +27,12 @@
 import static org.assertj.core.api.Assertions.assertThat;
 
 import io.opentelemetry.api.trace.SpanKind;
+import io.opentelemetry.instrumentation.testing.internal.AutoCleanupExtension;
 import io.opentelemetry.instrumentation.testing.junit.AgentInstrumentationExtension;
 import io.opentelemetry.instrumentation.testing.junit.InstrumentationExtension;
 import java.net.InetAddress;
 import java.net.UnknownHostException;
 import org.assertj.core.api.AbstractLongAssert;
-import org.junit.jupiter.api.AfterAll;
 import org.junit.jupiter.api.BeforeAll;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
@@ -45,6 +45,9 @@ class Jedis30ClientTest {
   @RegisterExtension
   private static final InstrumentationExtension testing = AgentInstrumentationExtension.create();
 
+  @RegisterExtension
+  private static final AutoCleanupExtension cleanup = AutoCleanupExtension.create();
+
   private static final GenericContainer<?> redisServer =
       new GenericContainer<>("redis:6.2.3-alpine").withExposedPorts(6379);
 
@@ -59,16 +62,12 @@ class Jedis30ClientTest {
   @BeforeAll
   static void setup() throws UnknownHostException {
     redisServer.start();
+    cleanup.deferAfterAll(redisServer::stop);
     host = redisServer.getHost();
     ip = InetAddress.getByName(host).getHostAddress();
     port = redisServer.getMappedPort(6379);
     jedis = new Jedis(host, port);
-  }
-
-  @AfterAll
-  static void cleanup() {
-    redisServer.stop();
-    jedis.close();
+    cleanup.deferAfterAll(jedis);
   }
 
   @BeforeEach

instrumentation/jedis/jedis-4.0/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/jedis/v4_0/Jedis40ClientTest.java

@@ -22,11 +22,11 @@
 import static org.assertj.core.api.Assertions.assertThat;
 
 import io.opentelemetry.api.trace.SpanKind;
+import io.opentelemetry.instrumentation.testing.internal.AutoCleanupExtension;
 import io.opentelemetry.instrumentation.testing.junit.AgentInstrumentationExtension;
 import io.opentelemetry.instrumentation.testing.junit.InstrumentationExtension;
 import java.net.InetAddress;
 import java.net.UnknownHostException;
-import org.junit.jupiter.api.AfterAll;
 import org.junit.jupiter.api.BeforeAll;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
@@ -37,29 +37,28 @@
 @SuppressWarnings("deprecation") // using deprecated semconv
 class Jedis40ClientTest {
   @RegisterExtension
-  static final InstrumentationExtension testing = AgentInstrumentationExtension.create();
+  private static final InstrumentationExtension testing = AgentInstrumentationExtension.create();
 
-  static GenericContainer<?> redisServer =
+  @RegisterExtension
+  private static final AutoCleanupExtension cleanup = AutoCleanupExtension.create();
+
+  private static final GenericContainer<?> redisServer =
       new GenericContainer<>("redis:6.2.3-alpine").withExposedPorts(6379);
 
-  static String ip;
+  private static String ip;
 
-  static int port;
+  private static int port;
 
-  static Jedis jedis;
+  private static Jedis jedis;
 
   @BeforeAll
   static void setup() throws UnknownHostException {
     redisServer.start();
+    cleanup.deferAfterAll(redisServer::stop);
     port = redisServer.getMappedPort(6379);
     ip = InetAddress.getByName(redisServer.getHost()).getHostAddress();
     jedis = new Jedis(redisServer.getHost(), port);
-  }
-
-  @AfterAll
-  static void cleanup() {
-    redisServer.stop();
-    jedis.close();
+    cleanup.deferAfterAll(jedis);
   }
 
   @BeforeEach

instrumentation/jetty-httpclient/jetty-httpclient-12.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jetty/httpclient/v12_0/JettyClient12ResponseListenersInstrumentation.java

@@ -59,11 +59,7 @@ public static Scope onEnterNotify(@Advice.Argument(0) Response response) {
     }
 
     @Advice.OnMethodExit(suppress = Throwable.class, onThrowable = Throwable.class, inline = false)
-    public static void onExitNotify(
-        @Advice.Argument(0) Response response,
-        @Advice.Thrown Throwable throwable,
-        @Advice.Enter @Nullable Scope scope) {
-
+    public static void onExitNotify(@Advice.Enter @Nullable Scope scope) {
       if (scope != null) {
         scope.close();
       }
@@ -82,10 +78,7 @@ public static Scope onEnterComplete(@Advice.Argument(0) Result result) {
     }
 
     @Advice.OnMethodExit(suppress = Throwable.class, onThrowable = Throwable.class, inline = false)
-    public static void onExitComplete(
-        @Advice.Argument(0) Result result,
-        @Advice.Thrown Throwable throwable,
-        @Advice.Enter @Nullable Scope scope) {
+    public static void onExitComplete(@Advice.Enter @Nullable Scope scope) {
       if (scope != null) {
         scope.close();
       }

instrumentation/jetty-httpclient/jetty-httpclient-12.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jetty/httpclient/v12_0/JettyHttpClient12Instrumentation.java

@@ -63,7 +63,7 @@ public static AdviceLocals onEnterSend(@Advice.This HttpRequest request) {
       if (context == null) {
         return null;
       }
-      // set context for responseListeners
+      // store the parent context for request/response listener callbacks
       request.attribute(JETTY_CLIENT_CONTEXT_KEY, parentContext);
 
       return new AdviceLocals(context, context.makeCurrent());