Update JMX metrics to support both a script and target systems (#1339)

Author: jaydeluca

jmx-metrics/README.md

@@ -1,14 +1,14 @@
 # JMX Metric Gatherer
 
 This utility provides an easy framework for gathering and reporting metrics based on queried
-MBeans from a JMX server.  It loads an included or custom Groovy script and establishes a helpful,
+MBeans from a JMX server.  It loads included and/or custom Groovy scripts and establishes a helpful,
 bound `otel` object with methods for obtaining MBeans and constructing OpenTelemetry instruments:
 
 ## Usage
 
 The JMX Metric Gatherer is intended to be run as an uber jar and configured with properties from the command line,
 properties file, and stdin (`-`).  Its metric-gathering scripts are specified by supported `otel.jmx.target.system`
-values or a `otel.jmx.groovy.script` path to run your own.
+values and/or a `otel.jmx.groovy.script` path to run your own.
 
 ```bash
 java -D<otel.jmx.property=value> -jar opentelemetry-jmx-metrics-<version>.jar [-config {session.properties, '-'}]
@@ -29,9 +29,11 @@ otel.exporter.otlp.endpoint = http://my-opentelemetry-collector:4317
 
 As configured in this example, the metric gatherer will establish an MBean server connection using the
 specified `otel.jmx.service.url` (required) and credentials and configure an OTLP gRPC metrics exporter reporting to
-`otel.exporter.otlp.endpoint`. If SSL is enabled on the RMI registry for your server, the `otel.jmx.remote.registry.ssl` property must be set to `true`. After loading the included JVM and Kafka metric-gathering scripts determined by
-the comma-separated list in `otel.jmx.target.system`, it will then run the scripts on the desired interval
-length of `otel.jmx.interval.milliseconds` and export the resulting metrics.
+`otel.exporter.otlp.endpoint`. If SSL is enabled on the RMI registry for your server, the
+`otel.jmx.remote.registry.ssl` property must be set to `true`. After loading the included JVM and
+Kafka metric-gathering scripts determined by the comma-separated list in `otel.jmx.target.system`,
+it will then run the scripts on the desired interval length of `otel.jmx.interval.milliseconds` and
+export the resulting metrics.
 
 For custom metrics and unsupported targets, you can provide your own MBean querying scripts to produce
 OpenTelemetry instruments:
@@ -63,8 +65,8 @@ attribute as queried in each interval.
 ## Target Systems
 
 The JMX Metric Gatherer provides built in metric producing Groovy scripts for supported target systems
-capable of being specified via the `otel.jmx.target.system` property as a comma-separated list.  This property is
-mutually exclusive with `otel.jmx.groovy.script`. The currently supported target systems are:
+capable of being specified via the `otel.jmx.target.system` property as a comma-separated list. The
+currently supported target systems are:
 
 | `otel.jmx.target.system` |
 |--------------------------|

jmx-metrics/src/main/groovy/io/opentelemetry/contrib/jmxmetrics/GroovyRunner.java

@@ -48,7 +48,8 @@ public class GroovyRunner {
           String systemResourcePath = "target-systems/" + target + ".groovy";
           scriptSources.add(getTargetSystemResourceAsString(systemResourcePath));
         }
-      } else {
+      }
+      if (config.groovyScript != null && !config.groovyScript.isEmpty()) {
         scriptSources.add(getFileAsString(config.groovyScript));
       }
     } catch (IOException e) {

jmx-metrics/src/main/groovy/io/opentelemetry/contrib/jmxmetrics/JmxConfig.java

@@ -33,7 +33,7 @@ class JmxConfig {
   static final String JMX_REALM = PREFIX + "jmx.realm";
 
   // These properties need to be copied into System Properties if provided via the property
-  // file so they are available to the JMX Connection builder
+  // file so that they are available to the JMX Connection builder
   static final List<String> JAVA_SYSTEM_PROPERTIES =
       Arrays.asList(
           "javax.net.ssl.keyStore",
@@ -175,11 +175,6 @@ void validate() {
           GROOVY_SCRIPT + " or " + TARGET_SYSTEM + " must be specified.");
     }
 
-    if (!isBlank(groovyScript) && !isBlank(targetSystem)) {
-      throw new ConfigurationException(
-          "Only one of " + GROOVY_SCRIPT + " or " + TARGET_SYSTEM + " can be specified.");
-    }
-
     if (targetSystems.size() != 0 && !AVAILABLE_TARGET_SYSTEMS.containsAll(targetSystems)) {
       throw new ConfigurationException(
           String.format(

jmx-metrics/src/test/java/io/opentelemetry/contrib/jmxmetrics/GroovyRunnerTest.java

@@ -44,11 +44,63 @@ public List<ObjectName> query(ObjectName objectName) {
   @Test
   @SetSystemProperty(key = "otel.jmx.service.url", value = "requiredValue")
   @SetSystemProperty(key = "otel.jmx.target.system", value = "notAProvidededTargetSystem")
-  void loadUnavailableTargetScript() throws Exception {
+  void loadUnavailableTargetScript() {
     JmxConfig config = new JmxConfig();
 
     assertThatThrownBy(() -> new GroovyRunner(config, null, null))
         .isInstanceOf(ConfigurationException.class)
         .hasMessage("Failed to load target-systems/notaprovidededtargetsystem.groovy");
   }
+
+  @Test
+  @SetSystemProperty(
+      key = "otel.jmx.service.url",
+      value = "service:jmx:rmi:///jndi/rmi://localhost:12345/jmxrmi")
+  @SetSystemProperty(key = "otel.jmx.target.system", value = "jvm,hadoop")
+  @SetSystemProperty(
+      key = "otel.jmx.groovy.script",
+      value = "src/integrationTest/resources/script.groovy")
+  void loadScriptAndTargetSystems() throws Exception {
+    JmxConfig config = new JmxConfig();
+
+    assertThatCode(config::validate).doesNotThrowAnyException();
+
+    JmxClient stub =
+        new JmxClient(config) {
+          @Override
+          public List<ObjectName> query(ObjectName objectName) {
+            return Collections.emptyList();
+          }
+        };
+
+    GroovyRunner runner = new GroovyRunner(config, stub, new GroovyMetricEnvironment(config));
+
+    assertThat(runner.getScripts()).hasSize(3);
+    runner.run();
+  }
+
+  @Test
+  @SetSystemProperty(
+      key = "otel.jmx.service.url",
+      value = "service:jmx:rmi:///jndi/rmi://localhost:12345/jmxrmi")
+  @SetSystemProperty(key = "otel.jmx.target.system", value = "jvm")
+  @SetSystemProperty(key = "otel.jmx.groovy.script", value = "")
+  void loadScriptAndTargetSystemWithBlankInputForScript() throws Exception {
+    JmxConfig config = new JmxConfig();
+
+    assertThatCode(config::validate).doesNotThrowAnyException();
+
+    JmxClient stub =
+        new JmxClient(config) {
+          @Override
+          public List<ObjectName> query(ObjectName objectName) {
+            return Collections.emptyList();
+          }
+        };
+
+    GroovyRunner runner = new GroovyRunner(config, stub, new GroovyMetricEnvironment(config));
+
+    assertThat(runner.getScripts()).hasSize(1);
+    runner.run();
+  }
 }

jmx-metrics/src/test/java/io/opentelemetry/contrib/jmxmetrics/JmxConfigTest.java

@@ -171,16 +171,16 @@ void invalidPrometheusPort() {
   }
 
   @Test
-  @SetSystemProperty(key = "otel.jmx.service.url", value = "requiredValue")
+  @SetSystemProperty(key = "otel.jmx.service.url", value = "myServiceUrl")
   @SetSystemProperty(key = "otel.jmx.groovy.script", value = "myGroovyScript")
   @SetSystemProperty(key = "otel.jmx.target.system", value = "myTargetSystem")
-  void conflictingScriptAndTargetSystem() {
+  void canSupportScriptAndTargetSystem() {
     JmxConfig config = new JmxConfig();
 
-    assertThatThrownBy(config::validate)
-        .isInstanceOf(ConfigurationException.class)
-        .hasMessage(
-            "Only one of otel.jmx.groovy.script or otel.jmx.target.system can be specified.");
+    assertThat(config.serviceUrl).isEqualTo("myServiceUrl");
+    assertThat(config.groovyScript).isEqualTo("myGroovyScript");
+    assertThat(config.targetSystem).isEqualTo("mytargetsystem");
+    assertThat(config.targetSystems).containsOnly("mytargetsystem");
   }
 
   @Test