Merge pull request #1 from SylvainJuge/jmx-scraper-integration-tests

refactor in a single method + reuse available port detect
robsunday · Oct 10, 2024 · da8f4d8 · da8f4d8
2 parents 26efa72 + 898c3c8
commit da8f4d8
Show file tree

Hide file tree

Showing 2 changed files with 25 additions and 15 deletions.
diff --git a/...src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/JmxConnectorBuilderTest.java b/...src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/JmxConnectorBuilderTest.java
@@ -32,8 +32,7 @@ static void afterAll() {
   @Test
   void noAuth() {
     int port = PortSelector.getAvailableRandomPort();
-    try (TestAppContainer app =
-        new TestAppContainer().withJmxPort(port).withJmxAccessibleFromHost()) {
+    try (TestAppContainer app = new TestAppContainer().withHostAccessFixedJmxPort(port)) {
       app.start();
       testConnector(
           () -> JmxConnectorBuilder.createNew(app.getHost(), app.getMappedPort(port)).build());
@@ -46,10 +45,7 @@ void loginPwdAuth() {
     String login = "user";
     String pwd = "t0p!Secret";
     try (TestAppContainer app =
-        new TestAppContainer()
-            .withJmxPort(port)
-            .withJmxAccessibleFromHost()
-            .withUserAuth(login, pwd)) {
+        new TestAppContainer().withHostAccessFixedJmxPort(port).withUserAuth(login, pwd)) {
       app.start();
       testConnector(
           () ->

diff --git a/...craper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/TestAppContainer.java b/...craper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/TestAppContainer.java
@@ -43,17 +43,15 @@ public TestAppContainer() {
         .withCommand("java", "-jar", "/app.jar");
   }
 
+  /**
+   * Configures app container for container-to-container access
+   *
+   * @param port mapped port to use
+   * @return this
+   */
   @CanIgnoreReturnValue
   public TestAppContainer withJmxPort(int port) {
     properties.put("com.sun.management.jmxremote.port", Integer.toString(port));
-    properties.put("com.sun.management.jmxremote.rmi.port", Integer.toString(port));
-
-    // To get host->container JMX connection working docker must expose JMX/RMI port under the same
-    // port number. Because of this testcontainers' standard exposed port randomization approach
-    // can't be used.
-    // Explanation:
-    // https://forums.docker.com/t/exposing-mapped-jmx-ports-from-multiple-containers/5287/6
-    addFixedExposedPort(port, port);
     return this;
   }
 
@@ -64,9 +62,25 @@ public TestAppContainer withUserAuth(String login, String pwd) {
     return this;
   }
 
+  /**
+   * Configures app container for host-to-container access, port will be used as-is from host to
+   * work-around JMX in docker. This is optional on Linux as there is a network route and the
+   * container is accessible, but not on Mac where the container runs in an isolated VM.
+   *
+   * @param port port to use, must be available on host.
+   * @return this
+   */
   @CanIgnoreReturnValue
-  public TestAppContainer withJmxAccessibleFromHost() {
+  public TestAppContainer withHostAccessFixedJmxPort(int port) {
+    // To get host->container JMX connection working docker must expose JMX/RMI port under the same
+    // port number. Because of this testcontainers' standard exposed port randomization approach
+    // can't be used.
+    // Explanation:
+    // https://forums.docker.com/t/exposing-mapped-jmx-ports-from-multiple-containers/5287/6
+    properties.put("com.sun.management.jmxremote.port", Integer.toString(port));
+    properties.put("com.sun.management.jmxremote.rmi.port", Integer.toString(port));
     properties.put("java.rmi.server.hostname", getHost());
+    addFixedExposedPort(port, port);
     return this;
   }