post-review changes

Author: SylvainJuge

jmx-scraper/src/main/java/io/opentelemetry/contrib/jmxscraper/ArgumentsParsingException.java

@@ -0,0 +1,23 @@
+/*
+ * Copyright The OpenTelemetry Authors
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+package io.opentelemetry.contrib.jmxscraper;
+
+/**
+ * Exception indicating something is wrong with the provided arguments or reading the configuration
+ * from them
+ */
+public class InvalidArgumentException extends Exception {
+
+  private static final long serialVersionUID = 0L;
+
+  public InvalidArgumentException(String msg) {
+    super(msg);
+  }
+
+  public InvalidArgumentException(String msg, Throwable cause) {
+    super(msg, cause);
+  }
+}

jmx-scraper/src/main/java/io/opentelemetry/contrib/jmxscraper/InvalidArgumentException.java

@@ -83,7 +83,7 @@ public static void main(String[] args) {
     } catch (ConfigurationException e) {
       logger.log(Level.SEVERE, "invalid configuration ", e);
       System.exit(1);
-    } catch (ArgumentsParsingException e) {
+    } catch (InvalidArgumentException e) {
       logger.log(Level.SEVERE, "invalid configuration provided through arguments", e);
       logger.info(
           "Usage: java -jar <path_to_jmxscraper.jar> "
@@ -116,18 +116,18 @@ static void propagateToSystemProperties(Properties properties) {
    *
    * @param args application commandline arguments
    */
-  static Properties parseArgs(List<String> args) throws ArgumentsParsingException {
+  static Properties parseArgs(List<String> args) throws InvalidArgumentException {
 
     if (args.isEmpty()) {
       // empty properties from stdin or external file
       // config could still be provided through JVM system properties
       return new Properties();
     }
     if (args.size() != 2) {
-      throw new ArgumentsParsingException("Exactly two arguments expected, got " + args.size());
+      throw new InvalidArgumentException("Exactly two arguments expected, got " + args.size());
     }
     if (!args.get(0).equalsIgnoreCase(CONFIG_ARG)) {
-      throw new ArgumentsParsingException("Unexpected first argument must be '" + CONFIG_ARG + "'");
+      throw new InvalidArgumentException("Unexpected first argument must be '" + CONFIG_ARG + "'");
     }
 
     String path = args.get(1);
@@ -138,28 +138,30 @@ static Properties parseArgs(List<String> args) throws ArgumentsParsingException
     }
   }
 
-  private static Properties loadPropertiesFromStdin() throws ArgumentsParsingException {
+  private static Properties loadPropertiesFromStdin() throws InvalidArgumentException {
     Properties properties = new Properties();
     try (InputStream is = new DataInputStream(System.in)) {
       properties.load(is);
       return properties;
     } catch (IOException e) {
-      throw new ArgumentsParsingException("Failed to read config properties from stdin", e);
+      // an IO error is very unlikely here
+      throw new InvalidArgumentException("Failed to read config properties from stdin", e);
     }
   }
 
-  private static Properties loadPropertiesFromPath(String path) throws ArgumentsParsingException {
+  private static Properties loadPropertiesFromPath(String path) throws InvalidArgumentException {
     Properties properties = new Properties();
     try (InputStream is = Files.newInputStream(Paths.get(path))) {
       properties.load(is);
       return properties;
     } catch (IOException e) {
-      throw new ArgumentsParsingException(
+      throw new InvalidArgumentException(
           "Failed to read config properties file: '" + path + "'", e);
     }
   }
 
-  JmxScraper(JmxConnectorBuilder client, JmxMetricInsight service, JmxScraperConfig config) {
+  private JmxScraper(
+      JmxConnectorBuilder client, JmxMetricInsight service, JmxScraperConfig config) {
     this.client = client;
     this.service = service;
     this.config = config;

jmx-scraper/src/main/java/io/opentelemetry/contrib/jmxscraper/JmxScraper.java

@@ -20,28 +20,33 @@
 import org.junitpioneer.jupiter.ClearSystemProperty;
 
 class JmxScraperTest {
+
   @Test
   void shouldThrowExceptionWhenInvalidCommandLineArgsProvided() {
-    // Given
-    List<String> emptyArgs = Collections.singletonList("-nonExistentOption");
+    testInvalidArguments("-nonExistentOption");
+    testInvalidArguments("-potato", "-config");
+    testInvalidArguments("-config", "path", "-nonExistentOption");
+  }
 
-    // When and Then
-    assertThatThrownBy(() -> JmxScraper.parseArgs(emptyArgs))
-        .isInstanceOf(ArgumentsParsingException.class);
+  @Test
+  void emptyArgumentsAllowed() throws InvalidArgumentException {
+    assertThat(JmxScraper.parseArgs(Collections.emptyList()))
+        .describedAs("empty arguments allowed to use JVM properties")
+        .isEmpty();
   }
 
   @Test
-  void shouldThrowExceptionWhenTooManyCommandLineArgsProvided() {
-    // Given
-    List<String> args = Arrays.asList("-config", "path", "-nonExistentOption");
+  void shouldThrowExceptionWhenMissingProperties() {
+    testInvalidArguments("-config", "missing.properties");
+  }
 
-    // When and Then
-    assertThatThrownBy(() -> JmxScraper.parseArgs(args))
-        .isInstanceOf(ArgumentsParsingException.class);
+  private static void testInvalidArguments(String... args) {
+    assertThatThrownBy(() -> JmxScraper.parseArgs(Arrays.asList(args)))
+        .isInstanceOf(InvalidArgumentException.class);
   }
 
   @Test
-  void shouldCreateConfig_propertiesLoadedFromFile() throws ArgumentsParsingException {
+  void shouldCreateConfig_propertiesLoadedFromFile() throws InvalidArgumentException {
     // Given
     String filePath =
         ClassLoader.getSystemClassLoader().getResource("validConfig.properties").getPath();
@@ -58,8 +63,7 @@ void shouldCreateConfig_propertiesLoadedFromFile() throws ArgumentsParsingExcept
   }
 
   @Test
-  void shouldCreateConfig_propertiesLoadedFromStdIn()
-      throws ArgumentsParsingException, IOException {
+  void shouldCreateConfig_propertiesLoadedFromStdIn() throws InvalidArgumentException, IOException {
     InputStream originalIn = System.in;
     try (InputStream stream =
         ClassLoader.getSystemClassLoader().getResourceAsStream("validConfig.properties")) {