From c56705bce8fe0d4aeb4b63881d1fa3d6c51b282f Mon Sep 17 00:00:00 2001 From: Peter Findeisen Date: Tue, 22 Nov 2022 17:11:52 -0800 Subject: [PATCH 1/3] Issue 7212 - Allow multiple YAML configuration files for JMX rules --- instrumentation/jmx-metrics/javaagent/README.md | 8 ++++---- .../javaagent/jmx/JmxMetricInsightInstaller.java | 15 ++++++++------- .../instrumentation/jmx/yaml/RuleParser.java | 7 ++++--- 3 files changed, 16 insertions(+), 14 deletions(-) diff --git a/instrumentation/jmx-metrics/javaagent/README.md b/instrumentation/jmx-metrics/javaagent/README.md index eaf2f69c54f5..218658422910 100644 --- a/instrumentation/jmx-metrics/javaagent/README.md +++ b/instrumentation/jmx-metrics/javaagent/README.md @@ -30,20 +30,20 @@ No targets are enabled by default. The supported target environments are listed - [wildfly](wildfly.md) - [hadoop](hadoop.md) -## Configuration File +## Configuration Files -To provide your own metric definitions, create a YAML configuration file, and specify its location using the `otel.jmx.config` property. For example +To provide your own metric definitions, create one or more YAML configuration files, and specify their location using the `otel.jmx.config` property. Absolute or relative pathnames can be specified. For example ```bash $ java -javaagent:path/to/opentelemetry-javaagent.jar \ - -Dotel.jmx.config=path/to/config_file.yaml \ + -Dotel.jmx.config=path/to/config_file.yaml,more_rules.yaml \ ... \ -jar myapp.jar ``` ### Basic Syntax -The configuration file can contain multiple entries (which we call _rules_), defining a number of metrics. Each rule must identify a set of MBeans and the name of the MBean attribute to query, along with additional information on how to report the values. Let's look at a simple example. +Each configuration file can contain multiple entries (which we call _rules_), defining a number of metrics. Each rule must identify a set of MBeans and the name of the MBean attribute to query, along with additional information on how to report the values. Let's look at a simple example. ```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 89cde5ae5db9..68cb6f482e34 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 @@ -59,7 +59,7 @@ private static void addRulesForPlatform(String platform, MetricConfiguration con if (inputStream != null) { JmxMetricInsight.getLogger().log(FINE, "Opened input stream {0}", yamlResource); RuleParser parserInstance = RuleParser.get(); - parserInstance.addMetricDefsTo(conf, inputStream); + parserInstance.addMetricDefsTo(conf, inputStream, platform); } else { JmxMetricInsight.getLogger().log(INFO, "No support found for {0}", platform); } @@ -80,14 +80,15 @@ private static void buildFromDefaultRules( private static void buildFromUserRules( MetricConfiguration conf, ConfigProperties configProperties) { - String jmxDir = configProperties.getString("otel.jmx.config"); - if (jmxDir != null) { - JmxMetricInsight.getLogger().log(FINE, "JMX config file name: {0}", jmxDir); + String files = configProperties.getString("otel.jmx.config", ""); + String[] configFiles = files.isEmpty() ? new String[0] : files.split(","); + for (String configFile : configFiles) { + JmxMetricInsight.getLogger().log(FINE, "JMX config file name: {0}", configFile); RuleParser parserInstance = RuleParser.get(); - try (InputStream inputStream = Files.newInputStream(new File(jmxDir.trim()).toPath())) { - parserInstance.addMetricDefsTo(conf, inputStream); + try (InputStream inputStream = Files.newInputStream(new File(configFile).toPath())) { + parserInstance.addMetricDefsTo(conf, inputStream, configFile); } catch (Exception e) { - JmxMetricInsight.getLogger().warning(e.getMessage()); + JmxMetricInsight.getLogger().warning(e.toString()); } } } 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 c5080db7b500..a7c12cd4d1bd 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 @@ -51,17 +51,18 @@ public JmxConfig loadConfig(InputStream is) throws Exception { * * @param conf the metric configuration * @param is the InputStream with the YAML rules + * @param id identifier of the YAML ruleset, such as a filename */ - public void addMetricDefsTo(MetricConfiguration conf, InputStream is) { + public void addMetricDefsTo(MetricConfiguration conf, InputStream is, String id) { try { JmxConfig config = loadConfig(is); if (config != null) { - logger.log(INFO, "Found {0} metric rules", config.getRules().size()); + logger.log(INFO, id + ": found {0} metric rules", config.getRules().size()); config.addMetricDefsTo(conf); } } catch (Exception exception) { - logger.log(WARNING, "Failed to parse YAML rules: " + rootCause(exception)); + logger.log(WARNING, "Failed to parse YAML rules from " + 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()); From 4f84f9efff496c12502aeee679b83f5f234d1fdf Mon Sep 17 00:00:00 2001 From: Peter Findeisen Date: Wed, 23 Nov 2022 10:37:26 -0800 Subject: [PATCH 2/3] Post-review changes --- .../javaagent/jmx/JmxMetricInsightInstaller.java | 13 ++++++------- .../instrumentation/jmx/yaml/RuleParser.java | 8 ++++++-- 2 files changed, 12 insertions(+), 9 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 68cb6f482e34..c545587d3a0d 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 @@ -16,9 +16,10 @@ import io.opentelemetry.javaagent.extension.AgentListener; import io.opentelemetry.sdk.autoconfigure.AutoConfiguredOpenTelemetrySdk; import io.opentelemetry.sdk.autoconfigure.spi.ConfigProperties; -import java.io.File; import java.io.InputStream; import java.nio.file.Files; +import java.nio.file.Paths; +import java.util.List; /** An {@link AgentListener} that enables JMX metrics during agent startup. */ @AutoService(AgentListener.class) @@ -70,9 +71,7 @@ private static void addRulesForPlatform(String platform, MetricConfiguration con private static void buildFromDefaultRules( MetricConfiguration conf, ConfigProperties configProperties) { - String targetSystem = configProperties.getString("otel.jmx.target.system", ""); - String[] platforms = targetSystem.isEmpty() ? new String[0] : targetSystem.split(","); - + List platforms = configProperties.getList("otel.jmx.target.system"); for (String platform : platforms) { addRulesForPlatform(platform, conf); } @@ -80,14 +79,14 @@ private static void buildFromDefaultRules( private static void buildFromUserRules( MetricConfiguration conf, ConfigProperties configProperties) { - String files = configProperties.getString("otel.jmx.config", ""); - String[] configFiles = files.isEmpty() ? new String[0] : files.split(","); + 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(new File(configFile).toPath())) { + try (InputStream inputStream = Files.newInputStream(Paths.get(configFile))) { parserInstance.addMetricDefsTo(conf, inputStream, configFile); } catch (Exception e) { + // NoSuchFileException, AccessDeniedException ? JmxMetricInsight.getLogger().warning(e.toString()); } } 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 a7c12cd4d1bd..659c7925854b 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 @@ -58,11 +58,15 @@ public void addMetricDefsTo(MetricConfiguration conf, InputStream is, String id) JmxConfig config = loadConfig(is); if (config != null) { - logger.log(INFO, id + ": found {0} metric rules", config.getRules().size()); + 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 " + id + ": " + rootCause(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()); From e9fecc8608f2503b3252997c937b41337758f861 Mon Sep 17 00:00:00 2001 From: Peter Findeisen Date: Wed, 23 Nov 2022 14:26:38 -0800 Subject: [PATCH 3/3] Update instrumentation/jmx-metrics/javaagent/src/main/java/io/opentelemetry/instrumentation/javaagent/jmx/JmxMetricInsightInstaller.java Co-authored-by: Trask Stalnaker --- .../javaagent/jmx/JmxMetricInsightInstaller.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) 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 c545587d3a0d..17687fec57ba 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 @@ -86,7 +86,8 @@ private static void buildFromUserRules( try (InputStream inputStream = Files.newInputStream(Paths.get(configFile))) { parserInstance.addMetricDefsTo(conf, inputStream, configFile); } catch (Exception e) { - // NoSuchFileException, AccessDeniedException ? + // yaml parsing errors are caught and logged inside of addMetricDefsTo + // only file access related exceptions are expected here JmxMetricInsight.getLogger().warning(e.toString()); } }