jmx-metrics: allow any resource path for classpath resources rules

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

@@ -13,6 +13,7 @@
 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.Paths;
 import java.time.Duration;
 import java.util.logging.Level;
@@ -32,19 +33,24 @@ public void afterAgent(AutoConfiguredOpenTelemetrySdk autoConfiguredSdk) {
       JmxTelemetryBuilder jmx =
           JmxTelemetry.builder(GlobalOpenTelemetry.get())
               .beanDiscoveryDelay(beanDiscoveryDelay(config));
-
       try {
-        config.getList("otel.jmx.config").stream().map(Paths::get).forEach(jmx::addCustomRules);
-        config.getList("otel.jmx.target.system").forEach(jmx::addClassPathRules);
+        config.getList("otel.jmx.config").stream().map(Paths::get).forEach(jmx::addRules);
+        config.getList("otel.jmx.target.system").forEach(target -> addClasspathRules(target, jmx));
       } catch (RuntimeException 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().start();
     }
   }
 
+  private static void addClasspathRules(String target, JmxTelemetryBuilder builder) {
+    ClassLoader classLoader = JmxTelemetryBuilder.class.getClassLoader();
+    String resource = String.format("jmx/rules/%s.yaml", target);
+    InputStream input = classLoader.getResourceAsStream(resource);
+    builder.addRules(input, "classpath " + resource);
+  }
+
   private static Duration beanDiscoveryDelay(ConfigProperties configProperties) {
     Duration discoveryDelay = configProperties.getDuration("otel.jmx.discovery.delay");
     if (discoveryDelay != null) {

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

@@ -41,7 +41,7 @@ void testToVerifyExistingRulesAreValid() throws Exception {
       assertThat(filePath).isRegularFile();
 
       // loading rules from direct file access
-      JmxTelemetry.builder(OpenTelemetry.noop()).addCustomRules(filePath);
+      JmxTelemetry.builder(OpenTelemetry.noop()).addRules(filePath);
     }
   }
 }

instrumentation/jmx-metrics/library/README.md

@@ -32,21 +32,24 @@ implementation("io.opentelemetry.instrumentation:opentelemetry-jmx-metrics:OPENT
 
 ```java
 import io.opentelemetry.api.OpenTelemetry;
+import io.opentelemetry.api.GlobalOpenTelemetry;
 import io.opentelemetry.instrumentation.jmx.JmxTelemetry;
 import io.opentelemetry.instrumentation.jmx.JmxTelemetryBuilder;
 
+import java.time.Duration;
+
 // Get an OpenTelemetry instance
-OpenTelemetry openTelemetry = ...
+OpenTelemetry openTelemetry = GlobalOpenTelemetry.get();
 
 JmxTelemetry jmxTelemetry = JmxTelemetry.builder(openTelemetry)
   // Configure included metrics (optional)
-  .addClasspathRules("tomcat")
-  .addClasspathRules("jetty")
+  .addRules(JmxTelemetry.class.getClassLoader().getResourceAsStream("jmx/rules/jetty.yaml"), "jetty")
+  .addRules(JmxTelemetry.class.getClassLoader().getResourceAsStream("jmx/rules/tomcat.yaml"), "tomcat")
   // Configure custom metrics (optional)
-  .addCustomRules("/path/to/custom-jmx.yaml")
+  .addRules(Paths.get("/path/to/custom-jmx.yaml"))
   // delay bean discovery by 5 seconds
-  .beanDiscoveryDelay(5000)
+  .beanDiscoveryDelay(Duration.ofSeconds(5))
   .build();
 
-jmxTelemetry.startLocal();
+jmxTelemetry.start();
 ```

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

@@ -49,47 +49,70 @@ public JmxTelemetryBuilder beanDiscoveryDelay(Duration delay) {
   }
 
   /**
-   * Adds built-in JMX rules from classpath resource
+   * 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
    */
+  // TODO: deprecate this method after 2.23.0 release in favor of addRules
   @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) {
+    String resourcePath = String.format("jmx/rules/%s.yaml", target);
+    ClassLoader classLoader = JmxTelemetryBuilder.class.getClassLoader();
+    try (InputStream inputStream = classLoader.getResourceAsStream(resourcePath)) {
+      return addRules(inputStream, resourcePath);
+    } catch (IOException e) {
       throw new IllegalArgumentException(
-          "Unable to load JMX rules from classpath: " + yamlResource, e);
+          "Unable to load JMX rules from resource " + resourcePath, e);
     }
+  }
+
+  /**
+   * Adds JMX rules from input stream
+   *
+   * @param input input to read rules from
+   * @param description input description, used for user-friendly logs and parsing error messages
+   * @throws IllegalArgumentException when input is {@literal null}
+   */
+  @CanIgnoreReturnValue
+  public JmxTelemetryBuilder addRules(InputStream input, String description) {
+    if (input == null) {
+      throw new IllegalArgumentException("JMX rules not found for " + description);
+    }
+    logger.log(FINE, "Adding JMX config from {0}", description);
+    RuleParser parserInstance = RuleParser.get();
+    parserInstance.addMetricDefsTo(metricConfiguration, input, description);
     return this;
   }
 
   /**
-   * Adds custom JMX rules from file system path
+   * Adds 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
+   * @throws IllegalArgumentException in case of parsing errors or when file does not exist
    */
   @CanIgnoreReturnValue
-  public JmxTelemetryBuilder addCustomRules(Path path) {
-    logger.log(FINE, "Adding JMX config from file: {0}", path);
-    RuleParser parserInstance = RuleParser.get();
+  public JmxTelemetryBuilder addRules(Path path) {
     try (InputStream inputStream = Files.newInputStream(path)) {
-      parserInstance.addMetricDefsTo(metricConfiguration, inputStream, path.toString());
+      return addRules(inputStream, "file " + path);
     } catch (IOException e) {
-      throw new IllegalArgumentException("Unable to load JMX rules in path: " + path, e);
+      throw new IllegalArgumentException("Unable to load JMX rules from: " + path, 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
+   */
+  // TODO: deprecate this method after 2.23.0 release in favor of addRules
+  @CanIgnoreReturnValue
+  public JmxTelemetryBuilder addCustomRules(Path path) {
+    return addRules(path);
   }
 
   public JmxTelemetry build() {

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

@@ -162,21 +162,24 @@ 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
+   * @param description description of the YAML ruleset like file name or classpath resource
    * @throws IllegalArgumentException when unable to parse YAML
    */
-  public void addMetricDefsTo(MetricConfiguration conf, InputStream is, String id) {
+  public void addMetricDefsTo(MetricConfiguration conf, InputStream is, String description) {
     try {
       JmxConfig config = loadConfig(is);
-      logger.log(INFO, "{0}: found {1} metric rules", new Object[] {id, config.getRules().size()});
+      logger.log(
+          INFO,
+          "{0}: found {1} metric rules",
+          new Object[] {description, config.getRules().size()});
       config.addMetricDefsTo(conf);
     } catch (Exception 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.
       String msg =
           String.format(
               "Failed to parse YAML rules from %s: %s %s",
-              id, rootCause(exception), exception.getMessage());
+              description, rootCause(exception), exception.getMessage());
       throw new IllegalArgumentException(msg, exception);
     }
   }

instrumentation/jmx-metrics/library/src/test/java/io/opentelemetry/instrumentation/jmx/JmxTelemetryTest.java

@@ -9,6 +9,7 @@
 import static org.assertj.core.api.Assertions.assertThatThrownBy;
 
 import io.opentelemetry.api.OpenTelemetry;
+import java.io.InputStream;
 import java.nio.charset.StandardCharsets;
 import java.nio.file.Files;
 import java.nio.file.Path;
@@ -27,29 +28,41 @@ void createDefault() {
   @Test
   void missingClasspathTarget() {
     JmxTelemetryBuilder builder = JmxTelemetry.builder(OpenTelemetry.noop());
-    assertThatThrownBy(() -> builder.addClassPathRules("should-not-exist"))
-        .isInstanceOf(IllegalArgumentException.class);
+    assertThatThrownBy(() -> builder.addRules(null, "something is missing"))
+        .isInstanceOf(IllegalArgumentException.class)
+        .hasMessageContaining("not found")
+        .hasMessageContaining("something is missing");
   }
 
   @Test
   void invalidClasspathTarget() {
     JmxTelemetryBuilder builder = JmxTelemetry.builder(OpenTelemetry.noop());
-    assertThatThrownBy(() -> builder.addClassPathRules("invalid"))
-        .isInstanceOf(IllegalArgumentException.class);
+    assertThatThrownBy(() -> addClasspathRules(builder, "jmx/rules/invalid.yaml"))
+        .isInstanceOf(IllegalArgumentException.class)
+        .describedAs("must have an exception message including the invalid resource path")
+        .hasMessageContaining("jmx/rules/invalid.yaml");
   }
 
   @Test
-  void knownClassPathTarget() {
-    JmxTelemetry.builder(OpenTelemetry.noop()).addClassPathRules("jvm").build();
+  void knownValidYaml() {
+    JmxTelemetryBuilder jmxtelemetry = JmxTelemetry.builder(OpenTelemetry.noop());
+    addClasspathRules(jmxtelemetry, "jmx/rules/jvm.yaml");
+    assertThat(jmxtelemetry.build()).isNotNull();
+  }
+
+  private static void addClasspathRules(JmxTelemetryBuilder builder, String path) {
+    InputStream input = JmxTelemetryTest.class.getClassLoader().getResourceAsStream(path);
+    builder.addRules(input, path);
   }
 
   @Test
   void invalidExternalYaml(@TempDir Path dir) throws Exception {
     Path invalid = Files.createTempFile(dir, "invalid", ".yaml");
     Files.write(invalid, ":this !is /not YAML".getBytes(StandardCharsets.UTF_8));
     JmxTelemetryBuilder builder = JmxTelemetry.builder(OpenTelemetry.noop());
-    assertThatThrownBy(() -> builder.addCustomRules(invalid))
-        .isInstanceOf(IllegalArgumentException.class);
+    assertThatThrownBy(() -> builder.addRules(invalid))
+        .isInstanceOf(IllegalArgumentException.class)
+        .hasMessageContaining(invalid.toString());
   }
 
   @Test