Code review sweep (run 25244636897) (open-telemetry#18505)

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

Author: otelbot[bot]

.github/agents/knowledge/general-rules.md

@@ -105,7 +105,8 @@ Reason about visibility from "what does the advice method directly reference?".
 ## [Style] `@SuppressWarnings` Usage
 
 - Place `@SuppressWarnings` on the single member that needs it, or on the class when two
-  or more members in the class need the same suppression.
+  or more members in the class need the same suppression. Do not move an existing
+  suppression from a member to the class unless multiple members need it.
 - **Do not add `@SuppressWarnings("deprecation")` unless the build fails without it.**
   The project disables javac's `-Xlint:deprecation` globally and uses a custom Error Prone
   check (`OtelDeprecatedApiUsage`) instead. Only add the annotation when it is actually

instrumentation/spring/spring-boot-resources/javaagent/src/main/java/io/opentelemetry/instrumentation/spring/resources/SystemHelper.java

@@ -14,6 +14,7 @@
 import java.nio.file.Paths;
 import java.util.Optional;
 import java.util.logging.Logger;
+import javax.annotation.Nullable;
 
 class SystemHelper {
   private static final Logger logger = Logger.getLogger(SystemHelper.class.getName());
@@ -31,10 +32,12 @@ class SystemHelper {
     }
   }
 
+  @Nullable
   String getenv(String name) {
     return System.getenv(name);
   }
 
+  @Nullable
   String getProperty(String key) {
     return System.getProperty(key);
   }
@@ -44,6 +47,7 @@ String getProperty(String key) {
    * bootJar layouts the application classes live under {@code BOOT-INF/classes/}, so the prefix is
    * applied when that layout is detected.
    */
+  @Nullable
   InputStream openClasspathResource(String filename) {
     String path = addBootInfPrefix ? "BOOT-INF/classes/" + filename : filename;
     return classLoader.getResourceAsStream(path);
@@ -54,6 +58,7 @@ InputStream openClasspathResource(String filename) {
    * This is used for things like {@code META-INF/build-info.properties}, which Spring Boot places
    * at the jar root rather than under {@code BOOT-INF/classes/}.
    */
+  @Nullable
   InputStream openJarRootResource(String path) {
     return classLoader.getResourceAsStream(path);
   }

instrumentation/spring/spring-cloud-aws-3.0/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/spring/cloud/aws/v3_0/AwsSqsTest.java

@@ -31,8 +31,6 @@
 import io.opentelemetry.instrumentation.testing.junit.AgentInstrumentationExtension;
 import io.opentelemetry.instrumentation.testing.junit.InstrumentationExtension;
 import java.util.concurrent.CompletableFuture;
-import java.util.concurrent.ExecutionException;
-import java.util.concurrent.TimeoutException;
 import org.apache.pekko.http.scaladsl.Http;
 import org.assertj.core.api.AbstractStringAssert;
 import org.elasticmq.rest.sqs.SQSRestServer;
@@ -71,7 +69,7 @@ static void cleanUp() {
   }
 
   @Test
-  void sqsListener() throws InterruptedException, ExecutionException, TimeoutException {
+  void sqsListener() throws Exception {
     String messageContent = "hello";
     CompletableFuture<String> messageFuture = new CompletableFuture<>();
     AwsSqsTestApplication.messageHandler =

instrumentation/spring/spring-cloud-gateway/spring-cloud-gateway-2.2/testing/build.gradle.kts

@@ -17,7 +17,7 @@ dependencies {
   latestDepTestLibrary("org.springframework.boot:spring-boot-starter-test:3.+") // see spring-cloud-gateway-4.3* module
 }
 
-tasks.withType<Test>().configureEach {
+tasks.test {
   jvmArgs("-Dotel.instrumentation.spring-cloud-gateway.experimental-span-attributes=true")
 
   // required on jdk17

instrumentation/spring/spring-core-2.0/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/spring/core/v2_0/SimpleAsyncTaskExecutorInstrumentationTest.java

@@ -39,7 +39,7 @@ class SimpleAsyncTaskExecutorInstrumentationTest {
   private static Method findMethod(String name, Class<?>... parameterTypes) {
     try {
       return SimpleAsyncTaskExecutor.class.getMethod(name, parameterTypes);
-    } catch (NoSuchMethodException e) {
+    } catch (NoSuchMethodException ignored) {
       return null;
     }
   }

instrumentation/spring/spring-data/spring-data-common/testing/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/data/AbstractSpringJpaTest.java

@@ -35,6 +35,7 @@
 import org.springframework.dao.IncorrectResultSizeDataAccessException;
 import org.springframework.data.jpa.repository.JpaRepository;
 
+@SuppressWarnings("deprecation") // TODO DB_CONNECTION_STRING deprecation
 public abstract class AbstractSpringJpaTest<
     ENTITY, REPOSITORY extends JpaRepository<ENTITY, Long>> {
 
@@ -74,7 +75,6 @@ void testObjectMethod() {
                 span -> span.hasName("toString test").hasTotalAttributeCount(0)));
   }
 
-  @SuppressWarnings("deprecation") // TODO DB_CONNECTION_STRING deprecation
   static void assertHibernate4Trace(TraceAssert trace, String repoClassName) {
     trace.hasSpansSatisfyingExactly(
         span ->
@@ -103,7 +103,6 @@ DB_CONNECTION_STRING, emitStableDatabaseSemconv() ? null : "hsqldb:mem:"),
                         emitStableDatabaseSemconv() ? null : "JpaCustomer")));
   }
 
-  @SuppressWarnings("deprecation") // TODO DB_CONNECTION_STRING deprecation
   static void assertHibernateTrace(TraceAssert trace, String repoClassName) {
     trace.hasSpansSatisfyingExactly(
         span ->
@@ -163,7 +162,6 @@ DB_CONNECTION_STRING, emitStableDatabaseSemconv() ? null : "hsqldb:mem:"),
                         emitStableDatabaseSemconv() ? null : "JpaCustomer")));
   }
 
-  @SuppressWarnings("deprecation") // TODO DB_CONNECTION_STRING deprecation
   @Test
   void testCrud() {
     boolean isHibernate4 = Version.getVersionString().startsWith("4.");
@@ -374,7 +372,6 @@ void testCrud() {
                                 emitStableDatabaseSemconv() ? null : "JpaCustomer"))));
   }
 
-  @SuppressWarnings("deprecation") // TODO DB_CONNECTION_STRING deprecation
   @Test
   void testCustomRepositoryMethod() {
     REPOSITORY repo = repository();
@@ -417,7 +414,6 @@ void testCustomRepositoryMethod() {
                                 emitStableDatabaseSemconv() ? null : "JpaCustomer"))));
   }
 
-  @SuppressWarnings("deprecation") // TODO DB_CONNECTION_STRING deprecation
   @Test
   void testFailedRepositoryMethod() {
     // given

instrumentation/spring/spring-integration-4.1/library/build.gradle.kts

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

instrumentation/spring/spring-integration-4.1/library/src/main/java/io/opentelemetry/instrumentation/spring/integration/v4_1/SpringIntegrationTelemetryBuilder.java

@@ -67,14 +67,6 @@ public SpringIntegrationTelemetryBuilder setProducerSpanEnabled(boolean producer
     return this;
   }
 
-  private static String consumerSpanName(MessageWithChannel messageWithChannel) {
-    return messageWithChannel.getChannelName() + " process";
-  }
-
-  private static String producerSpanName(MessageWithChannel messageWithChannel) {
-    return messageWithChannel.getChannelName() + " publish";
-  }
-
   /**
    * Returns a new {@link SpringIntegrationTelemetry} with the settings of this {@link
    * SpringIntegrationTelemetryBuilder}.
@@ -112,6 +104,14 @@ public SpringIntegrationTelemetry build() {
         producerSpanEnabled);
   }
 
+  private static String consumerSpanName(MessageWithChannel messageWithChannel) {
+    return messageWithChannel.getChannelName() + " process";
+  }
+
+  private static String producerSpanName(MessageWithChannel messageWithChannel) {
+    return messageWithChannel.getChannelName() + " publish";
+  }
+
   private static AttributesExtractor<MessageWithChannel, Void> buildMessagingAttributesExtractor(
       MessagingAttributesGetter<MessageWithChannel, Void> getter,
       MessageOperation operation,

instrumentation/spring/spring-integration-4.1/testing/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/integration/v4_1/AbstractComplexPropagationTest.java

@@ -9,6 +9,7 @@
 import static java.util.stream.Collectors.toMap;
 
 import io.opentelemetry.api.trace.SpanKind;
+import io.opentelemetry.instrumentation.testing.internal.AutoCleanupExtension;
 import io.opentelemetry.instrumentation.testing.junit.InstrumentationExtension;
 import java.util.ArrayList;
 import java.util.List;
@@ -17,9 +18,9 @@
 import java.util.concurrent.ExecutorService;
 import java.util.concurrent.Executors;
 import java.util.concurrent.LinkedBlockingQueue;
-import org.junit.jupiter.api.AfterEach;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.RegisterExtension;
 import org.springframework.boot.SpringApplication;
 import org.springframework.boot.SpringBootConfiguration;
 import org.springframework.boot.autoconfigure.EnableAutoConfiguration;
@@ -35,6 +36,8 @@
 
 abstract class AbstractComplexPropagationTest {
 
+  @RegisterExtension static final AutoCleanupExtension cleanup = AutoCleanupExtension.create();
+
   private final Class<?> additionalContextClass;
   private final InstrumentationExtension testing;
 
@@ -58,13 +61,7 @@ void setUp() {
     springApplication.setDefaultProperties(
         singletonMap("spring.main.web-application-type", "none"));
     applicationContext = springApplication.run();
-  }
-
-  @AfterEach
-  void tearDown() {
-    if (applicationContext != null) {
-      applicationContext.close();
-    }
+    cleanup.deferCleanup(applicationContext);
   }
 
   @Test

instrumentation/spring/spring-integration-4.1/testing/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/integration/v4_1/AbstractSpringIntegrationTracingTest.java

@@ -10,15 +10,16 @@
 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.InstrumentationExtension;
 import io.opentelemetry.sdk.trace.data.SpanData;
 import java.util.ArrayList;
 import java.util.List;
 import java.util.concurrent.ExecutorService;
 import java.util.concurrent.Executors;
-import org.junit.jupiter.api.AfterEach;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.RegisterExtension;
 import org.junit.jupiter.params.ParameterizedTest;
 import org.junit.jupiter.params.provider.CsvSource;
 import org.springframework.boot.SpringApplication;
@@ -37,6 +38,8 @@
 
 abstract class AbstractSpringIntegrationTracingTest {
 
+  @RegisterExtension static final AutoCleanupExtension cleanup = AutoCleanupExtension.create();
+
   private final InstrumentationExtension testing;
 
   private final Class<?> additionalContextClass;
@@ -61,13 +64,7 @@ void setUp() {
     springApplication.setDefaultProperties(
         singletonMap("spring.main.web-application-type", "none"));
     applicationContext = springApplication.run();
-  }
-
-  @AfterEach
-  void tearDown() {
-    if (applicationContext != null) {
-      applicationContext.close();
-    }
+    cleanup.deferCleanup(applicationContext);
   }
 
   @ParameterizedTest