remove ResourceDiscoveryCache

open-telemetry · Mar 11, 2024 · 2672296 · 2672296
1 parent f352396
commit 2672296
Show file tree

Hide file tree

Showing 6 changed files with 64 additions and 90 deletions.
diff --git a/...urces/library/src/main/java/io/opentelemetry/instrumentation/resources/JarPathFinder.java b/...urces/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<String[]> getProcessHandleArguments;
   private final Function<String, String> getSystemProperty;
   private final Predicate<Path> fileExists;
-  private final Function<Path, Optional<Manifest>> manifestReader;
 
-  private static final Logger logger = Logger.getLogger(JarPathFinder.class.getName());
+  private static Optional<Optional<Path>> 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<String[]> getProcessHandleArguments,
       Function<String, String> getSystemProperty,
-      Predicate<Path> fileExists,
-      Function<Path, Optional<Manifest>> manifestReader) {
+      Predicate<Path> 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<Manifest> getManifestFromJarFile() {
-    Path jarPath = getJarPath();
-    if (jarPath == null) {
-      return Optional.empty();
+  Optional<Path> getJarPath() {
+    if (jarPath.isPresent()) {
+      return jarPath.get();
     }
-    return manifestReader.apply(jarPath);
+    jarPath = Optional.of(Optional.ofNullable(readJarPath()));
+    return jarPath.get();
   }
 
-  private static Optional<Manifest> 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/...rary/src/main/java/io/opentelemetry/instrumentation/resources/JarServiceNameDetector.java b/...rary/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/...ry/src/main/java/io/opentelemetry/instrumentation/resources/ManifestResourceProvider.java b/...ry/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 <code>service.name</code> and <code>
@@ -18,18 +25,21 @@
 @AutoService(ResourceProvider.class)
 public final class ManifestResourceProvider extends AttributeResourceProvider<Manifest> {
 
+  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<Path, Optional<Manifest>> manifestReader) {
     super(
         new AttributeProvider<Manifest>() {
           @Override
           public Optional<Manifest> readData() {
-            return jarPathFinder.getManifestFromJarFile();
+            return jarPathFinder.getJarPath().flatMap(manifestReader);
           }
 
           @Override
@@ -53,6 +63,18 @@ public void registerAttributes(Builder<Manifest> builder) {
         });
   }
 
+  private static Optional<Manifest> 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/...rary/src/main/java/io/opentelemetry/instrumentation/resources/ResourceDiscoveryCache.java b/...rary/src/main/java/io/opentelemetry/instrumentation/resources/ResourceDiscoveryCache.java
diff --git a/.../src/test/java/io/opentelemetry/instrumentation/resources/JarServiceNameDetectorTest.java b/.../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<String, String> getProperty, Predicate<Path> fileExists) {
     return new JarServiceNameDetector(
-        new JarPathFinder(() -> processArgs, getProperty, fileExists, p -> Optional.empty()));
+        new JarPathFinder(() -> processArgs, getProperty, fileExists));
   }
 
   @Test

diff --git a/...rc/test/java/io/opentelemetry/instrumentation/resources/ManifestResourceProviderTest.java b/...rc/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<DynamicTest> 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);