Code review sweep (run 25083201560) (#18392)

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

Author: otelbot[bot]

instrumentation/spring/spring-boot-actuator-autoconfigure-2.0/javaagent/build.gradle.kts

@@ -8,8 +8,8 @@ muzzle {
     group.set("org.springframework.boot")
     module.set("spring-boot-actuator-autoconfigure")
     versions.set("[2.0.0.RELEASE,)")
-    extraDependency("io.micrometer:micrometer-core:1.5.0")
     assertInverse.set(true)
+    extraDependency("io.micrometer:micrometer-core:1.5.0")
   }
 }
 

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

@@ -37,14 +37,19 @@
  * strategy wins:
  *
  * <ul>
- *   <li>Check for the SPRING_APPLICATION_NAME environment variable
+ *   <li>Check for --spring.application.name program argument (not jvm arg) via ProcessHandle
+ *   <li>Check for --spring.application.name program argument via sun.java.command system property
  *   <li>Check for spring.application.name system property
- *   <li>Check for application.properties file on the classpath
+ *   <li>Check for the SPRING_APPLICATION_NAME environment variable
  *   <li>Check for application.properties in the current working dir
- *   <li>Check for application.yml on the classpath
  *   <li>Check for application.yml in the current working dir
- *   <li>Check for --spring.application.name program argument (not jvm arg) via ProcessHandle
- *   <li>Check for --spring.application.name program argument via sun.java.command system property
+ *   <li>Check for application.yaml in the current working dir
+ *   <li>Check for application.properties file on the classpath
+ *   <li>Check for application.yml on the classpath
+ *   <li>Check for application.yaml on the classpath
+ *   <li>Check for bootstrap.properties file on the classpath
+ *   <li>Check for bootstrap.yml on the classpath
+ *   <li>Check for bootstrap.yaml on the classpath
  * </ul>
  *
  * <p>Note: The spring starter already includes provider in
@@ -57,7 +62,7 @@ public class SpringBootServiceNameDetector implements ConditionalResourceProvide
       Logger.getLogger(SpringBootServiceNameDetector.class.getName());
   private static final String COMMANDLINE_ARG_PREFIX = "--spring.application.name=";
   private static final Pattern COMMANDLINE_PATTERN =
-      Pattern.compile("--spring\\.application\\.name=([a-zA-Z.\\-_]+)");
+      Pattern.compile("--spring\\.application\\.name=(\\S+)");
   private final SystemHelper system;
 
   @SuppressWarnings("unused")

instrumentation/spring/spring-boot-resources/javaagent/src/test/java/io/opentelemetry/instrumentation/spring/resources/SpringBootServiceNameDetectorTest.java

@@ -8,6 +8,7 @@
 import static io.opentelemetry.semconv.ServiceAttributes.SERVICE_NAME;
 import static java.nio.charset.StandardCharsets.UTF_8;
 import static java.util.Collections.singletonMap;
+import static java.util.Objects.requireNonNull;
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.mockito.Mockito.lenient;
 import static org.mockito.Mockito.when;
@@ -18,7 +19,6 @@
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.OutputStream;
-import java.net.URL;
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.nio.file.Paths;
@@ -132,12 +132,12 @@ void classpathApplicationYamlContainingMultipleYamlDefinitions(String fileName)
 
   @ParameterizedTest
   @ValueSource(strings = {"application.yaml", APPLICATION_YML})
-  void yamlFileInCurrentDir(String fileName) throws Exception {
+  void yamlFileInCurrentDir(String fileName) throws IOException {
     Path yamlPath = Paths.get(fileName);
     try {
-      URL url = getClass().getClassLoader().getResource(APPLICATION_YML);
-      String content = readString(Paths.get(url.toURI()));
-      writeString(yamlPath, content);
+      try (InputStream in = requireNonNull(openClasspathResource(APPLICATION_YML))) {
+        Files.copy(in, yamlPath);
+      }
       when(system.openFile(fileName)).thenCallRealMethod();
       SpringBootServiceNameDetector guesser = new SpringBootServiceNameDetector(system);
       Resource result = guesser.createResource(config);
@@ -165,10 +165,11 @@ void getFromCommandlineArgsWithProcessHandle() throws ReflectiveOperationExcepti
   @Test
   void getFromCommandlineArgsWithSystemProperty() {
     when(system.getProperty("sun.java.command"))
-        .thenReturn("/bin/java sweet-spring.jar --spring.application.name=bullpen --quiet=never");
+        .thenReturn(
+            "/bin/java sweet-spring.jar --spring.application.name=bullpen-v2 --quiet=never");
     SpringBootServiceNameDetector guesser = new SpringBootServiceNameDetector(system);
     Resource result = guesser.createResource(config);
-    expectServiceName(result, "bullpen");
+    expectServiceName(result, "bullpen-v2");
   }
 
   @Test
@@ -210,11 +211,6 @@ private static void writeString(Path path, String value) throws IOException {
     }
   }
 
-  private static String readString(Path path) throws Exception {
-    byte[] allBytes = Files.readAllBytes(path);
-    return new String(allBytes, UTF_8);
-  }
-
   private InputStream openClasspathResource(String resource) {
     return getClass().getClassLoader().getResourceAsStream(resource);
   }

instrumentation/spring/spring-cloud-gateway/spring-cloud-gateway-2.0/javaagent/build.gradle.kts

@@ -16,7 +16,7 @@ muzzle {
   pass {
     group.set("org.springframework.cloud")
     module.set("spring-cloud-starter-gateway-server-webflux")
-    versions.set("[4.3.0,]")
+    versions.set("[4.3.0,)")
     assertInverse.set(true)
   }
 }
@@ -39,7 +39,7 @@ dependencies {
   latestDepTestLibrary("org.springframework.boot:spring-boot-starter-test:2.1.+") // see spring-cloud-gateway-2.2:testing module
 }
 
-tasks.withType<Test>().configureEach {
+tasks.test {
   jvmArgs("-Dotel.instrumentation.spring-cloud-gateway.experimental-span-attributes=true")
 
   // required on jdk17

instrumentation/spring/spring-cloud-gateway/spring-cloud-gateway-2.0/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/spring/gateway/v2_0/GatewayTestApplication.java

@@ -13,10 +13,10 @@
 import org.springframework.context.annotation.Bean;
 
 @SpringBootApplication
-public class GatewayTestApplication {
+class GatewayTestApplication {
 
   @Bean
-  public RouteLocator customRouteLocator(RouteLocatorBuilder builder) {
+  RouteLocator customRouteLocator(RouteLocatorBuilder builder) {
     // A simple echo gateway.
     return builder
         .routes()

instrumentation/spring/spring-cloud-gateway/spring-cloud-gateway-webflux-4.3/testing/build.gradle.kts

@@ -18,7 +18,7 @@ dependencies {
   testLibrary("org.springframework.boot:spring-boot-webflux")
 }
 
-tasks.withType<Test>().configureEach {
+tasks.test {
   jvmArgs("-Dotel.instrumentation.spring-cloud-gateway.experimental-span-attributes=true")
   jvmArgs("-Dotel.instrumentation.common.experimental.controller-telemetry.enabled=true")
 }

instrumentation/spring/spring-cloud-gateway/spring-cloud-gateway-webmvc-4.3/javaagent/build.gradle.kts

@@ -26,7 +26,7 @@ dependencies {
   testLibrary("org.springframework.boot:spring-boot-starter-test:4.0.0")
 }
 
-tasks.withType<Test>().configureEach {
+tasks.test {
   jvmArgs("-Dotel.instrumentation.spring-cloud-gateway.experimental-span-attributes=true")
   jvmArgs("-Dotel.instrumentation.common.experimental.controller-telemetry.enabled=true")
 }

instrumentation/spring/spring-data/spring-data-1.8/javaagent/build.gradle.kts

@@ -8,14 +8,14 @@ muzzle {
   pass {
     group.set("org.springframework.data")
     module.set("spring-data-commons")
-    versions.set("[1.8.0.RELEASE,]")
+    versions.set("[1.8.0.RELEASE,)")
     extraDependency("org.springframework:spring-aop:1.2")
     assertInverse.set(true)
   }
   pass {
     group.set("org.springframework")
     module.set("spring-aop")
-    versions.set("[1.2,]")
+    versions.set("[1.2,)")
     extraDependency("org.springframework.data:spring-data-commons:1.8.0.RELEASE")
     assertInverse.set(true)
   }

instrumentation/spring/spring-data/spring-data-3.0/kotlin-testing/src/test/kotlin/io/opentelemetry/javaagent/instrumentation/spring/data/v3_0/KotlinSpringDataTest.kt

@@ -24,7 +24,7 @@ class KotlinSpringDataTest {
   companion object {
     @JvmStatic
     @RegisterExtension
-    val testing = AgentInstrumentationExtension.create()
+    private val testing = AgentInstrumentationExtension.create()
   }
 
   private lateinit var applicationContext: ConfigurableApplicationContext

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

@@ -33,6 +33,42 @@ final class TracingChannelInterceptor implements ExecutorChannelInterceptor {
   private static final ThreadLocal<Map<MessageChannel, ContextAndScope>> localContextAndScope =
       ThreadLocal.withInitial(IdentityHashMap::new);
 
+  @Nullable
+  private static final Class<?> directWithAttributesChannelClass =
+      getDirectWithAttributesChannelClass();
+
+  @Nullable
+  private static final MethodHandle channelGetAttributeMh =
+      getChannelAttributeMh(directWithAttributesChannelClass);
+
+  @Nullable
+  private static Class<?> getDirectWithAttributesChannelClass() {
+    try {
+      return Class.forName(
+          "org.springframework.cloud.stream.messaging.DirectWithAttributesChannel");
+    } catch (ClassNotFoundException ignored) {
+      return null;
+    }
+  }
+
+  @Nullable
+  private static MethodHandle getChannelAttributeMh(
+      @Nullable Class<?> directWithAttributesChannelClass) {
+    if (directWithAttributesChannelClass == null) {
+      return null;
+    }
+
+    try {
+      return MethodHandles.lookup()
+          .findVirtual(
+              directWithAttributesChannelClass,
+              "getAttribute",
+              MethodType.methodType(Object.class, String.class));
+    } catch (NoSuchMethodException | IllegalAccessException ignored) {
+      return null;
+    }
+  }
+
   private final ContextPropagators propagators;
   private final Instrumenter<MessageWithChannel, Void> consumerInstrumenter;
   private final Instrumenter<MessageWithChannel, Void> producerInstrumenter;
@@ -203,42 +239,6 @@ private static Message<?> createMessageWithHeaders(
         .build();
   }
 
-  @Nullable
-  private static final Class<?> directWithAttributesChannelClass =
-      getDirectWithAttributesChannelClass();
-
-  @Nullable
-  private static final MethodHandle channelGetAttributeMh =
-      getChannelAttributeMh(directWithAttributesChannelClass);
-
-  @Nullable
-  private static Class<?> getDirectWithAttributesChannelClass() {
-    try {
-      return Class.forName(
-          "org.springframework.cloud.stream.messaging.DirectWithAttributesChannel");
-    } catch (ClassNotFoundException ignored) {
-      return null;
-    }
-  }
-
-  @Nullable
-  private static MethodHandle getChannelAttributeMh(
-      @Nullable Class<?> directWithAttributesChannelClass) {
-    if (directWithAttributesChannelClass == null) {
-      return null;
-    }
-
-    try {
-      return MethodHandles.lookup()
-          .findVirtual(
-              directWithAttributesChannelClass,
-              "getAttribute",
-              MethodType.methodType(Object.class, String.class));
-    } catch (NoSuchMethodException | IllegalAccessException ignored) {
-      return null;
-    }
-  }
-
   private boolean createProducerSpan(MessageChannel messageChannel) {
     if (!producerSpanEnabled || directWithAttributesChannelClass == null) {
       return false;