diff --git a/instrumentation/spring/spring-boot-resources/library/src/main/java/io/opentelemetry/instrumentation/spring/resources/SpringBootServiceNameDetector.java b/instrumentation/spring/spring-boot-resources/library/src/main/java/io/opentelemetry/instrumentation/spring/resources/SpringBootServiceNameDetector.java index bfe7af906637..d8c470ff66a6 100644 --- a/instrumentation/spring/spring-boot-resources/library/src/main/java/io/opentelemetry/instrumentation/spring/resources/SpringBootServiceNameDetector.java +++ b/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 // 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 { diff --git a/instrumentation/spring/spring-boot-resources/library/src/test/java/io/opentelemetry/instrumentation/spring/resources/SpringBootServiceNameDetectorTest.java b/instrumentation/spring/spring-boot-resources/library/src/test/java/io/opentelemetry/instrumentation/spring/resources/SpringBootServiceNameDetectorTest.java index c7a4782cf51b..6a82f019411c 100644 --- a/instrumentation/spring/spring-boot-resources/library/src/test/java/io/opentelemetry/instrumentation/spring/resources/SpringBootServiceNameDetectorTest.java +++ b/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); + } } diff --git a/instrumentation/spring/spring-boot-resources/testing/build.gradle.kts b/instrumentation/spring/spring-boot-resources/testing/build.gradle.kts new file mode 100644 index 000000000000..7d9a1e72392d --- /dev/null +++ b/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")) +} diff --git a/instrumentation/spring/spring-boot-resources/testing/src/test/java/io/opentelemetry/instrumentation/spring/resources/TestBootInfClassesResource.java b/instrumentation/spring/spring-boot-resources/testing/src/test/java/io/opentelemetry/instrumentation/spring/resources/TestBootInfClassesResource.java new file mode 100644 index 000000000000..d2a01e929d60 --- /dev/null +++ b/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"); + } +} diff --git a/settings.gradle.kts b/settings.gradle.kts index 559e5d581733..b2a20a754f80 100644 --- a/settings.gradle.kts +++ b/settings.gradle.kts @@ -456,6 +456,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") diff --git a/smoke-tests/src/test/groovy/io/opentelemetry/smoketest/SmokeTest.groovy b/smoke-tests/src/test/groovy/io/opentelemetry/smoketest/SmokeTest.groovy index d11ffbf9e8ef..d8bdee7dfa3b 100644 --- a/smoke-tests/src/test/groovy/io/opentelemetry/smoketest/SmokeTest.groovy +++ b/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) diff --git a/smoke-tests/src/test/groovy/io/opentelemetry/smoketest/SpringBootSmokeTest.groovy b/smoke-tests/src/test/groovy/io/opentelemetry/smoketest/SpringBootSmokeTest.groovy index 4d651838f92c..eeb463d8d17c 100644 --- a/smoke-tests/src/test/groovy/io/opentelemetry/smoketest/SpringBootSmokeTest.groovy +++ b/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-20230321.4484174638" + } + + @Override + protected boolean getSetServiceName() { + return false } @Override @@ -82,10 +87,17 @@ 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] + jdk << [8, 11, 17, 19] } } diff --git a/smoke-tests/src/test/java/io/opentelemetry/smoketest/AbstractTestContainerManager.java b/smoke-tests/src/test/java/io/opentelemetry/smoketest/AbstractTestContainerManager.java index 1a12d2978391..79b163c3991b 100644 --- a/smoke-tests/src/test/java/io/opentelemetry/smoketest/AbstractTestContainerManager.java +++ b/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 getAgentEnvironment(String jvmArgsEnvVarName) { + protected Map getAgentEnvironment( + String jvmArgsEnvVarName, boolean setServiceName) { Map 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 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; } diff --git a/smoke-tests/src/test/java/io/opentelemetry/smoketest/LinuxTestContainerManager.java b/smoke-tests/src/test/java/io/opentelemetry/smoketest/LinuxTestContainerManager.java index dd709cace4cb..7ebc47b31f47 100644 --- a/smoke-tests/src/test/java/io/opentelemetry/smoketest/LinuxTestContainerManager.java +++ b/smoke-tests/src/test/java/io/opentelemetry/smoketest/LinuxTestContainerManager.java @@ -74,6 +74,7 @@ public Consumer startTarget( String agentPath, String jvmArgsEnvVarName, Map extraEnv, + boolean setServiceName, List extraResources, List extraPorts, TargetWaitStrategy waitStrategy, @@ -94,7 +95,7 @@ public Consumer 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) { diff --git a/smoke-tests/src/test/java/io/opentelemetry/smoketest/TestContainerManager.java b/smoke-tests/src/test/java/io/opentelemetry/smoketest/TestContainerManager.java index 895e495e99e7..f926c95bda26 100644 --- a/smoke-tests/src/test/java/io/opentelemetry/smoketest/TestContainerManager.java +++ b/smoke-tests/src/test/java/io/opentelemetry/smoketest/TestContainerManager.java @@ -23,6 +23,7 @@ Consumer startTarget( String agentPath, String jvmArgsEnvVarName, Map extraEnv, + boolean setServiceName, List extraResources, List extraPorts, TargetWaitStrategy waitStrategy, diff --git a/smoke-tests/src/test/java/io/opentelemetry/smoketest/windows/WindowsTestContainerManager.java b/smoke-tests/src/test/java/io/opentelemetry/smoketest/windows/WindowsTestContainerManager.java index 5dc11fcf3028..0026380032d2 100644 --- a/smoke-tests/src/test/java/io/opentelemetry/smoketest/windows/WindowsTestContainerManager.java +++ b/smoke-tests/src/test/java/io/opentelemetry/smoketest/windows/WindowsTestContainerManager.java @@ -133,6 +133,7 @@ public Consumer startTarget( String agentPath, String jvmArgsEnvVarName, Map extraEnv, + boolean setServiceName, List extraResources, List extraPorts, TargetWaitStrategy waitStrategy, @@ -148,7 +149,7 @@ public Consumer startTarget( } List environment = new ArrayList<>(); - getAgentEnvironment(jvmArgsEnvVarName) + getAgentEnvironment(jvmArgsEnvVarName, setServiceName) .forEach((key, value) -> environment.add(key + "=" + value)); extraEnv.forEach((key, value) -> environment.add(key + "=" + value));