Allow JMX Insight reuse for remote connections (#12178)

Author: SylvainJuge

instrumentation/jmx-metrics/javaagent/src/main/java/io/opentelemetry/instrumentation/javaagent/jmx/JmxMetricInsightInstaller.java

@@ -36,7 +36,7 @@ public void afterAgent(AutoConfiguredOpenTelemetrySdk autoConfiguredSdk) {
           JmxMetricInsight.createService(
               GlobalOpenTelemetry.get(), beanDiscoveryDelay(config).toMillis());
       MetricConfiguration conf = buildMetricConfiguration(config);
-      service.start(conf);
+      service.startLocal(conf);
     }
   }
 

instrumentation/jmx-metrics/javaagent/src/test/java/io/opentelemetry/instrumentation/javaagent/jmx/JmxMetricInsightInstallerTest.java

@@ -38,7 +38,7 @@ class JmxMetricInsightInstallerTest {
   @Test
   void testToVerifyExistingRulesAreValid() throws Exception {
     RuleParser parser = RuleParser.get();
-    assertThat(parser == null).isFalse();
+    assertThat(parser).isNotNull();
 
     Path path = Paths.get(PATH_TO_ALL_EXISTING_RULES);
     assertThat(Files.exists(path)).isTrue();

instrumentation/jmx-metrics/library/src/main/java/io/opentelemetry/instrumentation/jmx/engine/BeanAttributeExtractor.java

@@ -16,7 +16,7 @@
 import javax.management.InstanceNotFoundException;
 import javax.management.MBeanAttributeInfo;
 import javax.management.MBeanInfo;
-import javax.management.MBeanServer;
+import javax.management.MBeanServerConnection;
 import javax.management.ObjectName;
 import javax.management.openmbean.CompositeData;
 import javax.management.openmbean.TabularData;
@@ -132,18 +132,17 @@ public String getAttributeName() {
    * including the internals of CompositeData and TabularData, if applicable, and that the provided
    * values will be numerical.
    *
-   * @param server the MBeanServer that reported knowledge of the ObjectName
-   * @param objectName the ObjectName identifying the MBean
-   * @return AttributeInfo if the attribute is properly recognized, or null
+   * @param connection the {@link MBeanServerConnection} that reported knowledge of the ObjectName
+   * @param objectName the {@link ObjectName} identifying the MBean
    */
   @Nullable
-  AttributeInfo getAttributeInfo(MBeanServer server, ObjectName objectName) {
+  AttributeInfo getAttributeInfo(MBeanServerConnection connection, ObjectName objectName) {
     if (logger.isLoggable(FINE)) {
       logger.log(FINE, "Resolving {0} for {1}", new Object[] {getAttributeName(), objectName});
     }
 
     try {
-      MBeanInfo info = server.getMBeanInfo(objectName);
+      MBeanInfo info = connection.getMBeanInfo(objectName);
       MBeanAttributeInfo[] allAttributes = info.getAttributes();
 
       for (MBeanAttributeInfo attr : allAttributes) {
@@ -152,7 +151,7 @@ AttributeInfo getAttributeInfo(MBeanServer server, ObjectName objectName) {
 
           // Verify correctness of configuration by attempting to extract the metric value.
           // The value will be discarded, but its type will be checked.
-          Object sampleValue = extractAttributeValue(server, objectName, logger);
+          Object sampleValue = extractAttributeValue(connection, objectName, logger);
 
           // Only numbers can be used to generate metric values
           if (sampleValue instanceof Number) {
@@ -199,18 +198,20 @@ AttributeInfo getAttributeInfo(MBeanServer server, ObjectName objectName) {
    * Extracts the specified attribute value. In case the value is a CompositeData, drills down into
    * it to find the correct singleton value (usually a Number or a String).
    *
-   * @param server the MBeanServer to use
-   * @param objectName the ObjectName specifying the MBean to use, it should not be a pattern
+   * @param connection the {@link MBeanServerConnection} to use
+   * @param objectName the {@link ObjectName} specifying the MBean to use, it should not be a
+   *     pattern
    * @param logger the logger to use, may be null. Typically we want to log any issues with the
    *     attributes during MBean discovery, but once the attribute is successfully detected and
    *     confirmed to be eligble for metric evaluation, any further attribute extraction
    *     malfunctions will be silent to avoid flooding the log.
-   * @return the attribute value, if found, or null if an error occurred
+   * @return the attribute value, if found, or {@literal null} if an error occurred
    */
   @Nullable
-  private Object extractAttributeValue(MBeanServer server, ObjectName objectName, Logger logger) {
+  private Object extractAttributeValue(
+      MBeanServerConnection connection, ObjectName objectName, Logger logger) {
     try {
-      Object value = server.getAttribute(objectName, baseName);
+      Object value = connection.getAttribute(objectName, baseName);
 
       int k = 0;
       while (k < nameChain.length) {
@@ -247,13 +248,13 @@ private Object extractAttributeValue(MBeanServer server, ObjectName objectName,
   }
 
   @Nullable
-  private Object extractAttributeValue(MBeanServer server, ObjectName objectName) {
-    return extractAttributeValue(server, objectName, null);
+  private Object extractAttributeValue(MBeanServerConnection connection, ObjectName objectName) {
+    return extractAttributeValue(connection, objectName, null);
   }
 
   @Nullable
-  Number extractNumericalAttribute(MBeanServer server, ObjectName objectName) {
-    Object value = extractAttributeValue(server, objectName);
+  Number extractNumericalAttribute(MBeanServerConnection connection, ObjectName objectName) {
+    Object value = extractAttributeValue(connection, objectName);
     if (value instanceof Number) {
       return (Number) value;
     }
@@ -262,13 +263,13 @@ Number extractNumericalAttribute(MBeanServer server, ObjectName objectName) {
 
   @Override
   @Nullable
-  public String extractValue(MBeanServer server, ObjectName objectName) {
-    return extractStringAttribute(server, objectName);
+  public String extractValue(MBeanServerConnection connection, ObjectName objectName) {
+    return extractStringAttribute(connection, objectName);
   }
 
   @Nullable
-  private String extractStringAttribute(MBeanServer server, ObjectName objectName) {
-    Object value = extractAttributeValue(server, objectName);
+  private String extractStringAttribute(MBeanServerConnection connection, ObjectName objectName) {
+    Object value = extractAttributeValue(connection, objectName);
     if (value instanceof String) {
       return (String) value;
     }

instrumentation/jmx-metrics/library/src/main/java/io/opentelemetry/instrumentation/jmx/engine/BeanFinder.java

@@ -5,6 +5,7 @@
 
 package io.opentelemetry.instrumentation.jmx.engine;
 
+import java.io.IOException;
 import java.lang.management.ManagementFactory;
 import java.util.ArrayList;
 import java.util.HashSet;
@@ -13,8 +14,10 @@
 import java.util.concurrent.Executors;
 import java.util.concurrent.ScheduledExecutorService;
 import java.util.concurrent.TimeUnit;
-import javax.management.MBeanServer;
-import javax.management.MBeanServerFactory;
+import java.util.function.Supplier;
+import java.util.logging.Level;
+import java.util.logging.Logger;
+import javax.management.MBeanServerConnection;
 import javax.management.ObjectName;
 
 /**
@@ -23,6 +26,8 @@
  */
 class BeanFinder {
 
+  private static final Logger logger = Logger.getLogger(BeanFinder.class.getName());
+
   private final MetricRegistrar registrar;
   private MetricConfiguration conf;
   private final ScheduledExecutorService exec =
@@ -42,12 +47,20 @@ class BeanFinder {
     this.maxDelay = Math.max(60000, discoveryDelay);
   }
 
-  void discoverBeans(MetricConfiguration conf) {
+  /**
+   * Starts bean discovery on a list of local or remote connections
+   *
+   * @param conf metric configuration
+   * @param connections supplier for instances of {@link MBeanServerConnection}, will be invoked
+   *     after delayed JMX initialization once the {@link #discoveryDelay} has expired.
+   */
+  void discoverBeans(
+      MetricConfiguration conf, Supplier<List<? extends MBeanServerConnection>> connections) {
     this.conf = conf;
 
     exec.schedule(
         () -> {
-          // Issue 9336: Corner case: PlatformMBeanServer will remain unitialized until a direct
+          // Issue 9336: Corner case: PlatformMBeanServer will remain uninitialized until a direct
           // reference to it is made. This call makes sure that the PlatformMBeanServer will be in
           // the set of MBeanServers reported by MBeanServerFactory.
           // Issue 11143: This call initializes java.util.logging.LogManager. We should not call it
@@ -62,7 +75,7 @@ void discoverBeans(MetricConfiguration conf) {
         new Runnable() {
           @Override
           public void run() {
-            refreshState();
+            refreshState(connections);
             // Use discoveryDelay as the increment for the actual delay
             delay = Math.min(delay + discoveryDelay, maxDelay);
             exec.schedule(this, delay, TimeUnit.MILLISECONDS);
@@ -77,9 +90,11 @@ public void run() {
    * found for a given metric definition, submit the definition to MetricRegistrar for further
    * handling. Successive invocations of this method may find matches that were previously
    * unavailable, in such cases MetricRegistrar will extend the coverage for the new MBeans
+   *
+   * @param connections supplier providing {@link MBeanServerConnection} instances to query
    */
-  private void refreshState() {
-    List<MBeanServer> servers = MBeanServerFactory.findMBeanServer(null);
+  private void refreshState(Supplier<List<? extends MBeanServerConnection>> connections) {
+    List<? extends MBeanServerConnection> servers = connections.get();
 
     for (MetricDef metricDef : conf.getMetricDefs()) {
       resolveBeans(metricDef, servers);
@@ -92,22 +107,26 @@ private void refreshState() {
    * collection of corresponding metrics.
    *
    * @param metricDef the MetricDef used to find matching MBeans
-   * @param servers the list of MBeanServers to query
+   * @param connections the list of {@link MBeanServerConnection} to query
    */
-  private void resolveBeans(MetricDef metricDef, List<MBeanServer> servers) {
+  private void resolveBeans(
+      MetricDef metricDef, List<? extends MBeanServerConnection> connections) {
     BeanGroup beans = metricDef.getBeanGroup();
 
-    for (MBeanServer server : servers) {
+    for (MBeanServerConnection connection : connections) {
       // The set of all matching ObjectNames recognized by the server
       Set<ObjectName> allObjectNames = new HashSet<>();
 
       for (ObjectName pattern : beans.getNamePatterns()) {
-        Set<ObjectName> objectNames = server.queryNames(pattern, beans.getQueryExp());
-        allObjectNames.addAll(objectNames);
+        try {
+          allObjectNames.addAll(connection.queryNames(pattern, beans.getQueryExp()));
+        } catch (IOException e) {
+          logger.log(Level.WARNING, "IO error while resolving mbean", e);
+        }
       }
 
       if (!allObjectNames.isEmpty()) {
-        resolveAttributes(allObjectNames, server, metricDef);
+        resolveAttributes(allObjectNames, connection, metricDef);
 
         // Assuming that only one MBeanServer has the required MBeans
         break;
@@ -119,19 +138,20 @@ private void resolveBeans(MetricDef metricDef, List<MBeanServer> servers) {
    * Go over the collection of matching MBeans and try to find all matching attributes. For every
    * successful match, activate metric value collection.
    *
-   * @param objectNames the collection of ObjectNames identifying the MBeans
-   * @param server the MBeanServer which recognized the collection of ObjectNames
-   * @param metricDef the MetricDef describing the attributes to look for
+   * @param objectNames the collection of {@link ObjectName}s identifying the MBeans
+   * @param connection the {@link MBeanServerConnection} which recognized the collection of
+   *     ObjectNames
+   * @param metricDef the {@link MetricDef} describing the attributes to look for
    */
   private void resolveAttributes(
-      Set<ObjectName> objectNames, MBeanServer server, MetricDef metricDef) {
+      Set<ObjectName> objectNames, MBeanServerConnection connection, MetricDef metricDef) {
     for (MetricExtractor extractor : metricDef.getMetricExtractors()) {
       // For each MetricExtractor, find the subset of MBeans that have the required attribute
       List<ObjectName> validObjectNames = new ArrayList<>();
       AttributeInfo attributeInfo = null;
       for (ObjectName objectName : objectNames) {
         AttributeInfo attr =
-            extractor.getMetricValueExtractor().getAttributeInfo(server, objectName);
+            extractor.getMetricValueExtractor().getAttributeInfo(connection, objectName);
         if (attr != null) {
           if (attributeInfo == null) {
             attributeInfo = attr;
@@ -143,7 +163,7 @@ private void resolveAttributes(
       }
       if (!validObjectNames.isEmpty()) {
         // Ready to collect metric values
-        registrar.enrollExtractor(server, validObjectNames, extractor, attributeInfo);
+        registrar.enrollExtractor(connection, validObjectNames, extractor, attributeInfo);
       }
     }
   }

instrumentation/jmx-metrics/library/src/main/java/io/opentelemetry/instrumentation/jmx/engine/DetectionStatus.java

@@ -6,7 +6,7 @@
 package io.opentelemetry.instrumentation.jmx.engine;
 
 import java.util.Collection;
-import javax.management.MBeanServer;
+import javax.management.MBeanServerConnection;
 import javax.management.ObjectName;
 
 /**
@@ -15,16 +15,16 @@
  */
 class DetectionStatus {
 
-  private final MBeanServer server;
+  private final MBeanServerConnection connection;
   private final Collection<ObjectName> objectNames;
 
-  DetectionStatus(MBeanServer server, Collection<ObjectName> objectNames) {
-    this.server = server;
+  DetectionStatus(MBeanServerConnection connection, Collection<ObjectName> objectNames) {
+    this.connection = connection;
     this.objectNames = objectNames;
   }
 
-  MBeanServer getServer() {
-    return server;
+  MBeanServerConnection getConnection() {
+    return connection;
   }
 
   Collection<ObjectName> getObjectNames() {

instrumentation/jmx-metrics/library/src/main/java/io/opentelemetry/instrumentation/jmx/engine/JmxMetricInsight.java

@@ -8,7 +8,11 @@
 import static java.util.logging.Level.FINE;
 
 import io.opentelemetry.api.OpenTelemetry;
+import java.util.List;
+import java.util.function.Supplier;
 import java.util.logging.Logger;
+import javax.management.MBeanServerConnection;
+import javax.management.MBeanServerFactory;
 
 /** Collecting and exporting JMX metrics. */
 public class JmxMetricInsight {
@@ -33,7 +37,28 @@ private JmxMetricInsight(OpenTelemetry openTelemetry, long discoveryDelay) {
     this.discoveryDelay = discoveryDelay;
   }
 
-  public void start(MetricConfiguration conf) {
+  /**
+   * Starts metric registration for local JVM
+   *
+   * @param conf metric configuration
+   */
+  public void startLocal(MetricConfiguration conf) {
+    start(conf, () -> MBeanServerFactory.findMBeanServer(null));
+  }
+
+  /**
+   * Starts metric registration for a remote JVM connection
+   *
+   * @param conf metric configuration
+   * @param connections supplier for list of remote connections
+   */
+  public void startRemote(
+      MetricConfiguration conf, Supplier<List<? extends MBeanServerConnection>> connections) {
+    start(conf, connections);
+  }
+
+  private void start(
+      MetricConfiguration conf, Supplier<List<? extends MBeanServerConnection>> connections) {
     if (conf.isEmpty()) {
       logger.log(
           FINE,
@@ -42,7 +67,7 @@ public void start(MetricConfiguration conf) {
     } else {
       MetricRegistrar registrar = new MetricRegistrar(openTelemetry, INSTRUMENTATION_SCOPE);
       BeanFinder finder = new BeanFinder(registrar, discoveryDelay);
-      finder.discoverBeans(conf);
+      finder.discoverBeans(conf, connections);
     }
   }
 }

instrumentation/jmx-metrics/library/src/main/java/io/opentelemetry/instrumentation/jmx/engine/MetricAttribute.java

@@ -5,7 +5,7 @@
 
 package io.opentelemetry.instrumentation.jmx.engine;
 
-import javax.management.MBeanServer;
+import javax.management.MBeanServerConnection;
 import javax.management.ObjectName;
 
 /**
@@ -26,7 +26,7 @@ public String getAttributeName() {
     return name;
   }
 
-  String acquireAttributeValue(MBeanServer server, ObjectName objectName) {
-    return extractor.extractValue(server, objectName);
+  String acquireAttributeValue(MBeanServerConnection connection, ObjectName objectName) {
+    return extractor.extractValue(connection, objectName);
   }
 }