Merge pull request #2 from laurit/simplify-main-jar-holder

simplify main jar holder
zeitlinger · Mar 14, 2024 · 8e7bf54 · 8e7bf54
2 parents 8952683 + c5d0562
commit 8e7bf54
Show file tree

Hide file tree

Showing 5 changed files with 28 additions and 44 deletions.
diff --git a/...rary/src/main/java/io/opentelemetry/instrumentation/resources/JarServiceNameDetector.java b/...rary/src/main/java/io/opentelemetry/instrumentation/resources/JarServiceNameDetector.java
@@ -16,6 +16,8 @@
 import io.opentelemetry.semconv.ResourceAttributes;
 import java.nio.file.Path;
 import java.util.Map;
+import java.util.Optional;
+import java.util.function.Supplier;
 import java.util.logging.Logger;
 
 /**
@@ -26,21 +28,26 @@ public final class JarServiceNameDetector implements ConditionalResourceProvider
 
   private static final Logger logger = Logger.getLogger(JarServiceNameDetector.class.getName());
 
-  private final MainJarPathFinder jarPathFinder;
+  private final Supplier<Optional<Path>> jarPathSupplier;
 
   @SuppressWarnings("unused") // SPI
   public JarServiceNameDetector() {
-    this(new MainJarPathFinder());
+    this(MainJarPathHolder::getJarPath);
+  }
+
+  private JarServiceNameDetector(Supplier<Optional<Path>> jarPathSupplier) {
+    this.jarPathSupplier = jarPathSupplier;
   }
 
   // visible for tests
   JarServiceNameDetector(MainJarPathFinder jarPathFinder) {
-    this.jarPathFinder = jarPathFinder;
+    this(() -> Optional.ofNullable(jarPathFinder.detectJarPath()));
   }
 
   @Override
   public Resource createResource(ConfigProperties config) {
-    return MainJarPathHolder.getJarPath(jarPathFinder)
+    return jarPathSupplier
+        .get()
         .map(
             jarPath -> {
               String serviceName = getServiceName(jarPath);

diff --git a/...s/library/src/main/java/io/opentelemetry/instrumentation/resources/MainJarPathHolder.java b/...s/library/src/main/java/io/opentelemetry/instrumentation/resources/MainJarPathHolder.java
@@ -8,30 +8,13 @@
 import java.nio.file.Path;
 import java.util.Optional;
 
-public class MainJarPathHolder {
+class MainJarPathHolder {
+  private static final Optional<Path> jarPath =
+      Optional.ofNullable(new MainJarPathFinder().detectJarPath());
 
-  private MainJarPathHolder() {}
-
-  // visible for testing
-  static void resetForTest() {
-    detectionResult = Optional.empty();
-  }
-
-  private static class DetectionResult {
-    private final Optional<Path> jarPath;
-
-    private DetectionResult(Optional<Path> jarPath) {
-      this.jarPath = jarPath;
-    }
+  static Optional<Path> getJarPath() {
+    return jarPath;
   }
 
-  private static Optional<DetectionResult> detectionResult = Optional.empty();
-
-  static Optional<Path> getJarPath(MainJarPathFinder finder) {
-    if (!detectionResult.isPresent()) {
-      detectionResult =
-          Optional.of(new DetectionResult(Optional.ofNullable(finder.detectJarPath())));
-    }
-    return detectionResult.get().jarPath;
-  }
+  private MainJarPathHolder() {}
 }
diff --git a/...ry/src/main/java/io/opentelemetry/instrumentation/resources/ManifestResourceProvider.java b/...ry/src/main/java/io/opentelemetry/instrumentation/resources/ManifestResourceProvider.java
@@ -14,6 +14,7 @@
 import java.nio.file.Path;
 import java.util.Optional;
 import java.util.function.Function;
+import java.util.function.Supplier;
 import java.util.jar.JarFile;
 import java.util.jar.Manifest;
 import java.util.logging.Logger;
@@ -29,17 +30,16 @@ public final class ManifestResourceProvider extends AttributeResourceProvider<Ma
 
   @SuppressWarnings("unused") // SPI
   public ManifestResourceProvider() {
-    this(new MainJarPathFinder(), ManifestResourceProvider::readManifest);
+    this(MainJarPathHolder::getJarPath, ManifestResourceProvider::readManifest);
   }
 
-  // Visible for testing
-  ManifestResourceProvider(
-      MainJarPathFinder jarPathFinder, Function<Path, Optional<Manifest>> manifestReader) {
+  private ManifestResourceProvider(
+      Supplier<Optional<Path>> jarPathSupplier, Function<Path, Optional<Manifest>> manifestReader) {
     super(
         new AttributeProvider<Manifest>() {
           @Override
           public Optional<Manifest> readData() {
-            return MainJarPathHolder.getJarPath(jarPathFinder).flatMap(manifestReader);
+            return jarPathSupplier.get().flatMap(manifestReader);
           }
 
           @Override
@@ -63,6 +63,12 @@ public void registerAttributes(Builder<Manifest> builder) {
         });
   }
 
+  // Visible for testing
+  ManifestResourceProvider(
+      MainJarPathFinder jarPathFinder, Function<Path, Optional<Manifest>> manifestReader) {
+    this(() -> Optional.ofNullable(jarPathFinder.detectJarPath()), manifestReader);
+  }
+
   private static Optional<Manifest> readManifest(Path jarPath) {
     try (JarFile jarFile = new JarFile(jarPath.toFile(), false)) {
       return Optional.of(jarFile.getManifest());

diff --git a/.../src/test/java/io/opentelemetry/instrumentation/resources/JarServiceNameDetectorTest.java b/.../src/test/java/io/opentelemetry/instrumentation/resources/JarServiceNameDetectorTest.java
@@ -16,7 +16,6 @@
 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,11 +32,6 @@ class JarServiceNameDetectorTest {
 
   @Mock ConfigProperties config;
 
-  @BeforeEach
-  void setUp() {
-    MainJarPathHolder.resetForTest();
-  }
-
   @Test
   void createResource_empty() {
     String[] processArgs = new String[0];

diff --git a/...rc/test/java/io/opentelemetry/instrumentation/resources/ManifestResourceProviderTest.java b/...rc/test/java/io/opentelemetry/instrumentation/resources/ManifestResourceProviderTest.java
@@ -20,17 +20,11 @@
 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() {
-    MainJarPathHolder.resetForTest();
-  }
-
   private static class TestCase {
     private final String name;
     private final String expectedName;