diff --git a/instrumentation/jmx-metrics/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jmx/JmxMetricInsightInstaller.java b/instrumentation/jmx-metrics/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jmx/JmxMetricInsightInstaller.java index 411842f6a26a..3a67fc0da0c1 100644 --- a/instrumentation/jmx-metrics/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jmx/JmxMetricInsightInstaller.java +++ b/instrumentation/jmx-metrics/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jmx/JmxMetricInsightInstaller.java @@ -13,6 +13,8 @@ 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.Path; import java.nio.file.Paths; import java.time.Duration; import java.util.logging.Level; @@ -33,18 +35,37 @@ public void afterAgent(AutoConfiguredOpenTelemetrySdk autoConfiguredSdk) { 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); - } 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); - } + config.getList("otel.jmx.config").stream() + .map(Paths::get) + .forEach(path -> addFileRules(path, jmx)); + config.getList("otel.jmx.target.system").forEach(target -> addClasspathRules(target, jmx)); jmx.build().start(); } } + private static void addFileRules(Path path, JmxTelemetryBuilder builder) { + try { + builder.addRules(path); + } catch (RuntimeException e) { + // for now only log JMX metric configuration errors as they do not prevent agent startup + logger.log(Level.SEVERE, "Error while loading JMX configuration from " + path, e); + } + } + + 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); + try { + builder.addRules(input); + } catch (RuntimeException e) { + // for now only log JMX metric configuration errors as they do not prevent agent startup + logger.log( + Level.SEVERE, "Error while loading JMX configuration from classpath " + resource, e); + } + } + private static Duration beanDiscoveryDelay(ConfigProperties configProperties) { Duration discoveryDelay = configProperties.getDuration("otel.jmx.discovery.delay"); if (discoveryDelay != null) { diff --git a/instrumentation/jmx-metrics/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/jmx/JmxMetricInsightInstallerTest.java b/instrumentation/jmx-metrics/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/jmx/JmxMetricInsightInstallerTest.java index 41b9cbc836ea..bb2b5fb5abf3 100644 --- a/instrumentation/jmx-metrics/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/jmx/JmxMetricInsightInstallerTest.java +++ b/instrumentation/jmx-metrics/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/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); } } } diff --git a/instrumentation/jmx-metrics/library/README.md b/instrumentation/jmx-metrics/library/README.md index 2803f10d60cb..551450558b71 100644 --- a/instrumentation/jmx-metrics/library/README.md +++ b/instrumentation/jmx-metrics/library/README.md @@ -35,18 +35,20 @@ import io.opentelemetry.api.OpenTelemetry; import io.opentelemetry.instrumentation.jmx.JmxTelemetry; import io.opentelemetry.instrumentation.jmx.JmxTelemetryBuilder; +import java.time.Duration; + // Get an OpenTelemetry instance -OpenTelemetry openTelemetry = ... +OpenTelemetry openTelemetry = ...; 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(); ``` diff --git a/instrumentation/jmx-metrics/library/src/main/java/io/opentelemetry/instrumentation/jmx/JmxTelemetryBuilder.java b/instrumentation/jmx-metrics/library/src/main/java/io/opentelemetry/instrumentation/jmx/JmxTelemetryBuilder.java index 8f1c5a4d51f0..4f9ddc720bd9 100644 --- a/instrumentation/jmx-metrics/library/src/main/java/io/opentelemetry/instrumentation/jmx/JmxTelemetryBuilder.java +++ b/instrumentation/jmx-metrics/library/src/main/java/io/opentelemetry/instrumentation/jmx/JmxTelemetryBuilder.java @@ -49,47 +49,73 @@ 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(); + logger.log(FINE, "Adding JMX config from classpath {0}", resourcePath); + try (InputStream inputStream = classLoader.getResourceAsStream(resourcePath)) { + return addRules(inputStream); + } 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 + * @throws IllegalArgumentException when input is {@literal null} or can't be parsed + */ + @CanIgnoreReturnValue + public JmxTelemetryBuilder addRules(InputStream input) { + if (input == null) { + throw new IllegalArgumentException("missing JMX rules"); + } + RuleParser parserInstance = RuleParser.get(); + parserInstance.addMetricDefsTo(metricConfiguration, input); 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) { + if (path == null) { + throw new IllegalArgumentException("missing JMX rules"); + } try (InputStream inputStream = Files.newInputStream(path)) { - parserInstance.addMetricDefsTo(metricConfiguration, inputStream, path.toString()); + logger.log(FINE, "Adding JMX config from file {0}", path); + return addRules(inputStream); } 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() { diff --git a/instrumentation/jmx-metrics/library/src/main/java/io/opentelemetry/instrumentation/jmx/yaml/RuleParser.java b/instrumentation/jmx-metrics/library/src/main/java/io/opentelemetry/instrumentation/jmx/yaml/RuleParser.java index 1fc7f13dd78a..e22710555c8f 100644 --- a/instrumentation/jmx-metrics/library/src/main/java/io/opentelemetry/instrumentation/jmx/yaml/RuleParser.java +++ b/instrumentation/jmx-metrics/library/src/main/java/io/opentelemetry/instrumentation/jmx/yaml/RuleParser.java @@ -6,7 +6,7 @@ package io.opentelemetry.instrumentation.jmx.yaml; import static java.util.Collections.emptyList; -import static java.util.logging.Level.INFO; +import static java.util.logging.Level.FINE; import io.opentelemetry.instrumentation.jmx.engine.MetricConfiguration; import java.io.InputStream; @@ -162,21 +162,19 @@ private static void failOnExtraKeys(Map 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) { + public void addMetricDefsTo(MetricConfiguration conf, InputStream is) { try { JmxConfig config = loadConfig(is); - logger.log(INFO, "{0}: found {1} metric rules", new Object[] {id, config.getRules().size()}); + logger.log(FINE, "found {1} metric rules", 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()); + "Failed to parse YAML rules : %s %s", rootCause(exception), exception.getMessage()); throw new IllegalArgumentException(msg, exception); } } diff --git a/instrumentation/jmx-metrics/library/src/test/java/io/opentelemetry/instrumentation/jmx/JmxTelemetryTest.java b/instrumentation/jmx-metrics/library/src/test/java/io/opentelemetry/instrumentation/jmx/JmxTelemetryTest.java index e840b91b1be3..749d0737a5c5 100644 --- a/instrumentation/jmx-metrics/library/src/test/java/io/opentelemetry/instrumentation/jmx/JmxTelemetryTest.java +++ b/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; @@ -25,22 +26,31 @@ void createDefault() { } @Test - void missingClasspathTarget() { + void throwsExceptionOnNullInput() { JmxTelemetryBuilder builder = JmxTelemetry.builder(OpenTelemetry.noop()); - assertThatThrownBy(() -> builder.addClassPathRules("should-not-exist")) + assertThatThrownBy(() -> builder.addRules((InputStream) null)) + .isInstanceOf(IllegalArgumentException.class); + assertThatThrownBy(() -> builder.addRules((Path) null)) .isInstanceOf(IllegalArgumentException.class); } @Test void invalidClasspathTarget() { JmxTelemetryBuilder builder = JmxTelemetry.builder(OpenTelemetry.noop()); - assertThatThrownBy(() -> builder.addClassPathRules("invalid")) + assertThatThrownBy(() -> addClasspathRules(builder, "jmx/rules/invalid.yaml")) .isInstanceOf(IllegalArgumentException.class); } @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); } @Test @@ -48,7 +58,7 @@ 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)) + assertThatThrownBy(() -> builder.addRules(invalid)) .isInstanceOf(IllegalArgumentException.class); }