diff --git a/instrumentation/jmx-metrics/javaagent/src/main/java/io/opentelemetry/instrumentation/javaagent/jmx/JmxMetricInsightInstaller.java b/instrumentation/jmx-metrics/javaagent/src/main/java/io/opentelemetry/instrumentation/javaagent/jmx/JmxMetricInsightInstaller.java index 39d32e69e4a8..f8a4425e6c57 100644 --- a/instrumentation/jmx-metrics/javaagent/src/main/java/io/opentelemetry/instrumentation/javaagent/jmx/JmxMetricInsightInstaller.java +++ b/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 platforms = configProperties.getList("otel.jmx.target.system"); - for (String platform : platforms) { - addRulesForPlatform(platform, conf); - } - } - - private static void buildFromUserRules( - MetricConfiguration conf, ConfigProperties configProperties) { - List 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; - } } diff --git a/instrumentation/jmx-metrics/javaagent/src/test/java/io/opentelemetry/instrumentation/javaagent/jmx/JmxMetricInsightInstallerTest.java b/instrumentation/jmx-metrics/javaagent/src/test/java/io/opentelemetry/instrumentation/javaagent/jmx/JmxMetricInsightInstallerTest.java index 2f486ade2e54..aa823292fb65 100644 --- a/instrumentation/jmx-metrics/javaagent/src/test/java/io/opentelemetry/instrumentation/javaagent/jmx/JmxMetricInsightInstallerTest.java +++ b/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 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 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); } } } diff --git a/instrumentation/jmx-metrics/library/README.md b/instrumentation/jmx-metrics/library/README.md index c7df1232e73f..2803f10d60cb 100644 --- a/instrumentation/jmx-metrics/library/README.md +++ b/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(); ``` diff --git a/instrumentation/jmx-metrics/library/src/main/java/io/opentelemetry/instrumentation/jmx/JmxTelemetry.java b/instrumentation/jmx-metrics/library/src/main/java/io/opentelemetry/instrumentation/jmx/JmxTelemetry.java new file mode 100644 index 000000000000..dba4c60e6fe3 --- /dev/null +++ b/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> connections) { + service.startRemote(metricConfiguration, connections); + } + + public void startLocal() { + service.startLocal(metricConfiguration); + } +} 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 new file mode 100644 index 000000000000..ac6e772cf1b7 --- /dev/null +++ b/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); + } +} diff --git a/instrumentation/jmx-metrics/library/src/main/java/io/opentelemetry/instrumentation/jmx/engine/JmxMetricInsight.java b/instrumentation/jmx-metrics/library/src/main/java/io/opentelemetry/instrumentation/jmx/engine/JmxMetricInsight.java index acc2f5725fe3..b238e1d174b3 100644 --- a/instrumentation/jmx-metrics/library/src/main/java/io/opentelemetry/instrumentation/jmx/engine/JmxMetricInsight.java +++ b/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> connections) { start(conf, connections); 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 993fc0238211..a900f3b92a80 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 @@ -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,6 +163,7 @@ 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) { try { @@ -171,13 +171,13 @@ public void addMetricDefsTo(MetricConfiguration conf, InputStream is, String id) 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); } } 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 new file mode 100644 index 000000000000..e1ff5c3673e3 --- /dev/null +++ b/instrumentation/jmx-metrics/library/src/test/java/io/opentelemetry/instrumentation/jmx/JmxTelemetryTest.java @@ -0,0 +1,60 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.instrumentation.jmx; + +import static io.opentelemetry.sdk.testing.assertj.OpenTelemetryAssertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; + +import io.opentelemetry.api.OpenTelemetry; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Path; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; + +public class JmxTelemetryTest { + + @Test + void createDefault() { + JmxTelemetryBuilder builder = JmxTelemetry.builder(OpenTelemetry.noop()); + assertThat(builder.build()).isNotNull(); + } + + @Test + void missingClasspathTarget() { + JmxTelemetryBuilder builder = JmxTelemetry.builder(OpenTelemetry.noop()); + assertThatThrownBy(() -> builder.addClassPathRules("should-not-exist")) + .isInstanceOf(IllegalArgumentException.class); + } + + @Test + void invalidClasspathTarget() { + JmxTelemetryBuilder builder = JmxTelemetry.builder(OpenTelemetry.noop()); + assertThatThrownBy(() -> builder.addClassPathRules("invalid")) + .isInstanceOf(IllegalArgumentException.class); + } + + @Test + void knownClassPathTarget() { + JmxTelemetry.builder(OpenTelemetry.noop()).addClassPathRules("jvm").build(); + } + + @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); + } + + @Test + void invalidStartDelay() { + JmxTelemetryBuilder builder = JmxTelemetry.builder(OpenTelemetry.noop()); + assertThatThrownBy(() -> builder.beanDiscoveryDelay(-1)) + .isInstanceOf(IllegalArgumentException.class); + } +} diff --git a/instrumentation/jmx-metrics/library/src/test/resources/jmx/rules/invalid.yaml b/instrumentation/jmx-metrics/library/src/test/resources/jmx/rules/invalid.yaml new file mode 100644 index 000000000000..ab8298d00499 --- /dev/null +++ b/instrumentation/jmx-metrics/library/src/test/resources/jmx/rules/invalid.yaml @@ -0,0 +1 @@ +:this !is /not YAML