jmx idiomatic library API - part 1

instrumentation/jmx-metrics/javaagent/src/main/java/io/opentelemetry/instrumentation/javaagent/jmx/JmxMetricInsightInstaller.java

@@ -5,38 +5,43 @@
 
 package io.opentelemetry.instrumentation.javaagent.jmx;
 
-import static java.util.logging.Level.FINE;
-import static java.util.logging.Level.INFO;
-
 import com.google.auto.service.AutoService;
 import io.opentelemetry.api.GlobalOpenTelemetry;
-import io.opentelemetry.instrumentation.jmx.engine.JmxMetricInsight;
-import io.opentelemetry.instrumentation.jmx.engine.MetricConfiguration;
-import io.opentelemetry.instrumentation.jmx.yaml.RuleParser;
+import io.opentelemetry.instrumentation.jmx.JmxTelemetry;
+import io.opentelemetry.instrumentation.jmx.JmxTelemetryBuilder;
 import io.opentelemetry.javaagent.extension.AgentListener;
 import io.opentelemetry.sdk.autoconfigure.AutoConfiguredOpenTelemetrySdk;
 import io.opentelemetry.sdk.autoconfigure.internal.AutoConfigureUtil;
 import io.opentelemetry.sdk.autoconfigure.spi.ConfigProperties;
-import java.io.InputStream;
-import java.nio.file.Files;
 import java.nio.file.Paths;
 import java.time.Duration;
-import java.util.List;
+import java.util.logging.Level;
+import java.util.logging.Logger;
 
 /** An {@link AgentListener} that enables JMX metrics during agent startup. */
 @AutoService(AgentListener.class)
 public class JmxMetricInsightInstaller implements AgentListener {
 
+  private static final Logger logger = Logger.getLogger(JmxMetricInsightInstaller.class.getName());
+
   @Override
   public void afterAgent(AutoConfiguredOpenTelemetrySdk autoConfiguredSdk) {
     ConfigProperties config = AutoConfigureUtil.getConfig(autoConfiguredSdk);
 
     if (config.getBoolean("otel.jmx.enabled", true)) {
-      JmxMetricInsight service =
-          JmxMetricInsight.createService(
-              GlobalOpenTelemetry.get(), beanDiscoveryDelay(config).toMillis());
-      MetricConfiguration conf = buildMetricConfiguration(config);
-      service.startLocal(conf);
+      JmxTelemetryBuilder jmx =
+          JmxTelemetry.builder(GlobalOpenTelemetry.get())
+              .beanDiscoveryDelay(beanDiscoveryDelay(config).toMillis());
+
+      try {
+        config.getList("otel.jmx.config").stream().map(Paths::get).forEach(jmx::addCustomRules);
+        config.getList("otel.jmx.target.system").forEach(jmx::addClassPathRules);
+      } catch (IllegalArgumentException e) {
+        // for now only log JMX errors as they do not prevent agent startup
+        logger.log(Level.SEVERE, "Error while loading JMX configuration", e);
+      }
+
+      jmx.build().startLocal();
     }
   }
 
@@ -50,58 +55,4 @@ private static Duration beanDiscoveryDelay(ConfigProperties configProperties) {
     // It makes sense for both of these values to be similar.
     return configProperties.getDuration("otel.metric.export.interval", Duration.ofMinutes(1));
   }
-
-  private static String resourceFor(String platform) {
-    return "/jmx/rules/" + platform + ".yaml";
-  }
-
-  private static void addRulesForPlatform(String platform, MetricConfiguration conf) {
-    String yamlResource = resourceFor(platform);
-    try (InputStream inputStream =
-        JmxMetricInsightInstaller.class.getResourceAsStream(yamlResource)) {
-      if (inputStream != null) {
-        JmxMetricInsight.getLogger().log(FINE, "Opened input stream {0}", yamlResource);
-        RuleParser parserInstance = RuleParser.get();
-        parserInstance.addMetricDefsTo(conf, inputStream, platform);
-      } else {
-        JmxMetricInsight.getLogger().log(INFO, "No support found for {0}", platform);
-      }
-    } catch (Exception e) {
-      JmxMetricInsight.getLogger().warning(e.getMessage());
-    }
-  }
-
-  private static void buildFromDefaultRules(
-      MetricConfiguration conf, ConfigProperties configProperties) {
-    List<String> platforms = configProperties.getList("otel.jmx.target.system");
-    for (String platform : platforms) {
-      addRulesForPlatform(platform, conf);
-    }
-  }
-
-  private static void buildFromUserRules(
-      MetricConfiguration conf, ConfigProperties configProperties) {
-    List<String> configFiles = configProperties.getList("otel.jmx.config");
-    for (String configFile : configFiles) {
-      JmxMetricInsight.getLogger().log(FINE, "JMX config file name: {0}", configFile);
-      RuleParser parserInstance = RuleParser.get();
-      try (InputStream inputStream = Files.newInputStream(Paths.get(configFile))) {
-        parserInstance.addMetricDefsTo(conf, inputStream, configFile);
-      } catch (Exception e) {
-        // yaml parsing errors are caught and logged inside of addMetricDefsTo
-        // only file access related exceptions are expected here
-        JmxMetricInsight.getLogger().warning(e.toString());
-      }
-    }
-  }
-
-  private static MetricConfiguration buildMetricConfiguration(ConfigProperties configProperties) {
-    MetricConfiguration metricConfiguration = new MetricConfiguration();
-
-    buildFromDefaultRules(metricConfiguration, configProperties);
-
-    buildFromUserRules(metricConfiguration, configProperties);
-
-    return metricConfiguration;
-  }
 }

instrumentation/jmx-metrics/javaagent/src/test/java/io/opentelemetry/instrumentation/javaagent/jmx/JmxMetricInsightInstallerTest.java

@@ -7,18 +7,13 @@
 
 import static org.assertj.core.api.Assertions.assertThat;
 
-import io.opentelemetry.instrumentation.jmx.yaml.JmxConfig;
-import io.opentelemetry.instrumentation.jmx.yaml.JmxRule;
+import io.opentelemetry.api.OpenTelemetry;
+import io.opentelemetry.instrumentation.jmx.JmxTelemetry;
 import io.opentelemetry.instrumentation.jmx.yaml.RuleParser;
-import java.io.File;
-import java.io.FileInputStream;
-import java.io.InputStream;
-import java.nio.file.Files;
 import java.nio.file.Path;
 import java.nio.file.Paths;
 import java.util.Arrays;
 import java.util.HashSet;
-import java.util.List;
 import java.util.Set;
 import org.junit.jupiter.api.Test;
 
@@ -39,34 +34,14 @@ void testToVerifyExistingRulesAreValid() throws Exception {
     assertThat(parser).isNotNull();
 
     Path path = Paths.get(PATH_TO_ALL_EXISTING_RULES);
-    assertThat(Files.exists(path)).isTrue();
+    assertThat(path).isNotEmptyDirectory();
 
-    File existingRulesDir = path.toFile();
-    File[] existingRules = existingRulesDir.listFiles();
-    Set<String> filesChecked = new HashSet<>();
+    for (String file : FILES_TO_BE_TESTED) {
+      Path filePath = path.resolve(file);
+      assertThat(filePath).isRegularFile();
 
-    for (File file : existingRules) {
-      // make sure we only test the files that we supposed to test
-      String fileName = file.getName();
-      if (FILES_TO_BE_TESTED.contains(fileName)) {
-        testRulesAreValid(file, parser);
-        filesChecked.add(fileName);
-      }
-    }
-    // make sure we checked all the files that are supposed to be here
-    assertThat(filesChecked).isEqualTo(FILES_TO_BE_TESTED);
-  }
-
-  void testRulesAreValid(File file, RuleParser parser) throws Exception {
-    try (InputStream inputStream = new FileInputStream(file)) {
-      JmxConfig config = parser.loadConfig(inputStream);
-      assertThat(config).isNotNull();
-
-      List<JmxRule> defs = config.getRules();
-      // make sure all the rules in that file are valid
-      for (JmxRule rule : defs) {
-        rule.buildMetricDef();
-      }
+      // loading rules from direct file access
+      JmxTelemetry.builder(OpenTelemetry.noop()).addCustomRules(filePath);
     }
   }
 }

instrumentation/jmx-metrics/library/README.md

@@ -32,16 +32,21 @@ implementation("io.opentelemetry.instrumentation:opentelemetry-jmx-metrics:OPENT
 
 ```java
 import io.opentelemetry.api.OpenTelemetry;
-import io.opentelemetry.instrumentation.jmx.engine.JmxMetricInsight;
-import io.opentelemetry.instrumentation.jmx.engine.MetricConfiguration;
+import io.opentelemetry.instrumentation.jmx.JmxTelemetry;
+import io.opentelemetry.instrumentation.jmx.JmxTelemetryBuilder;
 
 // Get an OpenTelemetry instance
-OpenTelemetry openTelemetry = ...;
-
-JmxMetricInsight jmxMetricInsight = JmxMetricInsight.createService(openTelemetry, 5000);
-
-// Configure your JMX metrics
-MetricConfiguration config = new MetricConfiguration();
-
-jmxMetricInsight.startLocal(config);
+OpenTelemetry openTelemetry = ...
+
+JmxTelemetry jmxTelemetry = JmxTelemetry.builder(openTelemetry)
+  // Configure included metrics (optional)
+  .addClasspathRules("tomcat")
+  .addClasspathRules("jetty")
+  // Configure custom metrics (optional)
+  .addCustomRules("/path/to/custom-jmx.yaml")
+  // delay bean discovery by 5 seconds
+  .beanDiscoveryDelay(5000)
+  .build();
+
+jmxTelemetry.startLocal();
 ```

instrumentation/jmx-metrics/library/src/main/java/io/opentelemetry/instrumentation/jmx/JmxTelemetry.java

@@ -0,0 +1,38 @@
+/*
+ * Copyright The OpenTelemetry Authors
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+package io.opentelemetry.instrumentation.jmx;
+
+import io.opentelemetry.api.OpenTelemetry;
+import io.opentelemetry.instrumentation.jmx.engine.JmxMetricInsight;
+import io.opentelemetry.instrumentation.jmx.engine.MetricConfiguration;
+import java.util.List;
+import java.util.function.Supplier;
+import javax.management.MBeanServerConnection;
+
+public class JmxTelemetry {
+
+  private final JmxMetricInsight service;
+  private final MetricConfiguration metricConfiguration;
+
+  public static JmxTelemetryBuilder builder(OpenTelemetry openTelemetry) {
+    return new JmxTelemetryBuilder(openTelemetry);
+  }
+
+  JmxTelemetry(
+      OpenTelemetry openTelemetry, long discoveryDelayMs, MetricConfiguration metricConfiguration) {
+    this.service = JmxMetricInsight.createService(openTelemetry, discoveryDelayMs);
+    this.metricConfiguration = metricConfiguration;
+  }
+
+  @SuppressWarnings("unused") // used by jmx-scraper with remote connection
+  public void startRemote(Supplier<List<? extends MBeanServerConnection>> connections) {
+    service.startRemote(metricConfiguration, connections);
+  }
+
+  public void startLocal() {
+    service.startLocal(metricConfiguration);
+  }
+}

instrumentation/jmx-metrics/library/src/main/java/io/opentelemetry/instrumentation/jmx/JmxTelemetryBuilder.java

@@ -0,0 +1,96 @@
+/*
+ * Copyright The OpenTelemetry Authors
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+package io.opentelemetry.instrumentation.jmx;
+
+import static java.util.logging.Level.FINE;
+
+import com.google.errorprone.annotations.CanIgnoreReturnValue;
+import io.opentelemetry.api.OpenTelemetry;
+import io.opentelemetry.instrumentation.jmx.engine.MetricConfiguration;
+import io.opentelemetry.instrumentation.jmx.yaml.RuleParser;
+import java.io.IOException;
+import java.io.InputStream;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.util.logging.Logger;
+
+public class JmxTelemetryBuilder {
+
+  private static final Logger logger = Logger.getLogger(JmxTelemetryBuilder.class.getName());
+
+  private final OpenTelemetry openTelemetry;
+  private final MetricConfiguration metricConfiguration;
+  private long discoveryDelayMs;
+
+  JmxTelemetryBuilder(OpenTelemetry openTelemetry) {
+    this.openTelemetry = openTelemetry;
+    this.discoveryDelayMs = 0;
+    this.metricConfiguration = new MetricConfiguration();
+  }
+
+  /**
+   * Sets initial delay for MBean discovery
+   *
+   * @param delayMs delay in milliseconds
+   * @return builder instance
+   */
+  @CanIgnoreReturnValue
+  public JmxTelemetryBuilder beanDiscoveryDelay(long delayMs) {
+    if (delayMs < 0) {
+      throw new IllegalArgumentException("delay must be greater than 0");
+    }
+    this.discoveryDelayMs = delayMs;
+    return this;
+  }
+
+  /**
+   * Adds built-in JMX rules from classpath resource
+   *
+   * @param target name of target in /jmx/rules/{target}.yaml classpath resource
+   * @return builder instance
+   * @throws IllegalArgumentException when classpath resource does not exist or can't be parsed
+   */
+  @CanIgnoreReturnValue
+  public JmxTelemetryBuilder addClassPathRules(String target) {
+    String yamlResource = String.format("jmx/rules/%s.yaml", target);
+    try (InputStream inputStream =
+        JmxTelemetryBuilder.class.getClassLoader().getResourceAsStream(yamlResource)) {
+      if (inputStream == null) {
+        throw new IllegalArgumentException("JMX rules not found in classpath: " + yamlResource);
+      }
+      logger.log(FINE, "Adding JMX config from classpath for {0}", yamlResource);
+      RuleParser parserInstance = RuleParser.get();
+      parserInstance.addMetricDefsTo(metricConfiguration, inputStream, target);
+    } catch (Exception e) {
+      throw new IllegalArgumentException(
+          "Unable to load JMX rules from classpath: " + yamlResource, e);
+    }
+    return this;
+  }
+
+  /**
+   * Adds custom JMX rules from file system path
+   *
+   * @param path path to yaml file
+   * @return builder instance
+   * @throws IllegalArgumentException when classpath resource does not exist or can't be parsed
+   */
+  @CanIgnoreReturnValue
+  public JmxTelemetryBuilder addCustomRules(Path path) {
+    logger.log(FINE, "Adding JMX config from file: {0}", path);
+    RuleParser parserInstance = RuleParser.get();
+    try (InputStream inputStream = Files.newInputStream(path)) {
+      parserInstance.addMetricDefsTo(metricConfiguration, inputStream, path.toString());
+    } catch (IOException e) {
+      throw new IllegalArgumentException("Unable to load JMX rules in path: " + path, e);
+    }
+    return this;
+  }
+
+  public JmxTelemetry build() {
+    return new JmxTelemetry(openTelemetry, discoveryDelayMs, metricConfiguration);
+  }
+}

instrumentation/jmx-metrics/library/src/main/java/io/opentelemetry/instrumentation/jmx/engine/JmxMetricInsight.java

@@ -52,6 +52,7 @@ public void startLocal(MetricConfiguration conf) {
    * @param conf metric configuration
    * @param connections supplier for list of remote connections
    */
+  @SuppressWarnings("unused") // used by jmx-scraper with remote connection
   public void startRemote(
       MetricConfiguration conf, Supplier<List<? extends MBeanServerConnection>> connections) {
     start(conf, connections);

instrumentation/jmx-metrics/library/src/main/java/io/opentelemetry/instrumentation/jmx/yaml/RuleParser.java

@@ -7,7 +7,6 @@
 
 import static java.util.Collections.emptyList;
 import static java.util.logging.Level.INFO;
-import static java.util.logging.Level.WARNING;
 
 import io.opentelemetry.instrumentation.jmx.engine.MetricConfiguration;
 import java.io.InputStream;
@@ -164,20 +163,21 @@ private static void failOnExtraKeys(Map<String, Object> yaml) {
    * @param conf the metric configuration
    * @param is the InputStream with the YAML rules
    * @param id identifier of the YAML ruleset, such as a filename
+   * @throws IllegalArgumentException when unable to parse YAML
    */
   public void addMetricDefsTo(MetricConfiguration conf, InputStream is, String id) {
     try {
       JmxConfig config = loadConfig(is);
       logger.log(INFO, "{0}: found {1} metric rules", new Object[] {id, config.getRules().size()});
       config.addMetricDefsTo(conf);
     } catch (Exception exception) {
-      logger.log(
-          WARNING,
-          "Failed to parse YAML rules from {0}: {1}",
-          new Object[] {id, rootCause(exception)});
       // It is essential that the parser exception is made visible to the user.
       // It contains contextual information about any syntax issues found by the parser.
-      logger.log(WARNING, exception.toString());
+      String msg =
+          String.format(
+              "Failed to parse YAML rules from %s: %s %s",
+              id, rootCause(exception), exception.getMessage());
+      throw new IllegalArgumentException(msg, exception);
     }
   }