From e4f47691ac9d07e3fde79e80dce84a4138bfc70e Mon Sep 17 00:00:00 2001 From: Sylvain Juge <763082+SylvainJuge@users.noreply.github.com> Date: Tue, 17 Sep 2024 15:44:40 +0200 Subject: [PATCH 01/10] prepare for tomcat e2e test --- jmx-scraper/build.gradle.kts | 4 + .../TargetSystemIntegrationTest.java | 160 ++++++++++++++++++ .../target_systems/TomcatIntegrationTest.java | 41 +++++ 3 files changed, 205 insertions(+) create mode 100644 jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/target_systems/TargetSystemIntegrationTest.java create mode 100644 jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/target_systems/TomcatIntegrationTest.java diff --git a/jmx-scraper/build.gradle.kts b/jmx-scraper/build.gradle.kts index d3c7e4b58..498ffc0b9 100644 --- a/jmx-scraper/build.gradle.kts +++ b/jmx-scraper/build.gradle.kts @@ -29,6 +29,10 @@ testing { dependencies { implementation("org.testcontainers:junit-jupiter") implementation("org.slf4j:slf4j-simple") + implementation("com.linecorp.armeria:armeria-grpc") + implementation("com.linecorp.armeria:armeria-junit5") + implementation("com.linecorp.armeria:armeria-grpc") + implementation("io.opentelemetry.proto:opentelemetry-proto:0.20.0-alpha") } } } diff --git a/jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/target_systems/TargetSystemIntegrationTest.java b/jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/target_systems/TargetSystemIntegrationTest.java new file mode 100644 index 000000000..a142ab46e --- /dev/null +++ b/jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/target_systems/TargetSystemIntegrationTest.java @@ -0,0 +1,160 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.contrib.jmxscraper.target_systems; + +import static org.assertj.core.api.Assertions.assertThat; + +import com.linecorp.armeria.server.ServerBuilder; +import com.linecorp.armeria.server.grpc.GrpcService; +import com.linecorp.armeria.testing.junit5.server.ServerExtension; +import io.grpc.stub.StreamObserver; +import io.opentelemetry.contrib.jmxscraper.client.JmxRemoteClient; +import io.opentelemetry.proto.collector.metrics.v1.ExportMetricsServiceRequest; +import io.opentelemetry.proto.collector.metrics.v1.ExportMetricsServiceResponse; +import io.opentelemetry.proto.collector.metrics.v1.MetricsServiceGrpc; +import java.io.IOException; +import java.util.ArrayList; +import java.util.List; +import java.util.concurrent.BlockingQueue; +import java.util.concurrent.ExecutionException; +import java.util.concurrent.LinkedBlockingDeque; +import javax.management.remote.JMXConnector; +import org.junit.jupiter.api.AfterAll; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.Test; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import org.testcontainers.Testcontainers; +import org.testcontainers.containers.GenericContainer; +import org.testcontainers.containers.Network; +import org.testcontainers.containers.output.Slf4jLogConsumer; + +public abstract class TargetSystemIntegrationTest { + + private static final Logger logger = LoggerFactory.getLogger(TargetSystemIntegrationTest.class); + + /** + * Create target system container + * + * @param jmxPort JMX port target JVM should listen to + * @return target system container + */ + protected abstract GenericContainer createTargetContainer(int jmxPort); + + // assert on received metrics + + private static Network network; + private static OtlpGrpcServer otlpServer; + private GenericContainer target; + private GenericContainer scraper; + + // private static final String OTLP_HOST = "host.testcontainers.internal"; + private static final int JMX_PORT = 9999; + + @BeforeAll + static void beforeAll() { + network = Network.newNetwork(); + otlpServer = new OtlpGrpcServer(); + otlpServer.start(); + Testcontainers.exposeHostPorts(otlpServer.httpPort()); + // String otlpEndpoint = "http://" + OTLP_HOST + ":" + otlpServer.httpPort(); + } + + @AfterAll + static void afterAll() { + network.close(); + try { + otlpServer.stop().get(); + } catch (InterruptedException | ExecutionException e) { + throw new RuntimeException(e); + } + } + + @AfterEach + void afterEach() { + if (target != null && target.isRunning()) { + target.stop(); + } + if (scraper != null && scraper.isRunning()) { + scraper.stop(); + } + if (otlpServer != null) { + otlpServer.reset(); + } + } + + @Test + void endToEndTest() { + + target = + createTargetContainer(JMX_PORT) + .withLogConsumer(new Slf4jLogConsumer(logger)) + .withNetwork(network) + .withExposedPorts(JMX_PORT) + .withNetworkAliases("target_system"); + target.start(); + + logger.info( + "Target system started, JMX port: {} mapped to {}:{}", + JMX_PORT, + target.getHost(), + target.getMappedPort(JMX_PORT)); + + scraper = createScraperContainer(); + + // TODO: start scraper container + // scraper.start(); + + // TODO : wait for metrics to be sent and add assertions on what is being captured + // for now we just test that we can connect to remote JMX using our client. + try (JMXConnector connector = + JmxRemoteClient.createNew(target.getHost(), target.getMappedPort(JMX_PORT)).connect()) { + assertThat(connector.getMBeanServerConnection()).isNotNull(); + } catch (IOException e) { + throw new RuntimeException(e); + } + // TODO: replace with real assertions + assertThat(otlpServer.getMetrics()).isEmpty(); + } + + private static GenericContainer createScraperContainer() { + return null; + } + + private static class OtlpGrpcServer extends ServerExtension { + + private final BlockingQueue metricRequests = + new LinkedBlockingDeque<>(); + + List getMetrics() { + return new ArrayList<>(metricRequests); + } + + void reset() { + metricRequests.clear(); + } + + @Override + protected void configure(ServerBuilder sb) { + sb.service( + GrpcService.builder() + .addService( + new MetricsServiceGrpc.MetricsServiceImplBase() { + @Override + public void export( + ExportMetricsServiceRequest request, + StreamObserver responseObserver) { + metricRequests.add(request); + responseObserver.onNext(ExportMetricsServiceResponse.getDefaultInstance()); + responseObserver.onCompleted(); + } + }) + .build()); + sb.http(0); + } + } +} diff --git a/jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/target_systems/TomcatIntegrationTest.java b/jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/target_systems/TomcatIntegrationTest.java new file mode 100644 index 000000000..69e1bba27 --- /dev/null +++ b/jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/target_systems/TomcatIntegrationTest.java @@ -0,0 +1,41 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.contrib.jmxscraper.target_systems; + +import java.time.Duration; +import org.testcontainers.containers.GenericContainer; +import org.testcontainers.containers.wait.strategy.Wait; +import org.testcontainers.images.builder.ImageFromDockerfile; + +public class TomcatIntegrationTest extends TargetSystemIntegrationTest { + + @Override + protected GenericContainer createTargetContainer(int jmxPort) { + return new GenericContainer<>( + new ImageFromDockerfile() + .withDockerfileFromBuilder( + builder -> + builder + .from("tomcat:9.0") + .run("rm", "-fr", "/usr/local/tomcat/webapps/ROOT") + .add( + "https://tomcat.apache.org/tomcat-9.0-doc/appdev/sample/sample.war", + "/usr/local/tomcat/webapps/ROOT.war") + .build())) + .withEnv("LOCAL_JMX", "no") + .withEnv( + "CATALINA_OPTS", + "-Dcom.sun.management.jmxremote.local.only=false" + + " -Dcom.sun.management.jmxremote.authenticate=false" + + " -Dcom.sun.management.jmxremote.ssl=false" + + " -Dcom.sun.management.jmxremote.port=" + + jmxPort + + " -Dcom.sun.management.jmxremote.rmi.port=" + + jmxPort) + .withStartupTimeout(Duration.ofMinutes(2)) + .waitingFor(Wait.forListeningPort()); + } +} From 71e6258c949aa66cfa777d8fd22fab90b6199be8 Mon Sep 17 00:00:00 2001 From: Sylvain Juge <763082+SylvainJuge@users.noreply.github.com> Date: Tue, 17 Sep 2024 22:31:55 +0200 Subject: [PATCH 02/10] start wiring things together --- dependencyManagement/build.gradle.kts | 2 +- .../target_systems/JvmIntegrationTest.java | 4 + .../TargetSystemIntegrationTest.java | 80 +++++++++++++++---- .../target_systems/TomcatIntegrationTest.java | 5 ++ .../contrib/jmxscraper/JmxScraper.java | 63 ++++++++------- .../jmxscraper/client/JmxRemoteClient.java | 29 +++---- 6 files changed, 126 insertions(+), 57 deletions(-) create mode 100644 jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/target_systems/JvmIntegrationTest.java diff --git a/dependencyManagement/build.gradle.kts b/dependencyManagement/build.gradle.kts index c8e9fc865..9baba0736 100644 --- a/dependencyManagement/build.gradle.kts +++ b/dependencyManagement/build.gradle.kts @@ -7,7 +7,7 @@ data class DependencySet(val group: String, val version: String, val modules: Li val dependencyVersions = hashMapOf() rootProject.extra["versions"] = dependencyVersions -val otelInstrumentationVersion = "2.8.0-alpha" +val otelInstrumentationVersion = "2.9.0-alpha-SNAPSHOT" val DEPENDENCY_BOMS = listOf( "com.fasterxml.jackson:jackson-bom:2.17.2", diff --git a/jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/target_systems/JvmIntegrationTest.java b/jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/target_systems/JvmIntegrationTest.java new file mode 100644 index 000000000..3d5285aa8 --- /dev/null +++ b/jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/target_systems/JvmIntegrationTest.java @@ -0,0 +1,4 @@ +package io.opentelemetry.contrib.jmxscraper.target_systems; + +public class JvmIntegrationTest { +} diff --git a/jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/target_systems/TargetSystemIntegrationTest.java b/jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/target_systems/TargetSystemIntegrationTest.java index a142ab46e..119acb103 100644 --- a/jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/target_systems/TargetSystemIntegrationTest.java +++ b/jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/target_systems/TargetSystemIntegrationTest.java @@ -16,8 +16,11 @@ import io.opentelemetry.proto.collector.metrics.v1.ExportMetricsServiceResponse; import io.opentelemetry.proto.collector.metrics.v1.MetricsServiceGrpc; import java.io.IOException; +import java.time.Duration; import java.util.ArrayList; +import java.util.Arrays; import java.util.List; +import java.util.Locale; import java.util.concurrent.BlockingQueue; import java.util.concurrent.ExecutionException; import java.util.concurrent.LinkedBlockingDeque; @@ -32,10 +35,13 @@ import org.testcontainers.containers.GenericContainer; import org.testcontainers.containers.Network; import org.testcontainers.containers.output.Slf4jLogConsumer; +import org.testcontainers.containers.wait.strategy.Wait; +import org.testcontainers.utility.MountableFile; public abstract class TargetSystemIntegrationTest { private static final Logger logger = LoggerFactory.getLogger(TargetSystemIntegrationTest.class); + private static String otlpEndpoint; /** * Create target system container @@ -45,14 +51,14 @@ public abstract class TargetSystemIntegrationTest { */ protected abstract GenericContainer createTargetContainer(int jmxPort); - // assert on received metrics + protected abstract String getTargetSystem(); private static Network network; private static OtlpGrpcServer otlpServer; private GenericContainer target; private GenericContainer scraper; - // private static final String OTLP_HOST = "host.testcontainers.internal"; + private static final String OTLP_HOST = "host.testcontainers.internal"; private static final int JMX_PORT = 9999; @BeforeAll @@ -61,7 +67,7 @@ static void beforeAll() { otlpServer = new OtlpGrpcServer(); otlpServer.start(); Testcontainers.exposeHostPorts(otlpServer.httpPort()); - // String otlpEndpoint = "http://" + OTLP_HOST + ":" + otlpServer.httpPort(); + otlpEndpoint = "http://" + OTLP_HOST + ":" + otlpServer.httpPort(); } @AfterAll @@ -98,21 +104,21 @@ void endToEndTest() { .withNetworkAliases("target_system"); target.start(); + String targetHost = target.getHost(); + Integer targetPort = target.getMappedPort(JMX_PORT); logger.info( - "Target system started, JMX port: {} mapped to {}:{}", - JMX_PORT, - target.getHost(), - target.getMappedPort(JMX_PORT)); + "Target system started, JMX port: {} mapped to {}:{}", JMX_PORT, targetHost, targetPort); - scraper = createScraperContainer(); + scraper = + createScraperContainer(otlpEndpoint, getTargetSystem(), null, "target_system", JMX_PORT); + logger.info( + "starting scraper with JVM arguments : {}", String.join(" ", scraper.getCommandParts())); - // TODO: start scraper container - // scraper.start(); + scraper.start(); // TODO : wait for metrics to be sent and add assertions on what is being captured // for now we just test that we can connect to remote JMX using our client. - try (JMXConnector connector = - JmxRemoteClient.createNew(target.getHost(), target.getMappedPort(JMX_PORT)).connect()) { + try (JMXConnector connector = JmxRemoteClient.createNew(targetHost, targetPort).connect()) { assertThat(connector.getMBeanServerConnection()).isNotNull(); } catch (IOException e) { throw new RuntimeException(e); @@ -121,8 +127,54 @@ void endToEndTest() { assertThat(otlpServer.getMetrics()).isEmpty(); } - private static GenericContainer createScraperContainer() { - return null; + protected GenericContainer createScraperContainer( + String otlpEndpoint, + String targetSystem, + String customYaml, + String targetHost, + int targetPort) { + + String scraperJarPath = System.getProperty("shadow.jar.path"); + assertThat(scraperJarPath).isNotNull(); + + // TODO: adding a way to provide 'host:port' syntax would make this easier for common use + String url = + String.format( + Locale.getDefault(), + "service:jmx:rmi:///jndi/rmi://%s:%d/jmxrmi", + targetHost, + targetPort); + + // for now only configure through JVM args + List arguments = + new ArrayList<>( + Arrays.asList( + "java", + "-Dotel.exporter.otlp.endpoint=" + otlpEndpoint, + "-Dotel.jmx.target.system=" + targetSystem, + "-Dotel.jmx.interval.milliseconds=1000", + "-Dotel.jmx.service.url=" + url, + "-jar", + "/scraper.jar")); + + GenericContainer scraper = + new GenericContainer<>("openjdk:8u272-jre-slim") + .withNetwork(network) + .withCopyFileToContainer(MountableFile.forHostPath(scraperJarPath), "/scraper.jar") + .withLogConsumer(new Slf4jLogConsumer(logger)) + .waitingFor( + Wait.forLogMessage(".*JMX scraping started.*", 1) + .withStartupTimeout(Duration.ofSeconds(10))); + + if (customYaml != null) { + arguments.add("-Dotel.jmx.config=/custom.yaml"); + scraper.withCopyFileToContainer( + MountableFile.forClasspathResource(customYaml), "/custom.yaml"); + } + + scraper.withCommand(arguments.toArray(new String[0])); + + return scraper; } private static class OtlpGrpcServer extends ServerExtension { diff --git a/jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/target_systems/TomcatIntegrationTest.java b/jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/target_systems/TomcatIntegrationTest.java index 69e1bba27..dc1f9c0f9 100644 --- a/jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/target_systems/TomcatIntegrationTest.java +++ b/jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/target_systems/TomcatIntegrationTest.java @@ -12,6 +12,11 @@ public class TomcatIntegrationTest extends TargetSystemIntegrationTest { + @Override + protected String getTargetSystem() { + return "tomcat"; + } + @Override protected GenericContainer createTargetContainer(int jmxPort) { return new GenericContainer<>( 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 835eebd1e..627346a7a 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,29 +5,38 @@ package io.opentelemetry.contrib.jmxscraper; +import io.opentelemetry.api.GlobalOpenTelemetry; +import io.opentelemetry.contrib.jmxscraper.client.JmxRemoteClient; import io.opentelemetry.contrib.jmxscraper.config.ConfigurationException; import io.opentelemetry.contrib.jmxscraper.config.JmxScraperConfig; import io.opentelemetry.contrib.jmxscraper.config.JmxScraperConfigFactory; -import io.opentelemetry.contrib.jmxscraper.jmx.JmxClient; +import io.opentelemetry.instrumentation.jmx.engine.JmxMetricInsight; +import io.opentelemetry.instrumentation.jmx.engine.MetricConfiguration; import java.io.DataInputStream; import java.io.IOException; import java.io.InputStream; -import java.net.MalformedURLException; import java.nio.file.Files; import java.nio.file.Paths; import java.util.Arrays; +import java.util.Collections; import java.util.List; import java.util.Properties; import java.util.concurrent.Executors; import java.util.concurrent.ScheduledExecutorService; import java.util.concurrent.TimeUnit; import java.util.logging.Logger; +import javax.annotation.Nullable; +import javax.management.MBeanServerConnection; +import javax.management.remote.JMXConnector; public class JmxScraper { private static final Logger logger = Logger.getLogger(JmxScraper.class.getName()); private static final int EXECUTOR_TERMINATION_TIMEOUT_MS = 5000; private final ScheduledExecutorService exec = Executors.newSingleThreadScheduledExecutor(); private final JmxScraperConfig config; + private final JmxRemoteClient client; + private final JmxMetricInsight service; + @Nullable private MBeanServerConnection connection; /** * Main method to create and run a {@link JmxScraper} instance. @@ -43,14 +52,7 @@ public static void main(String[] args) { JmxScraper jmxScraper = new JmxScraper(config); jmxScraper.start(); - Runtime.getRuntime() - .addShutdownHook( - new Thread() { - @Override - public void run() { - jmxScraper.shutdown(); - } - }); + Runtime.getRuntime().addShutdownHook(new Thread(jmxScraper::shutdown)); } catch (ArgumentsParsingException e) { System.err.println( "Usage: java -jar " @@ -106,31 +108,36 @@ private static void loadPropertiesFromPath(Properties props, String path) JmxScraper(JmxScraperConfig config) throws ConfigurationException { this.config = config; - try { - @SuppressWarnings("unused") // TODO: Temporary - JmxClient jmxClient = new JmxClient(config); - } catch (MalformedURLException e) { - throw new ConfigurationException("Malformed serviceUrl: ", e); + String serviceUrl = config.getServiceUrl(); + if (serviceUrl == null) { + throw new ConfigurationException("missing service URL"); + } + int interval = config.getIntervalMilliseconds(); + if (interval < 0) { + throw new ConfigurationException("interval must be positive"); } + this.client = JmxRemoteClient.createNew(serviceUrl); + this.service = JmxMetricInsight.createService(GlobalOpenTelemetry.get(), interval); } - @SuppressWarnings("FutureReturnValueIgnored") // TODO: Temporary private void start() { - exec.scheduleWithFixedDelay( - () -> { - logger.fine("JMX scraping triggered"); - // try { - // runner.run(); - // } catch (Throwable e) { - // logger.log(Level.SEVERE, "Error gathering JMX metrics", e); - // } - }, - 0, - config.getIntervalMilliseconds(), - TimeUnit.MILLISECONDS); + try { + JMXConnector connector = client.connect(); + connection = connector.getMBeanServerConnection(); + } catch (IOException e) { + throw new IllegalStateException(e); + } + service.startRemote(getMetricConfig(config), () -> Collections.singletonList(connection)); logger.info("JMX scraping started"); } + @SuppressWarnings("unused") + private static MetricConfiguration getMetricConfig(JmxScraperConfig config) { + MetricConfiguration metricConfig = new MetricConfiguration(); + + return metricConfig; + } + private void shutdown() { logger.info("Shutting down JmxScraper and exporting final metrics."); // Prevent new tasks to be submitted diff --git a/jmx-scraper/src/main/java/io/opentelemetry/contrib/jmxscraper/client/JmxRemoteClient.java b/jmx-scraper/src/main/java/io/opentelemetry/contrib/jmxscraper/client/JmxRemoteClient.java index 449b05ba4..f6fea12af 100644 --- a/jmx-scraper/src/main/java/io/opentelemetry/contrib/jmxscraper/client/JmxRemoteClient.java +++ b/jmx-scraper/src/main/java/io/opentelemetry/contrib/jmxscraper/client/JmxRemoteClient.java @@ -11,10 +11,10 @@ import java.security.Provider; import java.security.Security; import java.util.HashMap; +import java.util.Locale; import java.util.Map; import java.util.logging.Level; import java.util.logging.Logger; -import javax.annotation.Nonnull; import javax.annotation.Nullable; import javax.management.remote.JMXConnector; import javax.management.remote.JMXConnectorFactory; @@ -30,21 +30,23 @@ public class JmxRemoteClient { private static final Logger logger = Logger.getLogger(JmxRemoteClient.class.getName()); - private final String host; - private final int port; + private final JMXServiceURL url; @Nullable private String userName; @Nullable private String password; @Nullable private String profile; @Nullable private String realm; private boolean sslRegistry; - private JmxRemoteClient(@Nonnull String host, int port) { - this.host = host; - this.port = port; + private JmxRemoteClient(JMXServiceURL url) { + this.url = url; } public static JmxRemoteClient createNew(String host, int port) { - return new JmxRemoteClient(host, port); + return new JmxRemoteClient(buildUrl(host, port)); + } + + public static JmxRemoteClient createNew(String url) { + return new JmxRemoteClient(buildUrl(url)); } @CanIgnoreReturnValue @@ -109,7 +111,6 @@ public JMXConnector connect() throws IOException { logger.log(Level.WARNING, "SASL unsupported in current environment: " + e.getMessage(), e); } - JMXServiceURL url = buildUrl(host, port); try { if (sslRegistry) { return doConnectSslRegistry(url, env); @@ -132,14 +133,14 @@ public JMXConnector doConnectSslRegistry(JMXServiceURL url, Map } private static JMXServiceURL buildUrl(String host, int port) { - StringBuilder sb = new StringBuilder("service:jmx:rmi:///jndi/rmi://"); - if (host != null) { - sb.append(host); - } - sb.append(":").append(port).append("/jmxrmi"); + return buildUrl( + String.format( + Locale.getDefault(), "service:jmx:rmi:///jndi/rmi://%s:%d/jmxrmi", host, port)); + } + private static JMXServiceURL buildUrl(String url) { try { - return new JMXServiceURL(sb.toString()); + return new JMXServiceURL(url); } catch (MalformedURLException e) { throw new IllegalArgumentException("invalid url", e); } From 586f7800630994d2305217a7cf4b15a984fd78df Mon Sep 17 00:00:00 2001 From: Sylvain Juge <763082+SylvainJuge@users.noreply.github.com> Date: Wed, 18 Sep 2024 10:35:18 +0200 Subject: [PATCH 03/10] remove stack trace when sasl not supported --- .../contrib/jmxscraper/client/JmxRemoteClient.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/jmx-scraper/src/main/java/io/opentelemetry/contrib/jmxscraper/client/JmxRemoteClient.java b/jmx-scraper/src/main/java/io/opentelemetry/contrib/jmxscraper/client/JmxRemoteClient.java index f6fea12af..9ff594f54 100644 --- a/jmx-scraper/src/main/java/io/opentelemetry/contrib/jmxscraper/client/JmxRemoteClient.java +++ b/jmx-scraper/src/main/java/io/opentelemetry/contrib/jmxscraper/client/JmxRemoteClient.java @@ -108,7 +108,7 @@ public JMXConnector connect() throws IOException { } }); } catch (ReflectiveOperationException e) { - logger.log(Level.WARNING, "SASL unsupported in current environment: " + e.getMessage(), e); + logger.log(Level.WARNING, "SASL unsupported in current environment: " + e.getMessage()); } try { From 966aee6b852f7bef50fb57d54e9db84000651cfc Mon Sep 17 00:00:00 2001 From: Sylvain Juge <763082+SylvainJuge@users.noreply.github.com> Date: Wed, 18 Sep 2024 10:38:21 +0200 Subject: [PATCH 04/10] enhance comments & test cmd log msg --- .../jmxscraper/target_systems/TargetSystemIntegrationTest.java | 2 +- .../contrib/jmxscraper/client/JmxRemoteClient.java | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/target_systems/TargetSystemIntegrationTest.java b/jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/target_systems/TargetSystemIntegrationTest.java index 119acb103..895775905 100644 --- a/jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/target_systems/TargetSystemIntegrationTest.java +++ b/jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/target_systems/TargetSystemIntegrationTest.java @@ -112,7 +112,7 @@ void endToEndTest() { scraper = createScraperContainer(otlpEndpoint, getTargetSystem(), null, "target_system", JMX_PORT); logger.info( - "starting scraper with JVM arguments : {}", String.join(" ", scraper.getCommandParts())); + "starting scraper with command: {}", String.join(" ", scraper.getCommandParts())); scraper.start(); diff --git a/jmx-scraper/src/main/java/io/opentelemetry/contrib/jmxscraper/client/JmxRemoteClient.java b/jmx-scraper/src/main/java/io/opentelemetry/contrib/jmxscraper/client/JmxRemoteClient.java index 9ff594f54..483f63b9d 100644 --- a/jmx-scraper/src/main/java/io/opentelemetry/contrib/jmxscraper/client/JmxRemoteClient.java +++ b/jmx-scraper/src/main/java/io/opentelemetry/contrib/jmxscraper/client/JmxRemoteClient.java @@ -86,6 +86,7 @@ public JMXConnector connect() throws IOException { try { // Not all supported versions of Java contain this Provider + // Also it might not be accessible due to java.security.sasl module not accessible Class klass = Class.forName("com.sun.security.sasl.Provider"); Provider provider = (Provider) klass.getDeclaredConstructor().newInstance(); Security.addProvider(provider); From 91976c06da0b4a10452760284ee5a3a1ecdb805d Mon Sep 17 00:00:00 2001 From: Sylvain Juge <763082+SylvainJuge@users.noreply.github.com> Date: Wed, 18 Sep 2024 13:49:45 +0200 Subject: [PATCH 05/10] refactor to use test containers --- .../jmxscraper/JmxScraperContainer.java | 108 ++++++++++++ .../contrib/jmxscraper/TestAppContainer.java | 139 +++++++++++++++ .../client/JmxRemoteClientTest.java | 166 ++---------------- .../target_systems/JvmIntegrationTest.java | 22 ++- .../TargetSystemIntegrationTest.java | 66 +------ .../target_systems/TomcatIntegrationTest.java | 11 +- 6 files changed, 291 insertions(+), 221 deletions(-) create mode 100644 jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/JmxScraperContainer.java create mode 100644 jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/TestAppContainer.java diff --git a/jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/JmxScraperContainer.java b/jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/JmxScraperContainer.java new file mode 100644 index 000000000..0be5ba9ca --- /dev/null +++ b/jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/JmxScraperContainer.java @@ -0,0 +1,108 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.contrib.jmxscraper; + +import static org.assertj.core.api.Assertions.assertThat; + +import java.time.Duration; +import java.util.ArrayList; +import java.util.HashSet; +import java.util.List; +import java.util.Locale; +import java.util.Set; +import org.testcontainers.containers.GenericContainer; +import org.testcontainers.containers.wait.strategy.Wait; +import org.testcontainers.utility.MountableFile; + +/** Test container that allows to execute {@link JmxScraper} in an isolated container */ +public class JmxScraperContainer extends GenericContainer { + + private final String endpoint; + private final Set targetSystems; + private String serviceUrl; + private int intervalMillis; + private final Set customYaml; + + public JmxScraperContainer(String otlpEndpoint) { + super("openjdk:8u272-jre-slim"); + + String scraperJarPath = System.getProperty("shadow.jar.path"); + assertThat(scraperJarPath).isNotNull(); + + this.withCopyFileToContainer(MountableFile.forHostPath(scraperJarPath), "/scraper.jar") + .waitingFor( + Wait.forLogMessage(".*JMX scraping started.*", 1) + .withStartupTimeout(Duration.ofSeconds(10))); + + this.endpoint = otlpEndpoint; + this.targetSystems = new HashSet<>(); + this.customYaml = new HashSet<>(); + this.intervalMillis = 1000; + } + + public JmxScraperContainer withTargetSystem(String targetSystem) { + targetSystems.add(targetSystem); + return this; + } + + public JmxScraperContainer withIntervalMillis(int intervalMillis) { + this.intervalMillis = intervalMillis; + return this; + } + + public JmxScraperContainer withService(String host, int port) { + // TODO: adding a way to provide 'host:port' syntax would make this easier for end users + this.serviceUrl = + String.format( + Locale.getDefault(), "service:jmx:rmi:///jndi/rmi://%s:%d/jmxrmi", host, port); + return this; + } + + public JmxScraperContainer withCustomYaml(String yamlPath) { + this.customYaml.add(yamlPath); + return this; + } + + @Override + public void start() { + // for now only configure through JVM args + List arguments = new ArrayList<>(); + arguments.add("java"); + arguments.add("-Dotel.exporter.otlp.endpoint=" + endpoint); + + if (!targetSystems.isEmpty()) { + arguments.add("-Dotel.jmx.target.system=" + String.join(",", targetSystems)); + } + + if (serviceUrl == null) { + throw new IllegalStateException("Missing service URL"); + } + arguments.add("-Dotel.jmx.service.url=" + serviceUrl); + arguments.add("-Dotel.jmx.interval.milliseconds=" + intervalMillis); + + if (!customYaml.isEmpty()) { + int i = 0; + StringBuilder sb = new StringBuilder("-Dotel.jmx.config="); + for (String yaml : customYaml) { + String containerPath = "/custom_" + i + ".yaml"; + this.withCopyFileToContainer(MountableFile.forClasspathResource(yaml), containerPath); + if (i > 0) { + sb.append(","); + } + sb.append(containerPath); + i++; + } + arguments.add(sb.toString()); + } + + arguments.add("-jar"); + arguments.add("/scraper.jar"); + + this.withCommand(arguments.toArray(new String[0])); + + super.start(); + } +} diff --git a/jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/TestAppContainer.java b/jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/TestAppContainer.java new file mode 100644 index 000000000..a5dff3006 --- /dev/null +++ b/jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/TestAppContainer.java @@ -0,0 +1,139 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.contrib.jmxscraper; + +import static org.assertj.core.api.Assertions.assertThat; + +import java.io.IOException; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.Paths; +import java.time.Duration; +import java.util.HashMap; +import java.util.Map; +import java.util.stream.Collectors; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import org.testcontainers.containers.GenericContainer; +import org.testcontainers.containers.wait.strategy.Wait; +import org.testcontainers.shaded.com.google.errorprone.annotations.CanIgnoreReturnValue; +import org.testcontainers.utility.MountableFile; + +/** Test container that allows to execute {@link TestApp} in an isolated container */ +public class TestAppContainer extends GenericContainer { + + private static final Logger logger = LoggerFactory.getLogger(TestAppContainer.class); + + private final Map properties; + private int port; + private String login; + private String pwd; + + public TestAppContainer() { + super("openjdk:8u272-jre-slim"); + + this.properties = new HashMap<>(); + + String appJar = System.getProperty("app.jar.path"); + assertThat(Paths.get(appJar)).isNotEmptyFile().isReadable(); + + this.withCopyFileToContainer(MountableFile.forHostPath(appJar), "/app.jar") + .waitingFor( + Wait.forLogMessage(TestApp.APP_STARTED_MSG + "\\n", 1) + .withStartupTimeout(Duration.ofSeconds(5))) + .withCommand("java", "-jar", "/app.jar"); + } + + @CanIgnoreReturnValue + public TestAppContainer withJmxPort(int port) { + this.port = port; + properties.put("com.sun.management.jmxremote.port", Integer.toString(port)); + return this.withExposedPorts(port); + } + + @CanIgnoreReturnValue + public TestAppContainer withUserAuth(String login, String pwd) { + this.login = login; + this.pwd = pwd; + return this; + } + + public int getPort() { + return getMappedPort(port); + } + + @Override + protected void doStart() { + super.doStart(); + } + + @Override + public void start() { + + // TODO: add support for ssl + properties.put("com.sun.management.jmxremote.ssl", "false"); + + if (pwd == null) { + properties.put("com.sun.management.jmxremote.authenticate", "false"); + } else { + properties.put("com.sun.management.jmxremote.authenticate", "true"); + + Path pwdFile = createPwdFile(login, pwd); + this.withCopyFileToContainer(MountableFile.forHostPath(pwdFile), "/jmx.password"); + properties.put("com.sun.management.jmxremote.password.file", "/jmx.password"); + + Path accessFile = createAccessFile(login); + this.withCopyFileToContainer(MountableFile.forHostPath(accessFile), "/jmx.access"); + properties.put("com.sun.management.jmxremote.access.file", "/jmx.access"); + } + + String confArgs = + properties.entrySet().stream() + .map( + e -> { + String s = "-D" + e.getKey(); + if (!e.getValue().isEmpty()) { + s += "=" + e.getValue(); + } + return s; + }) + .collect(Collectors.joining(" ")); + + this.withEnv("JAVA_TOOL_OPTIONS", confArgs); + + logger.info("Test application JAVA_TOOL_OPTIONS = " + confArgs); + + super.start(); + + logger.info("Test application JMX port mapped to {}:{}", getHost(), getMappedPort(port)); + } + + private static Path createPwdFile(String login, String pwd) { + try { + Path path = Files.createTempFile("test", ".pwd"); + writeLine(path, String.format("%s %s", login, pwd)); + return path; + } catch (IOException e) { + throw new RuntimeException(e); + } + } + + private static Path createAccessFile(String login) { + try { + Path path = Files.createTempFile("test", ".pwd"); + writeLine(path, String.format("%s %s", login, "readwrite")); + return path; + } catch (IOException e) { + throw new RuntimeException(e); + } + } + + private static void writeLine(Path path, String line) throws IOException { + line = line + "\n"; + Files.write(path, line.getBytes(StandardCharsets.UTF_8)); + } +} diff --git a/jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/client/JmxRemoteClientTest.java b/jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/client/JmxRemoteClientTest.java index 291fc9ac5..ec7b9ba61 100644 --- a/jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/client/JmxRemoteClientTest.java +++ b/jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/client/JmxRemoteClientTest.java @@ -8,61 +8,35 @@ import static org.assertj.core.api.Assertions.assertThat; import io.opentelemetry.contrib.jmxscraper.TestApp; -import java.io.Closeable; +import io.opentelemetry.contrib.jmxscraper.TestAppContainer; import java.io.IOException; -import java.nio.charset.StandardCharsets; -import java.nio.file.Files; -import java.nio.file.Path; -import java.nio.file.Paths; -import java.time.Duration; -import java.util.ArrayList; -import java.util.HashMap; -import java.util.List; -import java.util.Map; -import java.util.stream.Collectors; import javax.management.ObjectName; import javax.management.remote.JMXConnector; import org.junit.jupiter.api.AfterAll; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Test; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; -import org.testcontainers.containers.GenericContainer; import org.testcontainers.containers.Network; -import org.testcontainers.containers.output.Slf4jLogConsumer; -import org.testcontainers.containers.wait.strategy.Wait; -import org.testcontainers.shaded.com.google.errorprone.annotations.CanIgnoreReturnValue; -import org.testcontainers.utility.MountableFile; public class JmxRemoteClientTest { - private static final Logger logger = LoggerFactory.getLogger(JmxRemoteClientTest.class); - private static Network network; - private static final List toClose = new ArrayList<>(); - @BeforeAll static void beforeAll() { network = Network.newNetwork(); - toClose.add(network); } @AfterAll static void afterAll() { - for (AutoCloseable item : toClose) { - try { - item.close(); - } catch (Exception e) { - logger.warn("Error closing " + item, e); - } - } + network.close(); } @Test void noAuth() { - try (AppContainer app = new AppContainer().withJmxPort(9990).start()) { - testConnector(() -> JmxRemoteClient.createNew(app.getHost(), app.getPort()).connect()); + try (TestAppContainer app = new TestAppContainer().withNetwork(network).withJmxPort(9990)) { + app.start(); + testConnector( + () -> JmxRemoteClient.createNew(app.getHost(), app.getMappedPort(9990)).connect()); } } @@ -70,10 +44,12 @@ void noAuth() { void loginPwdAuth() { String login = "user"; String pwd = "t0p!Secret"; - try (AppContainer app = new AppContainer().withJmxPort(9999).withUserAuth(login, pwd).start()) { + try (TestAppContainer app = + new TestAppContainer().withNetwork(network).withJmxPort(9999).withUserAuth(login, pwd)) { + app.start(); testConnector( () -> - JmxRemoteClient.createNew(app.getHost(), app.getPort()) + JmxRemoteClient.createNew(app.getHost(), app.getMappedPort(9999)) .userCredentials(login, pwd) .connect()); } @@ -115,126 +91,4 @@ private static void testConnector(ConnectorSupplier connectorSupplier) { private interface ConnectorSupplier { JMXConnector get() throws IOException; } - - private static class AppContainer implements Closeable { - - private final GenericContainer appContainer; - private final Map properties; - private int port; - private String login; - private String pwd; - - private AppContainer() { - this.properties = new HashMap<>(); - - properties.put("com.sun.management.jmxremote.ssl", "false"); // TODO : - - // SSL registry : com.sun.management.jmxremote.registry.ssl - // client side ssl auth: com.sun.management.jmxremote.ssl.need.client.auth - - String appJar = System.getProperty("app.jar.path"); - assertThat(Paths.get(appJar)).isNotEmptyFile().isReadable(); - - this.appContainer = - new GenericContainer<>("openjdk:8u272-jre-slim") - .withCopyFileToContainer(MountableFile.forHostPath(appJar), "/app.jar") - .withLogConsumer(new Slf4jLogConsumer(logger)) - .withNetwork(network) - .waitingFor( - Wait.forLogMessage(TestApp.APP_STARTED_MSG + "\\n", 1) - .withStartupTimeout(Duration.ofSeconds(5))) - .withCommand("java", "-jar", "/app.jar"); - } - - @CanIgnoreReturnValue - public AppContainer withJmxPort(int port) { - this.port = port; - properties.put("com.sun.management.jmxremote.port", Integer.toString(port)); - appContainer.withExposedPorts(port); - return this; - } - - @CanIgnoreReturnValue - public AppContainer withUserAuth(String login, String pwd) { - this.login = login; - this.pwd = pwd; - return this; - } - - @CanIgnoreReturnValue - AppContainer start() { - if (pwd == null) { - properties.put("com.sun.management.jmxremote.authenticate", "false"); - } else { - properties.put("com.sun.management.jmxremote.authenticate", "true"); - - Path pwdFile = createPwdFile(login, pwd); - appContainer.withCopyFileToContainer(MountableFile.forHostPath(pwdFile), "/jmx.password"); - properties.put("com.sun.management.jmxremote.password.file", "/jmx.password"); - - Path accessFile = createAccessFile(login); - appContainer.withCopyFileToContainer(MountableFile.forHostPath(accessFile), "/jmx.access"); - properties.put("com.sun.management.jmxremote.access.file", "/jmx.access"); - } - - String confArgs = - properties.entrySet().stream() - .map( - e -> { - String s = "-D" + e.getKey(); - if (!e.getValue().isEmpty()) { - s += "=" + e.getValue(); - } - return s; - }) - .collect(Collectors.joining(" ")); - - appContainer.withEnv("JAVA_TOOL_OPTIONS", confArgs).start(); - - logger.info("Test application JMX port mapped to {}:{}", getHost(), getPort()); - - toClose.add(this); - return this; - } - - int getPort() { - return appContainer.getMappedPort(port); - } - - String getHost() { - return appContainer.getHost(); - } - - @Override - public void close() { - if (appContainer.isRunning()) { - appContainer.stop(); - } - } - - private static Path createPwdFile(String login, String pwd) { - try { - Path path = Files.createTempFile("test", ".pwd"); - writeLine(path, String.format("%s %s", login, pwd)); - return path; - } catch (IOException e) { - throw new RuntimeException(e); - } - } - - private static Path createAccessFile(String login) { - try { - Path path = Files.createTempFile("test", ".pwd"); - writeLine(path, String.format("%s %s", login, "readwrite")); - return path; - } catch (IOException e) { - throw new RuntimeException(e); - } - } - - private static void writeLine(Path path, String line) throws IOException { - line = line + "\n"; - Files.write(path, line.getBytes(StandardCharsets.UTF_8)); - } - } } diff --git a/jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/target_systems/JvmIntegrationTest.java b/jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/target_systems/JvmIntegrationTest.java index 3d5285aa8..d1972371f 100644 --- a/jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/target_systems/JvmIntegrationTest.java +++ b/jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/target_systems/JvmIntegrationTest.java @@ -1,4 +1,24 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + package io.opentelemetry.contrib.jmxscraper.target_systems; -public class JvmIntegrationTest { +import io.opentelemetry.contrib.jmxscraper.JmxScraperContainer; +import io.opentelemetry.contrib.jmxscraper.TestAppContainer; +import org.testcontainers.containers.GenericContainer; + +public class JvmIntegrationTest extends TargetSystemIntegrationTest { + + @Override + protected GenericContainer createTargetContainer(int jmxPort) { + // reusing test application for JVM metrics and custom yaml + return new TestAppContainer().withJmxPort(jmxPort); + } + + @Override + protected JmxScraperContainer customizeScraperContainer(JmxScraperContainer scraper) { + return scraper.withTargetSystem("jvm"); + } } diff --git a/jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/target_systems/TargetSystemIntegrationTest.java b/jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/target_systems/TargetSystemIntegrationTest.java index 895775905..a59ade88c 100644 --- a/jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/target_systems/TargetSystemIntegrationTest.java +++ b/jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/target_systems/TargetSystemIntegrationTest.java @@ -11,16 +11,14 @@ import com.linecorp.armeria.server.grpc.GrpcService; import com.linecorp.armeria.testing.junit5.server.ServerExtension; import io.grpc.stub.StreamObserver; +import io.opentelemetry.contrib.jmxscraper.JmxScraperContainer; import io.opentelemetry.contrib.jmxscraper.client.JmxRemoteClient; import io.opentelemetry.proto.collector.metrics.v1.ExportMetricsServiceRequest; import io.opentelemetry.proto.collector.metrics.v1.ExportMetricsServiceResponse; import io.opentelemetry.proto.collector.metrics.v1.MetricsServiceGrpc; import java.io.IOException; -import java.time.Duration; import java.util.ArrayList; -import java.util.Arrays; import java.util.List; -import java.util.Locale; import java.util.concurrent.BlockingQueue; import java.util.concurrent.ExecutionException; import java.util.concurrent.LinkedBlockingDeque; @@ -35,8 +33,6 @@ import org.testcontainers.containers.GenericContainer; import org.testcontainers.containers.Network; import org.testcontainers.containers.output.Slf4jLogConsumer; -import org.testcontainers.containers.wait.strategy.Wait; -import org.testcontainers.utility.MountableFile; public abstract class TargetSystemIntegrationTest { @@ -51,12 +47,10 @@ public abstract class TargetSystemIntegrationTest { */ protected abstract GenericContainer createTargetContainer(int jmxPort); - protected abstract String getTargetSystem(); - private static Network network; private static OtlpGrpcServer otlpServer; private GenericContainer target; - private GenericContainer scraper; + private JmxScraperContainer scraper; private static final String OTLP_HOST = "host.testcontainers.internal"; private static final int JMX_PORT = 9999; @@ -109,10 +103,10 @@ void endToEndTest() { logger.info( "Target system started, JMX port: {} mapped to {}:{}", JMX_PORT, targetHost, targetPort); - scraper = - createScraperContainer(otlpEndpoint, getTargetSystem(), null, "target_system", JMX_PORT); - logger.info( - "starting scraper with command: {}", String.join(" ", scraper.getCommandParts())); + scraper = new JmxScraperContainer(otlpEndpoint).withService("target_system", JMX_PORT); + + scraper = customizeScraperContainer(scraper); + logger.info("starting scraper with command: {}", String.join(" ", scraper.getCommandParts())); scraper.start(); @@ -127,53 +121,7 @@ void endToEndTest() { assertThat(otlpServer.getMetrics()).isEmpty(); } - protected GenericContainer createScraperContainer( - String otlpEndpoint, - String targetSystem, - String customYaml, - String targetHost, - int targetPort) { - - String scraperJarPath = System.getProperty("shadow.jar.path"); - assertThat(scraperJarPath).isNotNull(); - - // TODO: adding a way to provide 'host:port' syntax would make this easier for common use - String url = - String.format( - Locale.getDefault(), - "service:jmx:rmi:///jndi/rmi://%s:%d/jmxrmi", - targetHost, - targetPort); - - // for now only configure through JVM args - List arguments = - new ArrayList<>( - Arrays.asList( - "java", - "-Dotel.exporter.otlp.endpoint=" + otlpEndpoint, - "-Dotel.jmx.target.system=" + targetSystem, - "-Dotel.jmx.interval.milliseconds=1000", - "-Dotel.jmx.service.url=" + url, - "-jar", - "/scraper.jar")); - - GenericContainer scraper = - new GenericContainer<>("openjdk:8u272-jre-slim") - .withNetwork(network) - .withCopyFileToContainer(MountableFile.forHostPath(scraperJarPath), "/scraper.jar") - .withLogConsumer(new Slf4jLogConsumer(logger)) - .waitingFor( - Wait.forLogMessage(".*JMX scraping started.*", 1) - .withStartupTimeout(Duration.ofSeconds(10))); - - if (customYaml != null) { - arguments.add("-Dotel.jmx.config=/custom.yaml"); - scraper.withCopyFileToContainer( - MountableFile.forClasspathResource(customYaml), "/custom.yaml"); - } - - scraper.withCommand(arguments.toArray(new String[0])); - + protected JmxScraperContainer customizeScraperContainer(JmxScraperContainer scraper) { return scraper; } diff --git a/jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/target_systems/TomcatIntegrationTest.java b/jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/target_systems/TomcatIntegrationTest.java index dc1f9c0f9..f6b9870c5 100644 --- a/jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/target_systems/TomcatIntegrationTest.java +++ b/jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/target_systems/TomcatIntegrationTest.java @@ -5,6 +5,7 @@ package io.opentelemetry.contrib.jmxscraper.target_systems; +import io.opentelemetry.contrib.jmxscraper.JmxScraperContainer; import java.time.Duration; import org.testcontainers.containers.GenericContainer; import org.testcontainers.containers.wait.strategy.Wait; @@ -12,11 +13,6 @@ public class TomcatIntegrationTest extends TargetSystemIntegrationTest { - @Override - protected String getTargetSystem() { - return "tomcat"; - } - @Override protected GenericContainer createTargetContainer(int jmxPort) { return new GenericContainer<>( @@ -43,4 +39,9 @@ protected GenericContainer createTargetContainer(int jmxPort) { .withStartupTimeout(Duration.ofMinutes(2)) .waitingFor(Wait.forListeningPort()); } + + @Override + protected JmxScraperContainer customizeScraperContainer(JmxScraperContainer scraper) { + return scraper.withTargetSystem("tomcat"); + } } From d28fd4257c22d101dc00eec2fabd5e88ab4292ff Mon Sep 17 00:00:00 2001 From: Sylvain Juge <763082+SylvainJuge@users.noreply.github.com> Date: Wed, 18 Sep 2024 14:49:21 +0200 Subject: [PATCH 06/10] make it mergeable without snapshot --- dependencyManagement/build.gradle.kts | 2 +- .../jmxscraper/JmxScraperContainer.java | 5 ++ .../contrib/jmxscraper/TestAppContainer.java | 4 -- .../contrib/jmxscraper/JmxScraper.java | 58 +++++-------------- 4 files changed, 19 insertions(+), 50 deletions(-) diff --git a/dependencyManagement/build.gradle.kts b/dependencyManagement/build.gradle.kts index 9baba0736..c8e9fc865 100644 --- a/dependencyManagement/build.gradle.kts +++ b/dependencyManagement/build.gradle.kts @@ -7,7 +7,7 @@ data class DependencySet(val group: String, val version: String, val modules: Li val dependencyVersions = hashMapOf() rootProject.extra["versions"] = dependencyVersions -val otelInstrumentationVersion = "2.9.0-alpha-SNAPSHOT" +val otelInstrumentationVersion = "2.8.0-alpha" val DEPENDENCY_BOMS = listOf( "com.fasterxml.jackson:jackson-bom:2.17.2", diff --git a/jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/JmxScraperContainer.java b/jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/JmxScraperContainer.java index 0be5ba9ca..f1fc04f39 100644 --- a/jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/JmxScraperContainer.java +++ b/jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/JmxScraperContainer.java @@ -13,6 +13,7 @@ import java.util.List; import java.util.Locale; import java.util.Set; +import com.google.errorprone.annotations.CanIgnoreReturnValue; import org.testcontainers.containers.GenericContainer; import org.testcontainers.containers.wait.strategy.Wait; import org.testcontainers.utility.MountableFile; @@ -43,16 +44,19 @@ public JmxScraperContainer(String otlpEndpoint) { this.intervalMillis = 1000; } + @CanIgnoreReturnValue public JmxScraperContainer withTargetSystem(String targetSystem) { targetSystems.add(targetSystem); return this; } + @CanIgnoreReturnValue public JmxScraperContainer withIntervalMillis(int intervalMillis) { this.intervalMillis = intervalMillis; return this; } + @CanIgnoreReturnValue public JmxScraperContainer withService(String host, int port) { // TODO: adding a way to provide 'host:port' syntax would make this easier for end users this.serviceUrl = @@ -61,6 +65,7 @@ public JmxScraperContainer withService(String host, int port) { return this; } + @CanIgnoreReturnValue public JmxScraperContainer withCustomYaml(String yamlPath) { this.customYaml.add(yamlPath); return this; diff --git a/jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/TestAppContainer.java b/jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/TestAppContainer.java index a5dff3006..8a3d8d9b0 100644 --- a/jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/TestAppContainer.java +++ b/jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/TestAppContainer.java @@ -62,10 +62,6 @@ public TestAppContainer withUserAuth(String login, String pwd) { return this; } - public int getPort() { - return getMappedPort(port); - } - @Override protected void doStart() { super.doStart(); 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 627346a7a..22722b111 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,38 +5,29 @@ package io.opentelemetry.contrib.jmxscraper; -import io.opentelemetry.api.GlobalOpenTelemetry; import io.opentelemetry.contrib.jmxscraper.client.JmxRemoteClient; import io.opentelemetry.contrib.jmxscraper.config.ConfigurationException; import io.opentelemetry.contrib.jmxscraper.config.JmxScraperConfig; import io.opentelemetry.contrib.jmxscraper.config.JmxScraperConfigFactory; -import io.opentelemetry.instrumentation.jmx.engine.JmxMetricInsight; -import io.opentelemetry.instrumentation.jmx.engine.MetricConfiguration; import java.io.DataInputStream; import java.io.IOException; import java.io.InputStream; import java.nio.file.Files; import java.nio.file.Paths; import java.util.Arrays; -import java.util.Collections; import java.util.List; import java.util.Properties; -import java.util.concurrent.Executors; -import java.util.concurrent.ScheduledExecutorService; -import java.util.concurrent.TimeUnit; import java.util.logging.Logger; -import javax.annotation.Nullable; import javax.management.MBeanServerConnection; import javax.management.remote.JMXConnector; public class JmxScraper { private static final Logger logger = Logger.getLogger(JmxScraper.class.getName()); - private static final int EXECUTOR_TERMINATION_TIMEOUT_MS = 5000; - private final ScheduledExecutorService exec = Executors.newSingleThreadScheduledExecutor(); - private final JmxScraperConfig config; + private final JmxRemoteClient client; - private final JmxMetricInsight service; - @Nullable private MBeanServerConnection connection; + + // TODO depend on instrumentation 2.9.0 snapshot + // private final JmxMetricInsight service; /** * Main method to create and run a {@link JmxScraper} instance. @@ -52,7 +43,6 @@ public static void main(String[] args) { JmxScraper jmxScraper = new JmxScraper(config); jmxScraper.start(); - Runtime.getRuntime().addShutdownHook(new Thread(jmxScraper::shutdown)); } catch (ArgumentsParsingException e) { System.err.println( "Usage: java -jar " @@ -106,7 +96,6 @@ private static void loadPropertiesFromPath(Properties props, String path) } JmxScraper(JmxScraperConfig config) throws ConfigurationException { - this.config = config; String serviceUrl = config.getServiceUrl(); if (serviceUrl == null) { @@ -117,46 +106,25 @@ private static void loadPropertiesFromPath(Properties props, String path) throw new ConfigurationException("interval must be positive"); } this.client = JmxRemoteClient.createNew(serviceUrl); - this.service = JmxMetricInsight.createService(GlobalOpenTelemetry.get(), interval); + // TODO: depend on instrumentation 2.9.0 snapshot + // this.service = JmxMetricInsight.createService(GlobalOpenTelemetry.get(), interval); } private void start() { + @SuppressWarnings("unused") + MBeanServerConnection connection; try { JMXConnector connector = client.connect(); connection = connector.getMBeanServerConnection(); } catch (IOException e) { throw new IllegalStateException(e); } - service.startRemote(getMetricConfig(config), () -> Collections.singletonList(connection)); - logger.info("JMX scraping started"); - } - - @SuppressWarnings("unused") - private static MetricConfiguration getMetricConfig(JmxScraperConfig config) { - MetricConfiguration metricConfig = new MetricConfiguration(); - return metricConfig; - } + // TODO: depend on instrumentation 2.9.0 snapshot + // MetricConfiguration metricConfig = new MetricConfiguration(); + // TODO create JMX insight config from scraper config + // service.startRemote(metricConfig, () -> Collections.singletonList(connection)); - private void shutdown() { - logger.info("Shutting down JmxScraper and exporting final metrics."); - // Prevent new tasks to be submitted - exec.shutdown(); - try { - // Wait a while for existing tasks to terminate - if (!exec.awaitTermination(EXECUTOR_TERMINATION_TIMEOUT_MS, TimeUnit.MILLISECONDS)) { - // Cancel currently executing tasks - exec.shutdownNow(); - // Wait a while for tasks to respond to being cancelled - if (!exec.awaitTermination(EXECUTOR_TERMINATION_TIMEOUT_MS, TimeUnit.MILLISECONDS)) { - logger.warning("Thread pool did not terminate in time: " + exec); - } - } - } catch (InterruptedException e) { - // (Re-)Cancel if current thread also interrupted - exec.shutdownNow(); - // Preserve interrupt status - Thread.currentThread().interrupt(); - } + logger.info("JMX scraping started"); } } From 9072de06f593e232cb653526c990f46aeb609a27 Mon Sep 17 00:00:00 2001 From: Sylvain Juge <763082+SylvainJuge@users.noreply.github.com> Date: Wed, 18 Sep 2024 17:03:34 +0200 Subject: [PATCH 07/10] tidy a few things --- jmx-scraper/build.gradle.kts | 1 - .../jmxscraper/JmxScraperContainer.java | 4 +++- .../contrib/jmxscraper/TestAppContainer.java | 8 ++------ .../TargetSystemIntegrationTest.java | 19 +++++++++++-------- .../contrib/jmxscraper/JmxScraper.java | 7 +++++++ 5 files changed, 23 insertions(+), 16 deletions(-) diff --git a/jmx-scraper/build.gradle.kts b/jmx-scraper/build.gradle.kts index 498ffc0b9..1ffe28de6 100644 --- a/jmx-scraper/build.gradle.kts +++ b/jmx-scraper/build.gradle.kts @@ -29,7 +29,6 @@ testing { dependencies { implementation("org.testcontainers:junit-jupiter") implementation("org.slf4j:slf4j-simple") - implementation("com.linecorp.armeria:armeria-grpc") implementation("com.linecorp.armeria:armeria-junit5") implementation("com.linecorp.armeria:armeria-grpc") implementation("io.opentelemetry.proto:opentelemetry-proto:0.20.0-alpha") diff --git a/jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/JmxScraperContainer.java b/jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/JmxScraperContainer.java index f1fc04f39..fbd64c8ce 100644 --- a/jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/JmxScraperContainer.java +++ b/jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/JmxScraperContainer.java @@ -7,13 +7,13 @@ import static org.assertj.core.api.Assertions.assertThat; +import com.google.errorprone.annotations.CanIgnoreReturnValue; import java.time.Duration; import java.util.ArrayList; import java.util.HashSet; import java.util.List; import java.util.Locale; import java.util.Set; -import com.google.errorprone.annotations.CanIgnoreReturnValue; import org.testcontainers.containers.GenericContainer; import org.testcontainers.containers.wait.strategy.Wait; import org.testcontainers.utility.MountableFile; @@ -108,6 +108,8 @@ public void start() { this.withCommand(arguments.toArray(new String[0])); + logger().info("Starting scraper with command: " + String.join(" ", arguments)); + super.start(); } } diff --git a/jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/TestAppContainer.java b/jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/TestAppContainer.java index 8a3d8d9b0..243aa01bb 100644 --- a/jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/TestAppContainer.java +++ b/jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/TestAppContainer.java @@ -16,8 +16,6 @@ import java.util.HashMap; import java.util.Map; import java.util.stream.Collectors; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; import org.testcontainers.containers.GenericContainer; import org.testcontainers.containers.wait.strategy.Wait; import org.testcontainers.shaded.com.google.errorprone.annotations.CanIgnoreReturnValue; @@ -26,8 +24,6 @@ /** Test container that allows to execute {@link TestApp} in an isolated container */ public class TestAppContainer extends GenericContainer { - private static final Logger logger = LoggerFactory.getLogger(TestAppContainer.class); - private final Map properties; private int port; private String login; @@ -101,11 +97,11 @@ public void start() { this.withEnv("JAVA_TOOL_OPTIONS", confArgs); - logger.info("Test application JAVA_TOOL_OPTIONS = " + confArgs); + logger().info("Test application JAVA_TOOL_OPTIONS = " + confArgs); super.start(); - logger.info("Test application JMX port mapped to {}:{}", getHost(), getMappedPort(port)); + logger().info("Test application JMX port mapped to {}:{}", getHost(), getMappedPort(port)); } private static Path createPwdFile(String login, String pwd) { diff --git a/jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/target_systems/TargetSystemIntegrationTest.java b/jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/target_systems/TargetSystemIntegrationTest.java index a59ade88c..89f90ed57 100644 --- a/jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/target_systems/TargetSystemIntegrationTest.java +++ b/jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/target_systems/TargetSystemIntegrationTest.java @@ -37,6 +37,7 @@ public abstract class TargetSystemIntegrationTest { private static final Logger logger = LoggerFactory.getLogger(TargetSystemIntegrationTest.class); + public static final String TARGET_SYSTEM_NETWORK_ALIAS = "targetsystem"; private static String otlpEndpoint; /** @@ -95,7 +96,7 @@ void endToEndTest() { .withLogConsumer(new Slf4jLogConsumer(logger)) .withNetwork(network) .withExposedPorts(JMX_PORT) - .withNetworkAliases("target_system"); + .withNetworkAliases(TARGET_SYSTEM_NETWORK_ALIAS); target.start(); String targetHost = target.getHost(); @@ -103,13 +104,6 @@ void endToEndTest() { logger.info( "Target system started, JMX port: {} mapped to {}:{}", JMX_PORT, targetHost, targetPort); - scraper = new JmxScraperContainer(otlpEndpoint).withService("target_system", JMX_PORT); - - scraper = customizeScraperContainer(scraper); - logger.info("starting scraper with command: {}", String.join(" ", scraper.getCommandParts())); - - scraper.start(); - // TODO : wait for metrics to be sent and add assertions on what is being captured // for now we just test that we can connect to remote JMX using our client. try (JMXConnector connector = JmxRemoteClient.createNew(targetHost, targetPort).connect()) { @@ -117,6 +111,15 @@ void endToEndTest() { } catch (IOException e) { throw new RuntimeException(e); } + + scraper = + new JmxScraperContainer(otlpEndpoint) + .withNetwork(network) + .withService(TARGET_SYSTEM_NETWORK_ALIAS, JMX_PORT); + + scraper = customizeScraperContainer(scraper); + scraper.start(); + // TODO: replace with real assertions assertThat(otlpServer.getMetrics()).isEmpty(); } 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 22722b111..78588c1b5 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 @@ -126,5 +126,12 @@ private void start() { // service.startRemote(metricConfig, () -> Collections.singletonList(connection)); logger.info("JMX scraping started"); + + // TODO: wait a bit to keep the JVM running, this won't be needed once calling jmx insight + try { + Thread.sleep(5000); + } catch (InterruptedException e) { + throw new IllegalStateException(e); + } } } From c0ac43fed8036d1701d8f3504fb119d399d08103 Mon Sep 17 00:00:00 2001 From: Sylvain Juge <763082+SylvainJuge@users.noreply.github.com> Date: Wed, 18 Sep 2024 17:04:21 +0200 Subject: [PATCH 08/10] remove JmxClient --- .../TargetSystemIntegrationTest.java | 2 +- .../jmxscraper/jmx/ClientCallbackHandler.java | 49 -------- .../contrib/jmxscraper/jmx/JmxClient.java | 109 ------------------ 3 files changed, 1 insertion(+), 159 deletions(-) delete mode 100644 jmx-scraper/src/main/java/io/opentelemetry/contrib/jmxscraper/jmx/ClientCallbackHandler.java delete mode 100644 jmx-scraper/src/main/java/io/opentelemetry/contrib/jmxscraper/jmx/JmxClient.java diff --git a/jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/target_systems/TargetSystemIntegrationTest.java b/jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/target_systems/TargetSystemIntegrationTest.java index 89f90ed57..71754ea6f 100644 --- a/jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/target_systems/TargetSystemIntegrationTest.java +++ b/jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/target_systems/TargetSystemIntegrationTest.java @@ -37,7 +37,7 @@ public abstract class TargetSystemIntegrationTest { private static final Logger logger = LoggerFactory.getLogger(TargetSystemIntegrationTest.class); - public static final String TARGET_SYSTEM_NETWORK_ALIAS = "targetsystem"; + private static final String TARGET_SYSTEM_NETWORK_ALIAS = "targetsystem"; private static String otlpEndpoint; /** diff --git a/jmx-scraper/src/main/java/io/opentelemetry/contrib/jmxscraper/jmx/ClientCallbackHandler.java b/jmx-scraper/src/main/java/io/opentelemetry/contrib/jmxscraper/jmx/ClientCallbackHandler.java deleted file mode 100644 index 2dfa01115..000000000 --- a/jmx-scraper/src/main/java/io/opentelemetry/contrib/jmxscraper/jmx/ClientCallbackHandler.java +++ /dev/null @@ -1,49 +0,0 @@ -/* - * Copyright The OpenTelemetry Authors - * SPDX-License-Identifier: Apache-2.0 - */ - -package io.opentelemetry.contrib.jmxscraper.jmx; - -import javax.annotation.Nullable; -import javax.security.auth.callback.Callback; -import javax.security.auth.callback.CallbackHandler; -import javax.security.auth.callback.NameCallback; -import javax.security.auth.callback.PasswordCallback; -import javax.security.auth.callback.UnsupportedCallbackException; -import javax.security.sasl.RealmCallback; - -public class ClientCallbackHandler implements CallbackHandler { - private final String username; - @Nullable private final char[] password; - private final String realm; - - /** - * Constructor for the {@link ClientCallbackHandler}, a CallbackHandler implementation for - * authenticating with an MBean server. - * - * @param username - authenticating username - * @param password - authenticating password (plaintext) - * @param realm - authenticating realm - */ - public ClientCallbackHandler(String username, String password, String realm) { - this.username = username; - this.password = password != null ? password.toCharArray() : null; - this.realm = realm; - } - - @Override - public void handle(Callback[] callbacks) throws UnsupportedCallbackException { - for (Callback callback : callbacks) { - if (callback instanceof NameCallback) { - ((NameCallback) callback).setName(this.username); - } else if (callback instanceof PasswordCallback) { - ((PasswordCallback) callback).setPassword(this.password); - } else if (callback instanceof RealmCallback) { - ((RealmCallback) callback).setText(this.realm); - } else { - throw new UnsupportedCallbackException(callback); - } - } - } -} diff --git a/jmx-scraper/src/main/java/io/opentelemetry/contrib/jmxscraper/jmx/JmxClient.java b/jmx-scraper/src/main/java/io/opentelemetry/contrib/jmxscraper/jmx/JmxClient.java deleted file mode 100644 index 0c71d9cc9..000000000 --- a/jmx-scraper/src/main/java/io/opentelemetry/contrib/jmxscraper/jmx/JmxClient.java +++ /dev/null @@ -1,109 +0,0 @@ -/* - * Copyright The OpenTelemetry Authors - * SPDX-License-Identifier: Apache-2.0 - */ - -package io.opentelemetry.contrib.jmxscraper.jmx; - -import io.opentelemetry.contrib.jmxscraper.config.JmxScraperConfig; -import io.opentelemetry.contrib.jmxscraper.util.StringUtils; -import java.io.IOException; -import java.net.MalformedURLException; -import java.security.Provider; -import java.security.Security; -import java.util.ArrayList; -import java.util.Collections; -import java.util.HashMap; -import java.util.List; -import java.util.Map; -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; -import javax.management.remote.JMXServiceURL; - -@SuppressWarnings("unused") // TODO: Temporary -public class JmxClient { - private static final Logger logger = Logger.getLogger(JmxClient.class.getName()); - - private final JMXServiceURL url; - private final String username; - private final String password; - private final String realm; - private final String remoteProfile; - private final boolean registrySsl; - @Nullable private JMXConnector jmxConn; - - public JmxClient(JmxScraperConfig config) throws MalformedURLException { - this.url = new JMXServiceURL(config.getServiceUrl()); - this.username = config.getUsername(); - this.password = config.getPassword(); - this.realm = config.getRealm(); - this.remoteProfile = config.getRemoteProfile(); - this.registrySsl = config.isRegistrySsl(); - } - - @Nullable - public MBeanServerConnection getConnection() { - if (jmxConn != null) { - try { - return jmxConn.getMBeanServerConnection(); - } catch (IOException e) { - // Attempt to connect with authentication below. - } - } - try { - @SuppressWarnings("ModifiedButNotUsed") // TODO: Temporary - Map env = new HashMap<>(); - if (!StringUtils.isBlank(username)) { - env.put(JMXConnector.CREDENTIALS, new String[] {this.username, this.password}); - } - try { - // Not all supported versions of Java contain this Provider - Class klass = Class.forName("com.sun.security.sasl.Provider"); - Provider provider = (Provider) klass.getDeclaredConstructor().newInstance(); - Security.addProvider(provider); - - env.put("jmx.remote.profile", this.remoteProfile); - env.put( - "jmx.remote.sasl.callback.handler", - new ClientCallbackHandler(this.username, this.password, this.realm)); - } catch (ReflectiveOperationException e) { - logger.warning("SASL unsupported in current environment: " + e.getMessage()); - } - - // jmxConn = JmxConnectorHelper.connect(url, env, registrySsl); - // return jmxConn.getMBeanServerConnection(); - return jmxConn == null ? null : jmxConn.getMBeanServerConnection(); // Temporary - - } catch (IOException e) { - logger.log(Level.WARNING, "Could not connect to remote JMX server: ", e); - return null; - } - } - - /** - * Query the MBean server for a given ObjectName. - * - * @param objectName ObjectName to query - * @return the sorted list of applicable ObjectName instances found by server - */ - public List query(ObjectName objectName) { - MBeanServerConnection mBeanServerConnection = getConnection(); - if (mBeanServerConnection == null) { - return Collections.emptyList(); - } - - try { - List objectNames = - new ArrayList<>(mBeanServerConnection.queryNames(objectName, null)); - Collections.sort(objectNames); - return Collections.unmodifiableList(objectNames); - } catch (IOException e) { - logger.log(Level.WARNING, "Could not query remote JMX server: ", e); - return Collections.emptyList(); - } - } -} From 14c0c2d6cf902861de68332aa5e51d71f10c25d4 Mon Sep 17 00:00:00 2001 From: Sylvain Juge <763082+SylvainJuge@users.noreply.github.com> Date: Thu, 19 Sep 2024 10:06:55 +0200 Subject: [PATCH 09/10] post-review changes --- .../jmxscraper/JmxScraperContainer.java | 22 ++++--------- .../contrib/jmxscraper/TestAppContainer.java | 5 --- .../contrib/jmxscraper/JmxScraper.java | 32 +++++++++---------- 3 files changed, 23 insertions(+), 36 deletions(-) diff --git a/jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/JmxScraperContainer.java b/jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/JmxScraperContainer.java index fbd64c8ce..f85a5ba17 100644 --- a/jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/JmxScraperContainer.java +++ b/jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/JmxScraperContainer.java @@ -25,7 +25,7 @@ public class JmxScraperContainer extends GenericContainer { private final Set targetSystems; private String serviceUrl; private int intervalMillis; - private final Set customYaml; + private final Set customYamlFiles; public JmxScraperContainer(String otlpEndpoint) { super("openjdk:8u272-jre-slim"); @@ -40,7 +40,7 @@ public JmxScraperContainer(String otlpEndpoint) { this.endpoint = otlpEndpoint; this.targetSystems = new HashSet<>(); - this.customYaml = new HashSet<>(); + this.customYamlFiles = new HashSet<>(); this.intervalMillis = 1000; } @@ -67,7 +67,7 @@ public JmxScraperContainer withService(String host, int port) { @CanIgnoreReturnValue public JmxScraperContainer withCustomYaml(String yamlPath) { - this.customYaml.add(yamlPath); + this.customYamlFiles.add(yamlPath); return this; } @@ -88,19 +88,11 @@ public void start() { arguments.add("-Dotel.jmx.service.url=" + serviceUrl); arguments.add("-Dotel.jmx.interval.milliseconds=" + intervalMillis); - if (!customYaml.isEmpty()) { - int i = 0; - StringBuilder sb = new StringBuilder("-Dotel.jmx.config="); - for (String yaml : customYaml) { - String containerPath = "/custom_" + i + ".yaml"; - this.withCopyFileToContainer(MountableFile.forClasspathResource(yaml), containerPath); - if (i > 0) { - sb.append(","); - } - sb.append(containerPath); - i++; + if (!customYamlFiles.isEmpty()) { + for (String yaml : customYamlFiles) { + this.withCopyFileToContainer(MountableFile.forClasspathResource(yaml), yaml); } - arguments.add(sb.toString()); + arguments.add("-Dotel.jmx.config=" + String.join(",", customYamlFiles)); } arguments.add("-jar"); diff --git a/jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/TestAppContainer.java b/jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/TestAppContainer.java index 243aa01bb..a38dd7ace 100644 --- a/jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/TestAppContainer.java +++ b/jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/TestAppContainer.java @@ -58,11 +58,6 @@ public TestAppContainer withUserAuth(String login, String pwd) { return this; } - @Override - protected void doStart() { - super.doStart(); - } - @Override public void start() { 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 78588c1b5..3d3131a30 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 @@ -36,12 +36,12 @@ public class JmxScraper { */ @SuppressWarnings({"SystemOut", "SystemExitOutsideMain"}) public static void main(String[] args) { + JmxScraperConfig config = null; + JmxScraper jmxScraper = null; try { JmxScraperConfigFactory factory = new JmxScraperConfigFactory(); - JmxScraperConfig config = JmxScraper.createConfigFromArgs(Arrays.asList(args), factory); - - JmxScraper jmxScraper = new JmxScraper(config); - jmxScraper.start(); + config = JmxScraper.createConfigFromArgs(Arrays.asList(args), factory); + jmxScraper = new JmxScraper(config); } catch (ArgumentsParsingException e) { System.err.println( @@ -52,6 +52,13 @@ public static void main(String[] args) { System.err.println(e.getMessage()); System.exit(1); } + + try { + jmxScraper.start(); + } catch (IOException e) { + System.err.println("Unable to connect to " + config.getServiceUrl() + " " + e.getMessage()); + System.exit(2); + } } /** @@ -96,11 +103,7 @@ private static void loadPropertiesFromPath(Properties props, String path) } JmxScraper(JmxScraperConfig config) throws ConfigurationException { - String serviceUrl = config.getServiceUrl(); - if (serviceUrl == null) { - throw new ConfigurationException("missing service URL"); - } int interval = config.getIntervalMilliseconds(); if (interval < 0) { throw new ConfigurationException("interval must be positive"); @@ -110,15 +113,12 @@ private static void loadPropertiesFromPath(Properties props, String path) // this.service = JmxMetricInsight.createService(GlobalOpenTelemetry.get(), interval); } - private void start() { + private void start() throws IOException { + + JMXConnector connector = client.connect(); + @SuppressWarnings("unused") - MBeanServerConnection connection; - try { - JMXConnector connector = client.connect(); - connection = connector.getMBeanServerConnection(); - } catch (IOException e) { - throw new IllegalStateException(e); - } + MBeanServerConnection connection = connector.getMBeanServerConnection(); // TODO: depend on instrumentation 2.9.0 snapshot // MetricConfiguration metricConfig = new MetricConfiguration(); From 1a6533e046cc5e9e9f53a61678d5f482552b2da1 Mon Sep 17 00:00:00 2001 From: Sylvain Juge <763082+SylvainJuge@users.noreply.github.com> Date: Thu, 19 Sep 2024 10:39:43 +0200 Subject: [PATCH 10/10] remove warnings --- .../io/opentelemetry/contrib/jmxscraper/JmxScraper.java | 7 ++++--- 1 file changed, 4 insertions(+), 3 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 3d3131a30..9ab051d44 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 @@ -16,6 +16,7 @@ import java.nio.file.Paths; import java.util.Arrays; import java.util.List; +import java.util.Objects; import java.util.Properties; import java.util.logging.Logger; import javax.management.MBeanServerConnection; @@ -36,7 +37,7 @@ public class JmxScraper { */ @SuppressWarnings({"SystemOut", "SystemExitOutsideMain"}) public static void main(String[] args) { - JmxScraperConfig config = null; + JmxScraperConfig config; JmxScraper jmxScraper = null; try { JmxScraperConfigFactory factory = new JmxScraperConfigFactory(); @@ -54,9 +55,9 @@ public static void main(String[] args) { } try { - jmxScraper.start(); + Objects.requireNonNull(jmxScraper).start(); } catch (IOException e) { - System.err.println("Unable to connect to " + config.getServiceUrl() + " " + e.getMessage()); + System.err.println("Unable to connect " + e.getMessage()); System.exit(2); } }