Make spring boot service name detector handle BOOT-INF/classes

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

@@ -265,6 +265,18 @@ private String loadFromClasspath(String filename, Function<InputStream, String>
 
   // Exists for testing
   static class SystemHelper {
+    private final ClassLoader classLoader;
+    private final boolean addBootInfPrefix;
+
+    SystemHelper() {
+      ClassLoader contextClassLoader = Thread.currentThread().getContextClassLoader();
+      classLoader =
+          contextClassLoader != null ? contextClassLoader : ClassLoader.getSystemClassLoader();
+      addBootInfPrefix = classLoader.getResource("BOOT-INF/classes/") != null;
+      if (addBootInfPrefix) {
+        logger.log(Level.FINER, "Detected presence of BOOT-INF/classes/");
+      }
+    }
 
     String getenv(String name) {
       return System.getenv(name);
@@ -275,7 +287,8 @@ String getProperty(String key) {
     }
 
     InputStream openClasspathResource(String filename) {
-      return ClassLoader.getSystemClassLoader().getResourceAsStream(filename);
+      String path = addBootInfPrefix ? "BOOT-INF/classes/" + filename : filename;
+      return classLoader.getResourceAsStream(path);
     }
 
     InputStream openFile(String filename) throws Exception {

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

@@ -14,6 +14,7 @@
 import io.opentelemetry.api.common.Attributes;
 import io.opentelemetry.sdk.autoconfigure.spi.ConfigProperties;
 import io.opentelemetry.sdk.resources.Resource;
+import java.io.InputStream;
 import java.io.OutputStream;
 import java.net.URL;
 import java.nio.file.Files;
@@ -45,7 +46,7 @@ void findByEnvVar() {
 
   @Test
   void classpathApplicationProperties() {
-    when(system.openClasspathResource(PROPS)).thenCallRealMethod();
+    when(system.openClasspathResource(PROPS)).thenReturn(openClasspathResource(PROPS));
     SpringBootServiceNameDetector guesser = new SpringBootServiceNameDetector(system);
     Resource result = guesser.createResource(config);
     expectServiceName(result, "dog-store");
@@ -67,7 +68,8 @@ void propertiesFileInCurrentDir() throws Exception {
 
   @Test
   void classpathApplicationYaml() {
-    when(system.openClasspathResource(APPLICATION_YML)).thenCallRealMethod();
+    when(system.openClasspathResource(APPLICATION_YML))
+        .thenReturn(openClasspathResource(APPLICATION_YML));
     SpringBootServiceNameDetector guesser = new SpringBootServiceNameDetector(system);
     Resource result = guesser.createResource(config);
     expectServiceName(result, "cat-store");
@@ -156,4 +158,8 @@ 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-boot-resources/testing/build.gradle.kts

@@ -0,0 +1,12 @@
+plugins {
+  id("otel.java-conventions")
+}
+
+dependencies {
+  testCompileOnly("com.google.auto.service:auto-service-annotations")
+
+  testImplementation("org.mockito:mockito-junit-jupiter")
+  testImplementation(project(":instrumentation:spring:spring-boot-resources:library"))
+  testImplementation("io.opentelemetry:opentelemetry-sdk-extension-autoconfigure-spi")
+  testImplementation(project(path = ":smoke-tests:images:spring-boot", configuration = "springBootJar"))
+}

instrumentation/spring/spring-boot-resources/testing/src/test/java/io/opentelemetry/instrumentation/spring/resources/TestBootInfClassesResource.java

@@ -0,0 +1,33 @@
+/*
+ * Copyright The OpenTelemetry Authors
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+package io.opentelemetry.instrumentation.spring.resources;
+
+import static io.opentelemetry.semconv.resource.attributes.ResourceAttributes.SERVICE_NAME;
+import static org.assertj.core.api.Assertions.assertThat;
+
+import io.opentelemetry.sdk.autoconfigure.spi.ConfigProperties;
+import io.opentelemetry.sdk.resources.Resource;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.ExtendWith;
+import org.mockito.Mock;
+import org.mockito.junit.jupiter.MockitoExtension;
+
+@ExtendWith(MockitoExtension.class)
+class TestBootInfClassesResource {
+  @Mock ConfigProperties config;
+
+  @Test
+  void testServiceName() {
+    // verify that the test app, that is added as a dependency to this project, has the expected
+    // layout
+    assertThat(getClass().getResource("/application.properties")).isNull();
+    assertThat(getClass().getResource("/BOOT-INF/classes/application.properties")).isNotNull();
+
+    SpringBootServiceNameDetector guesser = new SpringBootServiceNameDetector();
+    Resource result = guesser.createResource(config);
+    assertThat(result.getAttribute(SERVICE_NAME)).isEqualTo("otel-spring-test-app");
+  }
+}

settings.gradle.kts

@@ -451,6 +451,7 @@ hideFromDependabot(":instrumentation:spark-2.3:javaagent")
 hideFromDependabot(":instrumentation:spring:spring-batch-3.0:javaagent")
 hideFromDependabot(":instrumentation:spring:spring-boot-actuator-autoconfigure-2.0:javaagent")
 hideFromDependabot(":instrumentation:spring:spring-boot-resources:library")
+hideFromDependabot(":instrumentation:spring:spring-boot-resources:testing")
 hideFromDependabot(":instrumentation:spring:spring-core-2.0:javaagent")
 hideFromDependabot(":instrumentation:spring:spring-data:spring-data-1.8:javaagent")
 hideFromDependabot(":instrumentation:spring:spring-data:spring-data-3.0:testing")

smoke-tests/src/test/groovy/io/opentelemetry/smoketest/SmokeTest.groovy

@@ -51,6 +51,13 @@ abstract class SmokeTest extends Specification {
     return Collections.emptyMap()
   }
 
+  /**
+   * Subclasses can override this method to disable setting default service name
+   */
+  protected boolean getSetServiceName() {
+    return true
+  }
+
   /**
    * Subclasses can override this method to provide additional files to copy to target container
    */
@@ -77,7 +84,7 @@ abstract class SmokeTest extends Specification {
 
   def startTarget(String jdk, String serverVersion, boolean windows) {
     def targetImage = getTargetImage(jdk, serverVersion, windows)
-    return containerManager.startTarget(targetImage, agentPath, jvmArgsEnvVarName, extraEnv, extraResources, extraPorts, getWaitStrategy(), getCommand())
+    return containerManager.startTarget(targetImage, agentPath, jvmArgsEnvVarName, extraEnv, setServiceName, extraResources, extraPorts, getWaitStrategy(), getCommand())
   }
 
   protected abstract String getTargetImage(String jdk)

smoke-tests/src/test/groovy/io/opentelemetry/smoketest/SpringBootSmokeTest.groovy

@@ -21,7 +21,12 @@ import static java.util.stream.Collectors.toSet
 class SpringBootSmokeTest extends SmokeTest {
 
   protected String getTargetImage(String jdk) {
-    "ghcr.io/open-telemetry/opentelemetry-java-instrumentation/smoke-test-spring-boot:jdk$jdk-20211213.1570880324"
+    "ghcr.io/open-telemetry/opentelemetry-java-instrumentation/smoke-test-spring-boot:jdk$jdk-20230320.4472351571"
+  }
+
+  @Override
+  protected boolean getSetServiceName() {
+    return false
   }
 
   @Override
@@ -82,10 +87,18 @@ class SpringBootSmokeTest extends SmokeTest {
     metrics.hasMetricsNamed("process.runtime.jvm.memory.committed")
     metrics.hasMetricsNamed("process.runtime.jvm.memory.limit")
 
+    then: "service name is autodetected"
+    def serviceName = findResourceAttribute(traces, "service.name")
+      .map { it.stringValue }
+      .findAny()
+    serviceName.isPresent()
+    serviceName.get() == "otel-spring-test-app"
+
     cleanup:
     stopTarget()
 
     where:
-    jdk << [8, 11, 17]
+    // FIXME: add back jdk8 when jdk8 image is fixed
+    jdk << [11, 17, 19]
   }
 }

smoke-tests/src/test/java/io/opentelemetry/smoketest/AbstractTestContainerManager.java

@@ -17,7 +17,8 @@ public abstract class AbstractTestContainerManager implements TestContainerManag
 
   private boolean started = false;
 
-  protected Map<String, String> getAgentEnvironment(String jvmArgsEnvVarName) {
+  protected Map<String, String> getAgentEnvironment(
+      String jvmArgsEnvVarName, boolean setServiceName) {
     Map<String, String> environment = new HashMap<>();
     // while modern JVMs understand linux container memory limits, they do not understand windows
     // container memory limits yet, so we need to explicitly set max heap in order to prevent the
@@ -27,7 +28,9 @@ protected Map<String, String> getAgentEnvironment(String jvmArgsEnvVarName) {
     environment.put("OTEL_BSP_SCHEDULE_DELAY", "10ms");
     environment.put("OTEL_METRIC_EXPORT_INTERVAL", "1000");
     environment.put("OTEL_EXPORTER_OTLP_ENDPOINT", "http://" + BACKEND_ALIAS + ":8080");
-    environment.put("OTEL_RESOURCE_ATTRIBUTES", "service.name=smoke-test");
+    if (setServiceName) {
+      environment.put("OTEL_RESOURCE_ATTRIBUTES", "service.name=smoke-test");
+    }
     environment.put("OTEL_JAVAAGENT_DEBUG", "true");
     return environment;
   }

smoke-tests/src/test/java/io/opentelemetry/smoketest/LinuxTestContainerManager.java

@@ -74,6 +74,7 @@ public Consumer<OutputFrame> startTarget(
       String agentPath,
       String jvmArgsEnvVarName,
       Map<String, String> extraEnv,
+      boolean setServiceName,
       List<ResourceMapping> extraResources,
       List<Integer> extraPorts,
       TargetWaitStrategy waitStrategy,
@@ -94,7 +95,7 @@ public Consumer<OutputFrame> startTarget(
             .withLogConsumer(new Slf4jLogConsumer(appLogger))
             .withCopyFileToContainer(
                 MountableFile.forHostPath(agentPath), "/" + TARGET_AGENT_FILENAME)
-            .withEnv(getAgentEnvironment(jvmArgsEnvVarName))
+            .withEnv(getAgentEnvironment(jvmArgsEnvVarName, setServiceName))
             .withEnv(extraEnv);
 
     for (ResourceMapping resource : extraResources) {

smoke-tests/src/test/java/io/opentelemetry/smoketest/TestContainerManager.java

@@ -23,6 +23,7 @@ Consumer<OutputFrame> startTarget(
       String agentPath,
       String jvmArgsEnvVarName,
       Map<String, String> extraEnv,
+      boolean setServiceName,
       List<ResourceMapping> extraResources,
       List<Integer> extraPorts,
       TargetWaitStrategy waitStrategy,

smoke-tests/src/test/java/io/opentelemetry/smoketest/windows/WindowsTestContainerManager.java

@@ -133,6 +133,7 @@ public Consumer<OutputFrame> startTarget(
       String agentPath,
       String jvmArgsEnvVarName,
       Map<String, String> extraEnv,
+      boolean setServiceName,
       List<ResourceMapping> extraResources,
       List<Integer> extraPorts,
       TargetWaitStrategy waitStrategy,
@@ -148,7 +149,7 @@ public Consumer<OutputFrame> startTarget(
     }
 
     List<String> environment = new ArrayList<>();
-    getAgentEnvironment(jvmArgsEnvVarName)
+    getAgentEnvironment(jvmArgsEnvVarName, setServiceName)
         .forEach((key, value) -> environment.add(key + "=" + value));
     extraEnv.forEach((key, value) -> environment.add(key + "=" + value));