jmx gatherer: Remove manual exporter flush (#190)

* jmx: Add missing tomcat target system

* jmx: tear down gatherer test container

* jmx: remove manual flush and rely on `otel.metric.export.interval` property

Author: rmfitzpatrick

jmx-metrics/README.md

@@ -73,6 +73,7 @@ mutually exclusive with `otel.jmx.groovy.script`. The currently supported target
 | [`kafka`](./docs/target-systems/kafka.md) |
 | [`kafka-consumer`](./docs/target-systems/kafka-consumer.md) |
 | [`kafka-producer`](./docs/target-systems/kafka-producer.md) |
+| [`tomcat`](./docs/target-systems/tomcat.md) |
 
 ### JMX Query Helpers
 
@@ -218,7 +219,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` | 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.jmx.interval.milliseconds` | no | How often, in milliseconds, the Groovy script should be run. Value will also be used for `otel.metric.export.interval`, if unset, to control asynchronous updates and metric exporting. 10000 by default. |
 | `otel.jmx.username` | no | Username for JMX authentication, if applicable. |
 | `otel.jmx.password` | no | Password for JMX authentication, if applicable. |
 | `otel.jmx.remote.profile` | no | Supported JMX remote profiles are TLS in combination with SASL profiles: SASL/PLAIN, SASL/DIGEST-MD5 and SASL/CRAM-MD5. Thus valid `jmxRemoteProfiles` values are: `SASL/PLAIN`, `SASL/DIGEST-MD5`, `SASL/CRAM-MD5`, `TLS SASL/PLAIN`, `TLS SASL/DIGEST-MD5` and `TLS SASL/CRAM-MD5`. |

jmx-metrics/src/integrationTest/java/io/opentelemetry/contrib/jmxmetrics/AbstractIntegrationTest.java

@@ -104,6 +104,7 @@ void beforeAll() {
   @SuppressWarnings("FutureReturnValueIgnored")
   void afterAll() {
     otlpServer.stop();
+    scraper.stop();
   }
 
   @BeforeEach

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

@@ -135,15 +135,6 @@ public void run() {
     for (final Script script : scripts) {
       script.run();
     }
-    flush();
-  }
-
-  public void flush() {
-    groovyMetricEnvironment.flush();
-  }
-
-  public void shutdown() {
-    flush();
   }
 
   // Visible for testing

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

@@ -20,7 +20,7 @@ class JmxConfig {
   static final String METRICS_EXPORTER_TYPE = PREFIX + "metrics.exporter";
   static final String EXPORTER = PREFIX + "exporter.";
 
-  static final String EXPORTER_INTERVAL = PREFIX + "imr.export.interval";
+  static final String EXPORTER_INTERVAL = PREFIX + "metric.export.interval";
 
   static final String OTLP_ENDPOINT = EXPORTER + "otlp.endpoint";
 

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

@@ -58,7 +58,6 @@ public void run() {
   private void shutdown() {
     logger.info("Shutting down JmxMetrics Groovy runner and exporting final metrics.");
     exec.shutdown();
-    runner.shutdown();
   }
 
   private static JmxConfig getConfigFromArgs(final String[] args) {

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

@@ -11,7 +11,6 @@
 
 import java.util.Collections;
 import java.util.List;
-import java.util.concurrent.atomic.AtomicBoolean;
 import javax.management.ObjectName;
 import org.junit.jupiter.api.Test;
 import org.junitpioneer.jupiter.SetSystemProperty;
@@ -28,8 +27,6 @@ void loadTargetScript() throws Exception {
 
     assertThatCode(config::validate).doesNotThrowAnyException();
 
-    AtomicBoolean exportCalled = new AtomicBoolean();
-
     JmxClient stub =
         new JmxClient(config) {
           @Override
@@ -38,20 +35,10 @@ public List<ObjectName> query(ObjectName objectName) {
           }
         };
 
-    GroovyRunner runner =
-        new GroovyRunner(
-            config,
-            stub,
-            new GroovyMetricEnvironment(config) {
-              @Override
-              public void flush() {
-                exportCalled.set(true);
-              }
-            });
+    GroovyRunner runner = new GroovyRunner(config, stub, new GroovyMetricEnvironment(config));
 
     assertThat(runner.getScripts()).hasSize(1);
     runner.run();
-    assertThat(exportCalled).isTrue();
   }
 
   @Test

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

@@ -9,6 +9,7 @@
 import static org.assertj.core.api.Assertions.assertThatThrownBy;
 
 import org.junit.jupiter.api.Test;
+import org.junitpioneer.jupiter.ClearSystemProperty;
 import org.junitpioneer.jupiter.SetSystemProperty;
 
 class JmxConfigTest {
@@ -20,13 +21,12 @@ void staticValues() {
   }
 
   @Test
+  @ClearSystemProperty(key = "otel.metric.export.interval")
   void defaultValues() {
     JmxConfig config = new JmxConfig();
 
     assertThat(config.serviceUrl).isNull();
-    ;
     assertThat(config.groovyScript).isNull();
-    ;
     assertThat(config.targetSystem).isEmpty();
     assertThat(config.targetSystems).isEmpty();
     assertThat(config.intervalMilliseconds).isEqualTo(10000);
@@ -38,6 +38,7 @@ void defaultValues() {
     assertThat(config.password).isNull();
     assertThat(config.remoteProfile).isNull();
     assertThat(config.realm).isNull();
+    assertThat(config.properties.getProperty("otel.metric.export.interval")).isEqualTo("10000");
   }
 
   @Test
@@ -116,4 +117,12 @@ void invalidTargetSystem() {
             "[jvm, unavailabletargetsystem] must specify targets from "
                 + "[cassandra, jvm, kafka, kafka-consumer, kafka-producer, tomcat]");
   }
+
+  @Test
+  @SetSystemProperty(key = "otel.metric.export.interval", value = "123")
+  void otelMetricExportIntervalRespected() {
+    JmxConfig config = new JmxConfig();
+    assertThat(config.intervalMilliseconds).isEqualTo(10000);
+    assertThat(config.properties.getProperty("otel.metric.export.interval")).isEqualTo("123");
+  }
 }