From 7d6e4406b7eb37190eebcd0c6f2d7fec985741b1 Mon Sep 17 00:00:00 2001 From: Sylvain Juge <763082+SylvainJuge@users.noreply.github.com> Date: Wed, 17 Sep 2025 13:51:45 +0200 Subject: [PATCH 1/7] move otel init after scraper config --- .../contrib/jmxscraper/JmxScraper.java | 23 ++++++++++--------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/jmx-scraper/src/main/java/io/opentelemetry/contrib/jmxscraper/JmxScraper.java b/jmx-scraper/src/main/java/io/opentelemetry/contrib/jmxscraper/JmxScraper.java index 85c47ba0e..aa421576b 100644 --- a/jmx-scraper/src/main/java/io/opentelemetry/contrib/jmxscraper/JmxScraper.java +++ b/jmx-scraper/src/main/java/io/opentelemetry/contrib/jmxscraper/JmxScraper.java @@ -6,12 +6,14 @@ package io.opentelemetry.contrib.jmxscraper; import io.opentelemetry.api.GlobalOpenTelemetry; +import io.opentelemetry.api.OpenTelemetry; 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.OpenTelemetrySdk; import io.opentelemetry.sdk.autoconfigure.AutoConfiguredOpenTelemetrySdk; import io.opentelemetry.sdk.autoconfigure.spi.ConfigurationException; import java.io.DataInputStream; @@ -62,22 +64,12 @@ 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(); - JmxScraperConfig scraperConfig = configCustomizer.getScraperConfig(); long exportSeconds = scraperConfig.getSamplingInterval().toMillis() / 1000; logger.log(Level.INFO, "metrics export interval (seconds) = " + exportSeconds); - JmxMetricInsight service = - JmxMetricInsight.createService( - GlobalOpenTelemetry.get(), scraperConfig.getSamplingInterval().toMillis()); JmxConnectorBuilder connectorBuilder = JmxConnectorBuilder.createNew(scraperConfig.getServiceUrl()); @@ -91,7 +83,16 @@ public static void main(String[] args) { if (testMode) { System.exit(testConnection(connectorBuilder) ? 0 : 1); } else { - JmxScraper jmxScraper = new JmxScraper(connectorBuilder, service, scraperConfig); + // auto-configure SDK + OpenTelemetry openTelemetry = AutoConfiguredOpenTelemetrySdk.builder() + .addPropertiesSupplier(new PropertiesSupplier(argsConfig)) + .addPropertiesCustomizer(configCustomizer) + .build() + .getOpenTelemetrySdk(); + JmxMetricInsight jmxInsight = + JmxMetricInsight.createService( + openTelemetry, scraperConfig.getSamplingInterval().toMillis()); + JmxScraper jmxScraper = new JmxScraper(connectorBuilder, jmxInsight, scraperConfig); jmxScraper.start(); } } catch (ConfigurationException e) { From f46cf5c3740e74687881aaa13c93d8f6ab6e6898 Mon Sep 17 00:00:00 2001 From: Sylvain Juge <763082+SylvainJuge@users.noreply.github.com> Date: Wed, 17 Sep 2025 17:25:07 +0200 Subject: [PATCH 2/7] implement stable service ID --- .../contrib/jmxscraper/JmxScraper.java | 78 +++++++++++++------ .../config/PropertiesCustomizer.java | 25 ++++++ 2 files changed, 81 insertions(+), 22 deletions(-) diff --git a/jmx-scraper/src/main/java/io/opentelemetry/contrib/jmxscraper/JmxScraper.java b/jmx-scraper/src/main/java/io/opentelemetry/contrib/jmxscraper/JmxScraper.java index aa421576b..9124ff016 100644 --- a/jmx-scraper/src/main/java/io/opentelemetry/contrib/jmxscraper/JmxScraper.java +++ b/jmx-scraper/src/main/java/io/opentelemetry/contrib/jmxscraper/JmxScraper.java @@ -5,17 +5,18 @@ package io.opentelemetry.contrib.jmxscraper; -import io.opentelemetry.api.GlobalOpenTelemetry; import io.opentelemetry.api.OpenTelemetry; +import io.opentelemetry.api.common.AttributeKey; +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.OpenTelemetrySdk; import io.opentelemetry.sdk.autoconfigure.AutoConfiguredOpenTelemetrySdk; 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; @@ -27,15 +28,20 @@ import java.util.Collections; import java.util.List; import java.util.Map; -import java.util.Optional; import java.util.Properties; +import java.util.UUID; import java.util.concurrent.atomic.AtomicBoolean; import java.util.logging.Level; import java.util.logging.Logger; import javax.management.MBeanServerConnection; +import javax.management.ObjectName; import javax.management.remote.JMXConnector; public class JmxScraper { + + private static final AttributeKey SERVICE_INSTANCE_ID = + AttributeKey.stringKey("service.instance.id"); + private static final Logger logger = Logger.getLogger(JmxScraper.class.getName()); private static final String CONFIG_ARG = "-config"; private static final String TEST_ARG = "-test"; @@ -65,30 +71,35 @@ public static void main(String[] args) { propagateToSystemProperties(argsConfig); PropertiesCustomizer configCustomizer = new PropertiesCustomizer(); - JmxScraperConfig scraperConfig = configCustomizer.getScraperConfig(); - - long exportSeconds = scraperConfig.getSamplingInterval().toMillis() / 1000; - logger.log(Level.INFO, "metrics export interval (seconds) = " + exportSeconds); - JmxConnectorBuilder connectorBuilder = - JmxConnectorBuilder.createNew(scraperConfig.getServiceUrl()); + // auto-configure SDK + OpenTelemetry openTelemetry = + AutoConfiguredOpenTelemetrySdk.builder() + .addPropertiesSupplier(new PropertiesSupplier(argsConfig)) + .addPropertiesCustomizer(configCustomizer) + // we rely on the config customizer to be executed first to get effective config + .addResourceCustomizer( + (resource, configProperties) -> { + UUID instanceId = + getRemoteServiceInstanceId(configCustomizer.getConnectorBuilder()); + if (resource.getAttribute(SERVICE_INSTANCE_ID) != null || instanceId == null) { + return resource; + } + logger.log(Level.INFO, "remote service instance ID: " + instanceId); + return resource.merge( + Resource.create(Attributes.of(SERVICE_INSTANCE_ID, instanceId.toString()))); + }) + .build() + .getOpenTelemetrySdk(); - Optional.ofNullable(scraperConfig.getUsername()).ifPresent(connectorBuilder::withUser); - Optional.ofNullable(scraperConfig.getPassword()).ifPresent(connectorBuilder::withPassword); - - if (scraperConfig.isRegistrySsl()) { - connectorBuilder.withSslRegistry(); - } + // 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(); + JmxConnectorBuilder connectorBuilder = configCustomizer.getConnectorBuilder(); if (testMode) { System.exit(testConnection(connectorBuilder) ? 0 : 1); } else { - // auto-configure SDK - OpenTelemetry openTelemetry = AutoConfiguredOpenTelemetrySdk.builder() - .addPropertiesSupplier(new PropertiesSupplier(argsConfig)) - .addPropertiesCustomizer(configCustomizer) - .build() - .getOpenTelemetrySdk(); JmxMetricInsight jmxInsight = JmxMetricInsight.createService( openTelemetry, scraperConfig.getSamplingInterval().toMillis()); @@ -116,7 +127,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) { @@ -132,6 +142,30 @@ private static boolean testConnection(JmxConnectorBuilder connectorBuilder) { } } + // TODO : test on local JVM and call it more than once for stability + 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", "Pid", "Name")) { + Object value = connection.getAttribute(objectName, attribute); + if (id.length() > 0) { + id.append(","); + } + id.append(value); + } + return UUID.nameUUIDFromBytes(id.toString().getBytes()); + } catch (Exception e) { + throw new RuntimeException(e); + } + } catch (IOException e) { + return null; + } + } + // package private for testing static void propagateToSystemProperties(Properties properties) { for (Map.Entry entry : properties.entrySet()) { diff --git a/jmx-scraper/src/main/java/io/opentelemetry/contrib/jmxscraper/config/PropertiesCustomizer.java b/jmx-scraper/src/main/java/io/opentelemetry/contrib/jmxscraper/config/PropertiesCustomizer.java index 9d6812146..672c16b2b 100644 --- a/jmx-scraper/src/main/java/io/opentelemetry/contrib/jmxscraper/config/PropertiesCustomizer.java +++ b/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 class PropertiesCustomizer implements Function apply(ConfigProperties config) { Map result = new HashMap<>(); @@ -45,6 +50,19 @@ public Map apply(ConfigProperties config) { } scraperConfig = JmxScraperConfig.fromConfig(config); + + long exportSeconds = scraperConfig.getSamplingInterval().toMillis() / 1000; + logger.log(Level.INFO, "metrics export interval (seconds) = " + exportSeconds); + + 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 result; } @@ -60,4 +78,11 @@ public JmxScraperConfig getScraperConfig() { } return scraperConfig; } + + public JmxConnectorBuilder getConnectorBuilder() { + if (connectorBuilder == null) { + throw new IllegalStateException("apply() must be called before getConnectorBuilder()"); + } + return connectorBuilder; + } } From 614ce54ee46e5938c5be8e0706a8e2acf8dcf9d3 Mon Sep 17 00:00:00 2001 From: Sylvain Juge <763082+SylvainJuge@users.noreply.github.com> Date: Wed, 17 Sep 2025 21:41:05 +0200 Subject: [PATCH 3/7] fix tests --- .../contrib/jmxscraper/JmxScraper.java | 12 +++++++----- .../config/PropertiesCustomizerTest.java | 16 ++++++++++------ 2 files changed, 17 insertions(+), 11 deletions(-) diff --git a/jmx-scraper/src/main/java/io/opentelemetry/contrib/jmxscraper/JmxScraper.java b/jmx-scraper/src/main/java/io/opentelemetry/contrib/jmxscraper/JmxScraper.java index 1e2a0ee5b..e8c417478 100644 --- a/jmx-scraper/src/main/java/io/opentelemetry/contrib/jmxscraper/JmxScraper.java +++ b/jmx-scraper/src/main/java/io/opentelemetry/contrib/jmxscraper/JmxScraper.java @@ -25,6 +25,7 @@ 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; @@ -37,6 +38,7 @@ import java.util.concurrent.atomic.AtomicBoolean; import java.util.logging.Level; import java.util.logging.Logger; +import javax.annotation.Nullable; import javax.management.MBeanServerConnection; import javax.management.ObjectName; import javax.management.remote.JMXConnector; @@ -146,7 +148,7 @@ private static boolean testConnection(JmxConnectorBuilder connectorBuilder) { } } - // TODO : test on local JVM and call it more than once for stability + @Nullable static UUID getRemoteServiceInstanceId(JmxConnectorBuilder connectorBuilder) { try (JMXConnector jmxConnector = connectorBuilder.build()) { MBeanServerConnection connection = jmxConnector.getMBeanServerConnection(); @@ -154,16 +156,16 @@ static UUID getRemoteServiceInstanceId(JmxConnectorBuilder connectorBuilder) { StringBuilder id = new StringBuilder(); try { ObjectName objectName = new ObjectName("java.lang:type=Runtime"); - for (String attribute : Arrays.asList("StartTime", "Pid", "Name")) { + for (String attribute : Arrays.asList("StartTime", "Name")) { Object value = connection.getAttribute(objectName, attribute); if (id.length() > 0) { - id.append(","); + id.append(" "); } id.append(value); } - return UUID.nameUUIDFromBytes(id.toString().getBytes()); + return UUID.nameUUIDFromBytes(id.toString().getBytes(StandardCharsets.UTF_8)); } catch (Exception e) { - throw new RuntimeException(e); + throw new IllegalStateException(e); } } catch (IOException e) { return null; diff --git a/jmx-scraper/src/test/java/io/opentelemetry/contrib/jmxscraper/config/PropertiesCustomizerTest.java b/jmx-scraper/src/test/java/io/opentelemetry/contrib/jmxscraper/config/PropertiesCustomizerTest.java index 6ee1e7ba1..77ce948cc 100644 --- a/jmx-scraper/src/test/java/io/opentelemetry/contrib/jmxscraper/config/PropertiesCustomizerTest.java +++ b/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 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 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 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 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 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"); From 12acf645d482adfd97a6280aba43fab30e88c6a1 Mon Sep 17 00:00:00 2001 From: Sylvain Juge <763082+SylvainJuge@users.noreply.github.com> Date: Wed, 17 Sep 2025 22:31:57 +0200 Subject: [PATCH 4/7] implement test based on scraper logs --- .../contrib/jmxscraper/JmxConnectionTest.java | 36 ++++++++++++++++++- 1 file changed, 35 insertions(+), 1 deletion(-) diff --git a/jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/JmxConnectionTest.java b/jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/JmxConnectionTest.java index c984724e3..229ac3b7d 100644 --- a/jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/JmxConnectionTest.java +++ b/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,39 @@ void serverSslClientSsl(JmxScraperContainer.ConfigSource configSource) { .withConfigSource(configSource)); } + @Test + void stableServiceInstanceServiceId() { + UUID expectedServiceId = null; + + // 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(); + for (int i = 0; i < 2; i++) { + 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).isNotNull(); + if (expectedServiceId == null) { + expectedServiceId = serviceId; + } else { + assertThat(serviceId).isEqualTo(expectedServiceId); + } + } + } + } + } + private static void connectionTest( Function customizeApp, Function customizeScraper) { From 04c675e38e20af12b96a1a8e668feb4f99017caa Mon Sep 17 00:00:00 2001 From: Sylvain Juge <763082+SylvainJuge@users.noreply.github.com> Date: Thu, 18 Sep 2025 10:22:34 +0200 Subject: [PATCH 5/7] use semconv constant for service ID --- jmx-scraper/build.gradle.kts | 2 ++ .../java/io/opentelemetry/contrib/jmxscraper/JmxScraper.java | 5 +---- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/jmx-scraper/build.gradle.kts b/jmx-scraper/build.gradle.kts index 687b30612..c80ff15d5 100644 --- a/jmx-scraper/build.gradle.kts +++ b/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") diff --git a/jmx-scraper/src/main/java/io/opentelemetry/contrib/jmxscraper/JmxScraper.java b/jmx-scraper/src/main/java/io/opentelemetry/contrib/jmxscraper/JmxScraper.java index e8c417478..328ac8636 100644 --- a/jmx-scraper/src/main/java/io/opentelemetry/contrib/jmxscraper/JmxScraper.java +++ b/jmx-scraper/src/main/java/io/opentelemetry/contrib/jmxscraper/JmxScraper.java @@ -5,13 +5,13 @@ 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.logging.Level.INFO; import static java.util.logging.Level.SEVERE; import io.opentelemetry.api.OpenTelemetry; -import io.opentelemetry.api.common.AttributeKey; import io.opentelemetry.api.common.Attributes; import io.opentelemetry.contrib.jmxscraper.config.JmxScraperConfig; import io.opentelemetry.contrib.jmxscraper.config.PropertiesCustomizer; @@ -45,9 +45,6 @@ public final class JmxScraper { - private static final AttributeKey SERVICE_INSTANCE_ID = - AttributeKey.stringKey("service.instance.id"); - private static final Logger logger = Logger.getLogger(JmxScraper.class.getName()); private static final String CONFIG_ARG = "-config"; private static final String TEST_ARG = "-test"; From 48bbe803dd5872e9d10c5ac07e1428145149cd7a Mon Sep 17 00:00:00 2001 From: Sylvain Juge <763082+SylvainJuge@users.noreply.github.com> Date: Fri, 19 Sep 2025 11:30:01 +0200 Subject: [PATCH 6/7] post-review changes --- .../contrib/jmxscraper/JmxConnectionTest.java | 49 ++++++++++--------- .../config/PropertiesCustomizer.java | 13 +++-- 2 files changed, 35 insertions(+), 27 deletions(-) diff --git a/jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/JmxConnectionTest.java b/jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/JmxConnectionTest.java index 229ac3b7d..00d7fc124 100644 --- a/jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/JmxConnectionTest.java +++ b/jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/JmxConnectionTest.java @@ -178,34 +178,37 @@ void serverSslClientSsl(JmxScraperContainer.ConfigSource configSource) { @Test void stableServiceInstanceServiceId() { - UUID expectedServiceId = null; - // 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(); - for (int i = 0; i < 2; i++) { - 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).isNotNull(); - if (expectedServiceId == null) { - expectedServiceId = serviceId; - } else { - assertThat(serviceId).isEqualTo(expectedServiceId); - } + + 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; } } diff --git a/jmx-scraper/src/main/java/io/opentelemetry/contrib/jmxscraper/config/PropertiesCustomizer.java b/jmx-scraper/src/main/java/io/opentelemetry/contrib/jmxscraper/config/PropertiesCustomizer.java index 0a1dcd1aa..dc6f06952 100644 --- a/jmx-scraper/src/main/java/io/opentelemetry/contrib/jmxscraper/config/PropertiesCustomizer.java +++ b/jmx-scraper/src/main/java/io/opentelemetry/contrib/jmxscraper/config/PropertiesCustomizer.java @@ -49,21 +49,26 @@ public Map 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); - connectorBuilder = JmxConnectorBuilder.createNew(scraperConfig.getServiceUrl()); + 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 result; + return connectorBuilder; } /** From 487ca2d21faf62badce5f81eb81331de899d76a0 Mon Sep 17 00:00:00 2001 From: Sylvain Juge <763082+SylvainJuge@users.noreply.github.com> Date: Fri, 19 Sep 2025 13:57:17 +0200 Subject: [PATCH 7/7] post-review changes --- .../contrib/jmxscraper/JmxScraper.java | 30 ++++++++++--------- 1 file changed, 16 insertions(+), 14 deletions(-) diff --git a/jmx-scraper/src/main/java/io/opentelemetry/contrib/jmxscraper/JmxScraper.java b/jmx-scraper/src/main/java/io/opentelemetry/contrib/jmxscraper/JmxScraper.java index 328ac8636..678368d7b 100644 --- a/jmx-scraper/src/main/java/io/opentelemetry/contrib/jmxscraper/JmxScraper.java +++ b/jmx-scraper/src/main/java/io/opentelemetry/contrib/jmxscraper/JmxScraper.java @@ -20,6 +20,7 @@ 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; @@ -36,7 +37,7 @@ import java.util.Properties; import java.util.UUID; import java.util.concurrent.atomic.AtomicBoolean; -import java.util.logging.Level; +import java.util.function.BiFunction; import java.util.logging.Logger; import javax.annotation.Nullable; import javax.management.MBeanServerConnection; @@ -75,23 +76,24 @@ public static void main(String[] args) { PropertiesCustomizer configCustomizer = new PropertiesCustomizer(); + // we rely on the config customizer to be executed first to get effective config. + BiFunction 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) - // we rely on the config customizer to be executed first to get effective config - .addResourceCustomizer( - (resource, configProperties) -> { - UUID instanceId = - getRemoteServiceInstanceId(configCustomizer.getConnectorBuilder()); - if (resource.getAttribute(SERVICE_INSTANCE_ID) != null || instanceId == null) { - return resource; - } - logger.log(Level.INFO, "remote service instance ID: " + instanceId); - return resource.merge( - Resource.create(Attributes.of(SERVICE_INSTANCE_ID, instanceId.toString()))); - }) + .addResourceCustomizer(resourceCustomizer) .build() .getOpenTelemetrySdk(); @@ -146,7 +148,7 @@ private static boolean testConnection(JmxConnectorBuilder connectorBuilder) { } @Nullable - static UUID getRemoteServiceInstanceId(JmxConnectorBuilder connectorBuilder) { + private static UUID getRemoteServiceInstanceId(JmxConnectorBuilder connectorBuilder) { try (JMXConnector jmxConnector = connectorBuilder.build()) { MBeanServerConnection connection = jmxConnector.getMBeanServerConnection();