From 0e0d1060ba4e13d91f9e4e59271445fd6f967484 Mon Sep 17 00:00:00 2001 From: Sylvain Juge <763082+SylvainJuge@users.noreply.github.com> Date: Fri, 31 Jan 2025 17:19:10 +0100 Subject: [PATCH 1/8] add test mode with CLI args --- .../contrib/jmxscraper/JmxScraper.java | 40 ++++++++++++++++--- .../contrib/jmxscraper/JmxScraperTest.java | 10 ++--- 2 files changed, 39 insertions(+), 11 deletions(-) diff --git a/jmx-scraper/src/main/java/io/opentelemetry/contrib/jmxscraper/JmxScraper.java b/jmx-scraper/src/main/java/io/opentelemetry/contrib/jmxscraper/JmxScraper.java index 29156c67c..759ab56fc 100644 --- a/jmx-scraper/src/main/java/io/opentelemetry/contrib/jmxscraper/JmxScraper.java +++ b/jmx-scraper/src/main/java/io/opentelemetry/contrib/jmxscraper/JmxScraper.java @@ -19,6 +19,7 @@ import java.io.InputStream; import java.nio.file.Files; import java.nio.file.Paths; +import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; import java.util.List; @@ -34,6 +35,7 @@ public class JmxScraper { private static final Logger logger = Logger.getLogger(JmxScraper.class.getName()); private static final String CONFIG_ARG = "-config"; + private static final String TEST_ARG = "-test"; private final JmxConnectorBuilder client; private final JmxMetricInsight service; @@ -52,8 +54,11 @@ public static void main(String[] args) { // set log format System.setProperty("java.util.logging.SimpleFormatter.format", "%1$tF %1$tT %4$s %5$s%n"); + List effectiveArgs = new ArrayList<>(Arrays.asList(args)); + boolean testMode = effectiveArgs.remove(TEST_ARG); + try { - Properties argsConfig = parseArgs(Arrays.asList(args)); + Properties argsConfig = parseArgsConfig(effectiveArgs); propagateToSystemProperties(argsConfig); // auto-configure and register SDK @@ -78,16 +83,21 @@ public static void main(String[] args) { Optional.ofNullable(scraperConfig.getUsername()).ifPresent(connectorBuilder::withUser); Optional.ofNullable(scraperConfig.getPassword()).ifPresent(connectorBuilder::withPassword); - JmxScraper jmxScraper = new JmxScraper(connectorBuilder, service, scraperConfig); - jmxScraper.start(); + if(testMode) { + System.exit(testConnection(connectorBuilder) ? 0 : 1); + } else { + JmxScraper jmxScraper = new JmxScraper(connectorBuilder, service, scraperConfig); + jmxScraper.start(); + } } catch (ConfigurationException e) { logger.log(Level.SEVERE, "invalid configuration ", e); System.exit(1); } catch (InvalidArgumentException e) { logger.log(Level.SEVERE, "invalid configuration provided through arguments", e); logger.info( - "Usage: java -jar " - + "-config "); + "Usage: java -jar [-test] [-config ]"); + logger.info(" -test test JMX connection with provided configuration and exit"); + logger.info(" -config provide configuration, where is - for stdin, or " ); System.exit(1); } catch (IOException e) { logger.log(Level.SEVERE, "Unable to connect ", e); @@ -98,6 +108,24 @@ public static void main(String[] args) { } } + private static boolean testConnection(JmxConnectorBuilder connectorBuilder) { + try (JMXConnector connector = connectorBuilder.build()) { + + MBeanServerConnection connection = connector.getMBeanServerConnection(); + Integer mbeanCount = connection.getMBeanCount(); + if (mbeanCount > 0) { + logger.log(Level.INFO, "JMX connection test OK"); + return true; + } else { + logger.log(Level.SEVERE, "JMX connection test ERROR"); + return false; + } + } catch (IOException e) { + logger.log(Level.SEVERE, "JMX connection test ERROR", e); + return false; + } + } + // package private for testing static void propagateToSystemProperties(Properties properties) { for (Map.Entry entry : properties.entrySet()) { @@ -116,7 +144,7 @@ static void propagateToSystemProperties(Properties properties) { * * @param args application commandline arguments */ - static Properties parseArgs(List args) throws InvalidArgumentException { + static Properties parseArgsConfig(List args) throws InvalidArgumentException { if (args.isEmpty()) { // empty properties from stdin or external file diff --git a/jmx-scraper/src/test/java/io/opentelemetry/contrib/jmxscraper/JmxScraperTest.java b/jmx-scraper/src/test/java/io/opentelemetry/contrib/jmxscraper/JmxScraperTest.java index 1532715d9..6b07e98dc 100644 --- a/jmx-scraper/src/test/java/io/opentelemetry/contrib/jmxscraper/JmxScraperTest.java +++ b/jmx-scraper/src/test/java/io/opentelemetry/contrib/jmxscraper/JmxScraperTest.java @@ -30,8 +30,8 @@ void shouldThrowExceptionWhenInvalidCommandLineArgsProvided() { @Test void emptyArgumentsAllowed() throws InvalidArgumentException { - assertThat(JmxScraper.parseArgs(Collections.emptyList())) - .describedAs("empty arguments allowed to use JVM properties") + assertThat(JmxScraper.parseArgsConfig(Collections.emptyList())) + .describedAs("empty config allowed to use JVM properties") .isEmpty(); } @@ -41,7 +41,7 @@ void shouldThrowExceptionWhenMissingProperties() { } private static void testInvalidArguments(String... args) { - assertThatThrownBy(() -> JmxScraper.parseArgs(Arrays.asList(args))) + assertThatThrownBy(() -> JmxScraper.parseArgsConfig(Arrays.asList(args))) .isInstanceOf(InvalidArgumentException.class); } @@ -53,7 +53,7 @@ void shouldCreateConfig_propertiesLoadedFromFile() throws InvalidArgumentExcepti List args = Arrays.asList("-config", filePath); // When - Properties parsedConfig = JmxScraper.parseArgs(args); + Properties parsedConfig = JmxScraper.parseArgsConfig(args); JmxScraperConfig config = JmxScraperConfig.fromConfig(TestUtil.configProperties(parsedConfig)); // Then @@ -72,7 +72,7 @@ void shouldCreateConfig_propertiesLoadedFromStdIn() throws InvalidArgumentExcept List args = Arrays.asList("-config", "-"); // When - Properties parsedConfig = JmxScraper.parseArgs(args); + Properties parsedConfig = JmxScraper.parseArgsConfig(args); JmxScraperConfig config = JmxScraperConfig.fromConfig(TestUtil.configProperties(parsedConfig)); From 63886843266e5accc97dd9b97c33a2d9ead56e57 Mon Sep 17 00:00:00 2001 From: Sylvain Juge <763082+SylvainJuge@users.noreply.github.com> Date: Fri, 31 Jan 2025 17:22:38 +0100 Subject: [PATCH 2/8] allow test mode with jmx scraper container --- .../contrib/jmxscraper/JmxScraperContainer.java | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/JmxScraperContainer.java b/jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/JmxScraperContainer.java index 455b53c73..eb8f1c9b2 100644 --- a/jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/JmxScraperContainer.java +++ b/jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/JmxScraperContainer.java @@ -28,6 +28,7 @@ public class JmxScraperContainer extends GenericContainer { private String user; private String password; private final List extraJars; + private boolean testJmx; public JmxScraperContainer(String otlpEndpoint, String baseImage) { super(baseImage); @@ -108,6 +109,12 @@ public JmxScraperContainer withCustomYaml(String yamlPath) { return this; } + @CanIgnoreReturnValue + public JmxScraperContainer withTestJmx() { + this.testJmx = true; + return this; + } + @Override public void start() { // for now only configure through JVM args @@ -152,6 +159,10 @@ public void start() { arguments.add("io.opentelemetry.contrib.jmxscraper.JmxScraper"); } + if(testJmx) { + arguments.add("-test"); + } + this.withCommand(arguments.toArray(new String[0])); logger().info("Starting scraper with command: " + String.join(" ", arguments)); From f213ecd45efd6b795cc31dc336ce6dc6bf4728f1 Mon Sep 17 00:00:00 2001 From: Sylvain Juge <763082+SylvainJuge@users.noreply.github.com> Date: Fri, 31 Jan 2025 22:33:31 +0100 Subject: [PATCH 3/8] test connection with container-to-container --- .../contrib/jmxscraper/JmxConnectionTest.java | 131 ++++++++++++++++++ .../jmxscraper/JmxScraperContainer.java | 12 +- .../contrib/jmxscraper/JmxScraper.java | 8 +- 3 files changed, 142 insertions(+), 9 deletions(-) create mode 100644 jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/JmxConnectionTest.java diff --git a/jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/JmxConnectionTest.java b/jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/JmxConnectionTest.java new file mode 100644 index 000000000..f7fce97cd --- /dev/null +++ b/jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/JmxConnectionTest.java @@ -0,0 +1,131 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.contrib.jmxscraper; + +import static org.assertj.core.api.Assertions.assertThat; + +import java.util.function.Function; +import org.junit.jupiter.api.AfterAll; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.Test; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import org.testcontainers.containers.GenericContainer; +import org.testcontainers.containers.Network; +import org.testcontainers.containers.output.Slf4jLogConsumer; + +/** + * Tests all supported ways to connect to remote JMX interface. This indirectly tests + * JmxConnectionBuilder and relies on containers to minimize the JMX/RMI network complications which + * are not NAT-friendly. + */ +public class JmxConnectionTest { + + // OTLP endpoint is not used in test mode, but still has to be provided + private static final String DUMMY_OTLP_ENDPOINT = "http://dummy-otlp-endpoint:8080/"; + private static final String SCRAPER_BASE_IMAGE = "openjdk:8u342-jre-slim"; + + private static final int JMX_PORT = 9999; + private static final String APP_HOST = "app"; + + private static final Logger jmxScraperLogger = LoggerFactory.getLogger("JmxScraperContainer"); + private static final Logger appLogger = LoggerFactory.getLogger("TestAppContainer"); + + private static Network network; + + @BeforeAll + static void beforeAll() { + network = Network.newNetwork(); + } + + @AfterAll + static void afterAll() { + network.close(); + } + + @Test + void connectionError() { + try (JmxScraperContainer scraper = scraperContainer().withRmiServiceUrl("unknown_host", 1234)) { + scraper.start(); + waitTerminated(scraper); + checkConnectionLogs(scraper, /* expectedOk= */ false); + } + } + + @Test + void connectNoAuth() { + connectionTest( + app -> app.withJmxPort(JMX_PORT), scraper -> scraper.withRmiServiceUrl(APP_HOST, JMX_PORT)); + } + + @Test + void userPassword() { + String login = "user"; + String pwd = "t0p!Secret"; + connectionTest( + app -> app.withJmxPort(JMX_PORT).withUserAuth(login, pwd), + scraper -> scraper.withRmiServiceUrl(APP_HOST, JMX_PORT).withUser(login).withPassword(pwd)); + } + + private static void connectionTest( + Function customizeApp, + Function customizeScraper) { + try (TestAppContainer app = customizeApp.apply(appContainer())) { + app.start(); + try (JmxScraperContainer scraper = customizeScraper.apply(scraperContainer())) { + scraper.start(); + waitTerminated(scraper); + checkConnectionLogs(scraper, /* expectedOk= */ true); + } + } + } + + private static void checkConnectionLogs(JmxScraperContainer scraper, boolean expectedOk) { + + String[] logLines = scraper.getLogs().split("\n"); + String lastLine = logLines[logLines.length - 1]; + + if (expectedOk) { + assertThat(lastLine) + .describedAs("should log connection success") + .endsWith("JMX connection test OK"); + } else { + assertThat(lastLine) + .describedAs("should log connection failure") + .endsWith("JMX connection test ERROR"); + } + } + + private static void waitTerminated(GenericContainer container) { + int retries = 10; + while (retries > 0 && container.isRunning()) { + retries--; + try { + Thread.sleep(100); + } catch (InterruptedException e) { + throw new RuntimeException(e); + } + } + assertThat(retries) + .describedAs("container should stop when testing connection") + .isNotEqualTo(0); + } + + private static JmxScraperContainer scraperContainer() { + return new JmxScraperContainer(DUMMY_OTLP_ENDPOINT, SCRAPER_BASE_IMAGE) + .withLogConsumer(new Slf4jLogConsumer(jmxScraperLogger)) + .withNetwork(network) + .withTargetSystem("jvm") + .withTestJmx(); + } + + private static TestAppContainer appContainer() { + return new TestAppContainer() + .withLogConsumer(new Slf4jLogConsumer(appLogger)) + .withNetwork(network) + .withNetworkAliases(APP_HOST); + } +} diff --git a/jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/JmxScraperContainer.java b/jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/JmxScraperContainer.java index eb8f1c9b2..691c2fa08 100644 --- a/jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/JmxScraperContainer.java +++ b/jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/JmxScraperContainer.java @@ -36,10 +36,7 @@ public JmxScraperContainer(String otlpEndpoint, String baseImage) { String scraperJarPath = System.getProperty("shadow.jar.path"); assertThat(scraperJarPath).isNotNull(); - this.withCopyFileToContainer(MountableFile.forHostPath(scraperJarPath), "/scraper.jar") - .waitingFor( - Wait.forLogMessage(".*JMX scraping started.*", 1) - .withStartupTimeout(Duration.ofSeconds(10))); + this.withCopyFileToContainer(MountableFile.forHostPath(scraperJarPath), "/scraper.jar"); this.endpoint = otlpEndpoint; this.targetSystems = new HashSet<>(); @@ -159,8 +156,13 @@ public void start() { arguments.add("io.opentelemetry.contrib.jmxscraper.JmxScraper"); } - if(testJmx) { + if (testJmx) { arguments.add("-test"); + this.waitingFor(Wait.forLogMessage(".*JMX connection test.*", 1)); + } else { + this.waitingFor( + Wait.forLogMessage(".*JMX scraping started.*", 1) + .withStartupTimeout(Duration.ofSeconds(10))); } this.withCommand(arguments.toArray(new String[0])); diff --git a/jmx-scraper/src/main/java/io/opentelemetry/contrib/jmxscraper/JmxScraper.java b/jmx-scraper/src/main/java/io/opentelemetry/contrib/jmxscraper/JmxScraper.java index 759ab56fc..0c88fdec0 100644 --- a/jmx-scraper/src/main/java/io/opentelemetry/contrib/jmxscraper/JmxScraper.java +++ b/jmx-scraper/src/main/java/io/opentelemetry/contrib/jmxscraper/JmxScraper.java @@ -83,7 +83,7 @@ public static void main(String[] args) { Optional.ofNullable(scraperConfig.getUsername()).ifPresent(connectorBuilder::withUser); Optional.ofNullable(scraperConfig.getPassword()).ifPresent(connectorBuilder::withPassword); - if(testMode) { + if (testMode) { System.exit(testConnection(connectorBuilder) ? 0 : 1); } else { JmxScraper jmxScraper = new JmxScraper(connectorBuilder, service, scraperConfig); @@ -94,10 +94,10 @@ public static void main(String[] args) { System.exit(1); } catch (InvalidArgumentException e) { logger.log(Level.SEVERE, "invalid configuration provided through arguments", e); - logger.info( - "Usage: java -jar [-test] [-config ]"); + logger.info("Usage: java -jar [-test] [-config ]"); logger.info(" -test test JMX connection with provided configuration and exit"); - logger.info(" -config provide configuration, where is - for stdin, or " ); + logger.info( + " -config provide configuration, where is - for stdin, or "); System.exit(1); } catch (IOException e) { logger.log(Level.SEVERE, "Unable to connect ", e); From 20753143c5df26cd604047718e47ed16be654c5c Mon Sep 17 00:00:00 2001 From: Sylvain Juge <763082+SylvainJuge@users.noreply.github.com> Date: Fri, 31 Jan 2025 22:36:50 +0100 Subject: [PATCH 4/8] remove connection test + fixed port mapping --- .../jmxscraper/JmxConnectorBuilderTest.java | 95 ------------------- .../contrib/jmxscraper/TestAppContainer.java | 22 ----- 2 files changed, 117 deletions(-) delete mode 100644 jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/JmxConnectorBuilderTest.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 deleted file mode 100644 index f3f931e58..000000000 --- a/jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/JmxConnectorBuilderTest.java +++ /dev/null @@ -1,95 +0,0 @@ -/* - * Copyright The OpenTelemetry Authors - * SPDX-License-Identifier: Apache-2.0 - */ - -package io.opentelemetry.contrib.jmxscraper; - -import static org.assertj.core.api.Assertions.assertThat; - -import java.io.IOException; -import javax.management.ObjectName; -import javax.management.remote.JMXConnector; -import org.junit.jupiter.api.AfterAll; -import org.junit.jupiter.api.BeforeAll; -import org.junit.jupiter.api.Test; -import org.testcontainers.containers.Network; - -public class JmxConnectorBuilderTest { - - private static Network network; - - @BeforeAll - static void beforeAll() { - network = Network.newNetwork(); - } - - @AfterAll - static void afterAll() { - network.close(); - } - - @Test - void noAuth() { - int port = PortSelector.getAvailableRandomPort(); - try (TestAppContainer app = new TestAppContainer().withHostAccessFixedJmxPort(port)) { - app.start(); - testConnector( - () -> 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().withHostAccessFixedJmxPort(port).withUserAuth(login, pwd)) { - app.start(); - testConnector( - () -> - JmxConnectorBuilder.createNew(app.getHost(), app.getMappedPort(port)) - .withUser(login) - .withPassword(pwd) - .build()); - } - } - - @Test - void serverSSL() { - // TODO: test with SSL enabled as RMI registry seems to work differently with SSL - - // create keypair (public,private) - // create server keystore with private key - // configure server keystore - // - // create client truststore with public key - // can we configure to use a custom truststore ??? - // connect to server - } - - private static void testConnector(ConnectorSupplier connectorSupplier) { - try (JMXConnector connector = connectorSupplier.get()) { - assertThat(connector.getMBeanServerConnection()) - .isNotNull() - .satisfies( - connection -> { - try { - ObjectName name = new ObjectName("io.opentelemetry.test:name=TestApp"); - Object value = connection.getAttribute(name, "IntValue"); - assertThat(value).isEqualTo(42); - } catch (Exception e) { - throw new RuntimeException(e); - } - }); - - } catch (IOException e) { - throw new RuntimeException(e); - } - } - - private interface ConnectorSupplier { - JMXConnector get() throws IOException; - } -} 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 649153a41..ee88451fe 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 @@ -61,28 +61,6 @@ 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 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; - } - @Override public void start() { // TODO: add support for ssl From f9841f29f33cea09ae60ff0d78bdbe571e574e3a Mon Sep 17 00:00:00 2001 From: Sylvain Juge <763082+SylvainJuge@users.noreply.github.com> Date: Fri, 31 Jan 2025 22:39:48 +0100 Subject: [PATCH 5/8] add a few comments for clarity --- .../io/opentelemetry/contrib/jmxscraper/JmxConnectionTest.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/JmxConnectionTest.java b/jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/JmxConnectionTest.java index f7fce97cd..8509ab043 100644 --- a/jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/JmxConnectionTest.java +++ b/jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/JmxConnectionTest.java @@ -118,7 +118,9 @@ private static JmxScraperContainer scraperContainer() { return new JmxScraperContainer(DUMMY_OTLP_ENDPOINT, SCRAPER_BASE_IMAGE) .withLogConsumer(new Slf4jLogConsumer(jmxScraperLogger)) .withNetwork(network) + // mandatory to have a target system even if we don't collect metrics .withTargetSystem("jvm") + // we are only testing JMX connection here .withTestJmx(); } From 5ade3bddcd278db794e57e71500712d7eaa6ace5 Mon Sep 17 00:00:00 2001 From: Sylvain Juge <763082+SylvainJuge@users.noreply.github.com> Date: Tue, 4 Feb 2025 14:26:21 +0100 Subject: [PATCH 6/8] rename a bit --- .../io/opentelemetry/contrib/jmxscraper/JmxScraper.java | 4 ++-- .../opentelemetry/contrib/jmxscraper/JmxScraperTest.java | 8 ++++---- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/jmx-scraper/src/main/java/io/opentelemetry/contrib/jmxscraper/JmxScraper.java b/jmx-scraper/src/main/java/io/opentelemetry/contrib/jmxscraper/JmxScraper.java index 0c88fdec0..af1cfb49f 100644 --- a/jmx-scraper/src/main/java/io/opentelemetry/contrib/jmxscraper/JmxScraper.java +++ b/jmx-scraper/src/main/java/io/opentelemetry/contrib/jmxscraper/JmxScraper.java @@ -58,7 +58,7 @@ public static void main(String[] args) { boolean testMode = effectiveArgs.remove(TEST_ARG); try { - Properties argsConfig = parseArgsConfig(effectiveArgs); + Properties argsConfig = argsToConfig(effectiveArgs); propagateToSystemProperties(argsConfig); // auto-configure and register SDK @@ -144,7 +144,7 @@ static void propagateToSystemProperties(Properties properties) { * * @param args application commandline arguments */ - static Properties parseArgsConfig(List args) throws InvalidArgumentException { + static Properties argsToConfig(List args) throws InvalidArgumentException { if (args.isEmpty()) { // empty properties from stdin or external file diff --git a/jmx-scraper/src/test/java/io/opentelemetry/contrib/jmxscraper/JmxScraperTest.java b/jmx-scraper/src/test/java/io/opentelemetry/contrib/jmxscraper/JmxScraperTest.java index 6b07e98dc..2d3178072 100644 --- a/jmx-scraper/src/test/java/io/opentelemetry/contrib/jmxscraper/JmxScraperTest.java +++ b/jmx-scraper/src/test/java/io/opentelemetry/contrib/jmxscraper/JmxScraperTest.java @@ -30,7 +30,7 @@ void shouldThrowExceptionWhenInvalidCommandLineArgsProvided() { @Test void emptyArgumentsAllowed() throws InvalidArgumentException { - assertThat(JmxScraper.parseArgsConfig(Collections.emptyList())) + assertThat(JmxScraper.argsToConfig(Collections.emptyList())) .describedAs("empty config allowed to use JVM properties") .isEmpty(); } @@ -41,7 +41,7 @@ void shouldThrowExceptionWhenMissingProperties() { } private static void testInvalidArguments(String... args) { - assertThatThrownBy(() -> JmxScraper.parseArgsConfig(Arrays.asList(args))) + assertThatThrownBy(() -> JmxScraper.argsToConfig(Arrays.asList(args))) .isInstanceOf(InvalidArgumentException.class); } @@ -53,7 +53,7 @@ void shouldCreateConfig_propertiesLoadedFromFile() throws InvalidArgumentExcepti List args = Arrays.asList("-config", filePath); // When - Properties parsedConfig = JmxScraper.parseArgsConfig(args); + Properties parsedConfig = JmxScraper.argsToConfig(args); JmxScraperConfig config = JmxScraperConfig.fromConfig(TestUtil.configProperties(parsedConfig)); // Then @@ -72,7 +72,7 @@ void shouldCreateConfig_propertiesLoadedFromStdIn() throws InvalidArgumentExcept List args = Arrays.asList("-config", "-"); // When - Properties parsedConfig = JmxScraper.parseArgsConfig(args); + Properties parsedConfig = JmxScraper.argsToConfig(args); JmxScraperConfig config = JmxScraperConfig.fromConfig(TestUtil.configProperties(parsedConfig)); From 28b04c0e8fd88055d0b4944b1d80f941432597d4 Mon Sep 17 00:00:00 2001 From: Sylvain Juge <763082+SylvainJuge@users.noreply.github.com> Date: Tue, 4 Feb 2025 14:38:00 +0100 Subject: [PATCH 7/8] provide error msg to end-user in CLI --- .../io/opentelemetry/contrib/jmxscraper/JmxScraper.java | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/jmx-scraper/src/main/java/io/opentelemetry/contrib/jmxscraper/JmxScraper.java b/jmx-scraper/src/main/java/io/opentelemetry/contrib/jmxscraper/JmxScraper.java index af1cfb49f..ad8c8b7f4 100644 --- a/jmx-scraper/src/main/java/io/opentelemetry/contrib/jmxscraper/JmxScraper.java +++ b/jmx-scraper/src/main/java/io/opentelemetry/contrib/jmxscraper/JmxScraper.java @@ -93,7 +93,7 @@ public static void main(String[] args) { logger.log(Level.SEVERE, "invalid configuration ", e); System.exit(1); } catch (InvalidArgumentException e) { - logger.log(Level.SEVERE, "invalid configuration provided through arguments", e); + logger.log(Level.SEVERE, e.getMessage(), e); logger.info("Usage: java -jar [-test] [-config ]"); logger.info(" -test test JMX connection with provided configuration and exit"); logger.info( @@ -152,10 +152,10 @@ static Properties argsToConfig(List args) throws InvalidArgumentExceptio return new Properties(); } if (args.size() != 2) { - throw new InvalidArgumentException("Exactly two arguments expected, got " + args.size()); + throw new InvalidArgumentException("Unexpected number of arguments"); } if (!args.get(0).equalsIgnoreCase(CONFIG_ARG)) { - throw new InvalidArgumentException("Unexpected first argument must be '" + CONFIG_ARG + "'"); + throw new InvalidArgumentException("Unexpected argument must be '" + CONFIG_ARG + "'"); } String path = args.get(1); @@ -183,8 +183,7 @@ private static Properties loadPropertiesFromPath(String path) throws InvalidArgu properties.load(is); return properties; } catch (IOException e) { - throw new InvalidArgumentException( - "Failed to read config properties file: '" + path + "'", e); + throw new InvalidArgumentException("Failed to read config from file: '" + path + "'", e); } } From adfcd48f221b638e8df3b2d94b0b14a38e78c744 Mon Sep 17 00:00:00 2001 From: Sylvain Juge <763082+SylvainJuge@users.noreply.github.com> Date: Fri, 7 Feb 2025 09:52:59 +0100 Subject: [PATCH 8/8] document test mode --- jmx-scraper/README.md | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/jmx-scraper/README.md b/jmx-scraper/README.md index 49d80e49c..46ce69441 100644 --- a/jmx-scraper/README.md +++ b/jmx-scraper/README.md @@ -81,8 +81,19 @@ be set through the standard `JAVA_TOOL_OPTIONS` environment variable using the ` ## Troubleshooting +### Exported metrics + In order to investigate when and what metrics are being captured and sent, setting the `otel.metrics.exporter` -configuration option to include `logging` exporter provides log messages when metrics are being exported. +configuration option to include `logging` exporter provides log messages when metrics are being exported + +### JMX connection test + +Connection to the remote JVM through the JMX can be tested by adding the `-test` argument. +When doing so, the JMX Scraper will only test the connection to the remote JVM with provided configuration +and exit. + +- Connection OK: `JMX connection test OK` message is written to standard output and exit status = `0` +- Connection ERROR: `JMX connection test ERROR` message is written to standard output and exit status = `1` ## Extra libraries in classpath