From 67afd61fc506e5bcbc69e1fdfbdd8bc0c70c8524 Mon Sep 17 00:00:00 2001 From: Gregor Zeitlinger Date: Wed, 21 Feb 2024 13:06:16 +0100 Subject: [PATCH 01/14] add ManifestResourceProvider --- .../resources/AttributeProvider.java | 22 ++++ .../resources/AttributeResourceProvider.java | 110 +++++++++++++++++ .../resources/ManifestResourceProvider.java | 68 +++++++++++ .../ManifestResourceProviderTest.java | 112 ++++++++++++++++++ 4 files changed, 312 insertions(+) create mode 100644 instrumentation/resources/library/src/main/java/io/opentelemetry/instrumentation/resources/AttributeProvider.java create mode 100644 instrumentation/resources/library/src/main/java/io/opentelemetry/instrumentation/resources/AttributeResourceProvider.java create mode 100644 instrumentation/resources/library/src/main/java/io/opentelemetry/instrumentation/resources/ManifestResourceProvider.java create mode 100644 instrumentation/resources/library/src/test/java/io/opentelemetry/instrumentation/resources/ManifestResourceProviderTest.java diff --git a/instrumentation/resources/library/src/main/java/io/opentelemetry/instrumentation/resources/AttributeProvider.java b/instrumentation/resources/library/src/main/java/io/opentelemetry/instrumentation/resources/AttributeProvider.java new file mode 100644 index 000000000000..ab1370cd1cb6 --- /dev/null +++ b/instrumentation/resources/library/src/main/java/io/opentelemetry/instrumentation/resources/AttributeProvider.java @@ -0,0 +1,22 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.instrumentation.resources; + +import com.google.errorprone.annotations.CanIgnoreReturnValue; +import io.opentelemetry.api.common.AttributeKey; +import java.util.Optional; +import java.util.function.Function; + +public interface AttributeProvider { + Optional readData(); + + void registerAttributes(Builder builder); + + public interface Builder { + @CanIgnoreReturnValue + public Builder add(AttributeKey key, Function> getter); + } +} diff --git a/instrumentation/resources/library/src/main/java/io/opentelemetry/instrumentation/resources/AttributeResourceProvider.java b/instrumentation/resources/library/src/main/java/io/opentelemetry/instrumentation/resources/AttributeResourceProvider.java new file mode 100644 index 000000000000..311304612517 --- /dev/null +++ b/instrumentation/resources/library/src/main/java/io/opentelemetry/instrumentation/resources/AttributeResourceProvider.java @@ -0,0 +1,110 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.instrumentation.resources; + +import com.google.errorprone.annotations.CanIgnoreReturnValue; +import io.opentelemetry.api.common.AttributeKey; +import io.opentelemetry.api.common.Attributes; +import io.opentelemetry.api.common.AttributesBuilder; +import io.opentelemetry.sdk.autoconfigure.spi.ConfigProperties; +import io.opentelemetry.sdk.autoconfigure.spi.internal.ConditionalResourceProvider; +import io.opentelemetry.sdk.resources.Resource; +import io.opentelemetry.semconv.ResourceAttributes; +import java.util.HashMap; +import java.util.Map; +import java.util.Objects; +import java.util.Optional; +import java.util.function.Function; + +@SuppressWarnings({"unchecked", "rawtypes"}) +public abstract class AttributeResourceProvider implements ConditionalResourceProvider { + + private final AttributeProvider attributeProvider; + + public class AttributeBuilder implements AttributeProvider.Builder { + + private AttributeBuilder() {} + + @CanIgnoreReturnValue + @Override + public AttributeBuilder add(AttributeKey key, Function> getter) { + attributeGetters.put((AttributeKey) key, Objects.requireNonNull((Function) getter)); + return this; + } + } + + private static final ThreadLocal existingResource = new ThreadLocal<>(); + + private final Map, Function>> attributeGetters = + new HashMap<>(); + + public AttributeResourceProvider(AttributeProvider attributeProvider) { + this.attributeProvider = attributeProvider; + attributeProvider.registerAttributes(new AttributeBuilder()); + } + + @Override + public final boolean shouldApply(ConfigProperties config, Resource existing) { + existingResource.set(existing); + + Map resourceAttributes = getResourceAttributes(config); + return attributeGetters.keySet().stream() + .allMatch(key -> shouldUpdate(config, existing, key, resourceAttributes)); + } + + @Override + public final Resource createResource(ConfigProperties config) { + return attributeProvider + .readData() + .map( + data -> { + // what should we do here? + // we don't have access to the existing resource + // if the resource provider produces a single key, we can rely on shouldApply + // i.e. this method won't be called if the key is already present + // the thread local is a hack to work around this + Resource existing = + Objects.requireNonNull(existingResource.get(), "call shouldApply first"); + Map resourceAttributes = getResourceAttributes(config); + AttributesBuilder builder = Attributes.builder(); + attributeGetters.entrySet().stream() + .filter(e -> shouldUpdate(config, existing, e.getKey(), resourceAttributes)) + .forEach( + e -> + e.getValue() + .apply(data) + .ifPresent(value -> putAttribute(builder, e.getKey(), value))); + return Resource.create(builder.build()); + }) + .orElse(Resource.empty()); + } + + private static void putAttribute(AttributesBuilder builder, AttributeKey key, T value) { + builder.put(key, value); + } + + private static Map getResourceAttributes(ConfigProperties config) { + return config.getMap("otel.resource.attributes"); + } + + private static boolean shouldUpdate( + ConfigProperties config, + Resource existing, + AttributeKey key, + Map resourceAttributes) { + if (resourceAttributes.containsKey(key.getKey())) { + return false; + } + + Object value = existing.getAttribute(key); + + if (key.equals(ResourceAttributes.SERVICE_NAME)) { + return config.getString("otel.service.name") == null && "unknown_service:java".equals(value); + } + + return value == null; + } +} diff --git a/instrumentation/resources/library/src/main/java/io/opentelemetry/instrumentation/resources/ManifestResourceProvider.java b/instrumentation/resources/library/src/main/java/io/opentelemetry/instrumentation/resources/ManifestResourceProvider.java new file mode 100644 index 000000000000..7e7502cb11f8 --- /dev/null +++ b/instrumentation/resources/library/src/main/java/io/opentelemetry/instrumentation/resources/ManifestResourceProvider.java @@ -0,0 +1,68 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.instrumentation.resources; + +import static java.util.logging.Level.WARNING; + +import com.google.auto.service.AutoService; +import io.opentelemetry.sdk.autoconfigure.spi.ResourceProvider; +import io.opentelemetry.semconv.ResourceAttributes; +import java.io.IOException; +import java.io.InputStream; +import java.util.Optional; +import java.util.jar.Manifest; +import java.util.logging.Logger; + +/** + * A {@link ResourceProvider} that will attempt to detect the service.name and + * service.version from META-INF/MANIFEST.MF. + */ +@AutoService(ResourceProvider.class) +public final class ManifestResourceProvider extends AttributeResourceProvider { + + private static final Logger logger = Logger.getLogger(ManifestResourceProvider.class.getName()); + + public ManifestResourceProvider() { + super( + new AttributeProvider() { + @Override + public Optional readData() { + try { + Manifest manifest = new Manifest(); + InputStream systemResourceAsStream = + ClassLoader.getSystemResourceAsStream("META-INF/MANIFEST.MF"); + if (systemResourceAsStream == null) { + return Optional.empty(); + } + manifest.read(systemResourceAsStream); + return Optional.of(manifest); + } catch (IOException e) { + logger.log(WARNING, "Error reading manifest", e); + return Optional.empty(); + } + } + + @Override + public void registerAttributes(Builder builder) { + builder + .add( + ResourceAttributes.SERVICE_NAME, + manifest -> { + String serviceName = + manifest.getMainAttributes().getValue("Implementation-Title"); + return Optional.ofNullable(serviceName); + }) + .add( + ResourceAttributes.SERVICE_VERSION, + manifest -> { + String serviceVersion = + manifest.getMainAttributes().getValue("Implementation-Version"); + return Optional.ofNullable(serviceVersion); + }); + } + }); + } +} diff --git a/instrumentation/resources/library/src/test/java/io/opentelemetry/instrumentation/resources/ManifestResourceProviderTest.java b/instrumentation/resources/library/src/test/java/io/opentelemetry/instrumentation/resources/ManifestResourceProviderTest.java new file mode 100644 index 000000000000..30fecc319331 --- /dev/null +++ b/instrumentation/resources/library/src/test/java/io/opentelemetry/instrumentation/resources/ManifestResourceProviderTest.java @@ -0,0 +1,112 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.instrumentation.resources; + +import static io.opentelemetry.semconv.ResourceAttributes.SERVICE_NAME; +import static io.opentelemetry.semconv.ResourceAttributes.SERVICE_VERSION; +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.Mockito.when; + +import io.opentelemetry.api.common.AttributeKey; +import io.opentelemetry.sdk.autoconfigure.spi.ConfigProperties; +import java.io.InputStream; +import java.util.Collection; +import java.util.function.Function; +import java.util.stream.Collectors; +import java.util.stream.Stream; +import org.junit.jupiter.api.DynamicTest; +import org.junit.jupiter.api.TestFactory; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; + +@ExtendWith(MockitoExtension.class) +class ManifestResourceProviderTest { + + static final String BUILD_PROPS = "build-info.properties"; + static final String META_INFO = "META-INF"; + + @Mock ConfigProperties config; + @Mock SystemHelper system; + + private static class TestCase { + private final String name; + private final Function factory; + private final AttributeKey key; + private final String expected; + private final InputStream input; + + public TestCase( + String name, + Function factory, + AttributeKey key, + String expected, + InputStream input) { + this.name = name; + this.factory = factory; + this.key = key; + this.expected = expected; + this.input = input; + } + } + + @TestFactory + Collection createResource() { + return Stream.of( + new TestCase( + "name ok", + SpringBootBuildInfoServiceNameDetector::new, + SERVICE_NAME, + "some-name", + openClasspathResource(META_INFO + "/" + BUILD_PROPS)), + new TestCase( + "version ok", + SpringBootServiceVersionDetector::new, + SERVICE_VERSION, + "0.0.2", + openClasspathResource(META_INFO + "/" + BUILD_PROPS)), + new TestCase( + "name - no resource", + SpringBootBuildInfoServiceNameDetector::new, + SERVICE_NAME, + null, + null), + new TestCase( + "version - no resource", + SpringBootServiceVersionDetector::new, + SERVICE_VERSION, + null, + null), + new TestCase( + "name - empty resource", + SpringBootBuildInfoServiceNameDetector::new, + SERVICE_NAME, + null, + openClasspathResource(BUILD_PROPS)), + new TestCase( + "version - empty resource", + SpringBootServiceVersionDetector::new, + SERVICE_VERSION, + null, + openClasspathResource(BUILD_PROPS))) + .map( + t -> + DynamicTest.dynamicTest( + t.name, + () -> { + when(system.openClasspathResource(META_INFO, BUILD_PROPS)) + .thenReturn(t.input); + + assertThat(t.factory.apply(system).createResource(config).getAttribute(t.key)) + .isEqualTo(t.expected); + })) + .collect(Collectors.toList()); + } + + private static InputStream openClasspathResource(String resource) { + return ManifestResourceProviderTest.class.getClassLoader().getResourceAsStream(resource); + } +} From 62a9953f96ae848d15c754447b34e7b9fa770aed Mon Sep 17 00:00:00 2001 From: Gregor Zeitlinger Date: Wed, 21 Feb 2024 13:44:30 +0100 Subject: [PATCH 02/14] add ManifestResourceProvider --- .../resources/ManifestResourceProvider.java | 32 +++++----- .../resources/SystemHelper.java | 18 +++--- .../ManifestResourceProviderTest.java | 63 +++++++------------ .../library/src/test/resources/MANIFEST.MF | 3 + .../src/test/resources/empty-MANIFEST.MF | 1 + .../library/build.gradle.kts | 1 + .../SpringBootServiceNameDetector.java | 1 + .../SpringBootServiceVersionDetector.java | 1 + 8 files changed, 55 insertions(+), 65 deletions(-) rename instrumentation/{spring/spring-boot-resources/library/src/main/java/io/opentelemetry/instrumentation/spring => resources/library/src/main/java/io/opentelemetry/instrumentation}/resources/SystemHelper.java (80%) create mode 100644 instrumentation/resources/library/src/test/resources/MANIFEST.MF create mode 100644 instrumentation/resources/library/src/test/resources/empty-MANIFEST.MF diff --git a/instrumentation/resources/library/src/main/java/io/opentelemetry/instrumentation/resources/ManifestResourceProvider.java b/instrumentation/resources/library/src/main/java/io/opentelemetry/instrumentation/resources/ManifestResourceProvider.java index 7e7502cb11f8..86395e5570bd 100644 --- a/instrumentation/resources/library/src/main/java/io/opentelemetry/instrumentation/resources/ManifestResourceProvider.java +++ b/instrumentation/resources/library/src/main/java/io/opentelemetry/instrumentation/resources/ManifestResourceProvider.java @@ -11,7 +11,6 @@ import io.opentelemetry.sdk.autoconfigure.spi.ResourceProvider; import io.opentelemetry.semconv.ResourceAttributes; import java.io.IOException; -import java.io.InputStream; import java.util.Optional; import java.util.jar.Manifest; import java.util.logging.Logger; @@ -26,23 +25,28 @@ public final class ManifestResourceProvider extends AttributeResourceProvider() { @Override public Optional readData() { - try { - Manifest manifest = new Manifest(); - InputStream systemResourceAsStream = - ClassLoader.getSystemResourceAsStream("META-INF/MANIFEST.MF"); - if (systemResourceAsStream == null) { - return Optional.empty(); - } - manifest.read(systemResourceAsStream); - return Optional.of(manifest); - } catch (IOException e) { - logger.log(WARNING, "Error reading manifest", e); - return Optional.empty(); - } + return Optional.ofNullable( + systemHelper.openClasspathResource("META-INF", "MANIFEST.MF")) + .flatMap( + s -> { + try { + Manifest manifest = new Manifest(); + manifest.read(s); + return Optional.of(manifest); + } catch (IOException e) { + logger.log(WARNING, "Error reading manifest", e); + return Optional.empty(); + } + }); } @Override diff --git a/instrumentation/spring/spring-boot-resources/library/src/main/java/io/opentelemetry/instrumentation/spring/resources/SystemHelper.java b/instrumentation/resources/library/src/main/java/io/opentelemetry/instrumentation/resources/SystemHelper.java similarity index 80% rename from instrumentation/spring/spring-boot-resources/library/src/main/java/io/opentelemetry/instrumentation/spring/resources/SystemHelper.java rename to instrumentation/resources/library/src/main/java/io/opentelemetry/instrumentation/resources/SystemHelper.java index 59606a62c296..97ead727f9d5 100644 --- a/instrumentation/spring/spring-boot-resources/library/src/main/java/io/opentelemetry/instrumentation/spring/resources/SystemHelper.java +++ b/instrumentation/resources/library/src/main/java/io/opentelemetry/instrumentation/resources/SystemHelper.java @@ -3,7 +3,7 @@ * SPDX-License-Identifier: Apache-2.0 */ -package io.opentelemetry.instrumentation.spring.resources; +package io.opentelemetry.instrumentation.resources; import java.io.InputStream; import java.lang.reflect.Method; @@ -13,13 +13,13 @@ import java.util.logging.Level; import java.util.logging.Logger; -class SystemHelper { +public class SystemHelper { private static final Logger logger = Logger.getLogger(SystemHelper.class.getName()); private final ClassLoader classLoader; private final boolean addBootInfPrefix; - SystemHelper() { + public SystemHelper() { ClassLoader contextClassLoader = Thread.currentThread().getContextClassLoader(); classLoader = contextClassLoader != null ? contextClassLoader : ClassLoader.getSystemClassLoader(); @@ -29,25 +29,25 @@ class SystemHelper { } } - String getenv(String name) { + public String getenv(String name) { return System.getenv(name); } - String getProperty(String key) { + public String getProperty(String key) { return System.getProperty(key); } - InputStream openClasspathResource(String filename) { + public InputStream openClasspathResource(String filename) { String path = addBootInfPrefix ? "BOOT-INF/classes/" + filename : filename; return classLoader.getResourceAsStream(path); } - InputStream openClasspathResource(String directory, String filename) { + public InputStream openClasspathResource(String directory, String filename) { String path = directory + "/" + filename; return classLoader.getResourceAsStream(path); } - InputStream openFile(String filename) throws Exception { + public InputStream openFile(String filename) throws Exception { return Files.newInputStream(Paths.get(filename)); } @@ -56,7 +56,7 @@ InputStream openFile(String filename) throws Exception { * main method arguments). Will only succeed on java 9+. */ @SuppressWarnings("unchecked") - String[] attemptGetCommandLineArgsViaReflection() throws Exception { + public String[] attemptGetCommandLineArgsViaReflection() throws Exception { Class clazz = Class.forName("java.lang.ProcessHandle"); Method currentMethod = clazz.getDeclaredMethod("current"); Method infoMethod = clazz.getDeclaredMethod("info"); diff --git a/instrumentation/resources/library/src/test/java/io/opentelemetry/instrumentation/resources/ManifestResourceProviderTest.java b/instrumentation/resources/library/src/test/java/io/opentelemetry/instrumentation/resources/ManifestResourceProviderTest.java index 30fecc319331..93ebbac23239 100644 --- a/instrumentation/resources/library/src/test/java/io/opentelemetry/instrumentation/resources/ManifestResourceProviderTest.java +++ b/instrumentation/resources/library/src/test/java/io/opentelemetry/instrumentation/resources/ManifestResourceProviderTest.java @@ -10,11 +10,10 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.Mockito.when; -import io.opentelemetry.api.common.AttributeKey; import io.opentelemetry.sdk.autoconfigure.spi.ConfigProperties; +import io.opentelemetry.sdk.resources.Resource; import java.io.InputStream; import java.util.Collection; -import java.util.function.Function; import java.util.stream.Collectors; import java.util.stream.Stream; import org.junit.jupiter.api.DynamicTest; @@ -26,7 +25,7 @@ @ExtendWith(MockitoExtension.class) class ManifestResourceProviderTest { - static final String BUILD_PROPS = "build-info.properties"; + static final String MANIFEST_MF = "MANIFEST.MF"; static final String META_INFO = "META-INF"; @Mock ConfigProperties config; @@ -34,22 +33,19 @@ class ManifestResourceProviderTest { private static class TestCase { private final String name; - private final Function factory; - private final AttributeKey key; - private final String expected; - private final InputStream input; + private final String expectedName; + private final String expectedVersion; + private final InputStream input; public TestCase( String name, - Function factory, - AttributeKey key, - String expected, + String expectedName, + String expectedVersion, InputStream input) { this.name = name; - this.factory = factory; - this.key = key; - this.expected = expected; - this.input = input; + this.expectedName = expectedName; + this.expectedVersion = expectedVersion; + this.input = input; } } @@ -58,50 +54,33 @@ Collection createResource() { return Stream.of( new TestCase( "name ok", - SpringBootBuildInfoServiceNameDetector::new, - SERVICE_NAME, - "some-name", - openClasspathResource(META_INFO + "/" + BUILD_PROPS)), - new TestCase( - "version ok", - SpringBootServiceVersionDetector::new, - SERVICE_VERSION, - "0.0.2", - openClasspathResource(META_INFO + "/" + BUILD_PROPS)), + "demo", + "0.0.1-SNAPSHOT", + openClasspathResource(MANIFEST_MF)), new TestCase( "name - no resource", - SpringBootBuildInfoServiceNameDetector::new, - SERVICE_NAME, null, - null), - new TestCase( - "version - no resource", - SpringBootServiceVersionDetector::new, - SERVICE_VERSION, null, null), new TestCase( "name - empty resource", - SpringBootBuildInfoServiceNameDetector::new, - SERVICE_NAME, null, - openClasspathResource(BUILD_PROPS)), - new TestCase( - "version - empty resource", - SpringBootServiceVersionDetector::new, - SERVICE_VERSION, null, - openClasspathResource(BUILD_PROPS))) + openClasspathResource("empty-MANIFEST.MF"))) .map( t -> DynamicTest.dynamicTest( t.name, () -> { - when(system.openClasspathResource(META_INFO, BUILD_PROPS)) + when(system.openClasspathResource(META_INFO, MANIFEST_MF)) .thenReturn(t.input); - assertThat(t.factory.apply(system).createResource(config).getAttribute(t.key)) - .isEqualTo(t.expected); + ManifestResourceProvider provider = new ManifestResourceProvider(system); + provider.shouldApply(config, Resource.getDefault()); + + Resource resource = provider.createResource(config); + assertThat(resource.getAttribute(SERVICE_NAME)).isEqualTo(t.expectedName); + assertThat(resource.getAttribute(SERVICE_VERSION)).isEqualTo(t.expectedVersion); })) .collect(Collectors.toList()); } diff --git a/instrumentation/resources/library/src/test/resources/MANIFEST.MF b/instrumentation/resources/library/src/test/resources/MANIFEST.MF new file mode 100644 index 000000000000..28f63e3a3517 --- /dev/null +++ b/instrumentation/resources/library/src/test/resources/MANIFEST.MF @@ -0,0 +1,3 @@ +Manifest-Version: 1.0 +Implementation-Title: demo +Implementation-Version: 0.0.1-SNAPSHOT diff --git a/instrumentation/resources/library/src/test/resources/empty-MANIFEST.MF b/instrumentation/resources/library/src/test/resources/empty-MANIFEST.MF new file mode 100644 index 000000000000..9d885be53412 --- /dev/null +++ b/instrumentation/resources/library/src/test/resources/empty-MANIFEST.MF @@ -0,0 +1 @@ +Manifest-Version: 1.0 diff --git a/instrumentation/spring/spring-boot-resources/library/build.gradle.kts b/instrumentation/spring/spring-boot-resources/library/build.gradle.kts index 6417f7c627c7..bce88a4f3f95 100644 --- a/instrumentation/spring/spring-boot-resources/library/build.gradle.kts +++ b/instrumentation/spring/spring-boot-resources/library/build.gradle.kts @@ -10,6 +10,7 @@ dependencies { testCompileOnly("com.google.auto.service:auto-service-annotations") implementation("org.snakeyaml:snakeyaml-engine") + implementation(project(":instrumentation:resources:library")) testImplementation("io.opentelemetry:opentelemetry-sdk-extension-autoconfigure-spi") } 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 6ec01260aeaa..5349155ef8c6 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 @@ -9,6 +9,7 @@ import static java.util.logging.Level.FINER; import com.google.auto.service.AutoService; +import io.opentelemetry.instrumentation.resources.SystemHelper; import io.opentelemetry.sdk.autoconfigure.spi.ConfigProperties; import io.opentelemetry.sdk.autoconfigure.spi.ResourceProvider; import io.opentelemetry.sdk.autoconfigure.spi.internal.ConditionalResourceProvider; diff --git a/instrumentation/spring/spring-boot-resources/library/src/main/java/io/opentelemetry/instrumentation/spring/resources/SpringBootServiceVersionDetector.java b/instrumentation/spring/spring-boot-resources/library/src/main/java/io/opentelemetry/instrumentation/spring/resources/SpringBootServiceVersionDetector.java index b858e28e8acd..181c3f20bccc 100644 --- a/instrumentation/spring/spring-boot-resources/library/src/main/java/io/opentelemetry/instrumentation/spring/resources/SpringBootServiceVersionDetector.java +++ b/instrumentation/spring/spring-boot-resources/library/src/main/java/io/opentelemetry/instrumentation/spring/resources/SpringBootServiceVersionDetector.java @@ -8,6 +8,7 @@ import static java.util.logging.Level.FINE; import com.google.auto.service.AutoService; +import io.opentelemetry.instrumentation.resources.SystemHelper; import io.opentelemetry.sdk.autoconfigure.spi.ConfigProperties; import io.opentelemetry.sdk.autoconfigure.spi.ResourceProvider; import io.opentelemetry.sdk.resources.Resource; From 640789115dbba79867964f3b0a346f8b8b952957 Mon Sep 17 00:00:00 2001 From: Gregor Zeitlinger Date: Wed, 21 Feb 2024 13:47:20 +0100 Subject: [PATCH 03/14] add ManifestResourceProvider --- .../resources/AttributeProvider.java | 6 +++ .../resources/AttributeResourceProvider.java | 6 +++ .../ManifestResourceProviderTest.java | 38 ++++++------------- 3 files changed, 24 insertions(+), 26 deletions(-) diff --git a/instrumentation/resources/library/src/main/java/io/opentelemetry/instrumentation/resources/AttributeProvider.java b/instrumentation/resources/library/src/main/java/io/opentelemetry/instrumentation/resources/AttributeProvider.java index ab1370cd1cb6..33abb6435350 100644 --- a/instrumentation/resources/library/src/main/java/io/opentelemetry/instrumentation/resources/AttributeProvider.java +++ b/instrumentation/resources/library/src/main/java/io/opentelemetry/instrumentation/resources/AttributeProvider.java @@ -10,6 +10,12 @@ import java.util.Optional; import java.util.function.Function; +/** + * An easier alternative to {@link io.opentelemetry.sdk.autoconfigure.spi.ResourceProvider}, which + * avoids some common pitfalls and boilerplate. + * + *

An example of how to use this interface can be found in {@link ManifestResourceProvider}. + */ public interface AttributeProvider { Optional readData(); diff --git a/instrumentation/resources/library/src/main/java/io/opentelemetry/instrumentation/resources/AttributeResourceProvider.java b/instrumentation/resources/library/src/main/java/io/opentelemetry/instrumentation/resources/AttributeResourceProvider.java index 311304612517..e8ab69d2d3c5 100644 --- a/instrumentation/resources/library/src/main/java/io/opentelemetry/instrumentation/resources/AttributeResourceProvider.java +++ b/instrumentation/resources/library/src/main/java/io/opentelemetry/instrumentation/resources/AttributeResourceProvider.java @@ -19,6 +19,12 @@ import java.util.Optional; import java.util.function.Function; +/** + * An easier alternative to {@link io.opentelemetry.sdk.autoconfigure.spi.ResourceProvider}, which + * avoids some common pitfalls and boilerplate. + * + *

An example of how to use this interface can be found in {@link ManifestResourceProvider}. + */ @SuppressWarnings({"unchecked", "rawtypes"}) public abstract class AttributeResourceProvider implements ConditionalResourceProvider { diff --git a/instrumentation/resources/library/src/test/java/io/opentelemetry/instrumentation/resources/ManifestResourceProviderTest.java b/instrumentation/resources/library/src/test/java/io/opentelemetry/instrumentation/resources/ManifestResourceProviderTest.java index 93ebbac23239..9aad4c2d1c96 100644 --- a/instrumentation/resources/library/src/test/java/io/opentelemetry/instrumentation/resources/ManifestResourceProviderTest.java +++ b/instrumentation/resources/library/src/test/java/io/opentelemetry/instrumentation/resources/ManifestResourceProviderTest.java @@ -33,40 +33,25 @@ class ManifestResourceProviderTest { private static class TestCase { private final String name; - private final String expectedName; - private final String expectedVersion; - private final InputStream input; + private final String expectedName; + private final String expectedVersion; + private final InputStream input; - public TestCase( - String name, - String expectedName, - String expectedVersion, - InputStream input) { + public TestCase(String name, String expectedName, String expectedVersion, InputStream input) { this.name = name; - this.expectedName = expectedName; - this.expectedVersion = expectedVersion; - this.input = input; + this.expectedName = expectedName; + this.expectedVersion = expectedVersion; + this.input = input; } } @TestFactory Collection createResource() { return Stream.of( + new TestCase("name ok", "demo", "0.0.1-SNAPSHOT", openClasspathResource(MANIFEST_MF)), + new TestCase("name - no resource", null, null, null), new TestCase( - "name ok", - "demo", - "0.0.1-SNAPSHOT", - openClasspathResource(MANIFEST_MF)), - new TestCase( - "name - no resource", - null, - null, - null), - new TestCase( - "name - empty resource", - null, - null, - openClasspathResource("empty-MANIFEST.MF"))) + "name - empty resource", null, null, openClasspathResource("empty-MANIFEST.MF"))) .map( t -> DynamicTest.dynamicTest( @@ -80,7 +65,8 @@ Collection createResource() { Resource resource = provider.createResource(config); assertThat(resource.getAttribute(SERVICE_NAME)).isEqualTo(t.expectedName); - assertThat(resource.getAttribute(SERVICE_VERSION)).isEqualTo(t.expectedVersion); + assertThat(resource.getAttribute(SERVICE_VERSION)) + .isEqualTo(t.expectedVersion); })) .collect(Collectors.toList()); } From c3044fd54db5900949ce2d4a03508c729d80738c Mon Sep 17 00:00:00 2001 From: Gregor Zeitlinger Date: Wed, 21 Feb 2024 13:55:05 +0100 Subject: [PATCH 04/14] add ManifestResourceProvider --- .../instrumentation/resources/ManifestResourceProvider.java | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/instrumentation/resources/library/src/main/java/io/opentelemetry/instrumentation/resources/ManifestResourceProvider.java b/instrumentation/resources/library/src/main/java/io/opentelemetry/instrumentation/resources/ManifestResourceProvider.java index 86395e5570bd..935f4af70464 100644 --- a/instrumentation/resources/library/src/main/java/io/opentelemetry/instrumentation/resources/ManifestResourceProvider.java +++ b/instrumentation/resources/library/src/main/java/io/opentelemetry/instrumentation/resources/ManifestResourceProvider.java @@ -69,4 +69,10 @@ public void registerAttributes(Builder builder) { } }); } + + @Override + public int order() { + // make it run later than SpringBootServiceNameDetector + return 300; + } } From 1d1eab93065ebf9a4b882fce6f9d2fe66325b3d4 Mon Sep 17 00:00:00 2001 From: Gregor Zeitlinger Date: Wed, 21 Feb 2024 14:25:03 +0100 Subject: [PATCH 05/14] add ManifestResourceProvider --- .../spring/resources/SpringBootServiceNameDetectorTest.java | 1 + .../spring/resources/SpringBootServiceVersionDetectorTest.java | 1 + 2 files changed, 2 insertions(+) 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 935cfab9a998..6b747577f627 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 @@ -12,6 +12,7 @@ import static org.mockito.Mockito.when; import io.opentelemetry.api.common.Attributes; +import io.opentelemetry.instrumentation.resources.SystemHelper; import io.opentelemetry.sdk.autoconfigure.spi.ConfigProperties; import io.opentelemetry.sdk.resources.Resource; import java.io.InputStream; diff --git a/instrumentation/spring/spring-boot-resources/library/src/test/java/io/opentelemetry/instrumentation/spring/resources/SpringBootServiceVersionDetectorTest.java b/instrumentation/spring/spring-boot-resources/library/src/test/java/io/opentelemetry/instrumentation/spring/resources/SpringBootServiceVersionDetectorTest.java index 9bce16b4fcd2..640682a46778 100644 --- a/instrumentation/spring/spring-boot-resources/library/src/test/java/io/opentelemetry/instrumentation/spring/resources/SpringBootServiceVersionDetectorTest.java +++ b/instrumentation/spring/spring-boot-resources/library/src/test/java/io/opentelemetry/instrumentation/spring/resources/SpringBootServiceVersionDetectorTest.java @@ -9,6 +9,7 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.Mockito.when; +import io.opentelemetry.instrumentation.resources.SystemHelper; import io.opentelemetry.sdk.autoconfigure.spi.ConfigProperties; import io.opentelemetry.sdk.resources.Resource; import java.io.InputStream; From b518ccaf819e02178b4c6e857b1601a30d5bb5d3 Mon Sep 17 00:00:00 2001 From: Gregor Zeitlinger Date: Fri, 23 Feb 2024 12:07:34 +0100 Subject: [PATCH 06/14] read the manifest only from application jar files --- .../resources/JarFileDetector.java | 126 ++++++++++++++++++ .../resources/JarServiceNameDetector.java | 73 +--------- .../resources/ManifestResourceProvider.java | 25 +--- .../resources/JarServiceNameDetectorTest.java | 36 +++-- .../ManifestResourceProviderTest.java | 38 +++--- 5 files changed, 180 insertions(+), 118 deletions(-) create mode 100644 instrumentation/resources/library/src/main/java/io/opentelemetry/instrumentation/resources/JarFileDetector.java diff --git a/instrumentation/resources/library/src/main/java/io/opentelemetry/instrumentation/resources/JarFileDetector.java b/instrumentation/resources/library/src/main/java/io/opentelemetry/instrumentation/resources/JarFileDetector.java new file mode 100644 index 000000000000..3fd8d6791896 --- /dev/null +++ b/instrumentation/resources/library/src/main/java/io/opentelemetry/instrumentation/resources/JarFileDetector.java @@ -0,0 +1,126 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.instrumentation.resources; + +import static java.util.logging.Level.WARNING; + +import java.io.InputStream; +import java.net.URL; +import java.nio.file.Files; +import java.nio.file.InvalidPathException; +import java.nio.file.Path; +import java.nio.file.Paths; +import java.util.Optional; +import java.util.function.Function; +import java.util.function.Predicate; +import java.util.function.Supplier; +import java.util.jar.Manifest; +import java.util.logging.Logger; +import javax.annotation.Nullable; + +class JarFileDetector { + private final Supplier getProcessHandleArguments; + private final Function getSystemProperty; + private final Predicate fileExists; + private final Function> manifestReader; + + private static final Logger logger = Logger.getLogger(JarFileDetector.class.getName()); + + public JarFileDetector() { + this( + ProcessArguments::getProcessArguments, + System::getProperty, + Files::isRegularFile, + JarFileDetector::readManifest); + } + + // visible for tests + JarFileDetector( + Supplier getProcessHandleArguments, + Function getSystemProperty, + Predicate fileExists, + Function> manifestReader) { + this.getProcessHandleArguments = getProcessHandleArguments; + this.getSystemProperty = getSystemProperty; + this.fileExists = fileExists; + this.manifestReader = manifestReader; + } + + @Nullable + Path getJarPath() { + Path jarPath = getJarPathFromProcessHandle(); + if (jarPath != null) { + return jarPath; + } + return getJarPathFromSunCommandLine(); + } + + Optional getManifestFromJarFile() { + Path jarPath = getJarPath(); + if (jarPath == null) { + return Optional.empty(); + } + return manifestReader.apply(jarPath); + } + + private static Optional readManifest(Path jarPath) { + try (InputStream s = + new URL(String.format("jar:%s!/META-INF/MANIFEST.MF", jarPath.toUri())).openStream()) { + Manifest manifest = new Manifest(); + manifest.read(s); + return Optional.of(manifest); + } catch (Exception e) { + logger.log(WARNING, "Error reading manifest", e); + return Optional.empty(); + } + } + + @Nullable + private Path getJarPathFromProcessHandle() { + String[] javaArgs = getProcessHandleArguments.get(); + for (int i = 0; i < javaArgs.length; ++i) { + if ("-jar".equals(javaArgs[i]) && (i < javaArgs.length - 1)) { + return Paths.get(javaArgs[i + 1]); + } + } + return null; + } + + @Nullable + private Path getJarPathFromSunCommandLine() { + // the jar file is the first argument in the command line string + String programArguments = getSystemProperty.apply("sun.java.command"); + if (programArguments == null) { + return null; + } + + // Take the path until the first space. If the path doesn't exist extend it up to the next + // space. Repeat until a path that exists is found or input runs out. + int next = 0; + while (true) { + int nextSpace = programArguments.indexOf(' ', next); + if (nextSpace == -1) { + return pathIfExists(programArguments); + } + Path path = pathIfExists(programArguments.substring(0, nextSpace)); + next = nextSpace + 1; + if (path != null) { + return path; + } + } + } + + @Nullable + private Path pathIfExists(String programArguments) { + Path candidate; + try { + candidate = Paths.get(programArguments); + } catch (InvalidPathException e) { + return null; + } + return fileExists.test(candidate) ? candidate : null; + } +} diff --git a/instrumentation/resources/library/src/main/java/io/opentelemetry/instrumentation/resources/JarServiceNameDetector.java b/instrumentation/resources/library/src/main/java/io/opentelemetry/instrumentation/resources/JarServiceNameDetector.java index f77b7e8abd46..f1eeb344f932 100644 --- a/instrumentation/resources/library/src/main/java/io/opentelemetry/instrumentation/resources/JarServiceNameDetector.java +++ b/instrumentation/resources/library/src/main/java/io/opentelemetry/instrumentation/resources/JarServiceNameDetector.java @@ -14,16 +14,9 @@ import io.opentelemetry.sdk.autoconfigure.spi.internal.ConditionalResourceProvider; import io.opentelemetry.sdk.resources.Resource; import io.opentelemetry.semconv.ResourceAttributes; -import java.nio.file.Files; -import java.nio.file.InvalidPathException; import java.nio.file.Path; -import java.nio.file.Paths; import java.util.Map; -import java.util.function.Function; -import java.util.function.Predicate; -import java.util.function.Supplier; import java.util.logging.Logger; -import javax.annotation.Nullable; /** * A {@link ResourceProvider} that will attempt to detect the application name from the jar name. @@ -33,31 +26,21 @@ public final class JarServiceNameDetector implements ConditionalResourceProvider private static final Logger logger = Logger.getLogger(JarServiceNameDetector.class.getName()); - private final Supplier getProcessHandleArguments; - private final Function getSystemProperty; - private final Predicate fileExists; + private final JarFileDetector jarFileDetector; @SuppressWarnings("unused") // SPI public JarServiceNameDetector() { - this(ProcessArguments::getProcessArguments, System::getProperty, Files::isRegularFile); + this(new JarFileDetector()); } // visible for tests - JarServiceNameDetector( - Supplier getProcessHandleArguments, - Function getSystemProperty, - Predicate fileExists) { - this.getProcessHandleArguments = getProcessHandleArguments; - this.getSystemProperty = getSystemProperty; - this.fileExists = fileExists; + JarServiceNameDetector(JarFileDetector jarFileDetector) { + this.jarFileDetector = jarFileDetector; } @Override public Resource createResource(ConfigProperties config) { - Path jarPath = getJarPathFromProcessHandle(); - if (jarPath == null) { - jarPath = getJarPathFromSunCommandLine(); - } + Path jarPath = jarFileDetector.getJarPath(); if (jarPath == null) { return Resource.empty(); } @@ -75,52 +58,6 @@ public boolean shouldApply(ConfigProperties config, Resource existing) { && "unknown_service:java".equals(existing.getAttribute(ResourceAttributes.SERVICE_NAME)); } - @Nullable - private Path getJarPathFromProcessHandle() { - String[] javaArgs = getProcessHandleArguments.get(); - for (int i = 0; i < javaArgs.length; ++i) { - if ("-jar".equals(javaArgs[i]) && (i < javaArgs.length - 1)) { - return Paths.get(javaArgs[i + 1]); - } - } - return null; - } - - @Nullable - private Path getJarPathFromSunCommandLine() { - // the jar file is the first argument in the command line string - String programArguments = getSystemProperty.apply("sun.java.command"); - if (programArguments == null) { - return null; - } - - // Take the path until the first space. If the path doesn't exist extend it up to the next - // space. Repeat until a path that exists is found or input runs out. - int next = 0; - while (true) { - int nextSpace = programArguments.indexOf(' ', next); - if (nextSpace == -1) { - return pathIfExists(programArguments); - } - Path path = pathIfExists(programArguments.substring(0, nextSpace)); - next = nextSpace + 1; - if (path != null) { - return path; - } - } - } - - @Nullable - private Path pathIfExists(String programArguments) { - Path candidate; - try { - candidate = Paths.get(programArguments); - } catch (InvalidPathException e) { - return null; - } - return fileExists.test(candidate) ? candidate : null; - } - private static String getServiceName(Path jarPath) { String jarName = jarPath.getFileName().toString(); int dotIndex = jarName.lastIndexOf("."); diff --git a/instrumentation/resources/library/src/main/java/io/opentelemetry/instrumentation/resources/ManifestResourceProvider.java b/instrumentation/resources/library/src/main/java/io/opentelemetry/instrumentation/resources/ManifestResourceProvider.java index 935f4af70464..e818a4fa6d14 100644 --- a/instrumentation/resources/library/src/main/java/io/opentelemetry/instrumentation/resources/ManifestResourceProvider.java +++ b/instrumentation/resources/library/src/main/java/io/opentelemetry/instrumentation/resources/ManifestResourceProvider.java @@ -5,15 +5,11 @@ package io.opentelemetry.instrumentation.resources; -import static java.util.logging.Level.WARNING; - import com.google.auto.service.AutoService; import io.opentelemetry.sdk.autoconfigure.spi.ResourceProvider; import io.opentelemetry.semconv.ResourceAttributes; -import java.io.IOException; import java.util.Optional; import java.util.jar.Manifest; -import java.util.logging.Logger; /** * A {@link ResourceProvider} that will attempt to detect the service.name and @@ -22,31 +18,18 @@ @AutoService(ResourceProvider.class) public final class ManifestResourceProvider extends AttributeResourceProvider { - private static final Logger logger = Logger.getLogger(ManifestResourceProvider.class.getName()); - + @SuppressWarnings("unused") // SPI public ManifestResourceProvider() { - this(new SystemHelper()); + this(new JarFileDetector()); } // Visible for testing - ManifestResourceProvider(SystemHelper systemHelper) { + ManifestResourceProvider(JarFileDetector jarFileDetector) { super( new AttributeProvider() { @Override public Optional readData() { - return Optional.ofNullable( - systemHelper.openClasspathResource("META-INF", "MANIFEST.MF")) - .flatMap( - s -> { - try { - Manifest manifest = new Manifest(); - manifest.read(s); - return Optional.of(manifest); - } catch (IOException e) { - logger.log(WARNING, "Error reading manifest", e); - return Optional.empty(); - } - }); + return jarFileDetector.getManifestFromJarFile(); } @Override diff --git a/instrumentation/resources/library/src/test/java/io/opentelemetry/instrumentation/resources/JarServiceNameDetectorTest.java b/instrumentation/resources/library/src/test/java/io/opentelemetry/instrumentation/resources/JarServiceNameDetectorTest.java index d9b6e8b9afe0..704551e15667 100644 --- a/instrumentation/resources/library/src/test/java/io/opentelemetry/instrumentation/resources/JarServiceNameDetectorTest.java +++ b/instrumentation/resources/library/src/test/java/io/opentelemetry/instrumentation/resources/JarServiceNameDetectorTest.java @@ -13,6 +13,7 @@ import io.opentelemetry.semconv.ResourceAttributes; import java.nio.file.Path; import java.nio.file.Paths; +import java.util.Optional; import java.util.function.Function; import java.util.function.Predicate; import java.util.stream.Stream; @@ -27,26 +28,34 @@ import org.mockito.junit.jupiter.MockitoExtension; @ExtendWith(MockitoExtension.class) +// todo split JarFileDetectorTest and JarServiceNameDetectorTest class JarServiceNameDetectorTest { @Mock ConfigProperties config; @Test void createResource_empty() { - JarServiceNameDetector serviceNameProvider = - new JarServiceNameDetector( - () -> new String[0], prop -> null, JarServiceNameDetectorTest::failPath); + String[] processArgs = new String[0]; + Function getProperty = prop -> null; + Predicate fileExists = JarServiceNameDetectorTest::failPath; + JarServiceNameDetector serviceNameProvider = getDetector(processArgs, getProperty, fileExists); Resource resource = serviceNameProvider.createResource(config); assertThat(resource.getAttributes()).isEmpty(); } + private static JarServiceNameDetector getDetector( + String[] processArgs, Function getProperty, Predicate fileExists) { + return new JarServiceNameDetector( + new JarFileDetector(() -> processArgs, getProperty, fileExists, p -> Optional.empty())); + } + @Test void createResource_noJarFileInArgs() { String[] args = new String[] {"-Dtest=42", "-Xmx666m", "-jar"}; JarServiceNameDetector serviceNameProvider = - new JarServiceNameDetector(() -> args, prop -> null, JarServiceNameDetectorTest::failPath); + getDetector(args, prop -> null, JarServiceNameDetectorTest::failPath); Resource resource = serviceNameProvider.createResource(config); @@ -55,10 +64,8 @@ void createResource_noJarFileInArgs() { @Test void createResource_processHandleJar() { - String path = Paths.get("path", "to", "app", "my-service.jar").toString(); - String[] args = new String[] {"-Dtest=42", "-Xmx666m", "-jar", path, "abc", "def"}; JarServiceNameDetector serviceNameProvider = - new JarServiceNameDetector(() -> args, prop -> null, JarServiceNameDetectorTest::failPath); + getDetector(getArgs("my-service.jar"), prop -> null, JarServiceNameDetectorTest::failPath); Resource resource = serviceNameProvider.createResource(config); @@ -69,10 +76,8 @@ void createResource_processHandleJar() { @Test void createResource_processHandleJarWithoutExtension() { - String path = Paths.get("path", "to", "app", "my-service.jar").toString(); - String[] args = new String[] {"-Dtest=42", "-Xmx666m", "-jar", path}; JarServiceNameDetector serviceNameProvider = - new JarServiceNameDetector(() -> args, prop -> null, JarServiceNameDetectorTest::failPath); + getDetector(getArgs("my-service"), prop -> null, JarServiceNameDetectorTest::failPath); Resource resource = serviceNameProvider.createResource(config); @@ -81,6 +86,11 @@ void createResource_processHandleJarWithoutExtension() { .containsEntry(ResourceAttributes.SERVICE_NAME, "my-service"); } + static String[] getArgs(String jarName) { + String path = Paths.get("path", "to", "app", jarName).toString(); + return new String[] {"-Dtest=42", "-Xmx666m", "-jar", path, "abc", "def"}; + } + @ParameterizedTest @ArgumentsSource(SunCommandLineProvider.class) void createResource_sunCommandLine(String commandLine, Path jarPath) { @@ -89,7 +99,7 @@ void createResource_sunCommandLine(String commandLine, Path jarPath) { Predicate fileExists = jarPath::equals; JarServiceNameDetector serviceNameProvider = - new JarServiceNameDetector(() -> new String[0], getProperty, fileExists); + getDetector(new String[0], getProperty, fileExists); Resource resource = serviceNameProvider.createResource(config); @@ -107,7 +117,7 @@ void createResource_sunCommandLineProblematicArgs() { Predicate fileExists = path -> false; JarServiceNameDetector serviceNameProvider = - new JarServiceNameDetector(() -> new String[0], getProperty, fileExists); + getDetector(new String[0], getProperty, fileExists); Resource resource = serviceNameProvider.createResource(config); @@ -128,7 +138,7 @@ public Stream provideArguments(ExtensionContext context) { } } - private static boolean failPath(Path file) { + static boolean failPath(Path file) { throw new AssertionError("Unexpected call to Files.isRegularFile()"); } } diff --git a/instrumentation/resources/library/src/test/java/io/opentelemetry/instrumentation/resources/ManifestResourceProviderTest.java b/instrumentation/resources/library/src/test/java/io/opentelemetry/instrumentation/resources/ManifestResourceProviderTest.java index 9aad4c2d1c96..a1945571f14b 100644 --- a/instrumentation/resources/library/src/test/java/io/opentelemetry/instrumentation/resources/ManifestResourceProviderTest.java +++ b/instrumentation/resources/library/src/test/java/io/opentelemetry/instrumentation/resources/ManifestResourceProviderTest.java @@ -8,29 +8,22 @@ import static io.opentelemetry.semconv.ResourceAttributes.SERVICE_NAME; import static io.opentelemetry.semconv.ResourceAttributes.SERVICE_VERSION; import static org.assertj.core.api.Assertions.assertThat; -import static org.mockito.Mockito.when; import io.opentelemetry.sdk.autoconfigure.spi.ConfigProperties; +import io.opentelemetry.sdk.autoconfigure.spi.internal.DefaultConfigProperties; import io.opentelemetry.sdk.resources.Resource; import java.io.InputStream; import java.util.Collection; +import java.util.Collections; +import java.util.Optional; +import java.util.jar.Manifest; import java.util.stream.Collectors; import java.util.stream.Stream; import org.junit.jupiter.api.DynamicTest; import org.junit.jupiter.api.TestFactory; -import org.junit.jupiter.api.extension.ExtendWith; -import org.mockito.Mock; -import org.mockito.junit.jupiter.MockitoExtension; -@ExtendWith(MockitoExtension.class) class ManifestResourceProviderTest { - static final String MANIFEST_MF = "MANIFEST.MF"; - static final String META_INFO = "META-INF"; - - @Mock ConfigProperties config; - @Mock SystemHelper system; - private static class TestCase { private final String name; private final String expectedName; @@ -47,8 +40,10 @@ public TestCase(String name, String expectedName, String expectedVersion, InputS @TestFactory Collection createResource() { + ConfigProperties config = DefaultConfigProperties.createFromMap(Collections.emptyMap()); + return Stream.of( - new TestCase("name ok", "demo", "0.0.1-SNAPSHOT", openClasspathResource(MANIFEST_MF)), + new TestCase("name ok", "demo", "0.0.1-SNAPSHOT", openClasspathResource("MANIFEST.MF")), new TestCase("name - no resource", null, null, null), new TestCase( "name - empty resource", null, null, openClasspathResource("empty-MANIFEST.MF"))) @@ -57,10 +52,21 @@ Collection createResource() { DynamicTest.dynamicTest( t.name, () -> { - when(system.openClasspathResource(META_INFO, MANIFEST_MF)) - .thenReturn(t.input); - - ManifestResourceProvider provider = new ManifestResourceProvider(system); + ManifestResourceProvider provider = + new ManifestResourceProvider( + new JarFileDetector( + () -> JarServiceNameDetectorTest.getArgs("app.jar"), + prop -> null, + JarServiceNameDetectorTest::failPath, + p -> { + try { + Manifest manifest = new Manifest(); + manifest.read(t.input); + return Optional.of(manifest); + } catch (Exception e) { + return Optional.empty(); + } + })); provider.shouldApply(config, Resource.getDefault()); Resource resource = provider.createResource(config); From 5a47580b930f7a3d63826861b5efdff62980be4e Mon Sep 17 00:00:00 2001 From: Gregor Zeitlinger Date: Fri, 23 Feb 2024 15:36:30 +0100 Subject: [PATCH 07/14] read the manifest only from application jar files --- .../smoketest/QuarkusSmokeTest.groovy | 22 ++++++++++++------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/smoke-tests/src/test/groovy/io/opentelemetry/smoketest/QuarkusSmokeTest.groovy b/smoke-tests/src/test/groovy/io/opentelemetry/smoketest/QuarkusSmokeTest.groovy index 3eb71b6e1b21..9279197eefb2 100644 --- a/smoke-tests/src/test/groovy/io/opentelemetry/smoketest/QuarkusSmokeTest.groovy +++ b/smoke-tests/src/test/groovy/io/opentelemetry/smoketest/QuarkusSmokeTest.groovy @@ -5,7 +5,7 @@ package io.opentelemetry.smoketest -import io.opentelemetry.proto.collector.trace.v1.ExportTraceServiceRequest +import io.opentelemetry.semconv.ResourceAttributes import spock.lang.IgnoreIf import spock.lang.Unroll @@ -14,7 +14,6 @@ import java.util.jar.Attributes import java.util.jar.JarFile import static io.opentelemetry.smoketest.TestContainerManager.useWindowsContainers -import static java.util.stream.Collectors.toSet @IgnoreIf({ useWindowsContainers() }) class QuarkusSmokeTest extends SmokeTest { @@ -28,6 +27,11 @@ class QuarkusSmokeTest extends SmokeTest { return new TargetWaitStrategy.Log(Duration.ofMinutes(1), ".*Listening on.*") } + @Override + protected boolean getSetServiceName() { + return false + } + @Unroll def "quarkus smoke test on JDK #jdk"(int jdk) { setup: @@ -37,14 +41,16 @@ class QuarkusSmokeTest extends SmokeTest { when: client().get("/hello").aggregate().join() - Collection traces = waitForTraces() + TraceInspector traces = new TraceInspector(waitForTraces()) + + then: "Expected span names" + traces.countSpansByName('GET /hello') == 1 - then: - countSpansByName(traces, 'GET /hello') == 1 + and: "telemetry.distro.version is set" + traces.countFilteredResourceAttributes("telemetry.distro.version", currentAgentVersion) == 1 - [currentAgentVersion] as Set == findResourceAttribute(traces, "telemetry.distro.version") - .map { it.stringValue } - .collect(toSet()) + and: "service.name is detected from manifest" + traces.countFilteredResourceAttributes(ResourceAttributes.SERVICE_NAME.key, "smoke-test-quarkus-images") == 1 cleanup: stopTarget() From edbbe265c7c194613dbb0104fb9956516d88ad99 Mon Sep 17 00:00:00 2001 From: Gregor Zeitlinger Date: Fri, 23 Feb 2024 15:40:17 +0100 Subject: [PATCH 08/14] system helper isn't needed for manifest provider --- .../library/build.gradle.kts | 1 - .../SpringBootServiceNameDetector.java | 1 - .../SpringBootServiceVersionDetector.java | 1 - .../spring}/resources/SystemHelper.java | 18 +++++++++--------- .../SpringBootServiceNameDetectorTest.java | 1 - .../SpringBootServiceVersionDetectorTest.java | 1 - 6 files changed, 9 insertions(+), 14 deletions(-) rename instrumentation/{resources/library/src/main/java/io/opentelemetry/instrumentation => spring/spring-boot-resources/library/src/main/java/io/opentelemetry/instrumentation/spring}/resources/SystemHelper.java (80%) diff --git a/instrumentation/spring/spring-boot-resources/library/build.gradle.kts b/instrumentation/spring/spring-boot-resources/library/build.gradle.kts index bce88a4f3f95..6417f7c627c7 100644 --- a/instrumentation/spring/spring-boot-resources/library/build.gradle.kts +++ b/instrumentation/spring/spring-boot-resources/library/build.gradle.kts @@ -10,7 +10,6 @@ dependencies { testCompileOnly("com.google.auto.service:auto-service-annotations") implementation("org.snakeyaml:snakeyaml-engine") - implementation(project(":instrumentation:resources:library")) testImplementation("io.opentelemetry:opentelemetry-sdk-extension-autoconfigure-spi") } 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 5349155ef8c6..6ec01260aeaa 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 @@ -9,7 +9,6 @@ import static java.util.logging.Level.FINER; import com.google.auto.service.AutoService; -import io.opentelemetry.instrumentation.resources.SystemHelper; import io.opentelemetry.sdk.autoconfigure.spi.ConfigProperties; import io.opentelemetry.sdk.autoconfigure.spi.ResourceProvider; import io.opentelemetry.sdk.autoconfigure.spi.internal.ConditionalResourceProvider; diff --git a/instrumentation/spring/spring-boot-resources/library/src/main/java/io/opentelemetry/instrumentation/spring/resources/SpringBootServiceVersionDetector.java b/instrumentation/spring/spring-boot-resources/library/src/main/java/io/opentelemetry/instrumentation/spring/resources/SpringBootServiceVersionDetector.java index 181c3f20bccc..b858e28e8acd 100644 --- a/instrumentation/spring/spring-boot-resources/library/src/main/java/io/opentelemetry/instrumentation/spring/resources/SpringBootServiceVersionDetector.java +++ b/instrumentation/spring/spring-boot-resources/library/src/main/java/io/opentelemetry/instrumentation/spring/resources/SpringBootServiceVersionDetector.java @@ -8,7 +8,6 @@ import static java.util.logging.Level.FINE; import com.google.auto.service.AutoService; -import io.opentelemetry.instrumentation.resources.SystemHelper; import io.opentelemetry.sdk.autoconfigure.spi.ConfigProperties; import io.opentelemetry.sdk.autoconfigure.spi.ResourceProvider; import io.opentelemetry.sdk.resources.Resource; diff --git a/instrumentation/resources/library/src/main/java/io/opentelemetry/instrumentation/resources/SystemHelper.java b/instrumentation/spring/spring-boot-resources/library/src/main/java/io/opentelemetry/instrumentation/spring/resources/SystemHelper.java similarity index 80% rename from instrumentation/resources/library/src/main/java/io/opentelemetry/instrumentation/resources/SystemHelper.java rename to instrumentation/spring/spring-boot-resources/library/src/main/java/io/opentelemetry/instrumentation/spring/resources/SystemHelper.java index 97ead727f9d5..59606a62c296 100644 --- a/instrumentation/resources/library/src/main/java/io/opentelemetry/instrumentation/resources/SystemHelper.java +++ b/instrumentation/spring/spring-boot-resources/library/src/main/java/io/opentelemetry/instrumentation/spring/resources/SystemHelper.java @@ -3,7 +3,7 @@ * SPDX-License-Identifier: Apache-2.0 */ -package io.opentelemetry.instrumentation.resources; +package io.opentelemetry.instrumentation.spring.resources; import java.io.InputStream; import java.lang.reflect.Method; @@ -13,13 +13,13 @@ import java.util.logging.Level; import java.util.logging.Logger; -public class SystemHelper { +class SystemHelper { private static final Logger logger = Logger.getLogger(SystemHelper.class.getName()); private final ClassLoader classLoader; private final boolean addBootInfPrefix; - public SystemHelper() { + SystemHelper() { ClassLoader contextClassLoader = Thread.currentThread().getContextClassLoader(); classLoader = contextClassLoader != null ? contextClassLoader : ClassLoader.getSystemClassLoader(); @@ -29,25 +29,25 @@ public SystemHelper() { } } - public String getenv(String name) { + String getenv(String name) { return System.getenv(name); } - public String getProperty(String key) { + String getProperty(String key) { return System.getProperty(key); } - public InputStream openClasspathResource(String filename) { + InputStream openClasspathResource(String filename) { String path = addBootInfPrefix ? "BOOT-INF/classes/" + filename : filename; return classLoader.getResourceAsStream(path); } - public InputStream openClasspathResource(String directory, String filename) { + InputStream openClasspathResource(String directory, String filename) { String path = directory + "/" + filename; return classLoader.getResourceAsStream(path); } - public InputStream openFile(String filename) throws Exception { + InputStream openFile(String filename) throws Exception { return Files.newInputStream(Paths.get(filename)); } @@ -56,7 +56,7 @@ public InputStream openFile(String filename) throws Exception { * main method arguments). Will only succeed on java 9+. */ @SuppressWarnings("unchecked") - public String[] attemptGetCommandLineArgsViaReflection() throws Exception { + String[] attemptGetCommandLineArgsViaReflection() throws Exception { Class clazz = Class.forName("java.lang.ProcessHandle"); Method currentMethod = clazz.getDeclaredMethod("current"); Method infoMethod = clazz.getDeclaredMethod("info"); 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 6b747577f627..935cfab9a998 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 @@ -12,7 +12,6 @@ import static org.mockito.Mockito.when; import io.opentelemetry.api.common.Attributes; -import io.opentelemetry.instrumentation.resources.SystemHelper; import io.opentelemetry.sdk.autoconfigure.spi.ConfigProperties; import io.opentelemetry.sdk.resources.Resource; import java.io.InputStream; diff --git a/instrumentation/spring/spring-boot-resources/library/src/test/java/io/opentelemetry/instrumentation/spring/resources/SpringBootServiceVersionDetectorTest.java b/instrumentation/spring/spring-boot-resources/library/src/test/java/io/opentelemetry/instrumentation/spring/resources/SpringBootServiceVersionDetectorTest.java index 640682a46778..9bce16b4fcd2 100644 --- a/instrumentation/spring/spring-boot-resources/library/src/test/java/io/opentelemetry/instrumentation/spring/resources/SpringBootServiceVersionDetectorTest.java +++ b/instrumentation/spring/spring-boot-resources/library/src/test/java/io/opentelemetry/instrumentation/spring/resources/SpringBootServiceVersionDetectorTest.java @@ -9,7 +9,6 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.Mockito.when; -import io.opentelemetry.instrumentation.resources.SystemHelper; import io.opentelemetry.sdk.autoconfigure.spi.ConfigProperties; import io.opentelemetry.sdk.resources.Resource; import java.io.InputStream; From ab1c61e56d974113cbb59c4e89357e9dfd8a0c84 Mon Sep 17 00:00:00 2001 From: Gregor Zeitlinger Date: Fri, 23 Feb 2024 16:13:40 +0100 Subject: [PATCH 09/14] cache discovered jar path --- .../resources/JarFileDetector.java | 14 +++++++---- .../resources/ResourceDiscoveryCache.java | 25 +++++++++++++++++++ .../resources/JarServiceNameDetectorTest.java | 6 +++++ .../ManifestResourceProviderTest.java | 6 +++++ 4 files changed, 46 insertions(+), 5 deletions(-) create mode 100644 instrumentation/resources/library/src/main/java/io/opentelemetry/instrumentation/resources/ResourceDiscoveryCache.java diff --git a/instrumentation/resources/library/src/main/java/io/opentelemetry/instrumentation/resources/JarFileDetector.java b/instrumentation/resources/library/src/main/java/io/opentelemetry/instrumentation/resources/JarFileDetector.java index 3fd8d6791896..280a9a7a3973 100644 --- a/instrumentation/resources/library/src/main/java/io/opentelemetry/instrumentation/resources/JarFileDetector.java +++ b/instrumentation/resources/library/src/main/java/io/opentelemetry/instrumentation/resources/JarFileDetector.java @@ -51,11 +51,15 @@ public JarFileDetector() { @Nullable Path getJarPath() { - Path jarPath = getJarPathFromProcessHandle(); - if (jarPath != null) { - return jarPath; - } - return getJarPathFromSunCommandLine(); + return ResourceDiscoveryCache.get( + "jarPath", + () -> { + Path jarPath = getJarPathFromProcessHandle(); + if (jarPath != null) { + return jarPath; + } + return getJarPathFromSunCommandLine(); + }); } Optional getManifestFromJarFile() { diff --git a/instrumentation/resources/library/src/main/java/io/opentelemetry/instrumentation/resources/ResourceDiscoveryCache.java b/instrumentation/resources/library/src/main/java/io/opentelemetry/instrumentation/resources/ResourceDiscoveryCache.java new file mode 100644 index 000000000000..32d5b7337352 --- /dev/null +++ b/instrumentation/resources/library/src/main/java/io/opentelemetry/instrumentation/resources/ResourceDiscoveryCache.java @@ -0,0 +1,25 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.instrumentation.resources; + +import java.util.concurrent.ConcurrentHashMap; +import java.util.function.Supplier; + +public class ResourceDiscoveryCache { + private static final ConcurrentHashMap cache = new ConcurrentHashMap<>(); + + private ResourceDiscoveryCache() {} + + // visible for testing + public static void resetForTest() { + cache.clear(); + } + + @SuppressWarnings("unchecked") + public static T get(String key, Supplier supplier) { + return (T) cache.computeIfAbsent(key, k -> supplier.get()); + } +} diff --git a/instrumentation/resources/library/src/test/java/io/opentelemetry/instrumentation/resources/JarServiceNameDetectorTest.java b/instrumentation/resources/library/src/test/java/io/opentelemetry/instrumentation/resources/JarServiceNameDetectorTest.java index 704551e15667..dd3032fdb385 100644 --- a/instrumentation/resources/library/src/test/java/io/opentelemetry/instrumentation/resources/JarServiceNameDetectorTest.java +++ b/instrumentation/resources/library/src/test/java/io/opentelemetry/instrumentation/resources/JarServiceNameDetectorTest.java @@ -17,6 +17,7 @@ import java.util.function.Function; import java.util.function.Predicate; import java.util.stream.Stream; +import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; import org.junit.jupiter.api.extension.ExtensionContext; @@ -33,6 +34,11 @@ class JarServiceNameDetectorTest { @Mock ConfigProperties config; + @BeforeEach + void setUp() { + ResourceDiscoveryCache.resetForTest(); + } + @Test void createResource_empty() { String[] processArgs = new String[0]; diff --git a/instrumentation/resources/library/src/test/java/io/opentelemetry/instrumentation/resources/ManifestResourceProviderTest.java b/instrumentation/resources/library/src/test/java/io/opentelemetry/instrumentation/resources/ManifestResourceProviderTest.java index a1945571f14b..557c0bf5a44b 100644 --- a/instrumentation/resources/library/src/test/java/io/opentelemetry/instrumentation/resources/ManifestResourceProviderTest.java +++ b/instrumentation/resources/library/src/test/java/io/opentelemetry/instrumentation/resources/ManifestResourceProviderTest.java @@ -19,11 +19,17 @@ import java.util.jar.Manifest; import java.util.stream.Collectors; import java.util.stream.Stream; +import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.DynamicTest; import org.junit.jupiter.api.TestFactory; class ManifestResourceProviderTest { + @BeforeEach + void setUp() { + ResourceDiscoveryCache.resetForTest(); + } + private static class TestCase { private final String name; private final String expectedName; From 2d47e69b0cc3fbd8348f8702f7193b36e089447e Mon Sep 17 00:00:00 2001 From: Gregor Zeitlinger Date: Tue, 5 Mar 2024 18:09:17 +0100 Subject: [PATCH 10/14] PR feedback --- .../instrumentation/resources/AttributeProvider.java | 6 +++--- .../{JarFileDetector.java => JarPathFinder.java} | 10 +++++----- .../resources/JarServiceNameDetector.java | 10 +++++----- .../resources/ManifestResourceProvider.java | 6 +++--- .../resources/ResourceDiscoveryCache.java | 2 +- .../resources/JarServiceNameDetectorTest.java | 2 +- .../resources/ManifestResourceProviderTest.java | 2 +- 7 files changed, 19 insertions(+), 19 deletions(-) rename instrumentation/resources/library/src/main/java/io/opentelemetry/instrumentation/resources/{JarFileDetector.java => JarPathFinder.java} (94%) diff --git a/instrumentation/resources/library/src/main/java/io/opentelemetry/instrumentation/resources/AttributeProvider.java b/instrumentation/resources/library/src/main/java/io/opentelemetry/instrumentation/resources/AttributeProvider.java index 33abb6435350..0e74368ff3fd 100644 --- a/instrumentation/resources/library/src/main/java/io/opentelemetry/instrumentation/resources/AttributeProvider.java +++ b/instrumentation/resources/library/src/main/java/io/opentelemetry/instrumentation/resources/AttributeProvider.java @@ -16,13 +16,13 @@ * *

An example of how to use this interface can be found in {@link ManifestResourceProvider}. */ -public interface AttributeProvider { +interface AttributeProvider { Optional readData(); void registerAttributes(Builder builder); - public interface Builder { + interface Builder { @CanIgnoreReturnValue - public Builder add(AttributeKey key, Function> getter); + Builder add(AttributeKey key, Function> getter); } } diff --git a/instrumentation/resources/library/src/main/java/io/opentelemetry/instrumentation/resources/JarFileDetector.java b/instrumentation/resources/library/src/main/java/io/opentelemetry/instrumentation/resources/JarPathFinder.java similarity index 94% rename from instrumentation/resources/library/src/main/java/io/opentelemetry/instrumentation/resources/JarFileDetector.java rename to instrumentation/resources/library/src/main/java/io/opentelemetry/instrumentation/resources/JarPathFinder.java index 280a9a7a3973..e1269de31147 100644 --- a/instrumentation/resources/library/src/main/java/io/opentelemetry/instrumentation/resources/JarFileDetector.java +++ b/instrumentation/resources/library/src/main/java/io/opentelemetry/instrumentation/resources/JarPathFinder.java @@ -21,24 +21,24 @@ import java.util.logging.Logger; import javax.annotation.Nullable; -class JarFileDetector { +class JarPathFinder { private final Supplier getProcessHandleArguments; private final Function getSystemProperty; private final Predicate fileExists; private final Function> manifestReader; - private static final Logger logger = Logger.getLogger(JarFileDetector.class.getName()); + private static final Logger logger = Logger.getLogger(JarPathFinder.class.getName()); - public JarFileDetector() { + public JarPathFinder() { this( ProcessArguments::getProcessArguments, System::getProperty, Files::isRegularFile, - JarFileDetector::readManifest); + JarPathFinder::readManifest); } // visible for tests - JarFileDetector( + JarPathFinder( Supplier getProcessHandleArguments, Function getSystemProperty, Predicate fileExists, diff --git a/instrumentation/resources/library/src/main/java/io/opentelemetry/instrumentation/resources/JarServiceNameDetector.java b/instrumentation/resources/library/src/main/java/io/opentelemetry/instrumentation/resources/JarServiceNameDetector.java index f1eeb344f932..6ac3614aca0c 100644 --- a/instrumentation/resources/library/src/main/java/io/opentelemetry/instrumentation/resources/JarServiceNameDetector.java +++ b/instrumentation/resources/library/src/main/java/io/opentelemetry/instrumentation/resources/JarServiceNameDetector.java @@ -26,21 +26,21 @@ public final class JarServiceNameDetector implements ConditionalResourceProvider private static final Logger logger = Logger.getLogger(JarServiceNameDetector.class.getName()); - private final JarFileDetector jarFileDetector; + private final JarPathFinder jarPathFinder; @SuppressWarnings("unused") // SPI public JarServiceNameDetector() { - this(new JarFileDetector()); + this(new JarPathFinder()); } // visible for tests - JarServiceNameDetector(JarFileDetector jarFileDetector) { - this.jarFileDetector = jarFileDetector; + JarServiceNameDetector(JarPathFinder jarPathFinder) { + this.jarPathFinder = jarPathFinder; } @Override public Resource createResource(ConfigProperties config) { - Path jarPath = jarFileDetector.getJarPath(); + Path jarPath = jarPathFinder.getJarPath(); if (jarPath == null) { return Resource.empty(); } diff --git a/instrumentation/resources/library/src/main/java/io/opentelemetry/instrumentation/resources/ManifestResourceProvider.java b/instrumentation/resources/library/src/main/java/io/opentelemetry/instrumentation/resources/ManifestResourceProvider.java index e818a4fa6d14..bc8f01ed1d89 100644 --- a/instrumentation/resources/library/src/main/java/io/opentelemetry/instrumentation/resources/ManifestResourceProvider.java +++ b/instrumentation/resources/library/src/main/java/io/opentelemetry/instrumentation/resources/ManifestResourceProvider.java @@ -20,16 +20,16 @@ public final class ManifestResourceProvider extends AttributeResourceProvider() { @Override public Optional readData() { - return jarFileDetector.getManifestFromJarFile(); + return jarPathFinder.getManifestFromJarFile(); } @Override diff --git a/instrumentation/resources/library/src/main/java/io/opentelemetry/instrumentation/resources/ResourceDiscoveryCache.java b/instrumentation/resources/library/src/main/java/io/opentelemetry/instrumentation/resources/ResourceDiscoveryCache.java index 32d5b7337352..ffcf6533d2d9 100644 --- a/instrumentation/resources/library/src/main/java/io/opentelemetry/instrumentation/resources/ResourceDiscoveryCache.java +++ b/instrumentation/resources/library/src/main/java/io/opentelemetry/instrumentation/resources/ResourceDiscoveryCache.java @@ -8,7 +8,7 @@ import java.util.concurrent.ConcurrentHashMap; import java.util.function.Supplier; -public class ResourceDiscoveryCache { +class ResourceDiscoveryCache { private static final ConcurrentHashMap cache = new ConcurrentHashMap<>(); private ResourceDiscoveryCache() {} diff --git a/instrumentation/resources/library/src/test/java/io/opentelemetry/instrumentation/resources/JarServiceNameDetectorTest.java b/instrumentation/resources/library/src/test/java/io/opentelemetry/instrumentation/resources/JarServiceNameDetectorTest.java index dd3032fdb385..fe529bea76fe 100644 --- a/instrumentation/resources/library/src/test/java/io/opentelemetry/instrumentation/resources/JarServiceNameDetectorTest.java +++ b/instrumentation/resources/library/src/test/java/io/opentelemetry/instrumentation/resources/JarServiceNameDetectorTest.java @@ -54,7 +54,7 @@ void createResource_empty() { private static JarServiceNameDetector getDetector( String[] processArgs, Function getProperty, Predicate fileExists) { return new JarServiceNameDetector( - new JarFileDetector(() -> processArgs, getProperty, fileExists, p -> Optional.empty())); + new JarPathFinder(() -> processArgs, getProperty, fileExists, p -> Optional.empty())); } @Test diff --git a/instrumentation/resources/library/src/test/java/io/opentelemetry/instrumentation/resources/ManifestResourceProviderTest.java b/instrumentation/resources/library/src/test/java/io/opentelemetry/instrumentation/resources/ManifestResourceProviderTest.java index 557c0bf5a44b..2b20b339797b 100644 --- a/instrumentation/resources/library/src/test/java/io/opentelemetry/instrumentation/resources/ManifestResourceProviderTest.java +++ b/instrumentation/resources/library/src/test/java/io/opentelemetry/instrumentation/resources/ManifestResourceProviderTest.java @@ -60,7 +60,7 @@ Collection createResource() { () -> { ManifestResourceProvider provider = new ManifestResourceProvider( - new JarFileDetector( + new JarPathFinder( () -> JarServiceNameDetectorTest.getArgs("app.jar"), prop -> null, JarServiceNameDetectorTest::failPath, From e6ae4877676df17d9b13e8c1d0aa61caa9a1badd Mon Sep 17 00:00:00 2001 From: Gregor Zeitlinger Date: Mon, 11 Mar 2024 16:37:45 +0100 Subject: [PATCH 11/14] remove ResourceDiscoveryCache --- .../resources/JarPathFinder.java | 57 ++++++------------- .../resources/JarServiceNameDetector.java | 17 +++--- .../resources/ManifestResourceProvider.java | 28 ++++++++- .../resources/ResourceDiscoveryCache.java | 25 -------- .../resources/JarServiceNameDetectorTest.java | 5 +- .../ManifestResourceProviderTest.java | 22 +++---- 6 files changed, 64 insertions(+), 90 deletions(-) delete mode 100644 instrumentation/resources/library/src/main/java/io/opentelemetry/instrumentation/resources/ResourceDiscoveryCache.java diff --git a/instrumentation/resources/library/src/main/java/io/opentelemetry/instrumentation/resources/JarPathFinder.java b/instrumentation/resources/library/src/main/java/io/opentelemetry/instrumentation/resources/JarPathFinder.java index e1269de31147..6ddc9f71e959 100644 --- a/instrumentation/resources/library/src/main/java/io/opentelemetry/instrumentation/resources/JarPathFinder.java +++ b/instrumentation/resources/library/src/main/java/io/opentelemetry/instrumentation/resources/JarPathFinder.java @@ -5,10 +5,6 @@ package io.opentelemetry.instrumentation.resources; -import static java.util.logging.Level.WARNING; - -import java.io.InputStream; -import java.net.URL; import java.nio.file.Files; import java.nio.file.InvalidPathException; import java.nio.file.Path; @@ -17,69 +13,48 @@ import java.util.function.Function; import java.util.function.Predicate; import java.util.function.Supplier; -import java.util.jar.Manifest; -import java.util.logging.Logger; import javax.annotation.Nullable; class JarPathFinder { private final Supplier getProcessHandleArguments; private final Function getSystemProperty; private final Predicate fileExists; - private final Function> manifestReader; - private static final Logger logger = Logger.getLogger(JarPathFinder.class.getName()); + private static Optional> jarPath = Optional.empty(); public JarPathFinder() { - this( - ProcessArguments::getProcessArguments, - System::getProperty, - Files::isRegularFile, - JarPathFinder::readManifest); + this(ProcessArguments::getProcessArguments, System::getProperty, Files::isRegularFile); } // visible for tests JarPathFinder( Supplier getProcessHandleArguments, Function getSystemProperty, - Predicate fileExists, - Function> manifestReader) { + Predicate fileExists) { this.getProcessHandleArguments = getProcessHandleArguments; this.getSystemProperty = getSystemProperty; this.fileExists = fileExists; - this.manifestReader = manifestReader; } - @Nullable - Path getJarPath() { - return ResourceDiscoveryCache.get( - "jarPath", - () -> { - Path jarPath = getJarPathFromProcessHandle(); - if (jarPath != null) { - return jarPath; - } - return getJarPathFromSunCommandLine(); - }); + // visible for testing + static void resetForTest() { + jarPath = Optional.empty(); } - Optional getManifestFromJarFile() { - Path jarPath = getJarPath(); - if (jarPath == null) { - return Optional.empty(); + Optional getJarPath() { + if (jarPath.isPresent()) { + return jarPath.get(); } - return manifestReader.apply(jarPath); + jarPath = Optional.of(Optional.ofNullable(readJarPath())); + return jarPath.get(); } - private static Optional readManifest(Path jarPath) { - try (InputStream s = - new URL(String.format("jar:%s!/META-INF/MANIFEST.MF", jarPath.toUri())).openStream()) { - Manifest manifest = new Manifest(); - manifest.read(s); - return Optional.of(manifest); - } catch (Exception e) { - logger.log(WARNING, "Error reading manifest", e); - return Optional.empty(); + private Path readJarPath() { + Path jarPath = getJarPathFromProcessHandle(); + if (jarPath != null) { + return jarPath; } + return getJarPathFromSunCommandLine(); } @Nullable diff --git a/instrumentation/resources/library/src/main/java/io/opentelemetry/instrumentation/resources/JarServiceNameDetector.java b/instrumentation/resources/library/src/main/java/io/opentelemetry/instrumentation/resources/JarServiceNameDetector.java index 6ac3614aca0c..51b379f2926a 100644 --- a/instrumentation/resources/library/src/main/java/io/opentelemetry/instrumentation/resources/JarServiceNameDetector.java +++ b/instrumentation/resources/library/src/main/java/io/opentelemetry/instrumentation/resources/JarServiceNameDetector.java @@ -40,13 +40,16 @@ public JarServiceNameDetector() { @Override public Resource createResource(ConfigProperties config) { - Path jarPath = jarPathFinder.getJarPath(); - if (jarPath == null) { - return Resource.empty(); - } - String serviceName = getServiceName(jarPath); - logger.log(FINE, "Auto-detected service name from the jar file name: {0}", serviceName); - return Resource.create(Attributes.of(ResourceAttributes.SERVICE_NAME, serviceName)); + return jarPathFinder + .getJarPath() + .map( + jarPath -> { + String serviceName = getServiceName(jarPath); + logger.log( + FINE, "Auto-detected service name from the jar file name: {0}", serviceName); + return Resource.create(Attributes.of(ResourceAttributes.SERVICE_NAME, serviceName)); + }) + .orElseGet(Resource::empty); } @Override diff --git a/instrumentation/resources/library/src/main/java/io/opentelemetry/instrumentation/resources/ManifestResourceProvider.java b/instrumentation/resources/library/src/main/java/io/opentelemetry/instrumentation/resources/ManifestResourceProvider.java index bc8f01ed1d89..010954831d12 100644 --- a/instrumentation/resources/library/src/main/java/io/opentelemetry/instrumentation/resources/ManifestResourceProvider.java +++ b/instrumentation/resources/library/src/main/java/io/opentelemetry/instrumentation/resources/ManifestResourceProvider.java @@ -5,11 +5,18 @@ package io.opentelemetry.instrumentation.resources; +import static java.util.logging.Level.WARNING; + import com.google.auto.service.AutoService; import io.opentelemetry.sdk.autoconfigure.spi.ResourceProvider; import io.opentelemetry.semconv.ResourceAttributes; +import java.io.InputStream; +import java.net.URL; +import java.nio.file.Path; import java.util.Optional; +import java.util.function.Function; import java.util.jar.Manifest; +import java.util.logging.Logger; /** * A {@link ResourceProvider} that will attempt to detect the service.name and @@ -18,18 +25,21 @@ @AutoService(ResourceProvider.class) public final class ManifestResourceProvider extends AttributeResourceProvider { + private static final Logger logger = Logger.getLogger(ManifestResourceProvider.class.getName()); + @SuppressWarnings("unused") // SPI public ManifestResourceProvider() { - this(new JarPathFinder()); + this(new JarPathFinder(), ManifestResourceProvider::readManifest); } // Visible for testing - ManifestResourceProvider(JarPathFinder jarPathFinder) { + ManifestResourceProvider( + JarPathFinder jarPathFinder, Function> manifestReader) { super( new AttributeProvider() { @Override public Optional readData() { - return jarPathFinder.getManifestFromJarFile(); + return jarPathFinder.getJarPath().flatMap(manifestReader); } @Override @@ -53,6 +63,18 @@ public void registerAttributes(Builder builder) { }); } + private static Optional readManifest(Path jarPath) { + try (InputStream s = + new URL(String.format("jar:%s!/META-INF/MANIFEST.MF", jarPath.toUri())).openStream()) { + Manifest manifest = new Manifest(); + manifest.read(s); + return Optional.of(manifest); + } catch (Exception e) { + logger.log(WARNING, "Error reading manifest", e); + return Optional.empty(); + } + } + @Override public int order() { // make it run later than SpringBootServiceNameDetector diff --git a/instrumentation/resources/library/src/main/java/io/opentelemetry/instrumentation/resources/ResourceDiscoveryCache.java b/instrumentation/resources/library/src/main/java/io/opentelemetry/instrumentation/resources/ResourceDiscoveryCache.java deleted file mode 100644 index ffcf6533d2d9..000000000000 --- a/instrumentation/resources/library/src/main/java/io/opentelemetry/instrumentation/resources/ResourceDiscoveryCache.java +++ /dev/null @@ -1,25 +0,0 @@ -/* - * Copyright The OpenTelemetry Authors - * SPDX-License-Identifier: Apache-2.0 - */ - -package io.opentelemetry.instrumentation.resources; - -import java.util.concurrent.ConcurrentHashMap; -import java.util.function.Supplier; - -class ResourceDiscoveryCache { - private static final ConcurrentHashMap cache = new ConcurrentHashMap<>(); - - private ResourceDiscoveryCache() {} - - // visible for testing - public static void resetForTest() { - cache.clear(); - } - - @SuppressWarnings("unchecked") - public static T get(String key, Supplier supplier) { - return (T) cache.computeIfAbsent(key, k -> supplier.get()); - } -} diff --git a/instrumentation/resources/library/src/test/java/io/opentelemetry/instrumentation/resources/JarServiceNameDetectorTest.java b/instrumentation/resources/library/src/test/java/io/opentelemetry/instrumentation/resources/JarServiceNameDetectorTest.java index fe529bea76fe..dd9562d572cd 100644 --- a/instrumentation/resources/library/src/test/java/io/opentelemetry/instrumentation/resources/JarServiceNameDetectorTest.java +++ b/instrumentation/resources/library/src/test/java/io/opentelemetry/instrumentation/resources/JarServiceNameDetectorTest.java @@ -13,7 +13,6 @@ import io.opentelemetry.semconv.ResourceAttributes; import java.nio.file.Path; import java.nio.file.Paths; -import java.util.Optional; import java.util.function.Function; import java.util.function.Predicate; import java.util.stream.Stream; @@ -36,7 +35,7 @@ class JarServiceNameDetectorTest { @BeforeEach void setUp() { - ResourceDiscoveryCache.resetForTest(); + JarPathFinder.resetForTest(); } @Test @@ -54,7 +53,7 @@ void createResource_empty() { private static JarServiceNameDetector getDetector( String[] processArgs, Function getProperty, Predicate fileExists) { return new JarServiceNameDetector( - new JarPathFinder(() -> processArgs, getProperty, fileExists, p -> Optional.empty())); + new JarPathFinder(() -> processArgs, getProperty, fileExists)); } @Test diff --git a/instrumentation/resources/library/src/test/java/io/opentelemetry/instrumentation/resources/ManifestResourceProviderTest.java b/instrumentation/resources/library/src/test/java/io/opentelemetry/instrumentation/resources/ManifestResourceProviderTest.java index 2b20b339797b..d789dbb0ffff 100644 --- a/instrumentation/resources/library/src/test/java/io/opentelemetry/instrumentation/resources/ManifestResourceProviderTest.java +++ b/instrumentation/resources/library/src/test/java/io/opentelemetry/instrumentation/resources/ManifestResourceProviderTest.java @@ -27,7 +27,7 @@ class ManifestResourceProviderTest { @BeforeEach void setUp() { - ResourceDiscoveryCache.resetForTest(); + JarPathFinder.resetForTest(); } private static class TestCase { @@ -63,16 +63,16 @@ Collection createResource() { new JarPathFinder( () -> JarServiceNameDetectorTest.getArgs("app.jar"), prop -> null, - JarServiceNameDetectorTest::failPath, - p -> { - try { - Manifest manifest = new Manifest(); - manifest.read(t.input); - return Optional.of(manifest); - } catch (Exception e) { - return Optional.empty(); - } - })); + JarServiceNameDetectorTest::failPath), + p -> { + try { + Manifest manifest = new Manifest(); + manifest.read(t.input); + return Optional.of(manifest); + } catch (Exception e) { + return Optional.empty(); + } + }); provider.shouldApply(config, Resource.getDefault()); Resource resource = provider.createResource(config); From 9cc81e1c62d6b751246a0768c1710af683c53c18 Mon Sep 17 00:00:00 2001 From: Gregor Zeitlinger Date: Tue, 12 Mar 2024 13:20:12 +0100 Subject: [PATCH 12/14] make detection explicit --- .../resources/JarPathFinder.java | 21 ++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/instrumentation/resources/library/src/main/java/io/opentelemetry/instrumentation/resources/JarPathFinder.java b/instrumentation/resources/library/src/main/java/io/opentelemetry/instrumentation/resources/JarPathFinder.java index 6ddc9f71e959..0c25176a6701 100644 --- a/instrumentation/resources/library/src/main/java/io/opentelemetry/instrumentation/resources/JarPathFinder.java +++ b/instrumentation/resources/library/src/main/java/io/opentelemetry/instrumentation/resources/JarPathFinder.java @@ -20,7 +20,15 @@ class JarPathFinder { private final Function getSystemProperty; private final Predicate fileExists; - private static Optional> jarPath = Optional.empty(); + private static class DetectionResult { + private final Optional jarPath; + + private DetectionResult(Optional jarPath) { + this.jarPath = jarPath; + } + } + + private static Optional detectionResult = Optional.empty(); public JarPathFinder() { this(ProcessArguments::getProcessArguments, System::getProperty, Files::isRegularFile); @@ -38,18 +46,17 @@ public JarPathFinder() { // visible for testing static void resetForTest() { - jarPath = Optional.empty(); + detectionResult = Optional.empty(); } Optional getJarPath() { - if (jarPath.isPresent()) { - return jarPath.get(); + if (!detectionResult.isPresent()) { + detectionResult = Optional.of(new DetectionResult(Optional.ofNullable(detectJarPath()))); } - jarPath = Optional.of(Optional.ofNullable(readJarPath())); - return jarPath.get(); + return detectionResult.get().jarPath; } - private Path readJarPath() { + private Path detectJarPath() { Path jarPath = getJarPathFromProcessHandle(); if (jarPath != null) { return jarPath; From 8cabb44a3f8c06490a6eb9921d7f339621f8c1d5 Mon Sep 17 00:00:00 2001 From: Gregor Zeitlinger Date: Wed, 13 Mar 2024 15:58:51 +0100 Subject: [PATCH 13/14] Update instrumentation/resources/library/src/main/java/io/opentelemetry/instrumentation/resources/ManifestResourceProvider.java Co-authored-by: Trask Stalnaker --- .../instrumentation/resources/ManifestResourceProvider.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/instrumentation/resources/library/src/main/java/io/opentelemetry/instrumentation/resources/ManifestResourceProvider.java b/instrumentation/resources/library/src/main/java/io/opentelemetry/instrumentation/resources/ManifestResourceProvider.java index 010954831d12..2a5d88c53da1 100644 --- a/instrumentation/resources/library/src/main/java/io/opentelemetry/instrumentation/resources/ManifestResourceProvider.java +++ b/instrumentation/resources/library/src/main/java/io/opentelemetry/instrumentation/resources/ManifestResourceProvider.java @@ -77,7 +77,7 @@ private static Optional readManifest(Path jarPath) { @Override public int order() { - // make it run later than SpringBootServiceNameDetector + // make it run later than SpringBootServiceNameDetector but before JarServiceNameDetector return 300; } } From 5927b3c98af86e495b6fe55ba5f2145749a6189b Mon Sep 17 00:00:00 2001 From: Gregor Zeitlinger Date: Wed, 13 Mar 2024 16:01:52 +0100 Subject: [PATCH 14/14] comment about order --- .../instrumentation/resources/ManifestResourceProvider.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/instrumentation/resources/library/src/main/java/io/opentelemetry/instrumentation/resources/ManifestResourceProvider.java b/instrumentation/resources/library/src/main/java/io/opentelemetry/instrumentation/resources/ManifestResourceProvider.java index 2a5d88c53da1..461af3cd99c7 100644 --- a/instrumentation/resources/library/src/main/java/io/opentelemetry/instrumentation/resources/ManifestResourceProvider.java +++ b/instrumentation/resources/library/src/main/java/io/opentelemetry/instrumentation/resources/ManifestResourceProvider.java @@ -77,7 +77,7 @@ private static Optional readManifest(Path jarPath) { @Override public int order() { - // make it run later than SpringBootServiceNameDetector but before JarServiceNameDetector + // make it run later than ManifestResourceProvider and SpringBootServiceNameDetector return 300; } }