jmx: Allow specifying multiple target systems (#28)

Author: rmfitzpatrick

contrib/jmx-metrics/README.md

@@ -14,7 +14,7 @@ $ java -D<otel.jmx.property=value> -jar opentelemetry-java-contrib-jmx-metrics-<
 
 ```properties
 otel.jmx.service.url = service:jmx:rmi:///jndi/rmi://<my-jmx-host>:<my-jmx-port>/jmxrmi
-otel.jmx.groovy.script = /opt/script.groovy
+otel.jmx.target.system = jvm,kafka
 otel.jmx.interval.milliseconds = 5000
 otel.exporter = otlp
 otel.otlp.endpoint = my-opentelemetry-collector:55680
@@ -34,8 +34,8 @@ otel.instrument(storageLoadMBean, "cassandra.storage.load",
 
 As configured in the example, this metric gatherer will configure an otlp gRPC metric exporter
 at the `otel.otlp.endpoint` and establish an MBean server connection using the
-provided `otel.jmx.service.url`. After loading the Groovy script whose path is specified
-via `otel.jmx.groovy.script`, it will then run the script on the specified
+provided `otel.jmx.service.url`. After loading the included metric gathering scripts specified by the
+comma-separated list in `otel.jmx.target.system`, it will then run the scripts on the specified
 `otel.jmx.interval.milliseconds` and export the resulting metrics.
 
 ### JMX Query Helpers
@@ -138,8 +138,8 @@ This metric extension supports Java 7+, though SASL is only supported where
 ### Target Systems
 
 The JMX Metric Gatherer also provides built in metric producing Groovy scripts for supported target systems
-capable of being specified via the `otel.jmx.target.system` property (mutually exclusive with `otel.jmx.groovy.script`).
-The currently available target systems are:
+capable of being specified via the `otel.jmx.target.system` property as a comma-separated list (mutually exclusive with
+`otel.jmx.groovy.script`). The currently available target systems are:
 
 | `otel.jmx.target.system` |
 | ------------------------ |
@@ -159,7 +159,7 @@ file contents can also be provided via stdin on startup when using `-config -` a
 | ------------- | -------- | ----------- |
 | `otel.jmx.service.url` | **yes** | The service URL for the JMX RMI/JMXMP endpoint (generally of the form `service:jmx:rmi:///jndi/rmi://<host>:<port>/jmxrmi` or `service:jmx:jmxmp://<host>:<port>`).|
 | `otel.jmx.groovy.script` | if not using `otel.jmx.target.system` | The path for the desired Groovy script. |
-| `otel.jmx.target.system` | if not using `otel.jmx.groovy.script` | The supported target application with built in Groovy script. |
+| `otel.jmx.target.system` | if not using `otel.jmx.groovy.script` | A comma-separated list of the supported target applications with built in Groovy scripts. |
 | `otel.jmx.interval.milliseconds` | no | How often, in milliseconds, the Groovy script should be run and its resulting metrics exported. 10000 by default. |
 | `otel.exporter` | no | The type of metric exporter to use: (`otlp`, `prometheus`, `inmemory`, `logging`).  `logging` by default. |
 | `otel.exporter.otlp.endpoint` | no | The otlp exporter endpoint to use, Required for `otlp`.  |

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

@@ -32,7 +32,9 @@
 import java.nio.file.FileSystems;
 import java.nio.file.Files;
 import java.nio.file.Path;
+import java.util.ArrayList;
 import java.util.Collections;
+import java.util.List;
 import java.util.Map;
 import java.util.logging.Level;
 import java.util.logging.Logger;
@@ -41,7 +43,7 @@
 public class GroovyRunner {
   private static final Logger logger = Logger.getLogger(GroovyRunner.class.getName());
 
-  private final Script script;
+  private final List<Script> scripts;
   private final GroovyMetricEnvironment groovyMetricEnvironment;
 
   GroovyRunner(
@@ -50,21 +52,26 @@ public class GroovyRunner {
       final GroovyMetricEnvironment groovyMetricEnvironment) {
     this.groovyMetricEnvironment = groovyMetricEnvironment;
 
-    String scriptSource;
+    List<String> scriptSources = new ArrayList<>();
     try {
-      if (!JmxConfig.isBlank(config.targetSystem)) {
-        String systemResourcePath = "target-systems/" + config.targetSystem + ".groovy";
-        scriptSource = getTargetSystemResourceAsString(systemResourcePath);
+      if (config.targetSystems.size() != 0) {
+        for (final String target : config.targetSystems) {
+          String systemResourcePath = "target-systems/" + target + ".groovy";
+          scriptSources.add(getTargetSystemResourceAsString(systemResourcePath));
+        }
       } else {
-        scriptSource = getFileAsString(config.groovyScript);
+        scriptSources.add(getFileAsString(config.groovyScript));
       }
     } catch (IOException e) {
       logger.log(Level.SEVERE, "Failed to read groovy script", e);
       throw new ConfigurationException("Failed to read groovy script", e);
     }
 
+    this.scripts = new ArrayList<>();
     try {
-      this.script = new GroovyShell().parse(scriptSource);
+      for (final String source : scriptSources) {
+        this.scripts.add(new GroovyShell().parse(source));
+      }
     } catch (CompilationFailedException e) {
       logger.log(Level.SEVERE, "Failed to compile groovy script", e);
       throw new ConfigurationException("Failed to compile groovy script", e);
@@ -76,7 +83,9 @@ public class GroovyRunner {
     OtelHelper otelHelper = new OtelHelper(jmxClient, this.groovyMetricEnvironment);
     binding.setVariable("otel", otelHelper);
 
-    script.setBinding(binding);
+    for (final Script script : scripts) {
+      script.setBinding(binding);
+    }
   }
 
   private static String getFileAsString(final String fileName) throws IOException {
@@ -134,7 +143,9 @@ private static String getFileFromInputStream(final InputStream is) throws IOExce
   }
 
   public void run() {
-    script.run();
+    for (final Script script : scripts) {
+      script.run();
+    }
     flush();
   }
 

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

@@ -17,8 +17,10 @@
 package io.opentelemetry.contrib.jmxmetrics;
 
 import java.util.Arrays;
+import java.util.LinkedHashSet;
 import java.util.List;
 import java.util.Properties;
+import java.util.Set;
 
 class JmxConfig {
   static final String PREFIX = "otel.";
@@ -45,6 +47,7 @@ class JmxConfig {
   final String serviceUrl;
   final String groovyScript;
   final String targetSystem;
+  final Set<String> targetSystems;
   final int intervalMilliseconds;
   final String exporterType;
 
@@ -69,6 +72,10 @@ class JmxConfig {
     groovyScript = properties.getProperty(GROOVY_SCRIPT);
     targetSystem = properties.getProperty(TARGET_SYSTEM, "").toLowerCase().trim();
 
+    List<String> targets =
+        Arrays.asList(isBlank(targetSystem) ? new String[0] : targetSystem.split(","));
+    targetSystems = new LinkedHashSet<>(targets);
+
     int interval = getProperty(INTERVAL_MILLISECONDS, 10000);
     intervalMilliseconds = interval == 0 ? 10000 : interval;
 
@@ -117,9 +124,10 @@ void validate() {
           "Only one of " + GROOVY_SCRIPT + " or " + TARGET_SYSTEM + " can be specified.");
     }
 
-    if (!isBlank(targetSystem) && !AVAILABLE_TARGET_SYSTEMS.contains(targetSystem)) {
+    if (targetSystems.size() != 0 && !AVAILABLE_TARGET_SYSTEMS.containsAll(targetSystems)) {
       throw new ConfigurationException(
-          String.format("%s must be one of %s", targetSystem, AVAILABLE_TARGET_SYSTEMS));
+          String.format(
+              "%s must specify targets from %s", targetSystems, AVAILABLE_TARGET_SYSTEMS));
     }
 
     if (isBlank(otlpExporterEndpoint)

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

@@ -44,7 +44,7 @@ class GroovyRunnerTest extends UnitTest {
                 })
 
         then: 'it is successfully loaded and runnable'
-        groovyRunner.script != null
+        groovyRunner.scripts.size() == 1
         groovyRunner.run()
         exportCalled
     }

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

@@ -38,6 +38,7 @@ class JmxConfigTest extends UnitTest {
         config.serviceUrl == null
         config.groovyScript == null
         config.targetSystem == ""
+        config.targetSystems == [] as LinkedHashSet
         config.intervalMilliseconds == 10000
         config.exporterType == "logging"
         config.otlpExporterEndpoint == null
@@ -54,7 +55,7 @@ class JmxConfigTest extends UnitTest {
         def properties = [
             "jmx.service.url" : "myServiceUrl",
             "jmx.groovy.script" : "myGroovyScript",
-            "jmx.target.system" : "mytargetsystem",
+            "jmx.target.system" : "mytargetsystem,mytargetsystem,myothertargetsystem,myadditionaltargetsystem",
             "jmx.interval.milliseconds": "123",
             "exporter": "inmemory",
             "exporter.otlp.endpoint": "myOtlpEndpoint",
@@ -71,7 +72,12 @@ class JmxConfigTest extends UnitTest {
         then:
         config.serviceUrl == "myServiceUrl"
         config.groovyScript == "myGroovyScript"
-        config.targetSystem == "mytargetsystem"
+        config.targetSystem == "mytargetsystem,mytargetsystem,myothertargetsystem,myadditionaltargetsystem"
+        config.targetSystems == [
+            "mytargetsystem",
+            "myothertargetsystem",
+            "myadditionaltargetsystem"
+        ] as LinkedHashSet
         config.intervalMilliseconds == 123
         config.exporterType == "inmemory"
         config.otlpExporterEndpoint == "myOtlpEndpoint"
@@ -130,7 +136,7 @@ class JmxConfigTest extends UnitTest {
         setup: "config is set with nonexistant target system"
         [
             "service.url" : "requiredValue",
-            "target.system": "unavailableTargetSystem"
+            "target.system": "jvm,unavailableTargetSystem"
         ].each {
             System.setProperty("otel.jmx.${it.key}", it.value)
         }
@@ -145,6 +151,6 @@ class JmxConfigTest extends UnitTest {
 
         expect: 'config fails to validate'
         raised != null
-        raised.message ==  "unavailabletargetsystem must be one of [cassandra, jvm, kafka, kafka-consumer, kafka-producer]"
+        raised.message ==  "[jvm, unavailabletargetsystem] must specify targets from [cassandra, jvm, kafka, kafka-consumer, kafka-producer]"
     }
 }