post-review changes

Author: SylvainJuge

jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/JmxConnectionTest.java

@@ -13,7 +13,6 @@
 import java.util.function.Function;
 import org.junit.jupiter.api.AfterAll;
 import org.junit.jupiter.api.BeforeAll;
-import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.io.TempDir;
 import org.junit.jupiter.params.ParameterizedTest;
@@ -60,12 +59,6 @@ static void afterAll() {
     network.close();
   }
 
-  @BeforeEach
-  void beforeEach() {
-    // extra safety to ensure temp folder is empty before each test method
-    assertThat(tempDir).isEmptyDirectory();
-  }
-
   @Test
   void connectionError() {
     try (JmxScraperContainer scraper = scraperContainer().withRmiServiceUrl("unknown_host", 1234)) {

jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/JmxScraperContainer.java

@@ -20,6 +20,7 @@
 import java.util.Map;
 import java.util.Set;
 import java.util.stream.Collectors;
+import org.jetbrains.annotations.NotNull;
 import org.testcontainers.containers.GenericContainer;
 import org.testcontainers.containers.wait.strategy.Wait;
 import org.testcontainers.utility.MountableFile;
@@ -214,43 +215,47 @@ public JmxScraperContainer withConfigSource(ConfigSource source) {
   @Override
   public void start() {
 
-    Map<String, String> config = new HashMap<>();
-    config.put("otel.metrics.exporter", "otlp");
-    config.put("otel.exporter.otlp.endpoint", endpoint);
-
-    if (!targetSystems.isEmpty()) {
-      config.put("otel.jmx.target.system", String.join(",", targetSystems));
-    }
+    Map<String, String> config = initConfig();
 
-    if (serviceUrl == null) {
-      throw new IllegalStateException("Missing service URL");
-    }
-    config.put("otel.jmx.service.url", serviceUrl);
+    List<String> cmd = createCommand(config);
 
-    // always use a very short export interval for testing
-    config.put("otel.metric.export.interval", "1s");
+    if (configSource != ConfigSource.STDIN) {
+      this.withCommand(cmd.toArray(new String[0]));
+    } else {
+      Path script = generateShellScript(cmd, config);
 
-    if (user != null) {
-      config.put("otel.jmx.username", user);
-    }
-    if (password != null) {
-      config.put("otel.jmx.password", password);
+      this.withCopyFileToContainer(MountableFile.forHostPath(script, 500), "/scraper.sh");
+      this.withCommand("/scraper.sh");
     }
 
-    addSecureStore(keyStore, /* isKeyStore= */ true, config);
-    addSecureStore(trustStore, /* isKeyStore= */ false, config);
+    logger().info("Starting scraper with command: " + String.join(" ", this.getCommandParts()));
+    super.start();
+  }
 
-    if (sslRmiRegistry) {
-      config.put("otel.jmx.remote.registry.ssl", "true");
+  private Path generateShellScript(List<String> cmd, Map<String, String> config) {
+    // generate shell script to feed standard input with config
+    List<String> lines = new ArrayList<>();
+    lines.add("#!/bin/bash");
+    lines.add(String.join(" ", cmd) + "<<EOF");
+    lines.addAll(toKeyValueString(config));
+    lines.add("EOF");
+
+    Path script;
+    try {
+      script = Files.createTempFile("scraper", ".sh");
+      Files.write(script, lines);
+    } catch (IOException e) {
+      throw new IllegalStateException(e);
     }
 
-    if (!customYamlFiles.isEmpty()) {
-      for (String yaml : customYamlFiles) {
-        this.withCopyFileToContainer(MountableFile.forClasspathResource(yaml), yaml);
-      }
-      config.put("otel.jmx.config", String.join(",", customYamlFiles));
+    logger().info("Scraper executed with /scraper.sh shell script");
+    for (int i = 0; i < lines.size(); i++) {
+      logger().info("/scrapper.sh:{}  {}", i, lines.get(i));
     }
+    return script;
+  }
 
+  private List<String> createCommand(Map<String, String> config) {
     List<String> cmd = new ArrayList<>();
     cmd.add("java");
 
@@ -328,36 +333,47 @@ public void start() {
           Wait.forLogMessage(".*JMX scraping started.*", 1)
               .withStartupTimeout(Duration.ofSeconds(10)));
     }
+    return cmd;
+  }
 
-    if (configSource != ConfigSource.STDIN) {
-      this.withCommand(cmd.toArray(new String[0]));
-    } else {
-      // generate shell script to feed standard input with config
-      List<String> lines = new ArrayList<>();
-      lines.add("#!/bin/bash");
-      lines.add(String.join(" ", cmd) + "<<EOF");
-      lines.addAll(toKeyValueString(config));
-      lines.add("EOF");
-
-      Path script;
-      try {
-        script = Files.createTempFile("scraper", ".sh");
-        Files.write(script, lines);
-      } catch (IOException e) {
-        throw new IllegalStateException(e);
-      }
+  private @NotNull Map<String, String> initConfig() {
+    Map<String, String> config = new HashMap<>();
+    config.put("otel.metrics.exporter", "otlp");
+    config.put("otel.exporter.otlp.endpoint", endpoint);
 
-      logger().info("Scraper executed with /scraper.sh shell script");
-      for (int i = 0; i < lines.size(); i++) {
-        logger().info("/scrapper.sh:{}  {}", i, lines.get(i));
-      }
+    if (!targetSystems.isEmpty()) {
+      config.put("otel.jmx.target.system", String.join(",", targetSystems));
+    }
 
-      this.withCopyFileToContainer(MountableFile.forHostPath(script, 500), "/scraper.sh");
-      this.withCommand("/scraper.sh");
+    if (serviceUrl == null) {
+      throw new IllegalStateException("Missing service URL");
     }
+    config.put("otel.jmx.service.url", serviceUrl);
 
-    logger().info("Starting scraper with command: " + String.join(" ", this.getCommandParts()));
-    super.start();
+    // always use a very short export interval for testing
+    config.put("otel.metric.export.interval", "1s");
+
+    if (user != null) {
+      config.put("otel.jmx.username", user);
+    }
+    if (password != null) {
+      config.put("otel.jmx.password", password);
+    }
+
+    addSecureStore(keyStore, /* isKeyStore= */ true, config);
+    addSecureStore(trustStore, /* isKeyStore= */ false, config);
+
+    if (sslRmiRegistry) {
+      config.put("otel.jmx.remote.registry.ssl", "true");
+    }
+
+    if (!customYamlFiles.isEmpty()) {
+      for (String yaml : customYamlFiles) {
+        this.withCopyFileToContainer(MountableFile.forClasspathResource(yaml), yaml);
+      }
+      config.put("otel.jmx.config", String.join(",", customYamlFiles));
+    }
+    return config;
   }
 
   private void addSecureStore(