diff --git a/jmx-scraper/src/main/java/io/opentelemetry/contrib/jmxscraper/ArgumentsParsingException.java b/jmx-scraper/src/main/java/io/opentelemetry/contrib/jmxscraper/ArgumentsParsingException.java new file mode 100644 index 000000000..afe3460d6 --- /dev/null +++ b/jmx-scraper/src/main/java/io/opentelemetry/contrib/jmxscraper/ArgumentsParsingException.java @@ -0,0 +1,10 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.contrib.jmxscraper; + +public class ArgumentsParsingException extends Exception { + private static final long serialVersionUID = 0L; +} 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 1c82b60b5..835eebd1e 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 @@ -9,8 +9,15 @@ import io.opentelemetry.contrib.jmxscraper.config.JmxScraperConfig; import io.opentelemetry.contrib.jmxscraper.config.JmxScraperConfigFactory; import io.opentelemetry.contrib.jmxscraper.jmx.JmxClient; +import java.io.DataInputStream; +import java.io.IOException; +import java.io.InputStream; import java.net.MalformedURLException; +import java.nio.file.Files; +import java.nio.file.Paths; import java.util.Arrays; +import java.util.List; +import java.util.Properties; import java.util.concurrent.Executors; import java.util.concurrent.ScheduledExecutorService; import java.util.concurrent.TimeUnit; @@ -18,10 +25,85 @@ public class JmxScraper { private static final Logger logger = Logger.getLogger(JmxScraper.class.getName()); + private static final int EXECUTOR_TERMINATION_TIMEOUT_MS = 5000; private final ScheduledExecutorService exec = Executors.newSingleThreadScheduledExecutor(); private final JmxScraperConfig config; - JmxScraper(JmxScraperConfig config) { + /** + * Main method to create and run a {@link JmxScraper} instance. + * + * @param args - must be of the form "-config {jmx_config_path,'-'}" + */ + @SuppressWarnings({"SystemOut", "SystemExitOutsideMain"}) + public static void main(String[] args) { + try { + JmxScraperConfigFactory factory = new JmxScraperConfigFactory(); + JmxScraperConfig config = JmxScraper.createConfigFromArgs(Arrays.asList(args), factory); + + JmxScraper jmxScraper = new JmxScraper(config); + jmxScraper.start(); + + Runtime.getRuntime() + .addShutdownHook( + new Thread() { + @Override + public void run() { + jmxScraper.shutdown(); + } + }); + } catch (ArgumentsParsingException e) { + System.err.println( + "Usage: java -jar " + + "-config "); + System.exit(1); + } catch (ConfigurationException e) { + System.err.println(e.getMessage()); + System.exit(1); + } + } + + /** + * Create {@link JmxScraperConfig} object basing on command line options + * + * @param args application commandline arguments + */ + static JmxScraperConfig createConfigFromArgs(List args, JmxScraperConfigFactory factory) + throws ArgumentsParsingException, ConfigurationException { + if (!args.isEmpty() && (args.size() != 2 || !args.get(0).equalsIgnoreCase("-config"))) { + throw new ArgumentsParsingException(); + } + + Properties loadedProperties = new Properties(); + if (args.size() == 2) { + String path = args.get(1); + if (path.trim().equals("-")) { + loadPropertiesFromStdin(loadedProperties); + } else { + loadPropertiesFromPath(loadedProperties, path); + } + } + + return factory.createConfig(loadedProperties); + } + + private static void loadPropertiesFromStdin(Properties props) throws ConfigurationException { + try (InputStream is = new DataInputStream(System.in)) { + props.load(is); + } catch (IOException e) { + throw new ConfigurationException("Failed to read config properties from stdin", e); + } + } + + private static void loadPropertiesFromPath(Properties props, String path) + throws ConfigurationException { + try (InputStream is = Files.newInputStream(Paths.get(path))) { + props.load(is); + } catch (IOException e) { + throw new ConfigurationException("Failed to read config properties file: '" + path + "'", e); + } + } + + JmxScraper(JmxScraperConfig config) throws ConfigurationException { this.config = config; try { @@ -51,28 +133,23 @@ private void start() { private void shutdown() { logger.info("Shutting down JmxScraper and exporting final metrics."); + // Prevent new tasks to be submitted exec.shutdown(); - } - - /** - * Main method to create and run a {@link JmxScraper} instance. - * - * @param args - must be of the form "-config {jmx_config_path,'-'}" - */ - public static void main(String[] args) { - JmxScraperConfigFactory factory = new JmxScraperConfigFactory(); - JmxScraperConfig config = factory.createConfigFromArgs(Arrays.asList(args)); - - JmxScraper jmxScraper = new JmxScraper(config); - jmxScraper.start(); - - Runtime.getRuntime() - .addShutdownHook( - new Thread() { - @Override - public void run() { - jmxScraper.shutdown(); - } - }); + try { + // Wait a while for existing tasks to terminate + if (!exec.awaitTermination(EXECUTOR_TERMINATION_TIMEOUT_MS, TimeUnit.MILLISECONDS)) { + // Cancel currently executing tasks + exec.shutdownNow(); + // Wait a while for tasks to respond to being cancelled + if (!exec.awaitTermination(EXECUTOR_TERMINATION_TIMEOUT_MS, TimeUnit.MILLISECONDS)) { + logger.warning("Thread pool did not terminate in time: " + exec); + } + } + } catch (InterruptedException e) { + // (Re-)Cancel if current thread also interrupted + exec.shutdownNow(); + // Preserve interrupt status + Thread.currentThread().interrupt(); + } } } diff --git a/jmx-scraper/src/main/java/io/opentelemetry/contrib/jmxscraper/config/ConfigurationException.java b/jmx-scraper/src/main/java/io/opentelemetry/contrib/jmxscraper/config/ConfigurationException.java index 2e00d737d..76c69998a 100644 --- a/jmx-scraper/src/main/java/io/opentelemetry/contrib/jmxscraper/config/ConfigurationException.java +++ b/jmx-scraper/src/main/java/io/opentelemetry/contrib/jmxscraper/config/ConfigurationException.java @@ -5,7 +5,7 @@ package io.opentelemetry.contrib.jmxscraper.config; -public class ConfigurationException extends RuntimeException { +public class ConfigurationException extends Exception { private static final long serialVersionUID = 0L; public ConfigurationException(String message, Throwable cause) { diff --git a/jmx-scraper/src/main/java/io/opentelemetry/contrib/jmxscraper/config/JmxScraperConfigFactory.java b/jmx-scraper/src/main/java/io/opentelemetry/contrib/jmxscraper/config/JmxScraperConfigFactory.java index c74f7c2f8..12f054585 100644 --- a/jmx-scraper/src/main/java/io/opentelemetry/contrib/jmxscraper/config/JmxScraperConfigFactory.java +++ b/jmx-scraper/src/main/java/io/opentelemetry/contrib/jmxscraper/config/JmxScraperConfigFactory.java @@ -7,18 +7,12 @@ import static io.opentelemetry.contrib.jmxscraper.util.StringUtils.isBlank; -import java.io.DataInputStream; -import java.io.IOException; -import java.io.InputStream; -import java.nio.file.Files; -import java.nio.file.Paths; import java.util.Arrays; import java.util.List; import java.util.Locale; import java.util.Properties; import java.util.stream.Collectors; -@SuppressWarnings({"SystemOut", "SystemExitOutsideMain"}) public class JmxScraperConfigFactory { private static final String PREFIX = "otel."; static final String SERVICE_URL = PREFIX + "jmx.service.url"; @@ -64,56 +58,7 @@ public class JmxScraperConfigFactory { private Properties properties = new Properties(); - /** - * Create {@link JmxScraperConfig} object basing on command line options - * - * @param args application commandline arguments - */ - public JmxScraperConfig createConfigFromArgs(List args) { - if (!args.isEmpty() && (args.size() != 2 || !args.get(0).equalsIgnoreCase("-config"))) { - System.out.println( - "Usage: java io.opentelemetry.contrib.jmxscraper.JmxScraper " - + "-config "); - System.exit(1); - } - - Properties loadedProperties = new Properties(); - if (args.size() == 2) { - String path = args.get(1); - if (path.trim().equals("-")) { - loadPropertiesFromStdin(loadedProperties); - } else { - loadPropertiesFromPath(loadedProperties, path); - } - } - - JmxScraperConfig config = createConfig(loadedProperties); - validateConfig(config); - populateJmxSystemProperties(); - - return config; - } - - private static void loadPropertiesFromStdin(Properties props) { - try (InputStream is = new DataInputStream(System.in)) { - props.load(is); - } catch (IOException e) { - System.out.println("Failed to read config properties from stdin: " + e.getMessage()); - System.exit(1); - } - } - - private static void loadPropertiesFromPath(Properties props, String path) { - try (InputStream is = Files.newInputStream(Paths.get(path))) { - props.load(is); - } catch (IOException e) { - System.out.println( - "Failed to read config properties file at '" + path + "': " + e.getMessage()); - System.exit(1); - } - } - - JmxScraperConfig createConfig(Properties props) { + public JmxScraperConfig createConfig(Properties props) throws ConfigurationException { properties = new Properties(); // putAll() instead of using constructor defaults // to ensure they will be recorded to underlying map @@ -148,6 +93,9 @@ JmxScraperConfig createConfig(Properties props) { config.registrySsl = Boolean.parseBoolean(properties.getProperty(REGISTRY_SSL)); + validateConfig(config); + populateJmxSystemProperties(); + return config; } @@ -168,7 +116,7 @@ private void populateJmxSystemProperties() { }); } - private int getProperty(String key, int defaultValue) { + private int getProperty(String key, int defaultValue) throws ConfigurationException { String propVal = properties.getProperty(key); if (propVal == null) { return defaultValue; @@ -191,7 +139,8 @@ private String getAndSetPropertyIfUndefined(String key, String defaultValue) { return propVal; } - private int getAndSetPropertyIfUndefined(String key, int defaultValue) { + private int getAndSetPropertyIfUndefined(String key, int defaultValue) + throws ConfigurationException { int propVal = getProperty(key, defaultValue); if (propVal == defaultValue) { properties.setProperty(key, String.valueOf(defaultValue)); @@ -200,7 +149,7 @@ private int getAndSetPropertyIfUndefined(String key, int defaultValue) { } /** Will determine if parsed config is complete, setting any applicable values and defaults. */ - void validateConfig(JmxScraperConfig config) { + private static void validateConfig(JmxScraperConfig config) throws ConfigurationException { if (isBlank(config.serviceUrl)) { throw new ConfigurationException(SERVICE_URL + " must be specified."); } diff --git a/jmx-scraper/src/main/java/io/opentelemetry/contrib/jmxscraper/jmx/JmxClient.java b/jmx-scraper/src/main/java/io/opentelemetry/contrib/jmxscraper/jmx/JmxClient.java index 75f518d2d..0c71d9cc9 100644 --- a/jmx-scraper/src/main/java/io/opentelemetry/contrib/jmxscraper/jmx/JmxClient.java +++ b/jmx-scraper/src/main/java/io/opentelemetry/contrib/jmxscraper/jmx/JmxClient.java @@ -38,7 +38,6 @@ public class JmxClient { public JmxClient(JmxScraperConfig config) throws MalformedURLException { this.url = new JMXServiceURL(config.getServiceUrl()); - ; this.username = config.getUsername(); this.password = config.getPassword(); this.realm = config.getRealm(); 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 new file mode 100644 index 000000000..86b83fc27 --- /dev/null +++ b/jmx-scraper/src/test/java/io/opentelemetry/contrib/jmxscraper/JmxScraperTest.java @@ -0,0 +1,39 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.contrib.jmxscraper; + +import static org.assertj.core.api.Assertions.assertThatThrownBy; +import static org.mockito.Mockito.mock; + +import io.opentelemetry.contrib.jmxscraper.config.JmxScraperConfigFactory; +import java.util.Arrays; +import java.util.Collections; +import java.util.List; +import org.junit.jupiter.api.Test; + +class JmxScraperTest { + @Test + void shouldThrowExceptionWhenInvalidCommandLineArgsProvided() { + // Given + List emptyArgs = Collections.singletonList("-inexistingOption"); + JmxScraperConfigFactory configFactoryMock = mock(JmxScraperConfigFactory.class); + + // When and Then + assertThatThrownBy(() -> JmxScraper.createConfigFromArgs(emptyArgs, configFactoryMock)) + .isInstanceOf(ArgumentsParsingException.class); + } + + @Test + void shouldThrowExceptionWhenTooManyCommandLineArgsProvided() { + // Given + List emptyArgs = Arrays.asList("-config", "path", "-inexistingOption"); + JmxScraperConfigFactory configFactoryMock = mock(JmxScraperConfigFactory.class); + + // When and Then + assertThatThrownBy(() -> JmxScraper.createConfigFromArgs(emptyArgs, configFactoryMock)) + .isInstanceOf(ArgumentsParsingException.class); + } +} diff --git a/jmx-scraper/src/test/java/io/opentelemetry/contrib/jmxscraper/config/JmxScraperConfigFactoryTest.java b/jmx-scraper/src/test/java/io/opentelemetry/contrib/jmxscraper/config/JmxScraperConfigFactoryTest.java index bcd6ac574..28a2680d7 100644 --- a/jmx-scraper/src/test/java/io/opentelemetry/contrib/jmxscraper/config/JmxScraperConfigFactoryTest.java +++ b/jmx-scraper/src/test/java/io/opentelemetry/contrib/jmxscraper/config/JmxScraperConfigFactoryTest.java @@ -8,11 +8,11 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; -import java.util.Arrays; -import java.util.HashSet; import java.util.Properties; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Test; +import org.junitpioneer.jupiter.ClearSystemProperty; +import org.junitpioneer.jupiter.SetSystemProperty; class JmxScraperConfigFactoryTest { private static Properties validProperties; @@ -36,17 +36,21 @@ static void setUp() { } @Test - void verifyDefaultValues() { + void shouldCreateMinimalValidConfiguration() throws ConfigurationException { // Given JmxScraperConfigFactory configFactory = new JmxScraperConfigFactory(); - Properties emptyProperties = new Properties(); + Properties properties = new Properties(); + properties.setProperty( + JmxScraperConfigFactory.SERVICE_URL, + "jservice:jmx:rmi:///jndi/rmi://localhost:9010/jmxrmi"); + properties.setProperty(JmxScraperConfigFactory.CUSTOM_JMX_SCRAPING_CONFIG, "/file.properties"); // When - JmxScraperConfig config = configFactory.createConfig(emptyProperties); + JmxScraperConfig config = configFactory.createConfig(properties); // Then - assertThat(config.serviceUrl).isNull(); - assertThat(config.customJmxScrapingConfigPath).isNull(); + assertThat(config.serviceUrl).isEqualTo("jservice:jmx:rmi:///jndi/rmi://localhost:9010/jmxrmi"); + assertThat(config.customJmxScrapingConfigPath).isEqualTo("/file.properties"); assertThat(config.targetSystems).isEmpty(); assertThat(config.intervalMilliseconds).isEqualTo(10000); assertThat(config.metricsExporterType).isEqualTo("logging"); @@ -58,17 +62,31 @@ void verifyDefaultValues() { } @Test - void shouldUseValuesFromProperties() { + @ClearSystemProperty(key = "javax.net.ssl.keyStore") + @ClearSystemProperty(key = "javax.net.ssl.keyStorePassword") + @ClearSystemProperty(key = "javax.net.ssl.keyStoreType") + @ClearSystemProperty(key = "javax.net.ssl.trustStore") + @ClearSystemProperty(key = "javax.net.ssl.trustStorePassword") + @ClearSystemProperty(key = "javax.net.ssl.trustStoreType") + void shouldUseValuesFromProperties() throws ConfigurationException { // Given JmxScraperConfigFactory configFactory = new JmxScraperConfigFactory(); + // Properties to be propagated to system, properties + Properties properties = (Properties) validProperties.clone(); + properties.setProperty("javax.net.ssl.keyStore", "/my/key/store"); + properties.setProperty("javax.net.ssl.keyStorePassword", "abc123"); + properties.setProperty("javax.net.ssl.keyStoreType", "JKS"); + properties.setProperty("javax.net.ssl.trustStore", "/my/trust/store"); + properties.setProperty("javax.net.ssl.trustStorePassword", "def456"); + properties.setProperty("javax.net.ssl.trustStoreType", "JKS"); // When - JmxScraperConfig config = configFactory.createConfig(validProperties); + JmxScraperConfig config = configFactory.createConfig(properties); // Then assertThat(config.serviceUrl).isEqualTo("jservice:jmx:rmi:///jndi/rmi://localhost:9010/jmxrmi"); assertThat(config.customJmxScrapingConfigPath).isEqualTo(""); - assertThat(config.targetSystems).isEqualTo(new HashSet<>(Arrays.asList("tomcat", "activemq"))); + assertThat(config.targetSystems).containsOnly("tomcat", "activemq"); assertThat(config.intervalMilliseconds).isEqualTo(1410); assertThat(config.metricsExporterType).isEqualTo("otel"); assertThat(config.otlpExporterEndpoint).isEqualTo("http://localhost:4317"); @@ -77,6 +95,32 @@ void shouldUseValuesFromProperties() { assertThat(config.remoteProfile).isEqualTo("some-profile"); assertThat(config.realm).isEqualTo("some-realm"); assertThat(config.registrySsl).isTrue(); + + // These properties are set from the config file loading into JmxConfig + assertThat(System.getProperty("javax.net.ssl.keyStore")).isEqualTo("/my/key/store"); + assertThat(System.getProperty("javax.net.ssl.keyStorePassword")).isEqualTo("abc123"); + assertThat(System.getProperty("javax.net.ssl.keyStoreType")).isEqualTo("JKS"); + assertThat(System.getProperty("javax.net.ssl.trustStore")).isEqualTo("/my/trust/store"); + assertThat(System.getProperty("javax.net.ssl.trustStorePassword")).isEqualTo("def456"); + assertThat(System.getProperty("javax.net.ssl.trustStoreType")).isEqualTo("JKS"); + } + + @Test + @SetSystemProperty(key = "otel.jmx.service.url", value = "originalServiceUrl") + @SetSystemProperty(key = "javax.net.ssl.keyStorePassword", value = "originalPassword") + void shouldRetainPredefinedSystemProperties() throws ConfigurationException { + // Given + JmxScraperConfigFactory configFactory = new JmxScraperConfigFactory(); + // Properties to be propagated to system, properties + Properties properties = (Properties) validProperties.clone(); + properties.setProperty("javax.net.ssl.keyStorePassword", "abc123"); + + // When + configFactory.createConfig(properties); + + // Then + assertThat(System.getProperty("otel.jmx.service.url")).isEqualTo("originalServiceUrl"); + assertThat(System.getProperty("javax.net.ssl.keyStorePassword")).isEqualTo("originalPassword"); } @Test @@ -85,10 +129,9 @@ void shouldFailValidation_missingServiceUrl() { JmxScraperConfigFactory configFactory = new JmxScraperConfigFactory(); Properties properties = (Properties) validProperties.clone(); properties.remove(JmxScraperConfigFactory.SERVICE_URL); - JmxScraperConfig config = configFactory.createConfig(properties); // When and Then - assertThatThrownBy(() -> configFactory.validateConfig(config)) + assertThatThrownBy(() -> configFactory.createConfig(properties)) .isInstanceOf(ConfigurationException.class) .hasMessage("otel.jmx.service.url must be specified."); } @@ -100,10 +143,9 @@ void shouldFailValidation_missingConfigPathAndTargetSystem() { Properties properties = (Properties) validProperties.clone(); properties.remove(JmxScraperConfigFactory.CUSTOM_JMX_SCRAPING_CONFIG); properties.remove(JmxScraperConfigFactory.TARGET_SYSTEM); - JmxScraperConfig config = configFactory.createConfig(properties); // When and Then - assertThatThrownBy(() -> configFactory.validateConfig(config)) + assertThatThrownBy(() -> configFactory.createConfig(properties)) .isInstanceOf(ConfigurationException.class) .hasMessage( "otel.jmx.custom.jmx.scraping.config or otel.jmx.target.system must be specified."); @@ -115,10 +157,9 @@ void shouldFailValidation_invalidTargetSystem() { JmxScraperConfigFactory configFactory = new JmxScraperConfigFactory(); Properties properties = (Properties) validProperties.clone(); properties.setProperty(JmxScraperConfigFactory.TARGET_SYSTEM, "hal9000"); - JmxScraperConfig config = configFactory.createConfig(properties); // When and Then - assertThatThrownBy(() -> configFactory.validateConfig(config)) + assertThatThrownBy(() -> configFactory.createConfig(properties)) .isInstanceOf(ConfigurationException.class) .hasMessage( "[hal9000] must specify targets from " @@ -132,10 +173,9 @@ void shouldFailValidation_missingOtlpEndpoint() { Properties properties = (Properties) validProperties.clone(); properties.remove(JmxScraperConfigFactory.OTLP_ENDPOINT); properties.setProperty(JmxScraperConfigFactory.METRICS_EXPORTER_TYPE, "otlp"); - JmxScraperConfig config = configFactory.createConfig(properties); // When and Then - assertThatThrownBy(() -> configFactory.validateConfig(config)) + assertThatThrownBy(() -> configFactory.createConfig(properties)) .isInstanceOf(ConfigurationException.class) .hasMessage("otel.exporter.otlp.endpoint must be specified for otlp format."); } @@ -146,10 +186,9 @@ void shouldFailValidation_negativeInterval() { JmxScraperConfigFactory configFactory = new JmxScraperConfigFactory(); Properties properties = (Properties) validProperties.clone(); properties.setProperty(JmxScraperConfigFactory.INTERVAL_MILLISECONDS, "-1"); - JmxScraperConfig config = configFactory.createConfig(properties); // When and Then - assertThatThrownBy(() -> configFactory.validateConfig(config)) + assertThatThrownBy(() -> configFactory.createConfig(properties)) .isInstanceOf(ConfigurationException.class) .hasMessage("otel.jmx.interval.milliseconds must be positive."); }