From f30888cb6942dcd42eed05bea48bb41803a0f4f9 Mon Sep 17 00:00:00 2001 From: Sylvain Juge <763082+SylvainJuge@users.noreply.github.com> Date: Tue, 4 Nov 2025 12:04:50 +0100 Subject: [PATCH 1/6] introduce JmxTelemetry + builder --- .../jmx/JmxMetricInsightInstaller.java | 81 +++---------------- .../instrumentation/jmx/JmxTelemetry.java | 39 +++++++++ .../jmx/JmxTelemetryBuilder.java | 75 +++++++++++++++++ .../jmx/engine/JmxMetricInsight.java | 1 + 4 files changed, 127 insertions(+), 69 deletions(-) create mode 100644 instrumentation/jmx-metrics/library/src/main/java/io/opentelemetry/instrumentation/jmx/JmxTelemetry.java create mode 100644 instrumentation/jmx-metrics/library/src/main/java/io/opentelemetry/instrumentation/jmx/JmxTelemetryBuilder.java 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..55acc01ae85f 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,23 +5,15 @@ 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; /** An {@link AgentListener} that enables JMX metrics during agent startup. */ @AutoService(AgentListener.class) @@ -32,11 +24,16 @@ 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()); + + config.getList("otel.jmx.config").forEach(jmx::addCustomRules); + config + .getList("otel.jmx.target.system") + .forEach(system -> jmx.addClasspathRules(JmxMetricInsightInstaller.class, system)); + + jmx.build().startLocal(); } } @@ -50,58 +47,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/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..a79d777943af --- /dev/null +++ b/instrumentation/jmx-metrics/library/src/main/java/io/opentelemetry/instrumentation/jmx/JmxTelemetry.java @@ -0,0 +1,39 @@ +/* + * 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..b3219be19d85 --- /dev/null +++ b/instrumentation/jmx-metrics/library/src/main/java/io/opentelemetry/instrumentation/jmx/JmxTelemetryBuilder.java @@ -0,0 +1,75 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.instrumentation.jmx; + +import static java.util.logging.Level.FINE; +import static java.util.logging.Level.INFO; + +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.InputStream; +import java.nio.file.Files; +import java.nio.file.Paths; +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(); + } + + @CanIgnoreReturnValue + public JmxTelemetryBuilder beanDiscoveryDelay(long delayMs) { + this.discoveryDelayMs = delayMs; + return this; + } + + // TODO: can we use classloader as argument instead of baseClass? + @CanIgnoreReturnValue + public JmxTelemetryBuilder addClasspathRules(Class baseClass, String targetId) { + String yamlResource = String.format("/jmx/rules/%s.yaml", targetId); + try (InputStream inputStream = baseClass.getResourceAsStream(yamlResource)) { + if (inputStream != null) { + logger.log(FINE, "Adding JMX config from classpath for {0}", yamlResource); + RuleParser parserInstance = RuleParser.get(); + parserInstance.addMetricDefsTo(metricConfiguration, inputStream, targetId); + } else { + logger.log(INFO, "No support found for {0}", targetId); + } + } catch (Exception e) { + logger.warning(e.getMessage()); + } + return this; + } + + @CanIgnoreReturnValue + public JmxTelemetryBuilder addCustomRules(String path) { + logger.log(FINE, "Adding JMX config from file: {0}", path); + RuleParser parserInstance = RuleParser.get(); + try (InputStream inputStream = Files.newInputStream(Paths.get(path))) { + parserInstance.addMetricDefsTo(metricConfiguration, inputStream, path); + } catch (Exception e) { + // yaml parsing errors are caught and logged inside of addMetricDefsTo + // only file access related exceptions are expected here + logger.warning(e.toString()); + } + 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); From b9a1d90c996513a4cca39b759ca9ac1f1eac238a Mon Sep 17 00:00:00 2001 From: Sylvain Juge <763082+SylvainJuge@users.noreply.github.com> Date: Tue, 4 Nov 2025 14:53:47 +0100 Subject: [PATCH 2/6] throw exception in case of parsing error --- .../jmx/JmxMetricInsightInstaller.java | 16 ++++-- .../jmx/JmxMetricInsightInstallerTest.java | 3 +- instrumentation/jmx-metrics/library/README.md | 25 ++++---- .../jmx/JmxTelemetryBuilder.java | 57 ++++++++++++------- .../instrumentation/jmx/yaml/RuleParser.java | 12 ++-- 5 files changed, 71 insertions(+), 42 deletions(-) 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 55acc01ae85f..9a80da6daa83 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 @@ -13,12 +13,17 @@ import io.opentelemetry.sdk.autoconfigure.AutoConfiguredOpenTelemetrySdk; import io.opentelemetry.sdk.autoconfigure.internal.AutoConfigureUtil; import io.opentelemetry.sdk.autoconfigure.spi.ConfigProperties; +import java.nio.file.Paths; import java.time.Duration; +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); @@ -28,10 +33,13 @@ public void afterAgent(AutoConfiguredOpenTelemetrySdk autoConfiguredSdk) { JmxTelemetry.builder(GlobalOpenTelemetry.get()) .beanDiscoveryDelay(beanDiscoveryDelay(config).toMillis()); - config.getList("otel.jmx.config").forEach(jmx::addCustomRules); - config - .getList("otel.jmx.target.system") - .forEach(system -> jmx.addClasspathRules(JmxMetricInsightInstaller.class, system)); + 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(); } 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..a2ecd402b5b7 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 @@ -13,7 +13,6 @@ 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; @@ -39,7 +38,7 @@ 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(); 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/JmxTelemetryBuilder.java b/instrumentation/jmx-metrics/library/src/main/java/io/opentelemetry/instrumentation/jmx/JmxTelemetryBuilder.java index b3219be19d85..d65bd4ac47ee 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 @@ -6,15 +6,15 @@ package io.opentelemetry.instrumentation.jmx; import static java.util.logging.Level.FINE; -import static java.util.logging.Level.INFO; 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.Paths; +import java.nio.file.Path; import java.util.logging.Logger; public class JmxTelemetryBuilder { @@ -31,40 +31,57 @@ public class JmxTelemetryBuilder { this.metricConfiguration = new MetricConfiguration(); } + /** + * Sets initial delay for MBean discovery + * + * @param delayMs delay in milliseconds + * @return builder instance + */ @CanIgnoreReturnValue public JmxTelemetryBuilder beanDiscoveryDelay(long delayMs) { this.discoveryDelayMs = delayMs; return this; } - // TODO: can we use classloader as argument instead of baseClass? + /** + * 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(Class baseClass, String targetId) { - String yamlResource = String.format("/jmx/rules/%s.yaml", targetId); - try (InputStream inputStream = baseClass.getResourceAsStream(yamlResource)) { - if (inputStream != null) { - logger.log(FINE, "Adding JMX config from classpath for {0}", yamlResource); - RuleParser parserInstance = RuleParser.get(); - parserInstance.addMetricDefsTo(metricConfiguration, inputStream, targetId); - } else { - logger.log(INFO, "No support found for {0}", targetId); + public JmxTelemetryBuilder addClasspathRules(String target) { + String yamlResource = String.format("/jmx/rules/%s.yaml", target); + ClassLoader classLoader = JmxTelemetryBuilder.class.getClassLoader(); + try (InputStream inputStream = classLoader.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) { - logger.warning(e.getMessage()); + 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(String path) { + public JmxTelemetryBuilder addCustomRules(Path path) { logger.log(FINE, "Adding JMX config from file: {0}", path); RuleParser parserInstance = RuleParser.get(); - try (InputStream inputStream = Files.newInputStream(Paths.get(path))) { - parserInstance.addMetricDefsTo(metricConfiguration, inputStream, path); - } catch (Exception e) { - // yaml parsing errors are caught and logged inside of addMetricDefsTo - // only file access related exceptions are expected here - logger.warning(e.toString()); + 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; } 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); } } From bad1e7c7d908c40e68a1fbd1ef0bb44143c8ed0b Mon Sep 17 00:00:00 2001 From: Sylvain Juge <763082+SylvainJuge@users.noreply.github.com> Date: Tue, 4 Nov 2025 15:00:16 +0100 Subject: [PATCH 3/6] add some basic testing --- .../instrumentation/jmx/JmxTelemetry.java | 1 - .../jmx/JmxTelemetryBuilder.java | 6 ++- .../instrumentation/jmx/JmxTelemetryTest.java | 46 +++++++++++++++++++ 3 files changed, 51 insertions(+), 2 deletions(-) create mode 100644 instrumentation/jmx-metrics/library/src/test/java/io/opentelemetry/instrumentation/jmx/JmxTelemetryTest.java 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 index a79d777943af..dba4c60e6fe3 100644 --- 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 @@ -35,5 +35,4 @@ public void startRemote(Supplier> connecti 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 index d65bd4ac47ee..f4e61ca475a7 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 @@ -39,6 +39,9 @@ public class JmxTelemetryBuilder { */ @CanIgnoreReturnValue public JmxTelemetryBuilder beanDiscoveryDelay(long delayMs) { + if (delayMs < 0) { + throw new IllegalArgumentException("delay must be greater than 0"); + } this.discoveryDelayMs = delayMs; return this; } @@ -62,7 +65,8 @@ public JmxTelemetryBuilder addClasspathRules(String target) { RuleParser parserInstance = RuleParser.get(); parserInstance.addMetricDefsTo(metricConfiguration, inputStream, target); } catch (Exception e) { - throw new IllegalArgumentException("Unable to load JMX rules from classpath: " + yamlResource, e); + throw new IllegalArgumentException( + "Unable to load JMX rules from classpath: " + yamlResource, e); } return this; } 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..5163e1a20b06 --- /dev/null +++ b/instrumentation/jmx-metrics/library/src/test/java/io/opentelemetry/instrumentation/jmx/JmxTelemetryTest.java @@ -0,0 +1,46 @@ +/* + * 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.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 invalidExternalYaml(@TempDir Path dir) throws Exception { + Path invalid = Files.createTempFile(dir, "invalid", ".yaml"); + 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); + } +} From 96ade82e787059b827cfb5ceff947c787202951a Mon Sep 17 00:00:00 2001 From: Sylvain Juge <763082+SylvainJuge@users.noreply.github.com> Date: Tue, 4 Nov 2025 15:16:14 +0100 Subject: [PATCH 4/6] minor rename + enhance tests --- .../javaagent/jmx/JmxMetricInsightInstaller.java | 2 +- .../instrumentation/jmx/JmxTelemetryBuilder.java | 5 ++--- .../instrumentation/jmx/JmxTelemetryTest.java | 16 +++++++++++++++- .../src/test/resources/jmx/rules/invalid.yaml | 1 + 4 files changed, 19 insertions(+), 5 deletions(-) create mode 100644 instrumentation/jmx-metrics/library/src/test/resources/jmx/rules/invalid.yaml 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 9a80da6daa83..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 @@ -35,7 +35,7 @@ public void afterAgent(AutoConfiguredOpenTelemetrySdk autoConfiguredSdk) { 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.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); 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 f4e61ca475a7..a5aae9e5606c 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 @@ -54,10 +54,9 @@ public JmxTelemetryBuilder beanDiscoveryDelay(long delayMs) { * @throws IllegalArgumentException when classpath resource does not exist or can't be parsed */ @CanIgnoreReturnValue - public JmxTelemetryBuilder addClasspathRules(String target) { + public JmxTelemetryBuilder addClassPathRules(String target) { String yamlResource = String.format("/jmx/rules/%s.yaml", target); - ClassLoader classLoader = JmxTelemetryBuilder.class.getClassLoader(); - try (InputStream inputStream = classLoader.getResourceAsStream(yamlResource)) { + try (InputStream inputStream = JmxTelemetryBuilder.class.getResourceAsStream(yamlResource)) { if (inputStream == null) { throw new IllegalArgumentException("JMX rules not found in classpath: " + yamlResource); } 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 5163e1a20b06..e1ff5c3673e3 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.nio.charset.StandardCharsets; import java.nio.file.Files; import java.nio.file.Path; import org.junit.jupiter.api.Test; @@ -25,13 +26,26 @@ void createDefault() { @Test void missingClasspathTarget() { JmxTelemetryBuilder builder = JmxTelemetry.builder(OpenTelemetry.noop()); - assertThatThrownBy(() -> builder.addClasspathRules("should-not-exist")) + 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); 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 From cf6d7b778f5eb3f8fea5088312592c0bd82f4c17 Mon Sep 17 00:00:00 2001 From: Sylvain Juge <763082+SylvainJuge@users.noreply.github.com> Date: Tue, 4 Nov 2025 16:05:04 +0100 Subject: [PATCH 5/6] simplify test --- .../jmx/JmxMetricInsightInstallerTest.java | 39 ++++--------------- .../jmx/JmxTelemetryBuilder.java | 5 ++- 2 files changed, 11 insertions(+), 33 deletions(-) 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 a2ecd402b5b7..2a5576205485 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,17 +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.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; @@ -40,32 +36,13 @@ void testToVerifyExistingRulesAreValid() throws Exception { Path path = Paths.get(PATH_TO_ALL_EXISTING_RULES); 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(); - } + String target = file.substring(0, file.indexOf(".")); + // loading rules from direct file access + JmxTelemetry.builder(OpenTelemetry.noop()).addCustomRules(filePath); } } } 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 a5aae9e5606c..ac6e772cf1b7 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 @@ -55,8 +55,9 @@ public JmxTelemetryBuilder beanDiscoveryDelay(long delayMs) { */ @CanIgnoreReturnValue public JmxTelemetryBuilder addClassPathRules(String target) { - String yamlResource = String.format("/jmx/rules/%s.yaml", target); - try (InputStream inputStream = JmxTelemetryBuilder.class.getResourceAsStream(yamlResource)) { + 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); } From 190c303d1998d91c880c9c086e50ab7b6c7aec6e Mon Sep 17 00:00:00 2001 From: Sylvain Juge <763082+SylvainJuge@users.noreply.github.com> Date: Tue, 4 Nov 2025 16:36:28 +0100 Subject: [PATCH 6/6] remove unused variable --- .../javaagent/jmx/JmxMetricInsightInstallerTest.java | 1 - 1 file changed, 1 deletion(-) 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 2a5576205485..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 @@ -40,7 +40,6 @@ void testToVerifyExistingRulesAreValid() throws Exception { Path filePath = path.resolve(file); assertThat(filePath).isRegularFile(); - String target = file.substring(0, file.indexOf(".")); // loading rules from direct file access JmxTelemetry.builder(OpenTelemetry.noop()).addCustomRules(filePath); }