jmx-scraper implement stable service.instance.id (#2270)

Author: SylvainJuge

jmx-scraper/build.gradle.kts

@@ -26,6 +26,8 @@ dependencies {
 
   implementation("io.opentelemetry.instrumentation:opentelemetry-jmx-metrics")
 
+  implementation("io.opentelemetry.semconv:opentelemetry-semconv-incubating")
+
   testImplementation("org.junit-pioneer:junit-pioneer")
   testImplementation("io.opentelemetry:opentelemetry-sdk-testing")
   testImplementation("org.awaitility:awaitility")

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

@@ -9,6 +9,7 @@
 
 import java.nio.file.Path;
 import java.security.cert.X509Certificate;
+import java.util.UUID;
 import java.util.concurrent.TimeUnit;
 import java.util.function.Function;
 import org.junit.jupiter.api.AfterAll;
@@ -131,7 +132,7 @@ private void testServerSsl(
   }
 
   @ParameterizedTest
-  @EnumSource(value = JmxScraperContainer.ConfigSource.class)
+  @EnumSource
   void serverSslClientSsl(JmxScraperContainer.ConfigSource configSource) {
     // Note: this could have been made simpler by relying on the fact that keystore could be used
     // as a trust store, but having clear split provides also some extra clarity
@@ -175,6 +176,42 @@ void serverSslClientSsl(JmxScraperContainer.ConfigSource configSource) {
                 .withConfigSource(configSource));
   }
 
+  @Test
+  void stableServiceInstanceServiceId() {
+    // start a single app, connect twice to it and check that the service id is the same
+    try (TestAppContainer app = appContainer().withJmxPort(JMX_PORT)) {
+      app.start();
+
+      UUID firstId = startScraperAndGetServiceId();
+      UUID secondId = startScraperAndGetServiceId();
+
+      assertThat(firstId)
+          .describedAs(
+              "connecting twice to the same JVM should return the same service instance ID")
+          .isEqualTo(secondId);
+    }
+  }
+
+  private static UUID startScraperAndGetServiceId() {
+    try (JmxScraperContainer scraper =
+        scraperContainer()
+            .withRmiServiceUrl(APP_HOST, JMX_PORT)
+            // does not need to be tested on all config sources
+            .withConfigSource(JmxScraperContainer.ConfigSource.SYSTEM_PROPERTIES)) {
+      scraper.start();
+      waitTerminated(scraper);
+      String[] logLines = scraper.getLogs().split("\n");
+      UUID serviceId = null;
+      for (String logLine : logLines) {
+        if (logLine.contains("remote service instance ID")) {
+          serviceId = UUID.fromString(logLine.substring(logLine.lastIndexOf(":") + 1).trim());
+        }
+      }
+      assertThat(serviceId).describedAs("unable to get service instance ID from logs").isNotNull();
+      return serviceId;
+    }
+  }
+
   private static void connectionTest(
       Function<TestAppContainer, TestAppContainer> customizeApp,
       Function<JmxScraperContainer, JmxScraperContainer> customizeScraper) {

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

@@ -5,37 +5,47 @@
 
 package io.opentelemetry.contrib.jmxscraper;
 
+import static io.opentelemetry.semconv.incubating.ServiceIncubatingAttributes.SERVICE_INSTANCE_ID;
 import static java.util.Arrays.asList;
 import static java.util.Collections.singletonList;
-import static java.util.Optional.ofNullable;
 import static java.util.logging.Level.INFO;
 import static java.util.logging.Level.SEVERE;
 
-import io.opentelemetry.api.GlobalOpenTelemetry;
+import io.opentelemetry.api.OpenTelemetry;
+import io.opentelemetry.api.common.Attributes;
 import io.opentelemetry.contrib.jmxscraper.config.JmxScraperConfig;
 import io.opentelemetry.contrib.jmxscraper.config.PropertiesCustomizer;
 import io.opentelemetry.contrib.jmxscraper.config.PropertiesSupplier;
 import io.opentelemetry.instrumentation.jmx.engine.JmxMetricInsight;
 import io.opentelemetry.instrumentation.jmx.engine.MetricConfiguration;
 import io.opentelemetry.instrumentation.jmx.yaml.RuleParser;
 import io.opentelemetry.sdk.autoconfigure.AutoConfiguredOpenTelemetrySdk;
+import io.opentelemetry.sdk.autoconfigure.spi.ConfigProperties;
 import io.opentelemetry.sdk.autoconfigure.spi.ConfigurationException;
+import io.opentelemetry.sdk.resources.Resource;
 import java.io.DataInputStream;
 import java.io.IOException;
 import java.io.InputStream;
+import java.nio.charset.StandardCharsets;
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.nio.file.Paths;
 import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.List;
 import java.util.Map;
 import java.util.Properties;
+import java.util.UUID;
 import java.util.concurrent.atomic.AtomicBoolean;
+import java.util.function.BiFunction;
 import java.util.logging.Logger;
+import javax.annotation.Nullable;
 import javax.management.MBeanServerConnection;
+import javax.management.ObjectName;
 import javax.management.remote.JMXConnector;
 
 public final class JmxScraper {
+
   private static final Logger logger = Logger.getLogger(JmxScraper.class.getName());
   private static final String CONFIG_ARG = "-config";
   private static final String TEST_ARG = "-test";
@@ -64,36 +74,41 @@ public static void main(String[] args) {
       Properties argsConfig = argsToConfig(effectiveArgs);
       propagateToSystemProperties(argsConfig);
 
-      // auto-configure and register SDK
       PropertiesCustomizer configCustomizer = new PropertiesCustomizer();
-      AutoConfiguredOpenTelemetrySdk.builder()
-          .addPropertiesSupplier(new PropertiesSupplier(argsConfig))
-          .addPropertiesCustomizer(configCustomizer)
-          .setResultAsGlobal()
-          .build();
 
+      // we rely on the config customizer to be executed first to get effective config.
+      BiFunction<Resource, ConfigProperties, Resource> resourceCustomizer =
+          (resource, configProperties) -> {
+            UUID instanceId = getRemoteServiceInstanceId(configCustomizer.getConnectorBuilder());
+            if (resource.getAttribute(SERVICE_INSTANCE_ID) != null || instanceId == null) {
+              return resource;
+            }
+            logger.log(INFO, "remote service instance ID: " + instanceId);
+            return resource.merge(
+                Resource.create(Attributes.of(SERVICE_INSTANCE_ID, instanceId.toString())));
+          };
+
+      // auto-configure SDK
+      OpenTelemetry openTelemetry =
+          AutoConfiguredOpenTelemetrySdk.builder()
+              .addPropertiesSupplier(new PropertiesSupplier(argsConfig))
+              .addPropertiesCustomizer(configCustomizer)
+              .addResourceCustomizer(resourceCustomizer)
+              .build()
+              .getOpenTelemetrySdk();
+
+      // scraper configuration and connector builder are built using effective SDK configuration
+      // thus we have to get it after the SDK is built
       JmxScraperConfig scraperConfig = configCustomizer.getScraperConfig();
-
-      long exportSeconds = scraperConfig.getSamplingInterval().toMillis() / 1000;
-      logger.log(INFO, "metrics export interval (seconds) =  " + exportSeconds);
-
-      JmxMetricInsight service =
-          JmxMetricInsight.createService(
-              GlobalOpenTelemetry.get(), scraperConfig.getSamplingInterval().toMillis());
-      JmxConnectorBuilder connectorBuilder =
-          JmxConnectorBuilder.createNew(scraperConfig.getServiceUrl());
-
-      ofNullable(scraperConfig.getUsername()).ifPresent(connectorBuilder::withUser);
-      ofNullable(scraperConfig.getPassword()).ifPresent(connectorBuilder::withPassword);
-
-      if (scraperConfig.isRegistrySsl()) {
-        connectorBuilder.withSslRegistry();
-      }
+      JmxConnectorBuilder connectorBuilder = configCustomizer.getConnectorBuilder();
 
       if (testMode) {
         System.exit(testConnection(connectorBuilder) ? 0 : 1);
       } else {
-        JmxScraper jmxScraper = new JmxScraper(connectorBuilder, service, scraperConfig);
+        JmxMetricInsight jmxInsight =
+            JmxMetricInsight.createService(
+                openTelemetry, scraperConfig.getSamplingInterval().toMillis());
+        JmxScraper jmxScraper = new JmxScraper(connectorBuilder, jmxInsight, scraperConfig);
         jmxScraper.start();
       }
     } catch (ConfigurationException e) {
@@ -117,7 +132,6 @@ public static void main(String[] args) {
 
   private static boolean testConnection(JmxConnectorBuilder connectorBuilder) {
     try (JMXConnector connector = connectorBuilder.build()) {
-
       MBeanServerConnection connection = connector.getMBeanServerConnection();
       Integer mbeanCount = connection.getMBeanCount();
       if (mbeanCount > 0) {
@@ -133,6 +147,30 @@ private static boolean testConnection(JmxConnectorBuilder connectorBuilder) {
     }
   }
 
+  @Nullable
+  private static UUID getRemoteServiceInstanceId(JmxConnectorBuilder connectorBuilder) {
+    try (JMXConnector jmxConnector = connectorBuilder.build()) {
+      MBeanServerConnection connection = jmxConnector.getMBeanServerConnection();
+
+      StringBuilder id = new StringBuilder();
+      try {
+        ObjectName objectName = new ObjectName("java.lang:type=Runtime");
+        for (String attribute : Arrays.asList("StartTime", "Name")) {
+          Object value = connection.getAttribute(objectName, attribute);
+          if (id.length() > 0) {
+            id.append(" ");
+          }
+          id.append(value);
+        }
+        return UUID.nameUUIDFromBytes(id.toString().getBytes(StandardCharsets.UTF_8));
+      } catch (Exception e) {
+        throw new IllegalStateException(e);
+      }
+    } catch (IOException e) {
+      return null;
+    }
+  }
+
   // package private for testing
   static void propagateToSystemProperties(Properties properties) {
     for (Map.Entry<Object, Object> entry : properties.entrySet()) {

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

@@ -8,10 +8,13 @@
 import static io.opentelemetry.contrib.jmxscraper.config.JmxScraperConfig.JMX_INTERVAL_LEGACY;
 import static io.opentelemetry.contrib.jmxscraper.config.JmxScraperConfig.METRIC_EXPORT_INTERVAL;
 
+import io.opentelemetry.contrib.jmxscraper.JmxConnectorBuilder;
 import io.opentelemetry.sdk.autoconfigure.spi.ConfigProperties;
 import java.util.HashMap;
 import java.util.Map;
+import java.util.Optional;
 import java.util.function.Function;
+import java.util.logging.Level;
 import java.util.logging.Logger;
 import javax.annotation.Nullable;
 
@@ -24,6 +27,8 @@ public final class PropertiesCustomizer implements Function<ConfigProperties, Ma
 
   @Nullable private JmxScraperConfig scraperConfig;
 
+  @Nullable private JmxConnectorBuilder connectorBuilder;
+
   @Override
   public Map<String, String> apply(ConfigProperties config) {
     Map<String, String> result = new HashMap<>();
@@ -44,10 +49,28 @@ public Map<String, String> apply(ConfigProperties config) {
       result.put(METRIC_EXPORT_INTERVAL, intervalLegacy + "ms");
     }
 
+    // scraper config and connector builder must be initialized with the effective SDK configuration
+    // thus we need to initialize them here and then rely on getter being called after this method.
     scraperConfig = JmxScraperConfig.fromConfig(config);
+    connectorBuilder = createConnectorBuilder(scraperConfig);
+
+    long exportSeconds = scraperConfig.getSamplingInterval().toMillis() / 1000;
+    logger.log(Level.INFO, "metrics export interval (seconds) =  " + exportSeconds);
+
     return result;
   }
 
+  private static JmxConnectorBuilder createConnectorBuilder(JmxScraperConfig scraperConfig) {
+    JmxConnectorBuilder connectorBuilder =
+        JmxConnectorBuilder.createNew(scraperConfig.getServiceUrl());
+    Optional.ofNullable(scraperConfig.getUsername()).ifPresent(connectorBuilder::withUser);
+    Optional.ofNullable(scraperConfig.getPassword()).ifPresent(connectorBuilder::withPassword);
+    if (scraperConfig.isRegistrySsl()) {
+      connectorBuilder.withSslRegistry();
+    }
+    return connectorBuilder;
+  }
+
   /**
    * Get scraper configuration from the previous call to {@link #apply(ConfigProperties)}
    *
@@ -60,4 +83,11 @@ public JmxScraperConfig getScraperConfig() {
     }
     return scraperConfig;
   }
+
+  public JmxConnectorBuilder getConnectorBuilder() {
+    if (connectorBuilder == null) {
+      throw new IllegalStateException("apply() must be called before getConnectorBuilder()");
+    }
+    return connectorBuilder;
+  }
 }

jmx-scraper/src/test/java/io/opentelemetry/contrib/jmxscraper/config/PropertiesCustomizerTest.java

@@ -17,16 +17,20 @@
 
 class PropertiesCustomizerTest {
 
+  private static final String DUMMY_URL = "service:jmx:rmi:///jndi/rmi://host:999/jmxrmi";
+
   @Test
-  void tryGetConfigBeforeApply() {
+  void tryGetBeforeApply() {
     assertThatThrownBy(() -> new PropertiesCustomizer().getScraperConfig())
         .isInstanceOf(IllegalStateException.class);
+    assertThatThrownBy(() -> new PropertiesCustomizer().getConnectorBuilder())
+        .isInstanceOf(IllegalStateException.class);
   }
 
   @Test
   void defaultOtlpExporter() {
     Map<String, String> map = new HashMap<>();
-    map.put("otel.jmx.service.url", "dummy-url");
+    map.put("otel.jmx.service.url", DUMMY_URL);
     map.put("otel.jmx.target.system", "jvm");
     ConfigProperties config = DefaultConfigProperties.createFromMap(map);
 
@@ -37,7 +41,7 @@ void defaultOtlpExporter() {
   @Test
   void explicitExporterSet() {
     Map<String, String> map = new HashMap<>();
-    map.put("otel.jmx.service.url", "dummy-url");
+    map.put("otel.jmx.service.url", DUMMY_URL);
     map.put("otel.jmx.target.system", "jvm");
     map.put("otel.metrics.exporter", "otlp,logging");
     ConfigProperties config = DefaultConfigProperties.createFromMap(map);
@@ -49,7 +53,7 @@ void explicitExporterSet() {
   @Test
   void getSomeConfiguration() {
     Map<String, String> map = new HashMap<>();
-    map.put("otel.jmx.service.url", "dummy-url");
+    map.put("otel.jmx.service.url", DUMMY_URL);
     map.put("otel.jmx.target.system", "jvm");
     map.put("otel.metrics.exporter", "otlp");
     ConfigProperties config = DefaultConfigProperties.createFromMap(map);
@@ -67,7 +71,7 @@ void getSomeConfiguration() {
   @Test
   void setSdkMetricExportFromJmxInterval() {
     Map<String, String> map = new HashMap<>();
-    map.put("otel.jmx.service.url", "dummy-url");
+    map.put("otel.jmx.service.url", DUMMY_URL);
     map.put("otel.jmx.target.system", "jvm");
     map.put("otel.metrics.exporter", "otlp");
     map.put("otel.jmx.interval.milliseconds", "10000");
@@ -83,7 +87,7 @@ void setSdkMetricExportFromJmxInterval() {
   @Test
   void sdkMetricExportIntervalPriority() {
     Map<String, String> map = new HashMap<>();
-    map.put("otel.jmx.service.url", "dummy-url");
+    map.put("otel.jmx.service.url", DUMMY_URL);
     map.put("otel.jmx.target.system", "jvm");
     map.put("otel.metrics.exporter", "otlp");
     map.put("otel.jmx.interval.milliseconds", "10000");