From 49cfb99d7e4a59043cbd92c4cd983b8013fa0e52 Mon Sep 17 00:00:00 2001 From: robsunday Date: Mon, 30 Sep 2024 15:46:56 +0200 Subject: [PATCH 01/10] Additional logger call --- .../io/opentelemetry/contrib/jmxscraper/JmxConnectorBuilder.java | 1 + 1 file changed, 1 insertion(+) diff --git a/jmx-scraper/src/main/java/io/opentelemetry/contrib/jmxscraper/JmxConnectorBuilder.java b/jmx-scraper/src/main/java/io/opentelemetry/contrib/jmxscraper/JmxConnectorBuilder.java index dd509aa13..eebca5fd4 100644 --- a/jmx-scraper/src/main/java/io/opentelemetry/contrib/jmxscraper/JmxConnectorBuilder.java +++ b/jmx-scraper/src/main/java/io/opentelemetry/contrib/jmxscraper/JmxConnectorBuilder.java @@ -138,6 +138,7 @@ private Map buildEnv() { @SuppressWarnings("BanJNDI") private static JMXConnector doConnect(JMXServiceURL url, Map env) throws IOException { + logger.info("Connecting to " + url); return JMXConnectorFactory.connect(url, env); } From a8f8ace496d5166c9dea333da438d0485be4e853 Mon Sep 17 00:00:00 2001 From: robsunday Date: Mon, 30 Sep 2024 16:40:55 +0200 Subject: [PATCH 02/10] Test exception thrown --- .../contrib/jmxscraper/JmxConnectorBuilderTest.java | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/JmxConnectorBuilderTest.java b/jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/JmxConnectorBuilderTest.java index 132605242..4fd1fb6bf 100644 --- a/jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/JmxConnectorBuilderTest.java +++ b/jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/JmxConnectorBuilderTest.java @@ -34,7 +34,10 @@ void noAuth() { try (TestAppContainer app = new TestAppContainer().withNetwork(network).withJmxPort(9990)) { app.start(); testConnector( - () -> JmxConnectorBuilder.createNew(app.getHost(), app.getMappedPort(9990)).build()); + () -> { + throw new RuntimeException("TEST"); + } + ); } } From e1043d11f495db589fb91268533c1790ae237eae Mon Sep 17 00:00:00 2001 From: robsunday Date: Mon, 30 Sep 2024 16:41:55 +0200 Subject: [PATCH 03/10] Spotless fix --- .../contrib/jmxscraper/JmxConnectorBuilderTest.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/JmxConnectorBuilderTest.java b/jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/JmxConnectorBuilderTest.java index 4fd1fb6bf..c5ccd6639 100644 --- a/jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/JmxConnectorBuilderTest.java +++ b/jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/JmxConnectorBuilderTest.java @@ -36,8 +36,7 @@ void noAuth() { testConnector( () -> { throw new RuntimeException("TEST"); - } - ); + }); } } From 26efa72a88b975019f36f944a2856bd570cb993b Mon Sep 17 00:00:00 2001 From: robsunday Date: Thu, 3 Oct 2024 17:07:02 +0200 Subject: [PATCH 04/10] Integration tests are now working on macOS --- .../jmxscraper/JmxConnectorBuilderTest.java | 16 ++++--- .../contrib/jmxscraper/PortSelector.java | 42 +++++++++++++++++++ .../contrib/jmxscraper/TestAppContainer.java | 26 ++++++++---- .../TargetSystemIntegrationTest.java | 33 ++++----------- 4 files changed, 79 insertions(+), 38 deletions(-) create mode 100644 jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/PortSelector.java diff --git a/jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/JmxConnectorBuilderTest.java b/jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/JmxConnectorBuilderTest.java index c5ccd6639..f6589cc10 100644 --- a/jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/JmxConnectorBuilderTest.java +++ b/jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/JmxConnectorBuilderTest.java @@ -31,25 +31,29 @@ static void afterAll() { @Test void noAuth() { - try (TestAppContainer app = new TestAppContainer().withNetwork(network).withJmxPort(9990)) { + int port = PortSelector.getAvailableRandomPort(); + try (TestAppContainer app = + new TestAppContainer().withJmxPort(port).withJmxAccessibleFromHost()) { app.start(); testConnector( - () -> { - throw new RuntimeException("TEST"); - }); + () -> JmxConnectorBuilder.createNew(app.getHost(), app.getMappedPort(port)).build()); } } @Test void loginPwdAuth() { + int port = PortSelector.getAvailableRandomPort(); String login = "user"; String pwd = "t0p!Secret"; try (TestAppContainer app = - new TestAppContainer().withNetwork(network).withJmxPort(9999).withUserAuth(login, pwd)) { + new TestAppContainer() + .withJmxPort(port) + .withJmxAccessibleFromHost() + .withUserAuth(login, pwd)) { app.start(); testConnector( () -> - JmxConnectorBuilder.createNew(app.getHost(), app.getMappedPort(9999)) + JmxConnectorBuilder.createNew(app.getHost(), app.getMappedPort(port)) .userCredentials(login, pwd) .build()); } diff --git a/jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/PortSelector.java b/jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/PortSelector.java new file mode 100644 index 000000000..400b7bb5e --- /dev/null +++ b/jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/PortSelector.java @@ -0,0 +1,42 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.contrib.jmxscraper; + +import java.io.IOException; +import java.net.Socket; +import java.util.Random; + +/** Class used for finding random free network port from range 1024-65535 */ +public class PortSelector { + private static final Random random = new Random(System.currentTimeMillis()); + + private static final int MIN_PORT = 1024; + private static final int MAX_PORT = 65535; + + private PortSelector() {} + + /** + * @return random available TCP port + */ + public static synchronized int getAvailableRandomPort() { + int port; + + do { + port = random.nextInt(MAX_PORT - MIN_PORT + 1) + MIN_PORT; + } while (!isPortAvailable(port)); + + return port; + } + + private static boolean isPortAvailable(int port) { + // see https://stackoverflow.com/a/13826145 for the chosen implementation + try (Socket s = new Socket("localhost", port)) { + return false; + } catch (IOException e) { + return true; + } + } +} diff --git a/jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/TestAppContainer.java b/jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/TestAppContainer.java index a38dd7ace..663a73bd6 100644 --- a/jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/TestAppContainer.java +++ b/jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/TestAppContainer.java @@ -25,7 +25,6 @@ public class TestAppContainer extends GenericContainer { private final Map properties; - private int port; private String login; private String pwd; @@ -46,9 +45,16 @@ public TestAppContainer() { @CanIgnoreReturnValue public TestAppContainer withJmxPort(int port) { - this.port = port; properties.put("com.sun.management.jmxremote.port", Integer.toString(port)); - return this.withExposedPorts(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; } @CanIgnoreReturnValue @@ -58,9 +64,17 @@ public TestAppContainer withUserAuth(String login, String pwd) { return this; } + @CanIgnoreReturnValue + public TestAppContainer withJmxAccessibleFromHost() { + properties.put("java.rmi.server.hostname", getHost()); + return this; + } + @Override public void start() { - + // properties.put("com.sun.management.jmxremote.local.only", "false"); + // properties.put("java.rmi.server.logCalls", "true"); + // // TODO: add support for ssl properties.put("com.sun.management.jmxremote.ssl", "false"); @@ -92,11 +106,9 @@ public void start() { this.withEnv("JAVA_TOOL_OPTIONS", confArgs); - logger().info("Test application JAVA_TOOL_OPTIONS = " + confArgs); + logger().info("Test application JAVA_TOOL_OPTIONS = {}", confArgs); super.start(); - - logger().info("Test application JMX port mapped to {}:{}", getHost(), getMappedPort(port)); } private static Path createPwdFile(String login, String pwd) { diff --git a/jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/target_systems/TargetSystemIntegrationTest.java b/jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/target_systems/TargetSystemIntegrationTest.java index 0552aa3bd..50e7f476a 100644 --- a/jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/target_systems/TargetSystemIntegrationTest.java +++ b/jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/target_systems/TargetSystemIntegrationTest.java @@ -5,24 +5,20 @@ package io.opentelemetry.contrib.jmxscraper.target_systems; -import static org.assertj.core.api.Assertions.assertThat; - import com.linecorp.armeria.server.ServerBuilder; import com.linecorp.armeria.server.grpc.GrpcService; import com.linecorp.armeria.testing.junit5.server.ServerExtension; import io.grpc.stub.StreamObserver; -import io.opentelemetry.contrib.jmxscraper.JmxConnectorBuilder; import io.opentelemetry.contrib.jmxscraper.JmxScraperContainer; +import io.opentelemetry.contrib.jmxscraper.PortSelector; import io.opentelemetry.proto.collector.metrics.v1.ExportMetricsServiceRequest; import io.opentelemetry.proto.collector.metrics.v1.ExportMetricsServiceResponse; import io.opentelemetry.proto.collector.metrics.v1.MetricsServiceGrpc; -import java.io.IOException; import java.util.ArrayList; import java.util.List; import java.util.concurrent.BlockingQueue; import java.util.concurrent.ExecutionException; import java.util.concurrent.LinkedBlockingDeque; -import javax.management.remote.JMXConnector; import org.junit.jupiter.api.AfterAll; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeAll; @@ -35,8 +31,8 @@ import org.testcontainers.containers.output.Slf4jLogConsumer; public abstract class TargetSystemIntegrationTest { - - private static final Logger logger = LoggerFactory.getLogger(TargetSystemIntegrationTest.class); + private static final Logger targetSystemLogger = LoggerFactory.getLogger("TargetSystemContainer"); + private static final Logger jmxScraperLogger = LoggerFactory.getLogger("JmxScraperContainer"); private static final String TARGET_SYSTEM_NETWORK_ALIAS = "targetsystem"; private static String otlpEndpoint; @@ -54,7 +50,6 @@ public abstract class TargetSystemIntegrationTest { private JmxScraperContainer scraper; private static final String OTLP_HOST = "host.testcontainers.internal"; - private static final int JMX_PORT = 9999; @BeforeAll static void beforeAll() { @@ -90,32 +85,20 @@ void afterEach() { @Test void endToEndTest() { + int jmxPort = PortSelector.getAvailableRandomPort(); target = - createTargetContainer(JMX_PORT) - .withLogConsumer(new Slf4jLogConsumer(logger)) + createTargetContainer(jmxPort) + .withLogConsumer(new Slf4jLogConsumer(targetSystemLogger)) .withNetwork(network) - .withExposedPorts(JMX_PORT) .withNetworkAliases(TARGET_SYSTEM_NETWORK_ALIAS); target.start(); - String targetHost = target.getHost(); - Integer targetPort = target.getMappedPort(JMX_PORT); - logger.info( - "Target system started, JMX port: {} mapped to {}:{}", JMX_PORT, targetHost, targetPort); - - // TODO : wait for metrics to be sent and add assertions on what is being captured - // for now we just test that we can connect to remote JMX using our client. - try (JMXConnector connector = JmxConnectorBuilder.createNew(targetHost, targetPort).build()) { - assertThat(connector.getMBeanServerConnection()).isNotNull(); - } catch (IOException e) { - throw new RuntimeException(e); - } - scraper = new JmxScraperContainer(otlpEndpoint) + .withLogConsumer(new Slf4jLogConsumer(jmxScraperLogger)) .withNetwork(network) - .withService(TARGET_SYSTEM_NETWORK_ALIAS, JMX_PORT); + .withService(TARGET_SYSTEM_NETWORK_ALIAS, jmxPort); scraper = customizeScraperContainer(scraper); scraper.start(); From 12e65333a0cb3f4f99763288a998267abd84d080 Mon Sep 17 00:00:00 2001 From: Sylvain Juge <763082+SylvainJuge@users.noreply.github.com> Date: Wed, 9 Oct 2024 14:57:01 +0200 Subject: [PATCH 05/10] refactor in a single method --- .../jmxscraper/JmxConnectorBuilderTest.java | 5 ++- .../contrib/jmxscraper/TestAppContainer.java | 32 +++++++++++++------ 2 files changed, 25 insertions(+), 12 deletions(-) diff --git a/jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/JmxConnectorBuilderTest.java b/jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/JmxConnectorBuilderTest.java index f6589cc10..63483fd00 100644 --- a/jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/JmxConnectorBuilderTest.java +++ b/jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/JmxConnectorBuilderTest.java @@ -33,7 +33,7 @@ static void afterAll() { void noAuth() { int port = PortSelector.getAvailableRandomPort(); try (TestAppContainer app = - new TestAppContainer().withJmxPort(port).withJmxAccessibleFromHost()) { + new TestAppContainer().withHostAccessFixedJmxPort(port)) { app.start(); testConnector( () -> JmxConnectorBuilder.createNew(app.getHost(), app.getMappedPort(port)).build()); @@ -47,8 +47,7 @@ void loginPwdAuth() { String pwd = "t0p!Secret"; try (TestAppContainer app = new TestAppContainer() - .withJmxPort(port) - .withJmxAccessibleFromHost() + .withHostAccessFixedJmxPort(port) .withUserAuth(login, pwd)) { app.start(); testConnector( diff --git a/jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/TestAppContainer.java b/jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/TestAppContainer.java index 663a73bd6..c95eba3e7 100644 --- a/jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/TestAppContainer.java +++ b/jmx-scraper/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; } From 172846c5f2460e2c4578574b8e4404b87701e784 Mon Sep 17 00:00:00 2001 From: Sylvain Juge <763082+SylvainJuge@users.noreply.github.com> Date: Wed, 9 Oct 2024 15:14:09 +0200 Subject: [PATCH 06/10] cleanup and reuse free port impl. --- .../jmxscraper/JmxConnectorBuilderTest.java | 12 +++--- .../contrib/jmxscraper/PortSelector.java | 42 ------------------- .../contrib/jmxscraper/TestAppContainer.java | 4 +- .../TargetSystemIntegrationTest.java | 4 +- 4 files changed, 9 insertions(+), 53 deletions(-) delete mode 100644 jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/PortSelector.java diff --git a/jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/JmxConnectorBuilderTest.java b/jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/JmxConnectorBuilderTest.java index 63483fd00..c785f41ee 100644 --- a/jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/JmxConnectorBuilderTest.java +++ b/jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/JmxConnectorBuilderTest.java @@ -7,6 +7,7 @@ import static org.assertj.core.api.Assertions.assertThat; +import com.linecorp.armeria.internal.common.util.PortUtil; import java.io.IOException; import javax.management.ObjectName; import javax.management.remote.JMXConnector; @@ -31,9 +32,8 @@ static void afterAll() { @Test void noAuth() { - int port = PortSelector.getAvailableRandomPort(); - try (TestAppContainer app = - new TestAppContainer().withHostAccessFixedJmxPort(port)) { + int port = PortUtil.unusedTcpPort(); + try (TestAppContainer app = new TestAppContainer().withHostAccessFixedJmxPort(port)) { app.start(); testConnector( () -> JmxConnectorBuilder.createNew(app.getHost(), app.getMappedPort(port)).build()); @@ -42,13 +42,11 @@ void noAuth() { @Test void loginPwdAuth() { - int port = PortSelector.getAvailableRandomPort(); + int port = PortUtil.unusedTcpPort(); String login = "user"; String pwd = "t0p!Secret"; try (TestAppContainer app = - new TestAppContainer() - .withHostAccessFixedJmxPort(port) - .withUserAuth(login, pwd)) { + new TestAppContainer().withHostAccessFixedJmxPort(port).withUserAuth(login, pwd)) { app.start(); testConnector( () -> diff --git a/jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/PortSelector.java b/jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/PortSelector.java deleted file mode 100644 index 400b7bb5e..000000000 --- a/jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/PortSelector.java +++ /dev/null @@ -1,42 +0,0 @@ -/* - * Copyright The OpenTelemetry Authors - * SPDX-License-Identifier: Apache-2.0 - */ - -package io.opentelemetry.contrib.jmxscraper; - -import java.io.IOException; -import java.net.Socket; -import java.util.Random; - -/** Class used for finding random free network port from range 1024-65535 */ -public class PortSelector { - private static final Random random = new Random(System.currentTimeMillis()); - - private static final int MIN_PORT = 1024; - private static final int MAX_PORT = 65535; - - private PortSelector() {} - - /** - * @return random available TCP port - */ - public static synchronized int getAvailableRandomPort() { - int port; - - do { - port = random.nextInt(MAX_PORT - MIN_PORT + 1) + MIN_PORT; - } while (!isPortAvailable(port)); - - return port; - } - - private static boolean isPortAvailable(int port) { - // see https://stackoverflow.com/a/13826145 for the chosen implementation - try (Socket s = new Socket("localhost", port)) { - return false; - } catch (IOException e) { - return true; - } - } -} diff --git a/jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/TestAppContainer.java b/jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/TestAppContainer.java index c95eba3e7..be92a0a96 100644 --- a/jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/TestAppContainer.java +++ b/jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/TestAppContainer.java @@ -64,8 +64,8 @@ public TestAppContainer withUserAuth(String login, String pwd) { /** * 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. + * 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 diff --git a/jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/target_systems/TargetSystemIntegrationTest.java b/jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/target_systems/TargetSystemIntegrationTest.java index 50e7f476a..1dba3d70a 100644 --- a/jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/target_systems/TargetSystemIntegrationTest.java +++ b/jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/target_systems/TargetSystemIntegrationTest.java @@ -5,12 +5,12 @@ package io.opentelemetry.contrib.jmxscraper.target_systems; +import com.linecorp.armeria.internal.common.util.PortUtil; import com.linecorp.armeria.server.ServerBuilder; import com.linecorp.armeria.server.grpc.GrpcService; import com.linecorp.armeria.testing.junit5.server.ServerExtension; import io.grpc.stub.StreamObserver; import io.opentelemetry.contrib.jmxscraper.JmxScraperContainer; -import io.opentelemetry.contrib.jmxscraper.PortSelector; import io.opentelemetry.proto.collector.metrics.v1.ExportMetricsServiceRequest; import io.opentelemetry.proto.collector.metrics.v1.ExportMetricsServiceResponse; import io.opentelemetry.proto.collector.metrics.v1.MetricsServiceGrpc; @@ -85,7 +85,7 @@ void afterEach() { @Test void endToEndTest() { - int jmxPort = PortSelector.getAvailableRandomPort(); + int jmxPort = PortUtil.unusedTcpPort(); target = createTargetContainer(jmxPort) From 898c3c8500feee04910d07fd16dcf6ee0b93af9e Mon Sep 17 00:00:00 2001 From: Sylvain Juge <763082+SylvainJuge@users.noreply.github.com> Date: Thu, 10 Oct 2024 10:28:42 +0200 Subject: [PATCH 07/10] revert port selector class --- .../jmxscraper/JmxConnectorBuilderTest.java | 5 +-- .../contrib/jmxscraper/PortSelector.java | 42 +++++++++++++++++++ .../TargetSystemIntegrationTest.java | 4 +- 3 files changed, 46 insertions(+), 5 deletions(-) create mode 100644 jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/PortSelector.java diff --git a/jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/JmxConnectorBuilderTest.java b/jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/JmxConnectorBuilderTest.java index c785f41ee..5766cd890 100644 --- a/jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/JmxConnectorBuilderTest.java +++ b/jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/JmxConnectorBuilderTest.java @@ -7,7 +7,6 @@ import static org.assertj.core.api.Assertions.assertThat; -import com.linecorp.armeria.internal.common.util.PortUtil; import java.io.IOException; import javax.management.ObjectName; import javax.management.remote.JMXConnector; @@ -32,7 +31,7 @@ static void afterAll() { @Test void noAuth() { - int port = PortUtil.unusedTcpPort(); + int port = PortSelector.getAvailableRandomPort(); try (TestAppContainer app = new TestAppContainer().withHostAccessFixedJmxPort(port)) { app.start(); testConnector( @@ -42,7 +41,7 @@ void noAuth() { @Test void loginPwdAuth() { - int port = PortUtil.unusedTcpPort(); + int port = PortSelector.getAvailableRandomPort(); String login = "user"; String pwd = "t0p!Secret"; try (TestAppContainer app = diff --git a/jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/PortSelector.java b/jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/PortSelector.java new file mode 100644 index 000000000..400b7bb5e --- /dev/null +++ b/jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/PortSelector.java @@ -0,0 +1,42 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.contrib.jmxscraper; + +import java.io.IOException; +import java.net.Socket; +import java.util.Random; + +/** Class used for finding random free network port from range 1024-65535 */ +public class PortSelector { + private static final Random random = new Random(System.currentTimeMillis()); + + private static final int MIN_PORT = 1024; + private static final int MAX_PORT = 65535; + + private PortSelector() {} + + /** + * @return random available TCP port + */ + public static synchronized int getAvailableRandomPort() { + int port; + + do { + port = random.nextInt(MAX_PORT - MIN_PORT + 1) + MIN_PORT; + } while (!isPortAvailable(port)); + + return port; + } + + private static boolean isPortAvailable(int port) { + // see https://stackoverflow.com/a/13826145 for the chosen implementation + try (Socket s = new Socket("localhost", port)) { + return false; + } catch (IOException e) { + return true; + } + } +} diff --git a/jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/target_systems/TargetSystemIntegrationTest.java b/jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/target_systems/TargetSystemIntegrationTest.java index 1dba3d70a..50e7f476a 100644 --- a/jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/target_systems/TargetSystemIntegrationTest.java +++ b/jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/target_systems/TargetSystemIntegrationTest.java @@ -5,12 +5,12 @@ package io.opentelemetry.contrib.jmxscraper.target_systems; -import com.linecorp.armeria.internal.common.util.PortUtil; import com.linecorp.armeria.server.ServerBuilder; import com.linecorp.armeria.server.grpc.GrpcService; import com.linecorp.armeria.testing.junit5.server.ServerExtension; import io.grpc.stub.StreamObserver; import io.opentelemetry.contrib.jmxscraper.JmxScraperContainer; +import io.opentelemetry.contrib.jmxscraper.PortSelector; import io.opentelemetry.proto.collector.metrics.v1.ExportMetricsServiceRequest; import io.opentelemetry.proto.collector.metrics.v1.ExportMetricsServiceResponse; import io.opentelemetry.proto.collector.metrics.v1.MetricsServiceGrpc; @@ -85,7 +85,7 @@ void afterEach() { @Test void endToEndTest() { - int jmxPort = PortUtil.unusedTcpPort(); + int jmxPort = PortSelector.getAvailableRandomPort(); target = createTargetContainer(jmxPort) From 2260258dae67e2f81566c3167f57715d4229727a Mon Sep 17 00:00:00 2001 From: Sylvain Juge <763082+SylvainJuge@users.noreply.github.com> Date: Fri, 11 Oct 2024 09:43:46 +0200 Subject: [PATCH 08/10] use fixed port for container-to-container --- .../target_systems/TargetSystemIntegrationTest.java | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/target_systems/TargetSystemIntegrationTest.java b/jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/target_systems/TargetSystemIntegrationTest.java index 50e7f476a..e4542ce4b 100644 --- a/jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/target_systems/TargetSystemIntegrationTest.java +++ b/jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/target_systems/TargetSystemIntegrationTest.java @@ -51,6 +51,10 @@ public abstract class TargetSystemIntegrationTest { private static final String OTLP_HOST = "host.testcontainers.internal"; + // JMX communication only happens between container, and we don't have to use JMX + // from host to container, we can use a fixed port. + private static final int JMX_PORT = 9999; + @BeforeAll static void beforeAll() { network = Network.newNetwork(); @@ -85,10 +89,9 @@ void afterEach() { @Test void endToEndTest() { - int jmxPort = PortSelector.getAvailableRandomPort(); target = - createTargetContainer(jmxPort) + createTargetContainer(JMX_PORT) .withLogConsumer(new Slf4jLogConsumer(targetSystemLogger)) .withNetwork(network) .withNetworkAliases(TARGET_SYSTEM_NETWORK_ALIAS); @@ -98,7 +101,7 @@ void endToEndTest() { new JmxScraperContainer(otlpEndpoint) .withLogConsumer(new Slf4jLogConsumer(jmxScraperLogger)) .withNetwork(network) - .withService(TARGET_SYSTEM_NETWORK_ALIAS, jmxPort); + .withService(TARGET_SYSTEM_NETWORK_ALIAS, JMX_PORT); scraper = customizeScraperContainer(scraper); scraper.start(); From df4e1498777fed7eaa56238c7448086e701c7dd8 Mon Sep 17 00:00:00 2001 From: robsunday Date: Fri, 11 Oct 2024 10:30:08 +0200 Subject: [PATCH 09/10] Spotless fix --- .../jmxscraper/target_systems/TargetSystemIntegrationTest.java | 1 - 1 file changed, 1 deletion(-) diff --git a/jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/target_systems/TargetSystemIntegrationTest.java b/jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/target_systems/TargetSystemIntegrationTest.java index e4542ce4b..2cce282f9 100644 --- a/jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/target_systems/TargetSystemIntegrationTest.java +++ b/jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/target_systems/TargetSystemIntegrationTest.java @@ -10,7 +10,6 @@ import com.linecorp.armeria.testing.junit5.server.ServerExtension; import io.grpc.stub.StreamObserver; import io.opentelemetry.contrib.jmxscraper.JmxScraperContainer; -import io.opentelemetry.contrib.jmxscraper.PortSelector; import io.opentelemetry.proto.collector.metrics.v1.ExportMetricsServiceRequest; import io.opentelemetry.proto.collector.metrics.v1.ExportMetricsServiceResponse; import io.opentelemetry.proto.collector.metrics.v1.MetricsServiceGrpc; From 93944b77f34ff574623ac94adfa173fdff9ef8c6 Mon Sep 17 00:00:00 2001 From: robsunday Date: Tue, 15 Oct 2024 15:29:05 +0200 Subject: [PATCH 10/10] Code review changes --- .../io/opentelemetry/contrib/jmxscraper/TestAppContainer.java | 3 --- 1 file changed, 3 deletions(-) diff --git a/jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/TestAppContainer.java b/jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/TestAppContainer.java index be92a0a96..990a316c9 100644 --- a/jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/TestAppContainer.java +++ b/jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/TestAppContainer.java @@ -86,9 +86,6 @@ public TestAppContainer withHostAccessFixedJmxPort(int port) { @Override public void start() { - // properties.put("com.sun.management.jmxremote.local.only", "false"); - // properties.put("java.rmi.server.logCalls", "true"); - // // TODO: add support for ssl properties.put("com.sun.management.jmxremote.ssl", "false");