jmx reuse instrumentation metrics

jmx-scraper/README.md

@@ -40,6 +40,7 @@ For example the `otel.jmx.service.url` option can be set with the `OTEL_JMX_SERV
 |--------------------------------|---------------|-------------------------------------------------------------------------------------------------------------------------------------------------------------|
 | `otel.jmx.service.url`         | -             | mandatory JMX URL to connect to the remote JVM                                                                                                              |
 | `otel.jmx.target.system`       | -             | comma-separated list of systems to monitor, mandatory unless `otel.jmx.config` is set                                                                       |
+| `otel.jmx.target.source`       | `auto`        | source of metrics definitions to use for `otel.jmx.target.system`, supported values are `auto`, `instrumentation` and `legacy`                              |
 | `otel.jmx.config`              | empty         | comma-separated list of paths to custom YAML metrics definition, mandatory when `otel.jmx.target.system` is not set                                         |
 | `otel.jmx.username`            | -             | user name for JMX connection, mandatory when JMX authentication is set on target JVM with`com.sun.management.jmxremote.authenticate=true`                   |
 | `otel.jmx.password`            | -             | password for JMX connection, mandatory when JMX authentication is set on target JVM with `com.sun.management.jmxremote.authenticate=true`                   |
@@ -55,22 +56,34 @@ When both `otel.jmx.target.system` and `otel.jmx.config` configuration options a
 
 If there is a need to override existing ready-to-use metrics or to keep control on the metrics definitions, using a custom YAML definition with `otel.jmx.config` is the recommended option.
 
-Supported values for `otel.jmx.target.system`:
-
-| `otel.jmx.target.system` | description           |
-|--------------------------|-----------------------|
-| `activemq`               | Apache ActiveMQ       |
-| `cassandra`              | Apache Cassandra      |
-| `hbase`                  | Apache HBase          |
-| `hadoop`                 | Apache Hadoop         |
-| `jetty`                  | Eclipse Jetty         |
-| `jvm`                    | JVM runtime metrics   |
-| `kafka`                  | Apache Kafka          |
-| `kafka-consumer`         | Apache Kafka consumer |
-| `kafka-producer`         | Apache Kafka producer |
-| `solr`                   | Apache Solr           |
-| `tomcat`                 | Apache Tomcat         |
-| `wildfly`                | Wildfly               |
+Supported values for `otel.jmx.target.system` and support for `otel.jmx.target.source` and links to the metrics definitions:
+
+| `otel.jmx.target.system` | description           | `legacy`                                                        | `instrumentation` |
+|--------------------------|-----------------------|-----------------------------------------------------------------|-------------------|
+| `activemq`               | Apache ActiveMQ       | [`activemq.yaml`](src/main/resources/activemq.yaml)             |                   |
+| `cassandra`              | Apache Cassandra      | [`cassandra.yaml`](src/main/resources/cassandra.yaml)           |                   |
+| `hbase`                  | Apache HBase          | [`hbase.yaml`](src/main/resources/hbase.yaml)                   |                   |
+| `hadoop`                 | Apache Hadoop         | [`hadoop.yaml`](src/main/resources/hadoop.yaml)                 |                   |
+| `jetty`                  | Eclipse Jetty         | [`jetty.yaml`](src/main/resources/jetty.yaml)                   |                   |
+| `jvm`                    | JVM runtime metrics   | [`jvm.yaml`](src/main/resources/jvm.yaml)                       |                   |
+| `kafka`                  | Apache Kafka          | [`kafka.yaml`](src/main/resources/kafka.yaml)                   |                   |
+| `kafka-consumer`         | Apache Kafka consumer | [`kafka-consumer.yaml`](src/main/resources/kafka-consumer.yaml) |                   |
+| `kafka-producer`         | Apache Kafka producer | [`kafka-producer.yaml`](src/main/resources/kafka-producer.yaml) |                   |
+| `solr`                   | Apache Solr           | [`solr.yaml`](src/main/resources/solr.yaml)                     |                   |
+| `tomcat`                 | Apache Tomcat         | [`tomcat.yaml`](src/main/resources/tomcat.yaml)                 |                   |
+| `wildfly`                | Wildfly               | [`wildfly.yaml`](src/main/resources/wildfly.yaml)               |                   |
+
+The source of metrics definitions is controlled by `otel.jmx.target.source`:
+
+- `auto` (default) : metrics definitions from `instrumentation` with fallback on `legacy` when not available.
+- `legacy` : metrics definitions embedded in jmx-scraper, almost equivalent to [JMX Gatherer](https://github.com/open-telemetry/opentelemetry-java-contrib/tree/main/jmx-metrics).
+- `instrumentation` : metrics definitions embedded in [instrumentation/jmx-metrics](https://github.com/open-telemetry/opentelemetry-java-instrumentation/tree/main/instrumentation/jmx-metrics/library) library
+
+Setting the value of `otel.jmx.target.source` allows to fit the following use-cases:
+
+- `auto` will ensure that the latest metrics definitions in instrumentation (reference) is being used when available with a fallback on `legacy` otherwise. Metrics definitions will thus be updated whenever the dependency on instrumentation is updated.
+- `legacy` allows to keep using definitions that are very close to JMX Gatherer, this is the recommended option if preserving compatibility is required. Those definitions are in maintenance and are unlikely to evolve over time.
+- `instrumentation` forces using metrics definitions from instrumentation, hence only the reference. Metrics definitions and supported values of `otel.jmx.target.system` will be updated whenever the dependency on instrumentation is updated.
 
 The following SDK configuration options are also relevant
 

jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/JmxScraperContainer.java

@@ -26,6 +26,7 @@ public class JmxScraperContainer extends GenericContainer<JmxScraperContainer> {
 
   private final String endpoint;
   private final Set<String> targetSystems;
+  private String targetSystemSource;
   private String serviceUrl;
   private final Set<String> customYamlFiles;
   private String user;
@@ -46,6 +47,7 @@ public JmxScraperContainer(String otlpEndpoint, String baseImage) {
 
     this.endpoint = otlpEndpoint;
     this.targetSystems = new HashSet<>();
+    this.targetSystemSource = "auto";
     this.customYamlFiles = new HashSet<>();
     this.extraJars = new ArrayList<>();
   }
@@ -62,6 +64,18 @@ public JmxScraperContainer withTargetSystem(String targetSystem) {
     return this;
   }
 
+  /**
+   * Sets the target system source
+   *
+   * @param source target system source, valid values are "auto", "instrumentation" and "legacy"
+   * @return this
+   */
+  @CanIgnoreReturnValue
+  public JmxScraperContainer withTargetSystemSource(String source) {
+    this.targetSystemSource = source;
+    return this;
+  }
+
   /**
    * Set connection to a standard JMX service URL
    *
@@ -192,6 +206,7 @@ public void start() {
 
     if (!targetSystems.isEmpty()) {
       arguments.add("-Dotel.jmx.target.system=" + String.join(",", targetSystems));
+      arguments.add("-Dotel.jmx.target.source=" + targetSystemSource);
     }
 
     if (serviceUrl == null) {

jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/target_systems/JvmIntegrationTest.java

@@ -32,6 +32,12 @@ protected JmxScraperContainer customizeScraperContainer(
       JmxScraperContainer scraper, GenericContainer<?> target, Path tempDir) {
     return scraper
         .withTargetSystem("jvm")
+        // TODO when JVM metrics will be added to instrumentation, the default "auto" source
+        // means that the definitions in instrumentation will be used, and thus this test will fail
+        // due to metrics differences, adding an explicit "legacy" source is required to continue
+        // testing the JVM metrics defined in this project.
+        // https://github.com/open-telemetry/opentelemetry-java-instrumentation/pull/13392
+        // .withTargetSystem("legacy")
         // also testing custom yaml
         .withCustomYaml("custom-metrics.yaml");
   }

jmx-scraper/src/main/java/io/opentelemetry/contrib/jmxscraper/JmxScraper.java

@@ -228,28 +228,18 @@ private void start() throws IOException {
   private static MetricConfiguration getMetricConfig(JmxScraperConfig scraperConfig) {
     MetricConfiguration config = new MetricConfiguration();
     for (String system : scraperConfig.getTargetSystems()) {
-      addRulesForSystem(system, config);
+      try (InputStream yaml = scraperConfig.getTargetSystemYaml(system)) {
+        RuleParser.get().addMetricDefsTo(config, yaml, system);
+      } catch (IOException e) {
+        throw new IllegalStateException("Error while loading rules for system " + system, e);
+      }
     }
     for (String file : scraperConfig.getJmxConfig()) {
       addRulesFromFile(file, config);
     }
     return config;
   }
 
-  private static void addRulesForSystem(String system, MetricConfiguration conf) {
-    String yamlResource = system + ".yaml";
-    try (InputStream inputStream =
-        JmxScraper.class.getClassLoader().getResourceAsStream(yamlResource)) {
-      if (inputStream != null) {
-        RuleParser.get().addMetricDefsTo(conf, inputStream, system);
-      } else {
-        throw new IllegalArgumentException("No support for system " + system);
-      }
-    } catch (Exception e) {
-      throw new IllegalStateException("Error while loading rules for system " + system, e);
-    }
-  }
-
   private static void addRulesFromFile(String file, MetricConfiguration conf) {
     Path path = Paths.get(file);
     if (!Files.isReadable(path)) {

jmx-scraper/src/main/java/io/opentelemetry/contrib/jmxscraper/config/JmxScraperConfig.java

@@ -7,12 +7,13 @@
 
 import io.opentelemetry.sdk.autoconfigure.spi.ConfigProperties;
 import io.opentelemetry.sdk.autoconfigure.spi.ConfigurationException;
+import java.io.InputStream;
 import java.time.Duration;
 import java.util.ArrayList;
-import java.util.Arrays;
 import java.util.Collections;
 import java.util.HashSet;
 import java.util.List;
+import java.util.Locale;
 import java.util.Set;
 import java.util.logging.Logger;
 import javax.annotation.Nullable;
@@ -37,37 +38,23 @@ public class JmxScraperConfig {
   static final String JMX_CONFIG_LEGACY = "otel.jmx.custom.scraping.config";
 
   static final String JMX_TARGET_SYSTEM = "otel.jmx.target.system";
+  static final String JMX_TARGET_SOURCE = "otel.jmx.target.source";
 
   static final String JMX_USERNAME = "otel.jmx.username";
   static final String JMX_PASSWORD = "otel.jmx.password";
 
-  // TODO: document those when they will be supported
   static final String JMX_REGISTRY_SSL = "otel.jmx.remote.registry.ssl";
   static final String JMX_REMOTE_PROFILE = "otel.jmx.remote.profile";
   static final String JMX_REALM = "otel.jmx.realm";
 
-  private static final List<String> AVAILABLE_TARGET_SYSTEMS =
-      Collections.unmodifiableList(
-          Arrays.asList(
-              "activemq",
-              "cassandra",
-              "hbase",
-              "hadoop",
-              "jetty",
-              "jvm",
-              "kafka",
-              "kafka-consumer",
-              "kafka-producer",
-              "solr",
-              "tomcat",
-              "wildfly"));
-
   private String serviceUrl = "";
 
   private List<String> jmxConfig = Collections.emptyList();
 
   private Set<String> targetSystems = Collections.emptySet();
 
+  private TargetSystemSource targetSystemSource = TargetSystemSource.AUTO;
+
   private Duration samplingInterval = Duration.ofMinutes(1);
 
   @Nullable private String username;
@@ -79,6 +66,20 @@ public class JmxScraperConfig {
   @Nullable private String remoteProfile;
   private boolean registrySsl;
 
+  public enum TargetSystemSource {
+    AUTO,
+    INSTRUMENTATION,
+    LEGACY;
+
+    static TargetSystemSource fromString(String source) {
+      try {
+        return TargetSystemSource.valueOf(source.toUpperCase(Locale.ROOT));
+      } catch (IllegalArgumentException e) {
+        throw new IllegalArgumentException("Invalid target system source: " + source, e);
+      }
+    }
+  }
+
   private JmxScraperConfig() {}
 
   public String getServiceUrl() {
@@ -93,6 +94,57 @@ public Set<String> getTargetSystems() {
     return targetSystems;
   }
 
+  /**
+   * Resolves the target system yaml from configuration
+   *
+   * @param system target system
+   * @return input stream on target system yaml definitions
+   * @throws ConfigurationException when no yaml for system is available
+   */
+  public InputStream getTargetSystemYaml(String system) {
+    InputStream yaml;
+    switch (targetSystemSource) {
+      case LEGACY:
+        yaml = getTargetSystemYaml(system, TargetSystemSource.LEGACY);
+        break;
+      case INSTRUMENTATION:
+        yaml = getTargetSystemYaml(system, TargetSystemSource.INSTRUMENTATION);
+        break;
+      case AUTO:
+        yaml = getTargetSystemYaml(system, TargetSystemSource.INSTRUMENTATION);
+        if (yaml == null) {
+          yaml = getTargetSystemYaml(system, TargetSystemSource.LEGACY);
+        }
+        break;
+      default:
+        throw new IllegalStateException("unsupported target system source: " + targetSystemSource);
+    }
+
+    if (yaml == null) {
+      throw new ConfigurationException(
+          "unsupported target system: '" + system + "', source: " + targetSystemSource);
+    }
+    return yaml;
+  }
+
+  @Nullable
+  private static InputStream getTargetSystemYaml(String system, TargetSystemSource source) {
+    String path;
+    switch (source) {
+      case LEGACY:
+        path = String.format("%s.yaml", system);
+        break;
+      case INSTRUMENTATION:
+        path = String.format("jmx/rules/%s.yaml", system);
+        break;
+      case AUTO:
+      default:
+        throw new IllegalArgumentException("invalid source" + source);
+    }
+
+    return JmxScraperConfig.class.getClassLoader().getResourceAsStream(path);
+  }
+
   public Duration getSamplingInterval() {
     return samplingInterval;
   }
@@ -164,12 +216,6 @@ public static JmxScraperConfig fromConfig(ConfigProperties config) {
       throw new ConfigurationException(
           "at least one of '" + JMX_TARGET_SYSTEM + "' or '" + JMX_CONFIG + "' must be set");
     }
-    targetSystem.forEach(
-        s -> {
-          if (!AVAILABLE_TARGET_SYSTEMS.contains(s)) {
-            throw new ConfigurationException("unsupported target system: '" + s + "'");
-          }
-        });
 
     scraperConfig.jmxConfig = Collections.unmodifiableList(jmxConfig);
     scraperConfig.targetSystems = Collections.unmodifiableSet(new HashSet<>(targetSystem));
@@ -180,6 +226,13 @@ public static JmxScraperConfig fromConfig(ConfigProperties config) {
     scraperConfig.realm = config.getString("otel.jmx.realm");
     scraperConfig.registrySsl = config.getBoolean("otel.jmx.remote.registry.ssl", false);
 
+    // checks target system is supported by resolving the yaml resource, throws exception on
+    // missing/error
+    scraperConfig.targetSystems.forEach(scraperConfig::getTargetSystemYaml);
+
+    String source = config.getString(JMX_TARGET_SOURCE, TargetSystemSource.AUTO.name());
+    scraperConfig.targetSystemSource = TargetSystemSource.fromString(source);
+
     return scraperConfig;
   }
 }