From ba48c6d0c148c102247b9ecc74fe3acfdacceab8 Mon Sep 17 00:00:00 2001 From: Carlos Roman Date: Thu, 14 Dec 2023 10:31:52 +0000 Subject: [PATCH 01/24] Added ability to change the final Docker image for misbehaving-jmx-server --- pom.xml | 2 +- tools/misbehaving-jmx-server/Dockerfile | 26 +++++++++++++++++-- tools/misbehaving-jmx-server/scripts/start.sh | 13 ++++++++++ 3 files changed, 38 insertions(+), 3 deletions(-) create mode 100755 tools/misbehaving-jmx-server/scripts/start.sh diff --git a/pom.xml b/pom.xml index 18cd989b3..15b614823 100644 --- a/pom.xml +++ b/pom.xml @@ -200,7 +200,7 @@ org.testcontainers testcontainers - 1.18.0 + 1.19.3 test diff --git a/tools/misbehaving-jmx-server/Dockerfile b/tools/misbehaving-jmx-server/Dockerfile index 938e8c112..ae02c6e48 100644 --- a/tools/misbehaving-jmx-server/Dockerfile +++ b/tools/misbehaving-jmx-server/Dockerfile @@ -1,3 +1,6 @@ +# syntax=docker/dockerfile:1 +# Use by default the JDK image used to build the jar +ARG FINAL_JRE_IMAGE=base # Use the official JDK image as the base image FROM eclipse-temurin:17 AS base @@ -9,22 +12,41 @@ WORKDIR /app # Copy the pom.xml and Maven files and install the dependencies COPY .mvn .mvn/ COPY pom.xml mvnw mvnw.cmd ./ + +# Test containers bug doesn't seem to allow mount cache +# RUN --mount=type=cache,id=mavenCache,target=/root/.m2,sharing=locked \ RUN set -eu && \ ./mvnw dependency:resolve; # Copy the source code and build the JAR file COPY src/ src/ + +# Test containers bug doesn't seem to allow mount cache +# RUN --mount=type=cache,id=mavenCache,target=/root/.m2,sharing=locked \ RUN set -eu && \ ./mvnw clean package assembly:single; # Use the base image as the the final image -FROM base AS final +FROM ${FINAL_JRE_IMAGE} AS final # Set the working directory to /app WORKDIR /app +COPY scripts/start.sh /usr/bin/ + # Copy the JAR file from the Maven image to the final image COPY --from=build /app/target/misbehavingjmxserver-1.0-SNAPSHOT-jar-with-dependencies.jar . +# RMI Port +EXPOSE 9090 + +# Control Port +EXPOSE 9091 + +# Supervisor Port +EXPOSE 9092 + # Run the supervisor class from the jar -CMD ["java", "-cp", "misbehavingjmxserver-1.0-SNAPSHOT-jar-with-dependencies.jar", "org.datadog.supervisor.App"] \ No newline at end of file +ENTRYPOINT [ "/usr/bin/start.sh" ] + +CMD [ "org.datadog.supervisor.App" ] diff --git a/tools/misbehaving-jmx-server/scripts/start.sh b/tools/misbehaving-jmx-server/scripts/start.sh new file mode 100755 index 000000000..6fb93ebdb --- /dev/null +++ b/tools/misbehaving-jmx-server/scripts/start.sh @@ -0,0 +1,13 @@ +#!/usr/bin/env sh + +set -f + +echo "Running $@" + +[ -n "$JAVA_OPTS" ] || JAVA_OPTS="-Xmx128M -Xms128M" + +# shellcheck disable=SC2086 +java \ + ${JAVA_OPTS} \ + -cp misbehavingjmxserver-1.0-SNAPSHOT-jar-with-dependencies.jar \ + "$@" From e352022dc4b58d3e07810a033755d800ead33046 Mon Sep 17 00:00:00 2001 From: Carlos Roman Date: Thu, 14 Dec 2023 12:19:32 +0000 Subject: [PATCH 02/24] wip, adding GC tests --- .../org/datadog/jmxfetch/TestGCMetrics.java | 58 ++++++++++++++ .../jmxfetch/util/MisbehavingJMXServer.java | 78 +++++++++++++++++++ 2 files changed, 136 insertions(+) create mode 100644 src/test/java/org/datadog/jmxfetch/TestGCMetrics.java create mode 100644 src/test/java/org/datadog/jmxfetch/util/MisbehavingJMXServer.java diff --git a/src/test/java/org/datadog/jmxfetch/TestGCMetrics.java b/src/test/java/org/datadog/jmxfetch/TestGCMetrics.java new file mode 100644 index 000000000..dd2a0a614 --- /dev/null +++ b/src/test/java/org/datadog/jmxfetch/TestGCMetrics.java @@ -0,0 +1,58 @@ +package org.datadog.jmxfetch; + +import static org.junit.Assert.assertTrue; +import java.io.IOException; +import javax.management.MBeanServerConnection; +import javax.management.remote.JMXConnector; +import javax.management.remote.JMXConnectorFactory; +import javax.management.remote.JMXServiceURL; +import lombok.extern.slf4j.Slf4j; +import org.datadog.jmxfetch.util.MisbehavingJMXServer; +import org.junit.Test; +import org.testcontainers.containers.output.Slf4jLogConsumer; + +@Slf4j +public class TestGCMetrics extends TestCommon { + + private static final int RMI_PORT = 9090; + private static final int CONTROL_PORT = 9091; + private static final int SUPERVISOR_PORT = 9092; + private static final Slf4jLogConsumer logConsumer = new Slf4jLogConsumer(log); + + private static boolean isDomainPresent(final String domain, final MBeanServerConnection mbs) { + boolean found = false; + try { + final String[] domains = mbs.getDomains(); + for (String s : domains) + if(s.equals(domain)) { + found = true; + break; + } + } catch (IOException e) { + log.warn("Got an exception checking if domain is present", e); + } + return found; + } + + /* + Just here to make sure I've not broken anything + */ + @Test + public void testJMXDirectBasic() throws Exception { + try (final MisbehavingJMXServer server = new MisbehavingJMXServer( + RMI_PORT, + CONTROL_PORT, + SUPERVISOR_PORT)) { + server.start(); + final String ipAddress = server.getIp(); + final String remoteJmxServiceUrl = String.format( + "service:jmx:rmi:///jndi/rmi://%s:%s/jmxrmi", + ipAddress, RMI_PORT + ); + final JMXServiceURL jmxUrl = new JMXServiceURL(remoteJmxServiceUrl); + final JMXConnector conn = JMXConnectorFactory.connect(jmxUrl); + final MBeanServerConnection mBeanServerConnection = conn.getMBeanServerConnection(); + assertTrue(isDomainPresent("Bohnanza", mBeanServerConnection)); + } + } +} diff --git a/src/test/java/org/datadog/jmxfetch/util/MisbehavingJMXServer.java b/src/test/java/org/datadog/jmxfetch/util/MisbehavingJMXServer.java new file mode 100644 index 000000000..aa9b8f160 --- /dev/null +++ b/src/test/java/org/datadog/jmxfetch/util/MisbehavingJMXServer.java @@ -0,0 +1,78 @@ +package org.datadog.jmxfetch.util; + +import java.io.IOException; +import java.nio.file.Paths; +import java.util.Collections; +import lombok.extern.slf4j.Slf4j; +import org.datadog.jmxfetch.JMXServerControlClient; +import org.datadog.jmxfetch.JMXServerSupervisorClient; +import org.testcontainers.containers.GenericContainer; +import org.testcontainers.containers.wait.strategy.Wait; +import org.testcontainers.images.builder.ImageFromDockerfile; +import org.testcontainers.lifecycle.Startable; + +@Slf4j +public class MisbehavingJMXServer implements Startable { + + private static final String DEFAULT_JDK_IMAGE = "base"; + private final String jdkImage; + private final int controlPort; + private final int supervisorPort; + private final GenericContainer server; + private JMXServerControlClient controlClient; + private JMXServerSupervisorClient supervisorClient; + + public MisbehavingJMXServer( + final int rmiPort, + final int controlPort, + final int supervisorPort) { + this(DEFAULT_JDK_IMAGE, rmiPort, controlPort, supervisorPort); + } + + public MisbehavingJMXServer( + final String jdkImage, + final int rmiPort, + final int controlPort, + final int supervisorPort) { + this.controlPort = controlPort; + this.supervisorPort = supervisorPort; + this.jdkImage = jdkImage; + final ImageFromDockerfile img = new ImageFromDockerfile() + .withFileFromPath(".", Paths.get("./tools/misbehaving-jmx-server/")); + this.server = new GenericContainer<>(img) + .withEnv(Collections.singletonMap("RMI_PORT", "" + rmiPort)) + .withEnv(Collections.singletonMap("CONTROL_PORT", "" + controlPort)) + .withEnv(Collections.singletonMap("SUPERVISOR_PORT", "" + supervisorPort)) + .waitingFor(Wait.forLogMessage( + ".*Supervisor HTTP Server Started. Waiting for initialization payload POST to /init.*", + 1)); + } + + @Override + public void start() { + this.server.start(); + final String ipAddress = this.getIp(); + this.controlClient = new JMXServerControlClient(ipAddress, this.controlPort); + this.supervisorClient = new JMXServerSupervisorClient(ipAddress, this.supervisorPort); + try { + log.debug("Initializing JMXServer"); + this.supervisorClient.initializeJMXServer(ipAddress); + } catch (IOException e) { + log.error("Could not initialize JMX Server", e); + } + } + + @Override + public void stop() { + this.server.stop(); + } + + @Override + public void close() { + this.stop(); + } + + public String getIp() { + return this.server.getContainerInfo().getNetworkSettings().getIpAddress(); + } +} From 96a45bc1bd2780a327c21c47c1ff11a4ba39006a Mon Sep 17 00:00:00 2001 From: Carlos Roman Date: Thu, 14 Dec 2023 12:55:44 +0000 Subject: [PATCH 03/24] wip: Second test that checks with JMXFetch for sanity --- .../org/datadog/jmxfetch/TestGCMetrics.java | 35 ++++++++++++++++++- .../jmxfetch/util/MisbehavingJMXServer.java | 8 +++++ 2 files changed, 42 insertions(+), 1 deletion(-) diff --git a/src/test/java/org/datadog/jmxfetch/TestGCMetrics.java b/src/test/java/org/datadog/jmxfetch/TestGCMetrics.java index dd2a0a614..8b27a38a3 100644 --- a/src/test/java/org/datadog/jmxfetch/TestGCMetrics.java +++ b/src/test/java/org/datadog/jmxfetch/TestGCMetrics.java @@ -1,12 +1,16 @@ package org.datadog.jmxfetch; +import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertTrue; import java.io.IOException; +import java.util.List; +import java.util.Map; import javax.management.MBeanServerConnection; import javax.management.remote.JMXConnector; import javax.management.remote.JMXConnectorFactory; import javax.management.remote.JMXServiceURL; import lombok.extern.slf4j.Slf4j; +import org.datadog.jmxfetch.reporter.ConsoleReporter; import org.datadog.jmxfetch.util.MisbehavingJMXServer; import org.junit.Test; import org.testcontainers.containers.output.Slf4jLogConsumer; @@ -23,11 +27,12 @@ private static boolean isDomainPresent(final String domain, final MBeanServerCon boolean found = false; try { final String[] domains = mbs.getDomains(); - for (String s : domains) + for (String s : domains) { if(s.equals(domain)) { found = true; break; } + } } catch (IOException e) { log.warn("Got an exception checking if domain is present", e); } @@ -55,4 +60,32 @@ public void testJMXDirectBasic() throws Exception { assertTrue(isDomainPresent("Bohnanza", mBeanServerConnection)); } } + + @Test + public void testJMXFetchBasic() throws IOException { + try (final MisbehavingJMXServer server = new MisbehavingJMXServer( + RMI_PORT, + CONTROL_PORT, + SUPERVISOR_PORT)) { + server.start(); + final String ipAddress = server.getIp(); + this.initApplicationWithYamlLines( + "init_config:", + " is_jmx: true", + "", + "instances:", + " - name: jmxint_container", + " host: " + ipAddress, + " collect_default_jvm_metrics: false", + " max_returned_metrics: 300000", + " port: " + RMI_PORT, + " conf:", + " - include:", + " domain: Bohnanza" + ); + this.app.doIteration(); + List> metrics = ((ConsoleReporter) this.appConfig.getReporter()).getMetrics(); + assertEquals(1, metrics.size()); + } + } } diff --git a/src/test/java/org/datadog/jmxfetch/util/MisbehavingJMXServer.java b/src/test/java/org/datadog/jmxfetch/util/MisbehavingJMXServer.java index aa9b8f160..4c0331752 100644 --- a/src/test/java/org/datadog/jmxfetch/util/MisbehavingJMXServer.java +++ b/src/test/java/org/datadog/jmxfetch/util/MisbehavingJMXServer.java @@ -75,4 +75,12 @@ public void close() { public String getIp() { return this.server.getContainerInfo().getNetworkSettings().getIpAddress(); } + + public void cutNetwork() throws IOException { + this.controlClient.jmxCutNetwork(); + } + + public void restoreNetwork() throws IOException { + this.controlClient.jmxRestoreNetwork(); + } } From dc55bbb6da7d00131a513cf9def3e882b64a5ae2 Mon Sep 17 00:00:00 2001 From: Carlos Roman Date: Thu, 14 Dec 2023 17:33:36 +0000 Subject: [PATCH 04/24] wip: Added working Old GC test --- pom.xml | 6 ++ .../org/datadog/jmxfetch/TestGCMetrics.java | 94 +++++++++++++++---- .../jmxfetch/TestReconnectContainer.java | 1 - .../jmxfetch/util/MisbehavingJMXServer.java | 10 +- 4 files changed, 89 insertions(+), 22 deletions(-) diff --git a/pom.xml b/pom.xml index 15b614823..3b1ab826d 100644 --- a/pom.xml +++ b/pom.xml @@ -203,6 +203,12 @@ 1.19.3 test + + org.hamcrest + hamcrest-library + 1.3 + test + scm:git:git@github.com:Datadog/jmxfetch.git diff --git a/src/test/java/org/datadog/jmxfetch/TestGCMetrics.java b/src/test/java/org/datadog/jmxfetch/TestGCMetrics.java index 8b27a38a3..a8945fbcd 100644 --- a/src/test/java/org/datadog/jmxfetch/TestGCMetrics.java +++ b/src/test/java/org/datadog/jmxfetch/TestGCMetrics.java @@ -1,19 +1,27 @@ package org.datadog.jmxfetch; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertTrue; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.*; + import java.io.IOException; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.HashSet; import java.util.List; import java.util.Map; +import java.util.Set; + import javax.management.MBeanServerConnection; import javax.management.remote.JMXConnector; import javax.management.remote.JMXConnectorFactory; import javax.management.remote.JMXServiceURL; + +import org.junit.Test; + import lombok.extern.slf4j.Slf4j; + import org.datadog.jmxfetch.reporter.ConsoleReporter; import org.datadog.jmxfetch.util.MisbehavingJMXServer; -import org.junit.Test; -import org.testcontainers.containers.output.Slf4jLogConsumer; @Slf4j public class TestGCMetrics extends TestCommon { @@ -21,7 +29,6 @@ public class TestGCMetrics extends TestCommon { private static final int RMI_PORT = 9090; private static final int CONTROL_PORT = 9091; private static final int SUPERVISOR_PORT = 9092; - private static final Slf4jLogConsumer logConsumer = new Slf4jLogConsumer(log); private static boolean isDomainPresent(final String domain, final MBeanServerConnection mbs) { boolean found = false; @@ -44,28 +51,22 @@ private static boolean isDomainPresent(final String domain, final MBeanServerCon */ @Test public void testJMXDirectBasic() throws Exception { - try (final MisbehavingJMXServer server = new MisbehavingJMXServer( - RMI_PORT, - CONTROL_PORT, + try (final MisbehavingJMXServer server = new MisbehavingJMXServer(RMI_PORT, CONTROL_PORT, SUPERVISOR_PORT)) { server.start(); final String ipAddress = server.getIp(); final String remoteJmxServiceUrl = String.format( - "service:jmx:rmi:///jndi/rmi://%s:%s/jmxrmi", - ipAddress, RMI_PORT - ); + "service:jmx:rmi:///jndi/rmi://%s:%s/jmxrmi", ipAddress, RMI_PORT); final JMXServiceURL jmxUrl = new JMXServiceURL(remoteJmxServiceUrl); final JMXConnector conn = JMXConnectorFactory.connect(jmxUrl); final MBeanServerConnection mBeanServerConnection = conn.getMBeanServerConnection(); - assertTrue(isDomainPresent("Bohnanza", mBeanServerConnection)); + assertThat(isDomainPresent("Bohnanza", mBeanServerConnection), is(true)); } } @Test public void testJMXFetchBasic() throws IOException { - try (final MisbehavingJMXServer server = new MisbehavingJMXServer( - RMI_PORT, - CONTROL_PORT, + try (final MisbehavingJMXServer server = new MisbehavingJMXServer(RMI_PORT, CONTROL_PORT, SUPERVISOR_PORT)) { server.start(); final String ipAddress = server.getIp(); @@ -84,8 +85,67 @@ public void testJMXFetchBasic() throws IOException { " domain: Bohnanza" ); this.app.doIteration(); - List> metrics = ((ConsoleReporter) this.appConfig.getReporter()).getMetrics(); - assertEquals(1, metrics.size()); + final List> metrics = ((ConsoleReporter) this.appConfig.getReporter()).getMetrics(); + assertThat(metrics, hasSize(1)); + } + } + + @Test + public void testDefaultOldGC() throws IOException { + try (final MisbehavingJMXServer server = new MisbehavingJMXServer(RMI_PORT, CONTROL_PORT, + SUPERVISOR_PORT)) { + server.start(); + final String ipAddress = server.getIp(); + this.initApplicationWithYamlLines("init_config:", + " is_jmx: true", + "", + "instances:", + " - name: jmxint_container", + " host: " + ipAddress, + " collect_default_jvm_metrics: true", + " max_returned_metrics: 300000", + " port: " + RMI_PORT); + this.app.doIteration(); + final List> actualMetrics = ((ConsoleReporter) appConfig.getReporter()).getMetrics(); + List gcGenerations = Arrays.asList( + "G1 Old Generation", + "G1 Young Generation"); + assertGCMetric(actualMetrics, "jvm.gc.cms.count", gcGenerations); + assertGCMetric(actualMetrics, "jvm.gc.parnew.time", gcGenerations); + } + } + + private static void assertGCMetric(final List> actualMetrics, + final String expectedMetric, + final List gcGenerations) { + final List> filteredMetrics = new ArrayList<>(); + for (Map actualMetric : actualMetrics) { + final String name = (String) actualMetric.get("name"); + if(expectedMetric.equals(name)) { + filteredMetrics.add(actualMetric); + } + } + assertThat(filteredMetrics, hasSize(2)); + for (final String name : gcGenerations) { + log.debug("Asserting for metric '{}'", name); + boolean found = false; + for (Map filteredMetric : filteredMetrics) { + final Set mTags = new HashSet<>( + Arrays.asList((String[]) (filteredMetric.get("tags")))); + + if(mTags.contains(String.format("name:%s", name))) { + assertThat(mTags, not(empty())); + assertThat(mTags, hasSize(5)); + log.debug("mTags '{}' has size: {}\n{}", name, mTags.size(), mTags); + assertThat(mTags, hasItems( + "instance:jmxint_container", + "jmx_domain:java.lang", + "type:GarbageCollector", + String.format("name:%s", name))); + found = true; + } + } + assertThat(String.format("Did not find metric '%s'", name), found, is(true)); } } } diff --git a/src/test/java/org/datadog/jmxfetch/TestReconnectContainer.java b/src/test/java/org/datadog/jmxfetch/TestReconnectContainer.java index 518bb4b8c..ace89ccdf 100644 --- a/src/test/java/org/datadog/jmxfetch/TestReconnectContainer.java +++ b/src/test/java/org/datadog/jmxfetch/TestReconnectContainer.java @@ -12,7 +12,6 @@ import javax.management.remote.JMXConnectorFactory; import javax.management.remote.JMXServiceURL; -import org.junit.Before; import org.junit.Rule; import org.junit.Test; import org.junit.rules.TestRule; diff --git a/src/test/java/org/datadog/jmxfetch/util/MisbehavingJMXServer.java b/src/test/java/org/datadog/jmxfetch/util/MisbehavingJMXServer.java index 4c0331752..06896deb6 100644 --- a/src/test/java/org/datadog/jmxfetch/util/MisbehavingJMXServer.java +++ b/src/test/java/org/datadog/jmxfetch/util/MisbehavingJMXServer.java @@ -2,7 +2,6 @@ import java.io.IOException; import java.nio.file.Paths; -import java.util.Collections; import lombok.extern.slf4j.Slf4j; import org.datadog.jmxfetch.JMXServerControlClient; import org.datadog.jmxfetch.JMXServerSupervisorClient; @@ -15,6 +14,9 @@ public class MisbehavingJMXServer implements Startable { private static final String DEFAULT_JDK_IMAGE = "base"; + private static final String RMI_PORT = "RMI_PORT"; + private static final String CONTROL_PORT = "CONTROL_PORT"; + private static final String SUPERVISOR_PORT = "SUPERVISOR_PORT"; private final String jdkImage; private final int controlPort; private final int supervisorPort; @@ -40,9 +42,9 @@ public MisbehavingJMXServer( final ImageFromDockerfile img = new ImageFromDockerfile() .withFileFromPath(".", Paths.get("./tools/misbehaving-jmx-server/")); this.server = new GenericContainer<>(img) - .withEnv(Collections.singletonMap("RMI_PORT", "" + rmiPort)) - .withEnv(Collections.singletonMap("CONTROL_PORT", "" + controlPort)) - .withEnv(Collections.singletonMap("SUPERVISOR_PORT", "" + supervisorPort)) + .withEnv(RMI_PORT, String.valueOf(rmiPort)) + .withEnv(CONTROL_PORT, String.valueOf(controlPort)) + .withEnv(SUPERVISOR_PORT, String.valueOf(supervisorPort)) .waitingFor(Wait.forLogMessage( ".*Supervisor HTTP Server Started. Waiting for initialization payload POST to /init.*", 1)); From f9a86809a71db33e5849c430de4b9f22e8f589e1 Mon Sep 17 00:00:00 2001 From: Carlos Roman Date: Fri, 15 Dec 2023 13:25:02 +0000 Subject: [PATCH 05/24] Added option to pass JAVA_OPTS and image to use to MisbehavingJMXServer --- .../datadog/jmxfetch/util/MisbehavingJMXServer.java | 12 ++++++++++-- tools/misbehaving-jmx-server/scripts/start.sh | 3 +++ 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/src/test/java/org/datadog/jmxfetch/util/MisbehavingJMXServer.java b/src/test/java/org/datadog/jmxfetch/util/MisbehavingJMXServer.java index 06896deb6..9531caac7 100644 --- a/src/test/java/org/datadog/jmxfetch/util/MisbehavingJMXServer.java +++ b/src/test/java/org/datadog/jmxfetch/util/MisbehavingJMXServer.java @@ -17,7 +17,10 @@ public class MisbehavingJMXServer implements Startable { private static final String RMI_PORT = "RMI_PORT"; private static final String CONTROL_PORT = "CONTROL_PORT"; private static final String SUPERVISOR_PORT = "SUPERVISOR_PORT"; + public static final String JAVA_OPTS = "JAVA_OPTS"; private final String jdkImage; + + private final String javaOpts; private final int controlPort; private final int supervisorPort; private final GenericContainer server; @@ -28,23 +31,27 @@ public MisbehavingJMXServer( final int rmiPort, final int controlPort, final int supervisorPort) { - this(DEFAULT_JDK_IMAGE, rmiPort, controlPort, supervisorPort); + this(DEFAULT_JDK_IMAGE, "", rmiPort, controlPort, supervisorPort); } public MisbehavingJMXServer( final String jdkImage, + final String javaOpts, final int rmiPort, final int controlPort, final int supervisorPort) { + this.javaOpts = javaOpts; this.controlPort = controlPort; this.supervisorPort = supervisorPort; this.jdkImage = jdkImage; final ImageFromDockerfile img = new ImageFromDockerfile() - .withFileFromPath(".", Paths.get("./tools/misbehaving-jmx-server/")); + .withFileFromPath(".", Paths.get("./tools/misbehaving-jmx-server/")) + .withBuildArg("FINAL_JRE_IMAGE", this.jdkImage); this.server = new GenericContainer<>(img) .withEnv(RMI_PORT, String.valueOf(rmiPort)) .withEnv(CONTROL_PORT, String.valueOf(controlPort)) .withEnv(SUPERVISOR_PORT, String.valueOf(supervisorPort)) + .withEnv(JAVA_OPTS, this.javaOpts) .waitingFor(Wait.forLogMessage( ".*Supervisor HTTP Server Started. Waiting for initialization payload POST to /init.*", 1)); @@ -52,6 +59,7 @@ public MisbehavingJMXServer( @Override public void start() { + log.info("Starting MisbehavingJMXServer with Docker image '{}' with JAVA_OPTS '{}'", this.jdkImage, this.javaOpts); this.server.start(); final String ipAddress = this.getIp(); this.controlClient = new JMXServerControlClient(ipAddress, this.controlPort); diff --git a/tools/misbehaving-jmx-server/scripts/start.sh b/tools/misbehaving-jmx-server/scripts/start.sh index 6fb93ebdb..ccc8a8a89 100755 --- a/tools/misbehaving-jmx-server/scripts/start.sh +++ b/tools/misbehaving-jmx-server/scripts/start.sh @@ -6,6 +6,9 @@ echo "Running $@" [ -n "$JAVA_OPTS" ] || JAVA_OPTS="-Xmx128M -Xms128M" +echo "Using `java --version`" +echo "With JAVA_OPTS '${JAVA_OPTS}'" + # shellcheck disable=SC2086 java \ ${JAVA_OPTS} \ From a7fdff807d059448d889d8375a971e098d3aeeb6 Mon Sep 17 00:00:00 2001 From: Carlos Roman Date: Fri, 15 Dec 2023 13:31:30 +0000 Subject: [PATCH 06/24] Added test for default new JVM metrics --- .../org/datadog/jmxfetch/TestGCMetrics.java | 26 +++++++++++++++++++ .../jmxfetch/util/MisbehavingJMXServer.java | 12 ++++++--- 2 files changed, 34 insertions(+), 4 deletions(-) diff --git a/src/test/java/org/datadog/jmxfetch/TestGCMetrics.java b/src/test/java/org/datadog/jmxfetch/TestGCMetrics.java index a8945fbcd..73bcb2f84 100644 --- a/src/test/java/org/datadog/jmxfetch/TestGCMetrics.java +++ b/src/test/java/org/datadog/jmxfetch/TestGCMetrics.java @@ -115,6 +115,32 @@ public void testDefaultOldGC() throws IOException { } } + @Test + public void testDefaultNewGCMetrics() throws IOException { + try (final MisbehavingJMXServer server = new MisbehavingJMXServer(RMI_PORT, CONTROL_PORT, + SUPERVISOR_PORT)) { + server.start(); + final String ipAddress = server.getIp(); + this.initApplicationWithYamlLines("init_config:", + " is_jmx: true", + "", + "instances:", + " - name: jmxint_container", + " host: " + ipAddress, + " collect_default_jvm_metrics: true", + " new_gc_metrics: true", + " max_returned_metrics: 300000", + " port: " + RMI_PORT); + this.app.doIteration(); + final List> actualMetrics = ((ConsoleReporter) appConfig.getReporter()).getMetrics(); + List gcGenerations = Arrays.asList( + "G1 Old Generation", + "G1 Young Generation"); + assertGCMetric(actualMetrics, "jvm.gc.cms.count", gcGenerations); + assertGCMetric(actualMetrics, "jvm.gc.parnew.time", gcGenerations); + } + } + private static void assertGCMetric(final List> actualMetrics, final String expectedMetric, final List gcGenerations) { diff --git a/src/test/java/org/datadog/jmxfetch/util/MisbehavingJMXServer.java b/src/test/java/org/datadog/jmxfetch/util/MisbehavingJMXServer.java index 9531caac7..d513e35e0 100644 --- a/src/test/java/org/datadog/jmxfetch/util/MisbehavingJMXServer.java +++ b/src/test/java/org/datadog/jmxfetch/util/MisbehavingJMXServer.java @@ -2,14 +2,17 @@ import java.io.IOException; import java.nio.file.Paths; -import lombok.extern.slf4j.Slf4j; -import org.datadog.jmxfetch.JMXServerControlClient; -import org.datadog.jmxfetch.JMXServerSupervisorClient; + import org.testcontainers.containers.GenericContainer; import org.testcontainers.containers.wait.strategy.Wait; import org.testcontainers.images.builder.ImageFromDockerfile; import org.testcontainers.lifecycle.Startable; +import lombok.extern.slf4j.Slf4j; + +import org.datadog.jmxfetch.JMXServerControlClient; +import org.datadog.jmxfetch.JMXServerSupervisorClient; + @Slf4j public class MisbehavingJMXServer implements Startable { @@ -59,7 +62,8 @@ public MisbehavingJMXServer( @Override public void start() { - log.info("Starting MisbehavingJMXServer with Docker image '{}' with JAVA_OPTS '{}'", this.jdkImage, this.javaOpts); + log.info("Starting MisbehavingJMXServer with Docker image '{}' with JAVA_OPTS '{}'", + this.jdkImage, this.javaOpts); this.server.start(); final String ipAddress = this.getIp(); this.controlClient = new JMXServerControlClient(ipAddress, this.controlPort); From 22a91de4f4099959b12e9d977a488b4dbef97091 Mon Sep 17 00:00:00 2001 From: Carlos Roman Date: Fri, 15 Dec 2023 17:33:57 +0000 Subject: [PATCH 07/24] Added UseParallelGC test using new_gc_metrics: true# --- .../java/org/datadog/jmxfetch/Instance.java | 1 - .../org/datadog/jmxfetch/TestGCMetrics.java | 44 ++++++++++++------- .../jmxfetch/util/MisbehavingJMXServer.java | 2 + 3 files changed, 29 insertions(+), 18 deletions(-) diff --git a/src/main/java/org/datadog/jmxfetch/Instance.java b/src/main/java/org/datadog/jmxfetch/Instance.java index 1cc3b91f9..9da8de750 100644 --- a/src/main/java/org/datadog/jmxfetch/Instance.java +++ b/src/main/java/org/datadog/jmxfetch/Instance.java @@ -16,7 +16,6 @@ import java.io.InputStream; import java.lang.management.ManagementFactory; import java.util.ArrayList; -import java.util.Arrays; import java.util.HashMap; import java.util.HashSet; import java.util.Iterator; diff --git a/src/test/java/org/datadog/jmxfetch/TestGCMetrics.java b/src/test/java/org/datadog/jmxfetch/TestGCMetrics.java index 73bcb2f84..10781f1a0 100644 --- a/src/test/java/org/datadog/jmxfetch/TestGCMetrics.java +++ b/src/test/java/org/datadog/jmxfetch/TestGCMetrics.java @@ -4,12 +4,8 @@ import static org.hamcrest.Matchers.*; import java.io.IOException; -import java.util.ArrayList; -import java.util.Arrays; -import java.util.HashSet; -import java.util.List; -import java.util.Map; -import java.util.Set; +import java.util.*; +import java.util.concurrent.TimeUnit; import javax.management.MBeanServerConnection; import javax.management.remote.JMXConnector; @@ -116,31 +112,45 @@ public void testDefaultOldGC() throws IOException { } @Test - public void testDefaultNewGCMetrics() throws IOException { - try (final MisbehavingJMXServer server = new MisbehavingJMXServer(RMI_PORT, CONTROL_PORT, - SUPERVISOR_PORT)) { + public void testDefaultNewGCMetrics() throws IOException, InterruptedException { + try (final MisbehavingJMXServer server = new MisbehavingJMXServer( + MisbehavingJMXServer.JDK_11, + "-XX:+UseParallelGC", + RMI_PORT, + CONTROL_PORT, + SUPERVISOR_PORT)) { server.start(); final String ipAddress = server.getIp(); this.initApplicationWithYamlLines("init_config:", " is_jmx: true", + " new_gc_metrics: true", "", "instances:", " - name: jmxint_container", " host: " + ipAddress, " collect_default_jvm_metrics: true", - " new_gc_metrics: true", " max_returned_metrics: 300000", " port: " + RMI_PORT); + // Run one iteration first + this.app.doIteration(); + // And then pull get the metrics or else reporter does not have correct number of metrics + ((ConsoleReporter) appConfig.getReporter()).getMetrics(); + + // Actual iteration we care about this.app.doIteration(); final List> actualMetrics = ((ConsoleReporter) appConfig.getReporter()).getMetrics(); - List gcGenerations = Arrays.asList( - "G1 Old Generation", - "G1 Young Generation"); - assertGCMetric(actualMetrics, "jvm.gc.cms.count", gcGenerations); - assertGCMetric(actualMetrics, "jvm.gc.parnew.time", gcGenerations); + assertThat(actualMetrics, hasSize(13)); + final List gcYoungGenerations = Collections.singletonList( + "G1 Young Generation"); + assertGCMetric(actualMetrics, "jvm.gc.minor_collection_count", gcYoungGenerations); + assertGCMetric(actualMetrics, "jvm.gc.minor_collection_time", gcYoungGenerations); + final List gcOldGenerations = Collections.singletonList( + "G1 Old Generation"); + assertGCMetric(actualMetrics, "jvm.gc.major_collection_count", gcOldGenerations); + assertGCMetric(actualMetrics, "jvm.gc.major_collection_time", gcOldGenerations); } } - + private static void assertGCMetric(final List> actualMetrics, final String expectedMetric, final List gcGenerations) { @@ -151,7 +161,7 @@ private static void assertGCMetric(final List> actualMetrics filteredMetrics.add(actualMetric); } } - assertThat(filteredMetrics, hasSize(2)); + assertThat(filteredMetrics, hasSize(gcGenerations.size())); for (final String name : gcGenerations) { log.debug("Asserting for metric '{}'", name); boolean found = false; diff --git a/src/test/java/org/datadog/jmxfetch/util/MisbehavingJMXServer.java b/src/test/java/org/datadog/jmxfetch/util/MisbehavingJMXServer.java index d513e35e0..ec48a96b9 100644 --- a/src/test/java/org/datadog/jmxfetch/util/MisbehavingJMXServer.java +++ b/src/test/java/org/datadog/jmxfetch/util/MisbehavingJMXServer.java @@ -17,6 +17,8 @@ public class MisbehavingJMXServer implements Startable { private static final String DEFAULT_JDK_IMAGE = "base"; + public static final String JDK_11 = "eclipse-temurin:11"; + public static final String JDK_17 = "eclipse-temurin:17"; private static final String RMI_PORT = "RMI_PORT"; private static final String CONTROL_PORT = "CONTROL_PORT"; private static final String SUPERVISOR_PORT = "SUPERVISOR_PORT"; From ed430f9c4a7f6e8b79184f5e170f215ae8a74ca6 Mon Sep 17 00:00:00 2001 From: Carlos Roman Date: Fri, 15 Dec 2023 18:03:32 +0000 Subject: [PATCH 08/24] Added UseG1GC test using new_gc_metrics: true --- .../org/datadog/jmxfetch/TestGCMetrics.java | 44 ++++++++++++++++++- 1 file changed, 42 insertions(+), 2 deletions(-) diff --git a/src/test/java/org/datadog/jmxfetch/TestGCMetrics.java b/src/test/java/org/datadog/jmxfetch/TestGCMetrics.java index 10781f1a0..3aa5d7d49 100644 --- a/src/test/java/org/datadog/jmxfetch/TestGCMetrics.java +++ b/src/test/java/org/datadog/jmxfetch/TestGCMetrics.java @@ -112,7 +112,7 @@ public void testDefaultOldGC() throws IOException { } @Test - public void testDefaultNewGCMetrics() throws IOException, InterruptedException { + public void testDefaultNewGCMetricsUseParallelGC() throws IOException, InterruptedException { try (final MisbehavingJMXServer server = new MisbehavingJMXServer( MisbehavingJMXServer.JDK_11, "-XX:+UseParallelGC", @@ -150,7 +150,47 @@ public void testDefaultNewGCMetrics() throws IOException, InterruptedException { assertGCMetric(actualMetrics, "jvm.gc.major_collection_time", gcOldGenerations); } } - + + @Test + public void testDefaultNewGCMetricsUseG1GC() throws IOException, InterruptedException { + try (final MisbehavingJMXServer server = new MisbehavingJMXServer( + MisbehavingJMXServer.JDK_17, + "-XX:+UseG1GC", + RMI_PORT, + CONTROL_PORT, + SUPERVISOR_PORT)) { + server.start(); + final String ipAddress = server.getIp(); + this.initApplicationWithYamlLines("init_config:", + " is_jmx: true", + " new_gc_metrics: true", + "", + "instances:", + " - name: jmxint_container", + " host: " + ipAddress, + " collect_default_jvm_metrics: true", + " max_returned_metrics: 300000", + " port: " + RMI_PORT); + // Run one iteration first + this.app.doIteration(); + // And then pull get the metrics or else reporter does not have correct number of metrics + ((ConsoleReporter) appConfig.getReporter()).getMetrics(); + + // Actual iteration we care about + this.app.doIteration(); + final List> actualMetrics = ((ConsoleReporter) appConfig.getReporter()).getMetrics(); + assertThat(actualMetrics, hasSize(13)); + final List gcYoungGenerations = Collections.singletonList( + "G1 Young Generation"); + assertGCMetric(actualMetrics, "jvm.gc.minor_collection_count", gcYoungGenerations); + assertGCMetric(actualMetrics, "jvm.gc.minor_collection_time", gcYoungGenerations); + final List gcOldGenerations = Collections.singletonList( + "G1 Old Generation"); + assertGCMetric(actualMetrics, "jvm.gc.major_collection_count", gcOldGenerations); + assertGCMetric(actualMetrics, "jvm.gc.major_collection_time", gcOldGenerations); + } + } + private static void assertGCMetric(final List> actualMetrics, final String expectedMetric, final List gcGenerations) { From d006fd0c3c455157d357bd719220defd30e76483 Mon Sep 17 00:00:00 2001 From: Carlos Roman Date: Fri, 15 Dec 2023 18:07:49 +0000 Subject: [PATCH 09/24] Added UseZGC test using new_gc_metrics: true --- .../org/datadog/jmxfetch/TestGCMetrics.java | 40 +++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/src/test/java/org/datadog/jmxfetch/TestGCMetrics.java b/src/test/java/org/datadog/jmxfetch/TestGCMetrics.java index 3aa5d7d49..1c64f14a4 100644 --- a/src/test/java/org/datadog/jmxfetch/TestGCMetrics.java +++ b/src/test/java/org/datadog/jmxfetch/TestGCMetrics.java @@ -191,6 +191,46 @@ public void testDefaultNewGCMetricsUseG1GC() throws IOException, InterruptedExce } } + @Test + public void testDefaultNewGCMetricsUseZGC() throws IOException, InterruptedException { + try (final MisbehavingJMXServer server = new MisbehavingJMXServer( + MisbehavingJMXServer.JDK_21, + "-XX:+UseZGC", + RMI_PORT, + CONTROL_PORT, + SUPERVISOR_PORT)) { + server.start(); + final String ipAddress = server.getIp(); + this.initApplicationWithYamlLines("init_config:", + " is_jmx: true", + " new_gc_metrics: true", + "", + "instances:", + " - name: jmxint_container", + " host: " + ipAddress, + " collect_default_jvm_metrics: true", + " max_returned_metrics: 300000", + " port: " + RMI_PORT); + // Run one iteration first + this.app.doIteration(); + // And then pull get the metrics or else reporter does not have correct number of metrics + ((ConsoleReporter) appConfig.getReporter()).getMetrics(); + + // Actual iteration we care about + this.app.doIteration(); + final List> actualMetrics = ((ConsoleReporter) appConfig.getReporter()).getMetrics(); + assertThat(actualMetrics, hasSize(13)); + final List gcYoungGenerations = Collections.singletonList( + "G1 Young Generation"); + assertGCMetric(actualMetrics, "jvm.gc.minor_collection_count", gcYoungGenerations); + assertGCMetric(actualMetrics, "jvm.gc.minor_collection_time", gcYoungGenerations); + final List gcOldGenerations = Collections.singletonList( + "G1 Old Generation"); + assertGCMetric(actualMetrics, "jvm.gc.major_collection_count", gcOldGenerations); + assertGCMetric(actualMetrics, "jvm.gc.major_collection_time", gcOldGenerations); + } + } + private static void assertGCMetric(final List> actualMetrics, final String expectedMetric, final List gcGenerations) { From 50c60417f09caefe9c1c4bc129013d0a44a31943 Mon Sep 17 00:00:00 2001 From: Carlos Roman Date: Mon, 18 Dec 2023 09:14:19 +0000 Subject: [PATCH 10/24] Adding missing change to MisbehavingJMXServer.java --- .../java/org/datadog/jmxfetch/util/MisbehavingJMXServer.java | 1 + 1 file changed, 1 insertion(+) diff --git a/src/test/java/org/datadog/jmxfetch/util/MisbehavingJMXServer.java b/src/test/java/org/datadog/jmxfetch/util/MisbehavingJMXServer.java index ec48a96b9..c8f04b0e1 100644 --- a/src/test/java/org/datadog/jmxfetch/util/MisbehavingJMXServer.java +++ b/src/test/java/org/datadog/jmxfetch/util/MisbehavingJMXServer.java @@ -19,6 +19,7 @@ public class MisbehavingJMXServer implements Startable { private static final String DEFAULT_JDK_IMAGE = "base"; public static final String JDK_11 = "eclipse-temurin:11"; public static final String JDK_17 = "eclipse-temurin:17"; + public static final String JDK_21 = "eclipse-temurin:21"; private static final String RMI_PORT = "RMI_PORT"; private static final String CONTROL_PORT = "CONTROL_PORT"; private static final String SUPERVISOR_PORT = "SUPERVISOR_PORT"; From a1585b0a816f673a1436c5d458edda2fb1c50f4e Mon Sep 17 00:00:00 2001 From: Carlos Roman Date: Mon, 18 Dec 2023 12:06:43 +0000 Subject: [PATCH 11/24] Adding a working ZGC test --- .../org/datadog/jmxfetch/TestGCMetrics.java | 63 ++++++++++--- .../jmxfetch/util/server/SimpleApp.java | 94 +++++++++++++++++++ .../util/server/SimpleAppContainer.java | 66 +++++++++++++ .../jmxfetch/util/server/Dockerfile-SimpleApp | 15 +++ .../jmxfetch/util/server/run-SimpleApp.sh | 26 +++++ 5 files changed, 253 insertions(+), 11 deletions(-) create mode 100644 src/test/java/org/datadog/jmxfetch/util/server/SimpleApp.java create mode 100644 src/test/java/org/datadog/jmxfetch/util/server/SimpleAppContainer.java create mode 100644 src/test/resources/org/datadog/jmxfetch/util/server/Dockerfile-SimpleApp create mode 100755 src/test/resources/org/datadog/jmxfetch/util/server/run-SimpleApp.sh diff --git a/src/test/java/org/datadog/jmxfetch/TestGCMetrics.java b/src/test/java/org/datadog/jmxfetch/TestGCMetrics.java index 1c64f14a4..fb238269a 100644 --- a/src/test/java/org/datadog/jmxfetch/TestGCMetrics.java +++ b/src/test/java/org/datadog/jmxfetch/TestGCMetrics.java @@ -12,6 +12,8 @@ import javax.management.remote.JMXConnectorFactory; import javax.management.remote.JMXServiceURL; +import org.datadog.jmxfetch.util.server.SimpleAppContainer; +import org.junit.Ignore; import org.junit.Test; import lombok.extern.slf4j.Slf4j; @@ -192,10 +194,49 @@ public void testDefaultNewGCMetricsUseG1GC() throws IOException, InterruptedExce } @Test - public void testDefaultNewGCMetricsUseZGC() throws IOException, InterruptedException { + public void testDefaultNewGCMetricsUseZGC() throws IOException { + try (final SimpleAppContainer container = new SimpleAppContainer( + "eclipse-temurin:21", + "-XX:+UseZGC -Xmx128M -Xms128M", + RMI_PORT + )){ + container.start(); + final String ipAddress = container.getIp(); + this.initApplicationWithYamlLines("init_config:", + " is_jmx: true", + " new_gc_metrics: true", + "", + "instances:", + " - name: jmxint_container", + " host: " + ipAddress, + " collect_default_jvm_metrics: true", + " max_returned_metrics: 300000", + " port: " + RMI_PORT); + // Run one iteration first + this.app.doIteration(); + // And then pull get the metrics or else reporter does not have correct number of metrics + ((ConsoleReporter) appConfig.getReporter()).getMetrics(); + + // Actual iteration we care about + this.app.doIteration(); + final List> actualMetrics = ((ConsoleReporter) appConfig.getReporter()).getMetrics(); + assertThat(actualMetrics, hasSize(13)); + final List zgcPause = Collections.singletonList( + "ZGC Pauses"); + assertGCMetric(actualMetrics, "jvm.gc.zgc_pauses_collection_count", zgcPause); + assertGCMetric(actualMetrics, "jvm.gc.zgc_pauses_collection_time", zgcPause); + final List zgcCycles = Collections.singletonList( + "ZGC Cycles"); + assertGCMetric(actualMetrics, "jvm.gc.zgc_cycles_collection_count", zgcCycles); + assertGCMetric(actualMetrics, "jvm.gc.zgc_cycles_collection_time", zgcCycles); + } + } + @Test + @Ignore("Can not force ZGC to work using MisbehavingJMXServer") + public void testDefaultNewGCMetricsUseZGCOld() throws IOException, InterruptedException { try (final MisbehavingJMXServer server = new MisbehavingJMXServer( - MisbehavingJMXServer.JDK_21, - "-XX:+UseZGC", + MisbehavingJMXServer.JDK_17, + "-XX:+UnlockExperimentalVMOptions -XX:+UseZGC", RMI_PORT, CONTROL_PORT, SUPERVISOR_PORT)) { @@ -220,14 +261,14 @@ public void testDefaultNewGCMetricsUseZGC() throws IOException, InterruptedExcep this.app.doIteration(); final List> actualMetrics = ((ConsoleReporter) appConfig.getReporter()).getMetrics(); assertThat(actualMetrics, hasSize(13)); - final List gcYoungGenerations = Collections.singletonList( - "G1 Young Generation"); - assertGCMetric(actualMetrics, "jvm.gc.minor_collection_count", gcYoungGenerations); - assertGCMetric(actualMetrics, "jvm.gc.minor_collection_time", gcYoungGenerations); - final List gcOldGenerations = Collections.singletonList( - "G1 Old Generation"); - assertGCMetric(actualMetrics, "jvm.gc.major_collection_count", gcOldGenerations); - assertGCMetric(actualMetrics, "jvm.gc.major_collection_time", gcOldGenerations); + final List zgcPause = Collections.singletonList( + "ZGC Pauses"); + assertGCMetric(actualMetrics, "jvm.gc.zgc_pauses_collection_count", zgcPause); + assertGCMetric(actualMetrics, "jvm.gc.zgc_pauses_collection_time", zgcPause); + final List zgcCycles = Collections.singletonList( + "ZGC Cycles"); + assertGCMetric(actualMetrics, "jvm.gc.zgc_cycles_collection_count", zgcCycles); + assertGCMetric(actualMetrics, "jvm.gc.zgc_cycles_collection_time", zgcCycles); } } diff --git a/src/test/java/org/datadog/jmxfetch/util/server/SimpleApp.java b/src/test/java/org/datadog/jmxfetch/util/server/SimpleApp.java new file mode 100644 index 000000000..20522d28b --- /dev/null +++ b/src/test/java/org/datadog/jmxfetch/util/server/SimpleApp.java @@ -0,0 +1,94 @@ +package org.datadog.jmxfetch.util.server; + +import java.lang.management.ManagementFactory; +import java.util.Hashtable; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicInteger; +import javax.management.InstanceAlreadyExistsException; +import javax.management.MBeanRegistrationException; +import javax.management.MBeanServer; +import javax.management.MalformedObjectNameException; +import javax.management.NotCompliantMBeanException; +import javax.management.ObjectName; + +class SimpleApp { + public interface SampleMBean { + + Integer getShouldBe100(); + + Double getShouldBe1000(); + + Long getShouldBe1337(); + + Float getShouldBe1_1(); + + int getShouldBeCounter(); + } + + public static class Sample implements SampleMBean { + + private final AtomicInteger counter = new AtomicInteger(0); + + @Override + public Integer getShouldBe100() { + return 100; + } + + @Override + public Double getShouldBe1000() { + return 200.0; + } + + @Override + public Long getShouldBe1337() { + return 1337L; + } + + @Override + public Float getShouldBe1_1() { + return 1.1F; + } + + @Override + public int getShouldBeCounter() { + return this.counter.get(); + } + } + + public static void main(String[] args) { + System.out.println("Starting sample app..."); + try { + final Hashtable pairs = new Hashtable<>(); + pairs.put("name", "default"); + pairs.put("type", "simple"); + final Thread daemonThread = getThread(pairs); + daemonThread.start(); + daemonThread.join(); + } catch (MalformedObjectNameException | InstanceAlreadyExistsException | + MBeanRegistrationException | NotCompliantMBeanException | InterruptedException e) { + throw new RuntimeException(e); + } + } + + private static Thread getThread(final Hashtable pairs) + throws MalformedObjectNameException, InstanceAlreadyExistsException, MBeanRegistrationException, NotCompliantMBeanException { + final ObjectName objectName = new ObjectName("dd.test.sample", pairs); + final MBeanServer server = ManagementFactory.getPlatformMBeanServer(); + final Sample sample = new Sample(); + server.registerMBean(sample, objectName); + final Thread daemonThread = new Thread(new Runnable() { + @Override + public void run() { + while (sample.counter.incrementAndGet() > 0) { + try { + Thread.sleep(TimeUnit.SECONDS.toSeconds(5)); + } catch (InterruptedException e) { + throw new RuntimeException(e); + } + } + } + }); + daemonThread.setDaemon(true); + return daemonThread; + } +} diff --git a/src/test/java/org/datadog/jmxfetch/util/server/SimpleAppContainer.java b/src/test/java/org/datadog/jmxfetch/util/server/SimpleAppContainer.java new file mode 100644 index 000000000..f239fdbe6 --- /dev/null +++ b/src/test/java/org/datadog/jmxfetch/util/server/SimpleAppContainer.java @@ -0,0 +1,66 @@ +package org.datadog.jmxfetch.util.server; + +import java.nio.file.Paths; +import java.time.Duration; +import java.util.Set; + +import org.testcontainers.containers.GenericContainer; +import org.testcontainers.containers.wait.strategy.Wait; +import org.testcontainers.images.builder.ImageFromDockerfile; +import org.testcontainers.lifecycle.Startable; + +import lombok.extern.slf4j.Slf4j; + +@Slf4j +public class SimpleAppContainer implements Startable { + + private static final String JAVA_OPTS = "JAVA_OPTS"; + private static final String RMI_PORT = "RMI_PORT"; + private final String jreDockerImage; + private final String javaOpts; + private final int rmiPort; + private final GenericContainer server; + + public SimpleAppContainer(final String jreDockerImage, final String javaOpts, final int rmiPort) { + this.jreDockerImage = jreDockerImage; + this.javaOpts = javaOpts; + this.rmiPort = rmiPort; + final ImageFromDockerfile img = new ImageFromDockerfile() + .withFileFromPath("app.java", Paths.get("./src/test/java/org/datadog/jmxfetch/util/server/SimpleApp.java")) + .withFileFromClasspath("Dockerfile", "org/datadog/jmxfetch/util/server/Dockerfile-SimpleApp") + .withFileFromClasspath("run.sh", "org/datadog/jmxfetch/util/server/run-SimpleApp.sh") + .withBuildArg("JRE_DOCKER_IMAGE", jreDockerImage); + this.server = new GenericContainer<>(img) + .withEnv(JAVA_OPTS, this.javaOpts) + .withEnv(RMI_PORT, Integer.toString(this.rmiPort)) + .withExposedPorts(this.rmiPort) + .waitingFor(Wait.forListeningPorts(this.rmiPort).withStartupTimeout(Duration.ofSeconds(10))); + } + + @Override + public Set getDependencies() { + return Startable.super.getDependencies(); + } + + @Override + public void start() { + log.info("Starting SimpleApp with Docker image '{}' with JAVA_OPTS '{}'", + this.jreDockerImage, this.javaOpts); + this.server.start(); + log.info(this.server.getLogs()); + } + + @Override + public void stop() { + this.server.stop(); + } + + public String getIp() { + return this.server.getContainerInfo().getNetworkSettings().getIpAddress(); + } + + @Override + public void close() { + Startable.super.close(); + } +} diff --git a/src/test/resources/org/datadog/jmxfetch/util/server/Dockerfile-SimpleApp b/src/test/resources/org/datadog/jmxfetch/util/server/Dockerfile-SimpleApp new file mode 100644 index 000000000..81f60f0ef --- /dev/null +++ b/src/test/resources/org/datadog/jmxfetch/util/server/Dockerfile-SimpleApp @@ -0,0 +1,15 @@ +# syntax=docker/dockerfile:1 + +# Allows to cheange the JDK image used +ARG JRE_DOCKER_IMAGE=eclipse-temurin:11 + +# Use the official JDK image as the base image +FROM ${JRE_DOCKER_IMAGE} + +WORKDIR /app + +COPY run.sh app.java /app/ + +EXPOSE 9010 + +ENTRYPOINT [ "/app/run.sh" ] diff --git a/src/test/resources/org/datadog/jmxfetch/util/server/run-SimpleApp.sh b/src/test/resources/org/datadog/jmxfetch/util/server/run-SimpleApp.sh new file mode 100755 index 000000000..5e1f63b65 --- /dev/null +++ b/src/test/resources/org/datadog/jmxfetch/util/server/run-SimpleApp.sh @@ -0,0 +1,26 @@ +#!/usr/bin/env sh +set -f + +[ -n "$JAVA_OPTS" ] || JAVA_OPTS="-Xmx128M -Xms128M" +[ -n "$RMI_PORT" ] || RMI_PORT="9010" + +echo "Using `java --version`" +echo "With JAVA_OPTS '${JAVA_OPTS}'" +CONTAINER_IP=`awk 'END{print $1}' /etc/hosts` + +# shellcheck disable=SC2086 +javac -d app app.java + +echo "Starting app with hostname set to ${CONTAINER_IP}" + +java -cp ./app \ + ${JAVA_OPTS} \ + -Dcom.sun.management.jmxremote=true \ + -Dcom.sun.management.jmxremote.port=${RMI_PORT} \ + -Dcom.sun.management.jmxremote.rmi.port=${RMI_PORT} \ + -Dcom.sun.management.jmxremote.authenticate=false \ + -Dcom.sun.management.jmxremote.ssl=false \ + -Djava.rmi.server.hostname=${CONTAINER_IP} \ + org.datadog.jmxfetch.util.server.SimpleApp + +# java -jar jmxterm-1.0.2-uber.jar -l service:jmx:rmi:///jndi/rmi://localhost:9010/jmxrmi From 5ee4f545198ef4353bbb595c2402a377d054dab4 Mon Sep 17 00:00:00 2001 From: Carlos Roman Date: Mon, 18 Dec 2023 12:19:11 +0000 Subject: [PATCH 12/24] Fixed broken test --- .../org/datadog/jmxfetch/TestGCMetrics.java | 3 ++- .../util/server/SimpleAppContainer.java | 18 ++++++------------ 2 files changed, 8 insertions(+), 13 deletions(-) diff --git a/src/test/java/org/datadog/jmxfetch/TestGCMetrics.java b/src/test/java/org/datadog/jmxfetch/TestGCMetrics.java index fb238269a..400d28fba 100644 --- a/src/test/java/org/datadog/jmxfetch/TestGCMetrics.java +++ b/src/test/java/org/datadog/jmxfetch/TestGCMetrics.java @@ -231,8 +231,9 @@ public void testDefaultNewGCMetricsUseZGC() throws IOException { assertGCMetric(actualMetrics, "jvm.gc.zgc_cycles_collection_time", zgcCycles); } } - @Test + @Ignore("Can not force ZGC to work using MisbehavingJMXServer") + @Test public void testDefaultNewGCMetricsUseZGCOld() throws IOException, InterruptedException { try (final MisbehavingJMXServer server = new MisbehavingJMXServer( MisbehavingJMXServer.JDK_17, diff --git a/src/test/java/org/datadog/jmxfetch/util/server/SimpleAppContainer.java b/src/test/java/org/datadog/jmxfetch/util/server/SimpleAppContainer.java index f239fdbe6..7e6980e98 100644 --- a/src/test/java/org/datadog/jmxfetch/util/server/SimpleAppContainer.java +++ b/src/test/java/org/datadog/jmxfetch/util/server/SimpleAppContainer.java @@ -2,7 +2,6 @@ import java.nio.file.Paths; import java.time.Duration; -import java.util.Set; import org.testcontainers.containers.GenericContainer; import org.testcontainers.containers.wait.strategy.Wait; @@ -37,15 +36,10 @@ public SimpleAppContainer(final String jreDockerImage, final String javaOpts, fi .waitingFor(Wait.forListeningPorts(this.rmiPort).withStartupTimeout(Duration.ofSeconds(10))); } - @Override - public Set getDependencies() { - return Startable.super.getDependencies(); - } - @Override public void start() { - log.info("Starting SimpleApp with Docker image '{}' with JAVA_OPTS '{}'", - this.jreDockerImage, this.javaOpts); + log.info("Starting SimpleApp with Docker image '{}' with JAVA_OPTS '{}' in port '{}'", + this.jreDockerImage, this.javaOpts, this.rmiPort); this.server.start(); log.info(this.server.getLogs()); } @@ -55,12 +49,12 @@ public void stop() { this.server.stop(); } + public void close() { + this.stop(); + } + public String getIp() { return this.server.getContainerInfo().getNetworkSettings().getIpAddress(); } - @Override - public void close() { - Startable.super.close(); - } } From 57b4d53d97a38472b95c0f2c5ecf068560dfc019 Mon Sep 17 00:00:00 2001 From: Carlos Roman Date: Tue, 19 Dec 2023 17:26:24 +0000 Subject: [PATCH 13/24] Fixed it so Misbehaving server runs with correct GC now --- .../java/org/datadog/jmxfetch/TestGCMetrics.java | 14 ++++++-------- .../jmxfetch/util/MisbehavingJMXServer.java | 6 +++--- tools/misbehaving-jmx-server/pom.xml | 14 ++++++++++---- tools/misbehaving-jmx-server/scripts/start.sh | 7 +++---- .../src/main/java/org/datadog/supervisor/App.java | 11 +++++++---- 5 files changed, 29 insertions(+), 23 deletions(-) diff --git a/src/test/java/org/datadog/jmxfetch/TestGCMetrics.java b/src/test/java/org/datadog/jmxfetch/TestGCMetrics.java index 400d28fba..81bfc15c6 100644 --- a/src/test/java/org/datadog/jmxfetch/TestGCMetrics.java +++ b/src/test/java/org/datadog/jmxfetch/TestGCMetrics.java @@ -5,21 +5,19 @@ import java.io.IOException; import java.util.*; -import java.util.concurrent.TimeUnit; import javax.management.MBeanServerConnection; import javax.management.remote.JMXConnector; import javax.management.remote.JMXConnectorFactory; import javax.management.remote.JMXServiceURL; -import org.datadog.jmxfetch.util.server.SimpleAppContainer; -import org.junit.Ignore; import org.junit.Test; import lombok.extern.slf4j.Slf4j; import org.datadog.jmxfetch.reporter.ConsoleReporter; import org.datadog.jmxfetch.util.MisbehavingJMXServer; +import org.datadog.jmxfetch.util.server.SimpleAppContainer; @Slf4j public class TestGCMetrics extends TestCommon { @@ -143,11 +141,11 @@ public void testDefaultNewGCMetricsUseParallelGC() throws IOException, Interrupt final List> actualMetrics = ((ConsoleReporter) appConfig.getReporter()).getMetrics(); assertThat(actualMetrics, hasSize(13)); final List gcYoungGenerations = Collections.singletonList( - "G1 Young Generation"); + "PS Scavenge"); assertGCMetric(actualMetrics, "jvm.gc.minor_collection_count", gcYoungGenerations); assertGCMetric(actualMetrics, "jvm.gc.minor_collection_time", gcYoungGenerations); final List gcOldGenerations = Collections.singletonList( - "G1 Old Generation"); + "PS MarkSweep"); assertGCMetric(actualMetrics, "jvm.gc.major_collection_count", gcOldGenerations); assertGCMetric(actualMetrics, "jvm.gc.major_collection_time", gcOldGenerations); } @@ -196,7 +194,7 @@ public void testDefaultNewGCMetricsUseG1GC() throws IOException, InterruptedExce @Test public void testDefaultNewGCMetricsUseZGC() throws IOException { try (final SimpleAppContainer container = new SimpleAppContainer( - "eclipse-temurin:21", + "eclipse-temurin:17", "-XX:+UseZGC -Xmx128M -Xms128M", RMI_PORT )){ @@ -232,12 +230,12 @@ public void testDefaultNewGCMetricsUseZGC() throws IOException { } } - @Ignore("Can not force ZGC to work using MisbehavingJMXServer") +// @Ignore("Can not force ZGC to work using MisbehavingJMXServer") @Test public void testDefaultNewGCMetricsUseZGCOld() throws IOException, InterruptedException { try (final MisbehavingJMXServer server = new MisbehavingJMXServer( MisbehavingJMXServer.JDK_17, - "-XX:+UnlockExperimentalVMOptions -XX:+UseZGC", + "-XX:+UseZGC", RMI_PORT, CONTROL_PORT, SUPERVISOR_PORT)) { diff --git a/src/test/java/org/datadog/jmxfetch/util/MisbehavingJMXServer.java b/src/test/java/org/datadog/jmxfetch/util/MisbehavingJMXServer.java index c8f04b0e1..b4bcf0710 100644 --- a/src/test/java/org/datadog/jmxfetch/util/MisbehavingJMXServer.java +++ b/src/test/java/org/datadog/jmxfetch/util/MisbehavingJMXServer.java @@ -23,7 +23,7 @@ public class MisbehavingJMXServer implements Startable { private static final String RMI_PORT = "RMI_PORT"; private static final String CONTROL_PORT = "CONTROL_PORT"; private static final String SUPERVISOR_PORT = "SUPERVISOR_PORT"; - public static final String JAVA_OPTS = "JAVA_OPTS"; + public static final String MISBEHAVING_OPTS = "MISBEHAVING_OPTS"; private final String jdkImage; private final String javaOpts; @@ -57,7 +57,7 @@ public MisbehavingJMXServer( .withEnv(RMI_PORT, String.valueOf(rmiPort)) .withEnv(CONTROL_PORT, String.valueOf(controlPort)) .withEnv(SUPERVISOR_PORT, String.valueOf(supervisorPort)) - .withEnv(JAVA_OPTS, this.javaOpts) + .withEnv(MISBEHAVING_OPTS, this.javaOpts) .waitingFor(Wait.forLogMessage( ".*Supervisor HTTP Server Started. Waiting for initialization payload POST to /init.*", 1)); @@ -65,7 +65,7 @@ public MisbehavingJMXServer( @Override public void start() { - log.info("Starting MisbehavingJMXServer with Docker image '{}' with JAVA_OPTS '{}'", + log.info("Starting MisbehavingJMXServer with Docker image '{}' with MISBEHAVING_OPTS '{}'", this.jdkImage, this.javaOpts); this.server.start(); final String ipAddress = this.getIp(); diff --git a/tools/misbehaving-jmx-server/pom.xml b/tools/misbehaving-jmx-server/pom.xml index 9a8ef20b6..d19bcfd6e 100644 --- a/tools/misbehaving-jmx-server/pom.xml +++ b/tools/misbehaving-jmx-server/pom.xml @@ -53,10 +53,9 @@ - junit - junit - 4.11 - test + org.apache.commons + commons-lang3 + 3.12.0 @@ -64,6 +63,13 @@ snakeyaml ${snakeyaml.version} + + + junit + junit + 4.11 + test + diff --git a/tools/misbehaving-jmx-server/scripts/start.sh b/tools/misbehaving-jmx-server/scripts/start.sh index ccc8a8a89..87ec5a88c 100755 --- a/tools/misbehaving-jmx-server/scripts/start.sh +++ b/tools/misbehaving-jmx-server/scripts/start.sh @@ -4,13 +4,12 @@ set -f echo "Running $@" -[ -n "$JAVA_OPTS" ] || JAVA_OPTS="-Xmx128M -Xms128M" +[ -n "$MISBEHAVING_OPTS" ] || MISBEHAVING_OPTS="-Xmx128M -Xms128M" echo "Using `java --version`" -echo "With JAVA_OPTS '${JAVA_OPTS}'" +echo "With MISBEHAVING_OPTS '${MISBEHAVING_OPTS}'" # shellcheck disable=SC2086 -java \ - ${JAVA_OPTS} \ +java -Xmx64M -Xms64M \ -cp misbehavingjmxserver-1.0-SNAPSHOT-jar-with-dependencies.jar \ "$@" diff --git a/tools/misbehaving-jmx-server/src/main/java/org/datadog/supervisor/App.java b/tools/misbehaving-jmx-server/src/main/java/org/datadog/supervisor/App.java index b9d13933e..907b09d36 100644 --- a/tools/misbehaving-jmx-server/src/main/java/org/datadog/supervisor/App.java +++ b/tools/misbehaving-jmx-server/src/main/java/org/datadog/supervisor/App.java @@ -1,16 +1,15 @@ package org.datadog.supervisor; -import java.io.BufferedReader; import java.io.File; import java.io.IOException; -import java.io.InputStream; -import java.io.InputStreamReader; import java.net.ConnectException; import java.net.HttpURLConnection; import java.net.MalformedURLException; import java.net.URL; import java.util.concurrent.atomic.AtomicBoolean; +import org.apache.commons.lang3.ArrayUtils; +import org.apache.commons.lang3.StringUtils; import org.datadog.Defaults; import lombok.extern.slf4j.Slf4j; @@ -123,12 +122,16 @@ static void stopJMXServer() throws IOException, InterruptedException { } static void startJMXServer() throws IOException { - ProcessBuilder pb = new ProcessBuilder("java", + final String misbehavingOpts = System.getenv("MISBEHAVING_OPTS"); + final String[] extraOpts = misbehavingOpts !=null ? StringUtils.split(misbehavingOpts) : ArrayUtils.EMPTY_STRING_ARRAY; + final String[] command = ArrayUtils.addAll( ArrayUtils.insert(0, extraOpts, "java"), "-cp", selfJarPath, jmxServerEntrypoint, "--rmi-host", App.config.rmiHostname); + log.info("Running JMXServer with command '{}'", ArrayUtils.toString(command)); + final ProcessBuilder pb = new ProcessBuilder(command); pb.inheritIO(); process = pb.start(); running.set(true); From bcea1a32cc40d7ff5a9e4705f62f1231ebd492bf Mon Sep 17 00:00:00 2001 From: Carlos Roman Date: Wed, 20 Dec 2023 09:13:49 +0000 Subject: [PATCH 14/24] Small refactor --- .../org/datadog/jmxfetch/TestGCMetrics.java | 74 +++++++++---------- .../{ => server}/MisbehavingJMXServer.java | 15 ++-- 2 files changed, 44 insertions(+), 45 deletions(-) rename src/test/java/org/datadog/jmxfetch/util/{ => server}/MisbehavingJMXServer.java (94%) diff --git a/src/test/java/org/datadog/jmxfetch/TestGCMetrics.java b/src/test/java/org/datadog/jmxfetch/TestGCMetrics.java index 81bfc15c6..679d64b08 100644 --- a/src/test/java/org/datadog/jmxfetch/TestGCMetrics.java +++ b/src/test/java/org/datadog/jmxfetch/TestGCMetrics.java @@ -16,16 +16,12 @@ import lombok.extern.slf4j.Slf4j; import org.datadog.jmxfetch.reporter.ConsoleReporter; -import org.datadog.jmxfetch.util.MisbehavingJMXServer; +import org.datadog.jmxfetch.util.server.MisbehavingJMXServer; import org.datadog.jmxfetch.util.server.SimpleAppContainer; @Slf4j public class TestGCMetrics extends TestCommon { - private static final int RMI_PORT = 9090; - private static final int CONTROL_PORT = 9091; - private static final int SUPERVISOR_PORT = 9092; - private static boolean isDomainPresent(final String domain, final MBeanServerConnection mbs) { boolean found = false; try { @@ -47,12 +43,12 @@ private static boolean isDomainPresent(final String domain, final MBeanServerCon */ @Test public void testJMXDirectBasic() throws Exception { - try (final MisbehavingJMXServer server = new MisbehavingJMXServer(RMI_PORT, CONTROL_PORT, - SUPERVISOR_PORT)) { + try (final MisbehavingJMXServer server = new MisbehavingJMXServer(MisbehavingJMXServer.DEFAULT_RMI_PORT, MisbehavingJMXServer.DEFAULT_CONTROL_PORT, + MisbehavingJMXServer.DEFAULT_SUPERVISOR_PORT)) { server.start(); final String ipAddress = server.getIp(); final String remoteJmxServiceUrl = String.format( - "service:jmx:rmi:///jndi/rmi://%s:%s/jmxrmi", ipAddress, RMI_PORT); + "service:jmx:rmi:///jndi/rmi://%s:%s/jmxrmi", ipAddress, MisbehavingJMXServer.DEFAULT_RMI_PORT); final JMXServiceURL jmxUrl = new JMXServiceURL(remoteJmxServiceUrl); final JMXConnector conn = JMXConnectorFactory.connect(jmxUrl); final MBeanServerConnection mBeanServerConnection = conn.getMBeanServerConnection(); @@ -62,8 +58,8 @@ public void testJMXDirectBasic() throws Exception { @Test public void testJMXFetchBasic() throws IOException { - try (final MisbehavingJMXServer server = new MisbehavingJMXServer(RMI_PORT, CONTROL_PORT, - SUPERVISOR_PORT)) { + try (final MisbehavingJMXServer server = new MisbehavingJMXServer(MisbehavingJMXServer.DEFAULT_RMI_PORT, MisbehavingJMXServer.DEFAULT_CONTROL_PORT, + MisbehavingJMXServer.DEFAULT_SUPERVISOR_PORT)) { server.start(); final String ipAddress = server.getIp(); this.initApplicationWithYamlLines( @@ -75,7 +71,7 @@ public void testJMXFetchBasic() throws IOException { " host: " + ipAddress, " collect_default_jvm_metrics: false", " max_returned_metrics: 300000", - " port: " + RMI_PORT, + " port: " + MisbehavingJMXServer.DEFAULT_RMI_PORT, " conf:", " - include:", " domain: Bohnanza" @@ -88,11 +84,12 @@ public void testJMXFetchBasic() throws IOException { @Test public void testDefaultOldGC() throws IOException { - try (final MisbehavingJMXServer server = new MisbehavingJMXServer(RMI_PORT, CONTROL_PORT, - SUPERVISOR_PORT)) { + try (final MisbehavingJMXServer server = new MisbehavingJMXServer(MisbehavingJMXServer.DEFAULT_RMI_PORT, MisbehavingJMXServer.DEFAULT_CONTROL_PORT, + MisbehavingJMXServer.DEFAULT_SUPERVISOR_PORT)) { server.start(); final String ipAddress = server.getIp(); - this.initApplicationWithYamlLines("init_config:", + this.initApplicationWithYamlLines( + "init_config:", " is_jmx: true", "", "instances:", @@ -100,7 +97,7 @@ public void testDefaultOldGC() throws IOException { " host: " + ipAddress, " collect_default_jvm_metrics: true", " max_returned_metrics: 300000", - " port: " + RMI_PORT); + " port: " + MisbehavingJMXServer.DEFAULT_RMI_PORT); this.app.doIteration(); final List> actualMetrics = ((ConsoleReporter) appConfig.getReporter()).getMetrics(); List gcGenerations = Arrays.asList( @@ -116,9 +113,9 @@ public void testDefaultNewGCMetricsUseParallelGC() throws IOException, Interrupt try (final MisbehavingJMXServer server = new MisbehavingJMXServer( MisbehavingJMXServer.JDK_11, "-XX:+UseParallelGC", - RMI_PORT, - CONTROL_PORT, - SUPERVISOR_PORT)) { + MisbehavingJMXServer.DEFAULT_RMI_PORT, + MisbehavingJMXServer.DEFAULT_CONTROL_PORT, + MisbehavingJMXServer.DEFAULT_SUPERVISOR_PORT)) { server.start(); final String ipAddress = server.getIp(); this.initApplicationWithYamlLines("init_config:", @@ -130,7 +127,7 @@ public void testDefaultNewGCMetricsUseParallelGC() throws IOException, Interrupt " host: " + ipAddress, " collect_default_jvm_metrics: true", " max_returned_metrics: 300000", - " port: " + RMI_PORT); + " port: " + MisbehavingJMXServer.DEFAULT_RMI_PORT); // Run one iteration first this.app.doIteration(); // And then pull get the metrics or else reporter does not have correct number of metrics @@ -156,21 +153,22 @@ public void testDefaultNewGCMetricsUseG1GC() throws IOException, InterruptedExce try (final MisbehavingJMXServer server = new MisbehavingJMXServer( MisbehavingJMXServer.JDK_17, "-XX:+UseG1GC", - RMI_PORT, - CONTROL_PORT, - SUPERVISOR_PORT)) { + MisbehavingJMXServer.DEFAULT_RMI_PORT, + MisbehavingJMXServer.DEFAULT_CONTROL_PORT, + MisbehavingJMXServer.DEFAULT_SUPERVISOR_PORT)) { server.start(); final String ipAddress = server.getIp(); - this.initApplicationWithYamlLines("init_config:", - " is_jmx: true", - " new_gc_metrics: true", - "", - "instances:", - " - name: jmxint_container", - " host: " + ipAddress, - " collect_default_jvm_metrics: true", - " max_returned_metrics: 300000", - " port: " + RMI_PORT); + this.initApplicationWithYamlLines( + "init_config:", + " is_jmx: true", + " new_gc_metrics: true", + "", + "instances:", + " - name: jmxint_container", + " host: " + ipAddress, + " collect_default_jvm_metrics: true", + " max_returned_metrics: 300000", + " port: " + MisbehavingJMXServer.DEFAULT_RMI_PORT); // Run one iteration first this.app.doIteration(); // And then pull get the metrics or else reporter does not have correct number of metrics @@ -196,7 +194,7 @@ public void testDefaultNewGCMetricsUseZGC() throws IOException { try (final SimpleAppContainer container = new SimpleAppContainer( "eclipse-temurin:17", "-XX:+UseZGC -Xmx128M -Xms128M", - RMI_PORT + MisbehavingJMXServer.DEFAULT_RMI_PORT )){ container.start(); final String ipAddress = container.getIp(); @@ -209,7 +207,7 @@ public void testDefaultNewGCMetricsUseZGC() throws IOException { " host: " + ipAddress, " collect_default_jvm_metrics: true", " max_returned_metrics: 300000", - " port: " + RMI_PORT); + " port: " + MisbehavingJMXServer.DEFAULT_RMI_PORT); // Run one iteration first this.app.doIteration(); // And then pull get the metrics or else reporter does not have correct number of metrics @@ -236,9 +234,9 @@ public void testDefaultNewGCMetricsUseZGCOld() throws IOException, InterruptedEx try (final MisbehavingJMXServer server = new MisbehavingJMXServer( MisbehavingJMXServer.JDK_17, "-XX:+UseZGC", - RMI_PORT, - CONTROL_PORT, - SUPERVISOR_PORT)) { + MisbehavingJMXServer.DEFAULT_RMI_PORT, + MisbehavingJMXServer.DEFAULT_CONTROL_PORT, + MisbehavingJMXServer.DEFAULT_SUPERVISOR_PORT)) { server.start(); final String ipAddress = server.getIp(); this.initApplicationWithYamlLines("init_config:", @@ -250,7 +248,7 @@ public void testDefaultNewGCMetricsUseZGCOld() throws IOException, InterruptedEx " host: " + ipAddress, " collect_default_jvm_metrics: true", " max_returned_metrics: 300000", - " port: " + RMI_PORT); + " port: " + MisbehavingJMXServer.DEFAULT_RMI_PORT); // Run one iteration first this.app.doIteration(); // And then pull get the metrics or else reporter does not have correct number of metrics diff --git a/src/test/java/org/datadog/jmxfetch/util/MisbehavingJMXServer.java b/src/test/java/org/datadog/jmxfetch/util/server/MisbehavingJMXServer.java similarity index 94% rename from src/test/java/org/datadog/jmxfetch/util/MisbehavingJMXServer.java rename to src/test/java/org/datadog/jmxfetch/util/server/MisbehavingJMXServer.java index b4bcf0710..35d5203b5 100644 --- a/src/test/java/org/datadog/jmxfetch/util/MisbehavingJMXServer.java +++ b/src/test/java/org/datadog/jmxfetch/util/server/MisbehavingJMXServer.java @@ -1,21 +1,22 @@ -package org.datadog.jmxfetch.util; +package org.datadog.jmxfetch.util.server; import java.io.IOException; import java.nio.file.Paths; - +import lombok.extern.slf4j.Slf4j; +import org.datadog.jmxfetch.JMXServerControlClient; +import org.datadog.jmxfetch.JMXServerSupervisorClient; import org.testcontainers.containers.GenericContainer; import org.testcontainers.containers.wait.strategy.Wait; import org.testcontainers.images.builder.ImageFromDockerfile; import org.testcontainers.lifecycle.Startable; -import lombok.extern.slf4j.Slf4j; - -import org.datadog.jmxfetch.JMXServerControlClient; -import org.datadog.jmxfetch.JMXServerSupervisorClient; - @Slf4j public class MisbehavingJMXServer implements Startable { + + public static final int DEFAULT_RMI_PORT = 9090; + public static final int DEFAULT_CONTROL_PORT = 9091; + public static final int DEFAULT_SUPERVISOR_PORT = 9092; private static final String DEFAULT_JDK_IMAGE = "base"; public static final String JDK_11 = "eclipse-temurin:11"; public static final String JDK_17 = "eclipse-temurin:17"; From fc3a762b7c7eb55a59a77b4d731770c3c0c18e3b Mon Sep 17 00:00:00 2001 From: Carlos Roman Date: Wed, 20 Dec 2023 09:27:40 +0000 Subject: [PATCH 15/24] Simple tidy up --- .../org/datadog/jmxfetch/TestGCMetrics.java | 99 +++++++------------ .../util/server/SimpleAppContainer.java | 8 ++ 2 files changed, 42 insertions(+), 65 deletions(-) diff --git a/src/test/java/org/datadog/jmxfetch/TestGCMetrics.java b/src/test/java/org/datadog/jmxfetch/TestGCMetrics.java index 679d64b08..dab8467af 100644 --- a/src/test/java/org/datadog/jmxfetch/TestGCMetrics.java +++ b/src/test/java/org/datadog/jmxfetch/TestGCMetrics.java @@ -38,47 +38,56 @@ private static boolean isDomainPresent(final String domain, final MBeanServerCon return found; } - /* - Just here to make sure I've not broken anything - */ @Test public void testJMXDirectBasic() throws Exception { - try (final MisbehavingJMXServer server = new MisbehavingJMXServer(MisbehavingJMXServer.DEFAULT_RMI_PORT, MisbehavingJMXServer.DEFAULT_CONTROL_PORT, - MisbehavingJMXServer.DEFAULT_SUPERVISOR_PORT)) { - server.start(); - final String ipAddress = server.getIp(); + try (final SimpleAppContainer container = new SimpleAppContainer()){ + container.start(); + final String ipAddress = container.getIp(); final String remoteJmxServiceUrl = String.format( - "service:jmx:rmi:///jndi/rmi://%s:%s/jmxrmi", ipAddress, MisbehavingJMXServer.DEFAULT_RMI_PORT); + "service:jmx:rmi:///jndi/rmi://%s:%s/jmxrmi", ipAddress, container.getRMIPort()); final JMXServiceURL jmxUrl = new JMXServiceURL(remoteJmxServiceUrl); final JMXConnector conn = JMXConnectorFactory.connect(jmxUrl); final MBeanServerConnection mBeanServerConnection = conn.getMBeanServerConnection(); - assertThat(isDomainPresent("Bohnanza", mBeanServerConnection), is(true)); + assertThat(isDomainPresent("java.lang", mBeanServerConnection), is(true)); } } @Test - public void testJMXFetchBasic() throws IOException { - try (final MisbehavingJMXServer server = new MisbehavingJMXServer(MisbehavingJMXServer.DEFAULT_RMI_PORT, MisbehavingJMXServer.DEFAULT_CONTROL_PORT, - MisbehavingJMXServer.DEFAULT_SUPERVISOR_PORT)) { - server.start(); - final String ipAddress = server.getIp(); - this.initApplicationWithYamlLines( - "init_config:", + public void testDefaultNewGCMetricsUseZGCSimple() throws IOException { + try (final SimpleAppContainer container = new SimpleAppContainer( + "eclipse-temurin:17", + "-XX:+UseZGC -Xmx128M -Xms128M", + MisbehavingJMXServer.DEFAULT_RMI_PORT + )){ + container.start(); + final String ipAddress = container.getIp(); + this.initApplicationWithYamlLines("init_config:", " is_jmx: true", + " new_gc_metrics: true", "", "instances:", " - name: jmxint_container", " host: " + ipAddress, - " collect_default_jvm_metrics: false", + " collect_default_jvm_metrics: true", " max_returned_metrics: 300000", - " port: " + MisbehavingJMXServer.DEFAULT_RMI_PORT, - " conf:", - " - include:", - " domain: Bohnanza" - ); + " port: " + MisbehavingJMXServer.DEFAULT_RMI_PORT); + // Run one iteration first this.app.doIteration(); - final List> metrics = ((ConsoleReporter) this.appConfig.getReporter()).getMetrics(); - assertThat(metrics, hasSize(1)); + // And then pull get the metrics or else reporter does not have correct number of metrics + ((ConsoleReporter) appConfig.getReporter()).getMetrics(); + + // Actual iteration we care about + this.app.doIteration(); + final List> actualMetrics = ((ConsoleReporter) appConfig.getReporter()).getMetrics(); + assertThat(actualMetrics, hasSize(13)); + final List zgcPause = Collections.singletonList( + "ZGC Pauses"); + assertGCMetric(actualMetrics, "jvm.gc.zgc_pauses_collection_count", zgcPause); + assertGCMetric(actualMetrics, "jvm.gc.zgc_pauses_collection_time", zgcPause); + final List zgcCycles = Collections.singletonList( + "ZGC Cycles"); + assertGCMetric(actualMetrics, "jvm.gc.zgc_cycles_collection_count", zgcCycles); + assertGCMetric(actualMetrics, "jvm.gc.zgc_cycles_collection_time", zgcCycles); } } @@ -188,49 +197,9 @@ public void testDefaultNewGCMetricsUseG1GC() throws IOException, InterruptedExce assertGCMetric(actualMetrics, "jvm.gc.major_collection_time", gcOldGenerations); } } - - @Test - public void testDefaultNewGCMetricsUseZGC() throws IOException { - try (final SimpleAppContainer container = new SimpleAppContainer( - "eclipse-temurin:17", - "-XX:+UseZGC -Xmx128M -Xms128M", - MisbehavingJMXServer.DEFAULT_RMI_PORT - )){ - container.start(); - final String ipAddress = container.getIp(); - this.initApplicationWithYamlLines("init_config:", - " is_jmx: true", - " new_gc_metrics: true", - "", - "instances:", - " - name: jmxint_container", - " host: " + ipAddress, - " collect_default_jvm_metrics: true", - " max_returned_metrics: 300000", - " port: " + MisbehavingJMXServer.DEFAULT_RMI_PORT); - // Run one iteration first - this.app.doIteration(); - // And then pull get the metrics or else reporter does not have correct number of metrics - ((ConsoleReporter) appConfig.getReporter()).getMetrics(); - - // Actual iteration we care about - this.app.doIteration(); - final List> actualMetrics = ((ConsoleReporter) appConfig.getReporter()).getMetrics(); - assertThat(actualMetrics, hasSize(13)); - final List zgcPause = Collections.singletonList( - "ZGC Pauses"); - assertGCMetric(actualMetrics, "jvm.gc.zgc_pauses_collection_count", zgcPause); - assertGCMetric(actualMetrics, "jvm.gc.zgc_pauses_collection_time", zgcPause); - final List zgcCycles = Collections.singletonList( - "ZGC Cycles"); - assertGCMetric(actualMetrics, "jvm.gc.zgc_cycles_collection_count", zgcCycles); - assertGCMetric(actualMetrics, "jvm.gc.zgc_cycles_collection_time", zgcCycles); - } - } -// @Ignore("Can not force ZGC to work using MisbehavingJMXServer") @Test - public void testDefaultNewGCMetricsUseZGCOld() throws IOException, InterruptedException { + public void testDefaultNewGCMetricsUseZGC() throws IOException, InterruptedException { try (final MisbehavingJMXServer server = new MisbehavingJMXServer( MisbehavingJMXServer.JDK_17, "-XX:+UseZGC", diff --git a/src/test/java/org/datadog/jmxfetch/util/server/SimpleAppContainer.java b/src/test/java/org/datadog/jmxfetch/util/server/SimpleAppContainer.java index 7e6980e98..b6702252e 100644 --- a/src/test/java/org/datadog/jmxfetch/util/server/SimpleAppContainer.java +++ b/src/test/java/org/datadog/jmxfetch/util/server/SimpleAppContainer.java @@ -20,6 +20,11 @@ public class SimpleAppContainer implements Startable { private final int rmiPort; private final GenericContainer server; + public SimpleAppContainer() { + this("eclipse-temurin:17", "", MisbehavingJMXServer.DEFAULT_RMI_PORT); + + } + public SimpleAppContainer(final String jreDockerImage, final String javaOpts, final int rmiPort) { this.jreDockerImage = jreDockerImage; this.javaOpts = javaOpts; @@ -57,4 +62,7 @@ public String getIp() { return this.server.getContainerInfo().getNetworkSettings().getIpAddress(); } + public int getRMIPort() { + return this.rmiPort; + } } From 4224dc7ae8f05d0bb2501cceefb3ffe61e3e155b Mon Sep 17 00:00:00 2001 From: Carlos Roman Date: Wed, 20 Dec 2023 14:38:03 +0000 Subject: [PATCH 16/24] Small refactor to use main metrics assert function --- .../java/org/datadog/jmxfetch/TestCommon.java | 84 +++++++++------ .../org/datadog/jmxfetch/TestGCMetrics.java | 101 +++++++++--------- .../util/server/MisbehavingJMXServer.java | 11 ++ .../org/datadog/misbehavingjmxserver/App.java | 3 +- 4 files changed, 110 insertions(+), 89 deletions(-) diff --git a/src/test/java/org/datadog/jmxfetch/TestCommon.java b/src/test/java/org/datadog/jmxfetch/TestCommon.java index edb8bde68..c3b1edf14 100644 --- a/src/test/java/org/datadog/jmxfetch/TestCommon.java +++ b/src/test/java/org/datadog/jmxfetch/TestCommon.java @@ -238,36 +238,23 @@ protected List> getServiceChecks() { return ((ConsoleReporter) appConfig.getReporter()).getServiceChecks(); } - /** - * Assert that a specific metric was collected. Brand the metric so we can easily know which - * metric have/have not been tested - * - * @param name metric name - * @param value metric value - * @param lowerBound lower bound metric value - * @param upperBound upper bound metric value - * @param commonTags metric tags inherited from the instance configuration - * @param additionalTags metric tags inherited from the bean properties - * @param countTags number of metric tags - * @param metricType type of the metric (gauge, histogram, ...) - * @return fail if the metric was not found - */ - public void assertMetric( - String name, - Number value, - Number lowerBound, - Number upperBound, - List commonTags, - List additionalTags, - int countTags, - String metricType) { - List tags = new ArrayList(commonTags); + public static void assertMetric( + String name, + Number value, + Number lowerBound, + Number upperBound, + List commonTags, + List additionalTags, + int countTags, + String metricType, + List> actualMetrics) { + List tags = new ArrayList<>(commonTags); tags.addAll(additionalTags); - for (Map m : metrics) { + for (Map m : actualMetrics) { String mName = (String) (m.get("name")); Double mValue = (Double) (m.get("value")); - Set mTags = new HashSet(Arrays.asList((String[]) (m.get("tags")))); + Set mTags = new HashSet<>(Arrays.asList((String[]) (m.get("tags")))); if (mName.equals(name)) { @@ -295,15 +282,42 @@ public void assertMetric( } } fail( - "Metric assertion failed (name: " - + name - + ", value: " - + value - + ", tags: " - + tags - + ", #tags: " - + countTags - + ")."); + "Metric assertion failed (name: " + + name + + ", value: " + + value + + ", tags: " + + tags + + ", #tags: " + + countTags + + ")."); + + } + + /** + * Assert that a specific metric was collected. Brand the metric so we can easily know which + * metric have/have not been tested + * + * @param name metric name + * @param value metric value + * @param lowerBound lower bound metric value + * @param upperBound upper bound metric value + * @param commonTags metric tags inherited from the instance configuration + * @param additionalTags metric tags inherited from the bean properties + * @param countTags number of metric tags + * @param metricType type of the metric (gauge, histogram, ...) + * @return fail if the metric was not found + */ + public void assertMetric( + String name, + Number value, + Number lowerBound, + Number upperBound, + List commonTags, + List additionalTags, + int countTags, + String metricType) { + assertMetric(name, value, lowerBound, upperBound, commonTags, additionalTags, countTags, metricType, this.metrics); } public void assertMetric( diff --git a/src/test/java/org/datadog/jmxfetch/TestGCMetrics.java b/src/test/java/org/datadog/jmxfetch/TestGCMetrics.java index dab8467af..f40ac8f07 100644 --- a/src/test/java/org/datadog/jmxfetch/TestGCMetrics.java +++ b/src/test/java/org/datadog/jmxfetch/TestGCMetrics.java @@ -60,17 +60,16 @@ public void testDefaultNewGCMetricsUseZGCSimple() throws IOException { MisbehavingJMXServer.DEFAULT_RMI_PORT )){ container.start(); - final String ipAddress = container.getIp(); this.initApplicationWithYamlLines("init_config:", " is_jmx: true", " new_gc_metrics: true", "", "instances:", " - name: jmxint_container", - " host: " + ipAddress, + " host: " + container.getIp(), " collect_default_jvm_metrics: true", " max_returned_metrics: 300000", - " port: " + MisbehavingJMXServer.DEFAULT_RMI_PORT); + " port: " + container.getRMIPort()); // Run one iteration first this.app.doIteration(); // And then pull get the metrics or else reporter does not have correct number of metrics @@ -96,17 +95,22 @@ public void testDefaultOldGC() throws IOException { try (final MisbehavingJMXServer server = new MisbehavingJMXServer(MisbehavingJMXServer.DEFAULT_RMI_PORT, MisbehavingJMXServer.DEFAULT_CONTROL_PORT, MisbehavingJMXServer.DEFAULT_SUPERVISOR_PORT)) { server.start(); - final String ipAddress = server.getIp(); this.initApplicationWithYamlLines( "init_config:", " is_jmx: true", "", "instances:", " - name: jmxint_container", - " host: " + ipAddress, + " host: " + server.getIp(), " collect_default_jvm_metrics: true", " max_returned_metrics: 300000", - " port: " + MisbehavingJMXServer.DEFAULT_RMI_PORT); + " port: " + server.getRMIPort()); + // Run one iteration first + this.app.doIteration(); + // And then pull get the metrics or else reporter does not have correct number of metrics + ((ConsoleReporter) appConfig.getReporter()).getMetrics(); + + // Actual iteration we care about this.app.doIteration(); final List> actualMetrics = ((ConsoleReporter) appConfig.getReporter()).getMetrics(); List gcGenerations = Arrays.asList( @@ -118,25 +122,21 @@ public void testDefaultOldGC() throws IOException { } @Test - public void testDefaultNewGCMetricsUseParallelGC() throws IOException, InterruptedException { + public void testDefaultNewGCMetricsUseParallelGC() throws IOException { try (final MisbehavingJMXServer server = new MisbehavingJMXServer( MisbehavingJMXServer.JDK_11, - "-XX:+UseParallelGC", - MisbehavingJMXServer.DEFAULT_RMI_PORT, - MisbehavingJMXServer.DEFAULT_CONTROL_PORT, - MisbehavingJMXServer.DEFAULT_SUPERVISOR_PORT)) { + "-XX:+UseParallelGC -Xmx128M -Xms128M")) { server.start(); - final String ipAddress = server.getIp(); this.initApplicationWithYamlLines("init_config:", " is_jmx: true", " new_gc_metrics: true", "", "instances:", " - name: jmxint_container", - " host: " + ipAddress, + " host: " + server.getIp(), " collect_default_jvm_metrics: true", " max_returned_metrics: 300000", - " port: " + MisbehavingJMXServer.DEFAULT_RMI_PORT); + " port: " + server.getRMIPort()); // Run one iteration first this.app.doIteration(); // And then pull get the metrics or else reporter does not have correct number of metrics @@ -146,27 +146,19 @@ public void testDefaultNewGCMetricsUseParallelGC() throws IOException, Interrupt this.app.doIteration(); final List> actualMetrics = ((ConsoleReporter) appConfig.getReporter()).getMetrics(); assertThat(actualMetrics, hasSize(13)); - final List gcYoungGenerations = Collections.singletonList( - "PS Scavenge"); - assertGCMetric(actualMetrics, "jvm.gc.minor_collection_count", gcYoungGenerations); - assertGCMetric(actualMetrics, "jvm.gc.minor_collection_time", gcYoungGenerations); - final List gcOldGenerations = Collections.singletonList( - "PS MarkSweep"); - assertGCMetric(actualMetrics, "jvm.gc.major_collection_count", gcOldGenerations); - assertGCMetric(actualMetrics, "jvm.gc.major_collection_time", gcOldGenerations); + assertGCMetric(actualMetrics, "jvm.gc.minor_collection_count", "PS Scavenge", "counter"); + assertGCMetric(actualMetrics, "jvm.gc.minor_collection_time", "PS Scavenge", "counter"); + assertGCMetric(actualMetrics, "jvm.gc.major_collection_count", "PS MarkSweep", "counter"); + assertGCMetric(actualMetrics, "jvm.gc.major_collection_time", "PS MarkSweep", "counter"); } } @Test - public void testDefaultNewGCMetricsUseG1GC() throws IOException, InterruptedException { + public void testDefaultNewGCMetricsUseG1GC() throws IOException { try (final MisbehavingJMXServer server = new MisbehavingJMXServer( MisbehavingJMXServer.JDK_17, - "-XX:+UseG1GC", - MisbehavingJMXServer.DEFAULT_RMI_PORT, - MisbehavingJMXServer.DEFAULT_CONTROL_PORT, - MisbehavingJMXServer.DEFAULT_SUPERVISOR_PORT)) { + "-XX:+UseG1GC -Xmx128M -Xms128M")) { server.start(); - final String ipAddress = server.getIp(); this.initApplicationWithYamlLines( "init_config:", " is_jmx: true", @@ -174,10 +166,10 @@ public void testDefaultNewGCMetricsUseG1GC() throws IOException, InterruptedExce "", "instances:", " - name: jmxint_container", - " host: " + ipAddress, + " host: " + server.getIp(), " collect_default_jvm_metrics: true", " max_returned_metrics: 300000", - " port: " + MisbehavingJMXServer.DEFAULT_RMI_PORT); + " port: " + server.getRMIPort()); // Run one iteration first this.app.doIteration(); // And then pull get the metrics or else reporter does not have correct number of metrics @@ -189,35 +181,29 @@ public void testDefaultNewGCMetricsUseG1GC() throws IOException, InterruptedExce assertThat(actualMetrics, hasSize(13)); final List gcYoungGenerations = Collections.singletonList( "G1 Young Generation"); - assertGCMetric(actualMetrics, "jvm.gc.minor_collection_count", gcYoungGenerations); - assertGCMetric(actualMetrics, "jvm.gc.minor_collection_time", gcYoungGenerations); - final List gcOldGenerations = Collections.singletonList( - "G1 Old Generation"); - assertGCMetric(actualMetrics, "jvm.gc.major_collection_count", gcOldGenerations); - assertGCMetric(actualMetrics, "jvm.gc.major_collection_time", gcOldGenerations); + assertGCMetric(actualMetrics, "jvm.gc.minor_collection_count", "G1 Young Generation", "counter"); + assertGCMetric(actualMetrics, "jvm.gc.minor_collection_time", "G1 Young Generation", "counter"); + assertGCMetric(actualMetrics, "jvm.gc.major_collection_count", "G1 Old Generation", "counter"); + assertGCMetric(actualMetrics, "jvm.gc.major_collection_time", "G1 Old Generation", "counter"); } } @Test - public void testDefaultNewGCMetricsUseZGC() throws IOException, InterruptedException { + public void testDefaultNewGCMetricsUseZGC() throws IOException { try (final MisbehavingJMXServer server = new MisbehavingJMXServer( MisbehavingJMXServer.JDK_17, - "-XX:+UseZGC", - MisbehavingJMXServer.DEFAULT_RMI_PORT, - MisbehavingJMXServer.DEFAULT_CONTROL_PORT, - MisbehavingJMXServer.DEFAULT_SUPERVISOR_PORT)) { + "-XX:+UseZGC -Xmx128M -Xms128M")) { server.start(); - final String ipAddress = server.getIp(); this.initApplicationWithYamlLines("init_config:", " is_jmx: true", " new_gc_metrics: true", "", "instances:", " - name: jmxint_container", - " host: " + ipAddress, + " host: " + server.getIp(), " collect_default_jvm_metrics: true", " max_returned_metrics: 300000", - " port: " + MisbehavingJMXServer.DEFAULT_RMI_PORT); + " port: " + server.getRMIPort()); // Run one iteration first this.app.doIteration(); // And then pull get the metrics or else reporter does not have correct number of metrics @@ -227,16 +213,27 @@ public void testDefaultNewGCMetricsUseZGC() throws IOException, InterruptedExcep this.app.doIteration(); final List> actualMetrics = ((ConsoleReporter) appConfig.getReporter()).getMetrics(); assertThat(actualMetrics, hasSize(13)); - final List zgcPause = Collections.singletonList( - "ZGC Pauses"); - assertGCMetric(actualMetrics, "jvm.gc.zgc_pauses_collection_count", zgcPause); - assertGCMetric(actualMetrics, "jvm.gc.zgc_pauses_collection_time", zgcPause); - final List zgcCycles = Collections.singletonList( - "ZGC Cycles"); - assertGCMetric(actualMetrics, "jvm.gc.zgc_cycles_collection_count", zgcCycles); - assertGCMetric(actualMetrics, "jvm.gc.zgc_cycles_collection_time", zgcCycles); + assertGCMetric(actualMetrics, "jvm.gc.zgc_pauses_collection_count", "ZGC Pauses", "counter"); + assertGCMetric(actualMetrics, "jvm.gc.zgc_pauses_collection_time", "ZGC Pauses", "counter"); + assertGCMetric(actualMetrics, "jvm.gc.zgc_cycles_collection_count", "ZGC Cycles", "counter"); + assertGCMetric(actualMetrics, "jvm.gc.zgc_cycles_collection_time", "ZGC Cycles", "counter"); } } + private static void assertGCMetric(final List> actualMetrics, + final String expectedMetric, + final String gcGeneration, + final String metricType) { + TestCommon.assertMetric( + expectedMetric, + -1, + -1, + 10.0, + Collections.singletonList(String.format("name:%s", gcGeneration)), + Arrays.asList("instance:jmxint_container", "jmx_domain:java.lang", "type:GarbageCollector"), + 5, + metricType, + actualMetrics); + } private static void assertGCMetric(final List> actualMetrics, final String expectedMetric, diff --git a/src/test/java/org/datadog/jmxfetch/util/server/MisbehavingJMXServer.java b/src/test/java/org/datadog/jmxfetch/util/server/MisbehavingJMXServer.java index 35d5203b5..f378132c9 100644 --- a/src/test/java/org/datadog/jmxfetch/util/server/MisbehavingJMXServer.java +++ b/src/test/java/org/datadog/jmxfetch/util/server/MisbehavingJMXServer.java @@ -28,12 +28,18 @@ public class MisbehavingJMXServer implements Startable { private final String jdkImage; private final String javaOpts; + private final int rmiPort; private final int controlPort; private final int supervisorPort; private final GenericContainer server; private JMXServerControlClient controlClient; private JMXServerSupervisorClient supervisorClient; + public MisbehavingJMXServer( + final String jdkImage, + final String javaOpts) { + this(jdkImage, javaOpts, DEFAULT_RMI_PORT, DEFAULT_CONTROL_PORT, DEFAULT_SUPERVISOR_PORT); + } public MisbehavingJMXServer( final int rmiPort, final int controlPort, @@ -48,6 +54,7 @@ public MisbehavingJMXServer( final int controlPort, final int supervisorPort) { this.javaOpts = javaOpts; + this.rmiPort = rmiPort; this.controlPort = controlPort; this.supervisorPort = supervisorPort; this.jdkImage = jdkImage; @@ -101,4 +108,8 @@ public void cutNetwork() throws IOException { public void restoreNetwork() throws IOException { this.controlClient.jmxRestoreNetwork(); } + + public int getRMIPort() { + return this.rmiPort; + } } diff --git a/tools/misbehaving-jmx-server/src/main/java/org/datadog/misbehavingjmxserver/App.java b/tools/misbehaving-jmx-server/src/main/java/org/datadog/misbehavingjmxserver/App.java index 2597dbce5..31cf230a4 100644 --- a/tools/misbehaving-jmx-server/src/main/java/org/datadog/misbehavingjmxserver/App.java +++ b/tools/misbehaving-jmx-server/src/main/java/org/datadog/misbehavingjmxserver/App.java @@ -145,8 +145,7 @@ public class App { private static boolean started = false; final static String testDomain = "Bohnanza"; - public static void main( String[] args ) throws IOException, MalformedObjectNameException, InstanceAlreadyExistsException, MBeanRegistrationException, NotCompliantMBeanException - { + public static void main( String[] args ) throws IOException{ AppConfig config = new AppConfig(); JCommander jCommander = JCommander.newBuilder() From 156f305e0981a80c2c0b68ae75f0ebea89ef6545 Mon Sep 17 00:00:00 2001 From: Carlos Roman Date: Wed, 20 Dec 2023 15:12:34 +0000 Subject: [PATCH 17/24] Refactor to tidy up some metrics asserts --- .../org/datadog/jmxfetch/TestGCMetrics.java | 58 +------------------ .../jmxfetch/TestReconnectContainer.java | 26 +++------ .../datadog/jmxfetch/util/MetricsAssert.java | 33 +++++++++++ 3 files changed, 43 insertions(+), 74 deletions(-) create mode 100644 src/test/java/org/datadog/jmxfetch/util/MetricsAssert.java diff --git a/src/test/java/org/datadog/jmxfetch/TestGCMetrics.java b/src/test/java/org/datadog/jmxfetch/TestGCMetrics.java index f40ac8f07..12ca28240 100644 --- a/src/test/java/org/datadog/jmxfetch/TestGCMetrics.java +++ b/src/test/java/org/datadog/jmxfetch/TestGCMetrics.java @@ -1,5 +1,7 @@ package org.datadog.jmxfetch; +import static org.datadog.jmxfetch.util.MetricsAssert.assertDomainPresent; +import static org.datadog.jmxfetch.util.MetricsAssert.isDomainPresent; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.*; @@ -22,22 +24,6 @@ @Slf4j public class TestGCMetrics extends TestCommon { - private static boolean isDomainPresent(final String domain, final MBeanServerConnection mbs) { - boolean found = false; - try { - final String[] domains = mbs.getDomains(); - for (String s : domains) { - if(s.equals(domain)) { - found = true; - break; - } - } - } catch (IOException e) { - log.warn("Got an exception checking if domain is present", e); - } - return found; - } - @Test public void testJMXDirectBasic() throws Exception { try (final SimpleAppContainer container = new SimpleAppContainer()){ @@ -48,45 +34,7 @@ public void testJMXDirectBasic() throws Exception { final JMXServiceURL jmxUrl = new JMXServiceURL(remoteJmxServiceUrl); final JMXConnector conn = JMXConnectorFactory.connect(jmxUrl); final MBeanServerConnection mBeanServerConnection = conn.getMBeanServerConnection(); - assertThat(isDomainPresent("java.lang", mBeanServerConnection), is(true)); - } - } - - @Test - public void testDefaultNewGCMetricsUseZGCSimple() throws IOException { - try (final SimpleAppContainer container = new SimpleAppContainer( - "eclipse-temurin:17", - "-XX:+UseZGC -Xmx128M -Xms128M", - MisbehavingJMXServer.DEFAULT_RMI_PORT - )){ - container.start(); - this.initApplicationWithYamlLines("init_config:", - " is_jmx: true", - " new_gc_metrics: true", - "", - "instances:", - " - name: jmxint_container", - " host: " + container.getIp(), - " collect_default_jvm_metrics: true", - " max_returned_metrics: 300000", - " port: " + container.getRMIPort()); - // Run one iteration first - this.app.doIteration(); - // And then pull get the metrics or else reporter does not have correct number of metrics - ((ConsoleReporter) appConfig.getReporter()).getMetrics(); - - // Actual iteration we care about - this.app.doIteration(); - final List> actualMetrics = ((ConsoleReporter) appConfig.getReporter()).getMetrics(); - assertThat(actualMetrics, hasSize(13)); - final List zgcPause = Collections.singletonList( - "ZGC Pauses"); - assertGCMetric(actualMetrics, "jvm.gc.zgc_pauses_collection_count", zgcPause); - assertGCMetric(actualMetrics, "jvm.gc.zgc_pauses_collection_time", zgcPause); - final List zgcCycles = Collections.singletonList( - "ZGC Cycles"); - assertGCMetric(actualMetrics, "jvm.gc.zgc_cycles_collection_count", zgcCycles); - assertGCMetric(actualMetrics, "jvm.gc.zgc_cycles_collection_time", zgcCycles); + assertDomainPresent("java.lang", mBeanServerConnection); } } diff --git a/src/test/java/org/datadog/jmxfetch/TestReconnectContainer.java b/src/test/java/org/datadog/jmxfetch/TestReconnectContainer.java index ace89ccdf..6e8dc6d71 100644 --- a/src/test/java/org/datadog/jmxfetch/TestReconnectContainer.java +++ b/src/test/java/org/datadog/jmxfetch/TestReconnectContainer.java @@ -1,6 +1,9 @@ package org.datadog.jmxfetch; +import static org.datadog.jmxfetch.util.MetricsAssert.isDomainPresent; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; import java.io.IOException; import java.util.Collections; @@ -38,21 +41,6 @@ public class TestReconnectContainer extends TestCommon { private JMXServerSupervisorClient supervisorClient; private static Slf4jLogConsumer logConsumer = new Slf4jLogConsumer(log); - private static boolean isDomainPresent(String domain, MBeanServerConnection mbs) { - boolean found = false; - try { - String[] domains = mbs.getDomains(); - for (int i = 0; i < domains.length; i++) { - if (domains[i].equals(domain)) { - found = true; - } - } - } catch (IOException e) { - found = false; - } - return found; - } - private static ImageFromDockerfile img = new ImageFromDockerfile() .withFileFromPath(".", Paths.get("./tools/misbehaving-jmx-server/")); @@ -100,7 +88,7 @@ public void testJMXDirectBasic() throws Exception { JMXConnector conn = JMXConnectorFactory.connect(jmxUrl); MBeanServerConnection mBeanServerConnection = conn.getMBeanServerConnection(); - assertEquals(true, isDomainPresent("Bohnanza", mBeanServerConnection)); + assertTrue(isDomainPresent("Bohnanza", mBeanServerConnection)); } @Test @@ -116,15 +104,15 @@ public void testJMXDirectReconnect() throws Exception { JMXConnector conn = JMXConnectorFactory.connect(jmxUrl); MBeanServerConnection mBeanServerConnection = conn.getMBeanServerConnection(); - assertEquals(true, isDomainPresent("Bohnanza", mBeanServerConnection)); + assertTrue(isDomainPresent("Bohnanza", mBeanServerConnection)); this.controlClient.jmxCutNetwork(); - assertEquals(false, isDomainPresent("Bohnanza", mBeanServerConnection)); + assertFalse(isDomainPresent("Bohnanza", mBeanServerConnection)); this.controlClient.jmxRestoreNetwork(); - assertEquals(true, isDomainPresent("Bohnanza", mBeanServerConnection)); + assertTrue(isDomainPresent("Bohnanza", mBeanServerConnection)); } @Test diff --git a/src/test/java/org/datadog/jmxfetch/util/MetricsAssert.java b/src/test/java/org/datadog/jmxfetch/util/MetricsAssert.java new file mode 100644 index 000000000..1b4049752 --- /dev/null +++ b/src/test/java/org/datadog/jmxfetch/util/MetricsAssert.java @@ -0,0 +1,33 @@ +package org.datadog.jmxfetch.util; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.equalTo; + +import java.io.IOException; +import javax.management.MBeanServerConnection; +import lombok.extern.slf4j.Slf4j; + +@Slf4j +public class MetricsAssert { + + public static void assertDomainPresent(final String domain, final MBeanServerConnection mbs){ + assertThat(String.format("Could not find domain '%s'", domain), + isDomainPresent(domain, mbs), equalTo(true)); + } + + public static boolean isDomainPresent(final String domain, final MBeanServerConnection mbs) { + boolean found = false; + try { + final String[] domains = mbs.getDomains(); + for (String s : domains) { + if(s.equals(domain)) { + found = true; + break; + } + } + } catch (IOException e) { + log.warn("Got an exception checking if domain is present", e); + } + return found; + } +} From 90835f9b84e9cd5d36b3a086c5baa6211c40f441 Mon Sep 17 00:00:00 2001 From: Carlos Roman Date: Wed, 20 Dec 2023 15:15:55 +0000 Subject: [PATCH 18/24] Got TestReconnectContainer to use helper functions --- .../org/datadog/jmxfetch/TestReconnectContainer.java | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/src/test/java/org/datadog/jmxfetch/TestReconnectContainer.java b/src/test/java/org/datadog/jmxfetch/TestReconnectContainer.java index 6e8dc6d71..1c964529e 100644 --- a/src/test/java/org/datadog/jmxfetch/TestReconnectContainer.java +++ b/src/test/java/org/datadog/jmxfetch/TestReconnectContainer.java @@ -1,9 +1,10 @@ package org.datadog.jmxfetch; -import static org.datadog.jmxfetch.util.MetricsAssert.isDomainPresent; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertTrue; + +import static org.datadog.jmxfetch.util.MetricsAssert.assertDomainPresent; +import static org.datadog.jmxfetch.util.MetricsAssert.isDomainPresent; import java.io.IOException; import java.util.Collections; @@ -88,7 +89,7 @@ public void testJMXDirectBasic() throws Exception { JMXConnector conn = JMXConnectorFactory.connect(jmxUrl); MBeanServerConnection mBeanServerConnection = conn.getMBeanServerConnection(); - assertTrue(isDomainPresent("Bohnanza", mBeanServerConnection)); + assertDomainPresent("Bohnanza", mBeanServerConnection); } @Test @@ -104,7 +105,7 @@ public void testJMXDirectReconnect() throws Exception { JMXConnector conn = JMXConnectorFactory.connect(jmxUrl); MBeanServerConnection mBeanServerConnection = conn.getMBeanServerConnection(); - assertTrue(isDomainPresent("Bohnanza", mBeanServerConnection)); + assertDomainPresent("Bohnanza", mBeanServerConnection); this.controlClient.jmxCutNetwork(); @@ -112,7 +113,7 @@ public void testJMXDirectReconnect() throws Exception { this.controlClient.jmxRestoreNetwork(); - assertTrue(isDomainPresent("Bohnanza", mBeanServerConnection)); + assertDomainPresent("Bohnanza", mBeanServerConnection); } @Test From 2484b72b122092093e6be8a32d7a671f138fb0b7 Mon Sep 17 00:00:00 2001 From: Carlos Roman Date: Wed, 20 Dec 2023 18:01:12 +0000 Subject: [PATCH 19/24] Refactor + adding UseConcMarkSweepGC test --- .../java/org/datadog/jmxfetch/TestCommon.java | 72 +--------- .../org/datadog/jmxfetch/TestGCMetrics.java | 127 +++++++----------- .../datadog/jmxfetch/util/MetricsAssert.java | 66 +++++++++ 3 files changed, 117 insertions(+), 148 deletions(-) diff --git a/src/test/java/org/datadog/jmxfetch/TestCommon.java b/src/test/java/org/datadog/jmxfetch/TestCommon.java index c3b1edf14..49fb77a12 100644 --- a/src/test/java/org/datadog/jmxfetch/TestCommon.java +++ b/src/test/java/org/datadog/jmxfetch/TestCommon.java @@ -1,9 +1,5 @@ package org.datadog.jmxfetch; -import static org.hamcrest.CoreMatchers.hasItem; -import static org.hamcrest.MatcherAssert.assertThat; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; import static org.mockito.Mockito.spy; import static org.mockito.Mockito.when; @@ -19,11 +15,8 @@ import java.nio.file.Files; import java.nio.file.Path; import java.util.ArrayList; -import java.util.Arrays; -import java.util.HashSet; import java.util.List; import java.util.Map; -import java.util.Set; import javax.management.InstanceAlreadyExistsException; import javax.management.InstanceNotFoundException; import javax.management.MBeanRegistrationException; @@ -31,12 +24,15 @@ import javax.management.MalformedObjectNameException; import javax.management.NotCompliantMBeanException; import javax.management.ObjectName; + +import org.junit.After; +import org.junit.BeforeClass; + import org.datadog.jmxfetch.reporter.ConsoleReporter; import org.datadog.jmxfetch.reporter.Reporter; import org.datadog.jmxfetch.util.CustomLogger; +import org.datadog.jmxfetch.util.MetricsAssert; import org.datadog.jmxfetch.util.LogLevel; -import org.junit.After; -import org.junit.BeforeClass; final class ConfigUtil { public static Path writeConfigYamlToTemp(String content, String yamlName) throws IOException { @@ -238,62 +234,6 @@ protected List> getServiceChecks() { return ((ConsoleReporter) appConfig.getReporter()).getServiceChecks(); } - public static void assertMetric( - String name, - Number value, - Number lowerBound, - Number upperBound, - List commonTags, - List additionalTags, - int countTags, - String metricType, - List> actualMetrics) { - List tags = new ArrayList<>(commonTags); - tags.addAll(additionalTags); - - for (Map m : actualMetrics) { - String mName = (String) (m.get("name")); - Double mValue = (Double) (m.get("value")); - Set mTags = new HashSet<>(Arrays.asList((String[]) (m.get("tags")))); - - if (mName.equals(name)) { - - if (!value.equals(-1)) { - assertEquals((Double) value.doubleValue(), mValue); - } else if (!lowerBound.equals(-1) || !upperBound.equals(-1)) { - assertTrue(mValue > (Double) lowerBound.doubleValue()); - assertTrue(mValue < (Double) upperBound.doubleValue()); - } - - if (countTags != -1) { - assertEquals(countTags, mTags.size()); - } - for (String t : tags) { - assertThat(mTags, hasItem(t)); - } - - if (metricType != null) { - assertEquals(metricType, m.get("type")); - } - // Brand the metric - m.put("tested", true); - - return; - } - } - fail( - "Metric assertion failed (name: " - + name - + ", value: " - + value - + ", tags: " - + tags - + ", #tags: " - + countTags - + ")."); - - } - /** * Assert that a specific metric was collected. Brand the metric so we can easily know which * metric have/have not been tested @@ -317,7 +257,7 @@ public void assertMetric( List additionalTags, int countTags, String metricType) { - assertMetric(name, value, lowerBound, upperBound, commonTags, additionalTags, countTags, metricType, this.metrics); + MetricsAssert.assertMetric(name, value, lowerBound, upperBound, commonTags, additionalTags, countTags, metricType, this.metrics); } public void assertMetric( diff --git a/src/test/java/org/datadog/jmxfetch/TestGCMetrics.java b/src/test/java/org/datadog/jmxfetch/TestGCMetrics.java index 12ca28240..de3ef1836 100644 --- a/src/test/java/org/datadog/jmxfetch/TestGCMetrics.java +++ b/src/test/java/org/datadog/jmxfetch/TestGCMetrics.java @@ -1,7 +1,6 @@ package org.datadog.jmxfetch; import static org.datadog.jmxfetch.util.MetricsAssert.assertDomainPresent; -import static org.datadog.jmxfetch.util.MetricsAssert.isDomainPresent; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.*; @@ -20,6 +19,7 @@ import org.datadog.jmxfetch.reporter.ConsoleReporter; import org.datadog.jmxfetch.util.server.MisbehavingJMXServer; import org.datadog.jmxfetch.util.server.SimpleAppContainer; +import org.datadog.jmxfetch.util.MetricsAssert; @Slf4j public class TestGCMetrics extends TestCommon { @@ -42,25 +42,7 @@ public void testJMXDirectBasic() throws Exception { public void testDefaultOldGC() throws IOException { try (final MisbehavingJMXServer server = new MisbehavingJMXServer(MisbehavingJMXServer.DEFAULT_RMI_PORT, MisbehavingJMXServer.DEFAULT_CONTROL_PORT, MisbehavingJMXServer.DEFAULT_SUPERVISOR_PORT)) { - server.start(); - this.initApplicationWithYamlLines( - "init_config:", - " is_jmx: true", - "", - "instances:", - " - name: jmxint_container", - " host: " + server.getIp(), - " collect_default_jvm_metrics: true", - " max_returned_metrics: 300000", - " port: " + server.getRMIPort()); - // Run one iteration first - this.app.doIteration(); - // And then pull get the metrics or else reporter does not have correct number of metrics - ((ConsoleReporter) appConfig.getReporter()).getMetrics(); - - // Actual iteration we care about - this.app.doIteration(); - final List> actualMetrics = ((ConsoleReporter) appConfig.getReporter()).getMetrics(); + final List> actualMetrics = startAngGetMetrics(server, false); List gcGenerations = Arrays.asList( "G1 Old Generation", "G1 Young Generation"); @@ -74,25 +56,7 @@ public void testDefaultNewGCMetricsUseParallelGC() throws IOException { try (final MisbehavingJMXServer server = new MisbehavingJMXServer( MisbehavingJMXServer.JDK_11, "-XX:+UseParallelGC -Xmx128M -Xms128M")) { - server.start(); - this.initApplicationWithYamlLines("init_config:", - " is_jmx: true", - " new_gc_metrics: true", - "", - "instances:", - " - name: jmxint_container", - " host: " + server.getIp(), - " collect_default_jvm_metrics: true", - " max_returned_metrics: 300000", - " port: " + server.getRMIPort()); - // Run one iteration first - this.app.doIteration(); - // And then pull get the metrics or else reporter does not have correct number of metrics - ((ConsoleReporter) appConfig.getReporter()).getMetrics(); - - // Actual iteration we care about - this.app.doIteration(); - final List> actualMetrics = ((ConsoleReporter) appConfig.getReporter()).getMetrics(); + final List> actualMetrics = startAngGetMetrics(server, true); assertThat(actualMetrics, hasSize(13)); assertGCMetric(actualMetrics, "jvm.gc.minor_collection_count", "PS Scavenge", "counter"); assertGCMetric(actualMetrics, "jvm.gc.minor_collection_time", "PS Scavenge", "counter"); @@ -101,65 +65,40 @@ public void testDefaultNewGCMetricsUseParallelGC() throws IOException { } } + @Test + public void testDefaultNewGCMetricsUseConcMarkSweepGC() throws IOException { + try (final MisbehavingJMXServer server = new MisbehavingJMXServer( + MisbehavingJMXServer.JDK_11, + "-XX:+UseConcMarkSweepGC -Xmx128M -Xms128M")) { + final List> actualMetrics = startAngGetMetrics(server, true); + assertThat(actualMetrics, hasSize(13)); + assertGCMetric(actualMetrics, "jvm.gc.minor_collection_count", "ParNew", "counter"); + assertGCMetric(actualMetrics, "jvm.gc.minor_collection_time", "ParNew", "counter"); + assertGCMetric(actualMetrics, "jvm.gc.major_collection_count", "ConcurrentMarkSweep", "counter"); + assertGCMetric(actualMetrics, "jvm.gc.major_collection_time", "ConcurrentMarkSweep", "counter"); + } + } + @Test public void testDefaultNewGCMetricsUseG1GC() throws IOException { try (final MisbehavingJMXServer server = new MisbehavingJMXServer( MisbehavingJMXServer.JDK_17, "-XX:+UseG1GC -Xmx128M -Xms128M")) { - server.start(); - this.initApplicationWithYamlLines( - "init_config:", - " is_jmx: true", - " new_gc_metrics: true", - "", - "instances:", - " - name: jmxint_container", - " host: " + server.getIp(), - " collect_default_jvm_metrics: true", - " max_returned_metrics: 300000", - " port: " + server.getRMIPort()); - // Run one iteration first - this.app.doIteration(); - // And then pull get the metrics or else reporter does not have correct number of metrics - ((ConsoleReporter) appConfig.getReporter()).getMetrics(); - - // Actual iteration we care about - this.app.doIteration(); - final List> actualMetrics = ((ConsoleReporter) appConfig.getReporter()).getMetrics(); + final List> actualMetrics = startAngGetMetrics(server, true); assertThat(actualMetrics, hasSize(13)); - final List gcYoungGenerations = Collections.singletonList( - "G1 Young Generation"); assertGCMetric(actualMetrics, "jvm.gc.minor_collection_count", "G1 Young Generation", "counter"); assertGCMetric(actualMetrics, "jvm.gc.minor_collection_time", "G1 Young Generation", "counter"); assertGCMetric(actualMetrics, "jvm.gc.major_collection_count", "G1 Old Generation", "counter"); assertGCMetric(actualMetrics, "jvm.gc.major_collection_time", "G1 Old Generation", "counter"); } } - + @Test public void testDefaultNewGCMetricsUseZGC() throws IOException { try (final MisbehavingJMXServer server = new MisbehavingJMXServer( MisbehavingJMXServer.JDK_17, "-XX:+UseZGC -Xmx128M -Xms128M")) { - server.start(); - this.initApplicationWithYamlLines("init_config:", - " is_jmx: true", - " new_gc_metrics: true", - "", - "instances:", - " - name: jmxint_container", - " host: " + server.getIp(), - " collect_default_jvm_metrics: true", - " max_returned_metrics: 300000", - " port: " + server.getRMIPort()); - // Run one iteration first - this.app.doIteration(); - // And then pull get the metrics or else reporter does not have correct number of metrics - ((ConsoleReporter) appConfig.getReporter()).getMetrics(); - - // Actual iteration we care about - this.app.doIteration(); - final List> actualMetrics = ((ConsoleReporter) appConfig.getReporter()).getMetrics(); + final List> actualMetrics = startAngGetMetrics(server, true); assertThat(actualMetrics, hasSize(13)); assertGCMetric(actualMetrics, "jvm.gc.zgc_pauses_collection_count", "ZGC Pauses", "counter"); assertGCMetric(actualMetrics, "jvm.gc.zgc_pauses_collection_time", "ZGC Pauses", "counter"); @@ -167,11 +106,35 @@ public void testDefaultNewGCMetricsUseZGC() throws IOException { assertGCMetric(actualMetrics, "jvm.gc.zgc_cycles_collection_time", "ZGC Cycles", "counter"); } } + + private List> startAngGetMetrics(final MisbehavingJMXServer server, final boolean newGCMetrics) throws IOException { + server.start(); + this.initApplicationWithYamlLines( + "init_config:", + " is_jmx: true", + " new_gc_metrics: " + newGCMetrics, + "", + "instances:", + " - name: jmxint_container", + " host: " + server.getIp(), + " collect_default_jvm_metrics: true", + " max_returned_metrics: 300000", + " port: " + server.getRMIPort()); + // Run one iteration first + this.app.doIteration(); + // And then pull get the metrics or else reporter does not have correct number of metrics + ((ConsoleReporter) appConfig.getReporter()).getMetrics(); + + // Actual iteration we care about + this.app.doIteration(); + return ((ConsoleReporter) appConfig.getReporter()).getMetrics(); + } + private static void assertGCMetric(final List> actualMetrics, final String expectedMetric, final String gcGeneration, final String metricType) { - TestCommon.assertMetric( + MetricsAssert.assertMetric( expectedMetric, -1, -1, diff --git a/src/test/java/org/datadog/jmxfetch/util/MetricsAssert.java b/src/test/java/org/datadog/jmxfetch/util/MetricsAssert.java index 1b4049752..41ca0a8ad 100644 --- a/src/test/java/org/datadog/jmxfetch/util/MetricsAssert.java +++ b/src/test/java/org/datadog/jmxfetch/util/MetricsAssert.java @@ -1,15 +1,81 @@ package org.datadog.jmxfetch.util; +import static org.hamcrest.CoreMatchers.hasItem; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.equalTo; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; import java.io.IOException; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.HashSet; +import java.util.List; +import java.util.Map; +import java.util.Set; import javax.management.MBeanServerConnection; import lombok.extern.slf4j.Slf4j; @Slf4j public class MetricsAssert { + public static void assertMetric( + String name, + Number value, + Number lowerBound, + Number upperBound, + List commonTags, + List additionalTags, + int countTags, + String metricType, + List> actualMetrics) { + List tags = new ArrayList<>(commonTags); + tags.addAll(additionalTags); + + for (Map m : actualMetrics) { + String mName = (String) (m.get("name")); + Double mValue = (Double) (m.get("value")); + Set mTags = new HashSet<>(Arrays.asList((String[]) (m.get("tags")))); + + if (mName.equals(name)) { + + if (!value.equals(-1)) { + assertEquals((Double) value.doubleValue(), mValue); + } else if (!lowerBound.equals(-1) || !upperBound.equals(-1)) { + assertTrue(mValue > (Double) lowerBound.doubleValue()); + assertTrue(mValue < (Double) upperBound.doubleValue()); + } + + if (countTags != -1) { + assertEquals(countTags, mTags.size()); + } + for (String t : tags) { + assertThat(mTags, hasItem(t)); + } + + if (metricType != null) { + assertEquals(metricType, m.get("type")); + } + // Brand the metric + m.put("tested", true); + + return; + } + } + fail( + "Metric assertion failed (name: " + + name + + ", value: " + + value + + ", tags: " + + tags + + ", #tags: " + + countTags + + ")."); + + } + public static void assertDomainPresent(final String domain, final MBeanServerConnection mbs){ assertThat(String.format("Could not find domain '%s'", domain), isDomainPresent(domain, mbs), equalTo(true)); From 5ca171998369eda1eb56263b24158d4ebc8521ed Mon Sep 17 00:00:00 2001 From: Carlos Roman Date: Thu, 21 Dec 2023 10:44:31 +0000 Subject: [PATCH 20/24] Small refactor to remove some magic strings --- .../org/datadog/jmxfetch/TestGCMetrics.java | 114 +++++++++++------- .../util/server/MisbehavingJMXServer.java | 89 +++++++++++--- .../jmxfetch/util/server/JDKImage.java | 19 +++ 3 files changed, 159 insertions(+), 63 deletions(-) create mode 100644 src/test/java/org/datadog/jmxfetch/util/server/app/org/datadog/jmxfetch/util/server/JDKImage.java diff --git a/src/test/java/org/datadog/jmxfetch/TestGCMetrics.java b/src/test/java/org/datadog/jmxfetch/TestGCMetrics.java index de3ef1836..13016b6b5 100644 --- a/src/test/java/org/datadog/jmxfetch/TestGCMetrics.java +++ b/src/test/java/org/datadog/jmxfetch/TestGCMetrics.java @@ -1,32 +1,43 @@ package org.datadog.jmxfetch; import static org.datadog.jmxfetch.util.MetricsAssert.assertDomainPresent; +import static org.datadog.jmxfetch.util.server.app.org.datadog.jmxfetch.util.server.JDKImage.JDK_11; +import static org.datadog.jmxfetch.util.server.app.org.datadog.jmxfetch.util.server.JDKImage.JDK_17; import static org.hamcrest.MatcherAssert.assertThat; -import static org.hamcrest.Matchers.*; +import static org.hamcrest.Matchers.empty; +import static org.hamcrest.Matchers.hasItems; +import static org.hamcrest.Matchers.hasSize; +import static org.hamcrest.Matchers.is; +import static org.hamcrest.Matchers.not; import java.io.IOException; -import java.util.*; - +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collections; +import java.util.HashSet; +import java.util.List; +import java.util.Map; +import java.util.Set; import javax.management.MBeanServerConnection; import javax.management.remote.JMXConnector; import javax.management.remote.JMXConnectorFactory; import javax.management.remote.JMXServiceURL; -import org.junit.Test; - import lombok.extern.slf4j.Slf4j; +import org.junit.Test; + import org.datadog.jmxfetch.reporter.ConsoleReporter; +import org.datadog.jmxfetch.util.MetricsAssert; import org.datadog.jmxfetch.util.server.MisbehavingJMXServer; import org.datadog.jmxfetch.util.server.SimpleAppContainer; -import org.datadog.jmxfetch.util.MetricsAssert; @Slf4j public class TestGCMetrics extends TestCommon { @Test public void testJMXDirectBasic() throws Exception { - try (final SimpleAppContainer container = new SimpleAppContainer()){ + try (final SimpleAppContainer container = new SimpleAppContainer()) { container.start(); final String ipAddress = container.getIp(); final String remoteJmxServiceUrl = String.format( @@ -40,9 +51,8 @@ public void testJMXDirectBasic() throws Exception { @Test public void testDefaultOldGC() throws IOException { - try (final MisbehavingJMXServer server = new MisbehavingJMXServer(MisbehavingJMXServer.DEFAULT_RMI_PORT, MisbehavingJMXServer.DEFAULT_CONTROL_PORT, - MisbehavingJMXServer.DEFAULT_SUPERVISOR_PORT)) { - final List> actualMetrics = startAngGetMetrics(server, false); + try (final MisbehavingJMXServer server = new MisbehavingJMXServer.Builder().build()) { + final List> actualMetrics = startAndGetMetrics(server, false); List gcGenerations = Arrays.asList( "G1 Old Generation", "G1 Young Generation"); @@ -53,61 +63,74 @@ public void testDefaultOldGC() throws IOException { @Test public void testDefaultNewGCMetricsUseParallelGC() throws IOException { - try (final MisbehavingJMXServer server = new MisbehavingJMXServer( - MisbehavingJMXServer.JDK_11, - "-XX:+UseParallelGC -Xmx128M -Xms128M")) { - final List> actualMetrics = startAngGetMetrics(server, true); + try (final MisbehavingJMXServer server = new MisbehavingJMXServer.Builder().withJDKImage( + JDK_11).appendJavaOpts("-XX:+UseParallelGC").build()) { + final List> actualMetrics = startAndGetMetrics(server, true); assertThat(actualMetrics, hasSize(13)); - assertGCMetric(actualMetrics, "jvm.gc.minor_collection_count", "PS Scavenge", "counter"); - assertGCMetric(actualMetrics, "jvm.gc.minor_collection_time", "PS Scavenge", "counter"); - assertGCMetric(actualMetrics, "jvm.gc.major_collection_count", "PS MarkSweep", "counter"); - assertGCMetric(actualMetrics, "jvm.gc.major_collection_time", "PS MarkSweep", "counter"); + assertGCMetric(actualMetrics, + "jvm.gc.minor_collection_count", "PS Scavenge", "counter"); + assertGCMetric(actualMetrics, + "jvm.gc.minor_collection_time", "PS Scavenge", "counter"); + assertGCMetric(actualMetrics, + "jvm.gc.major_collection_count", "PS MarkSweep", "counter"); + assertGCMetric(actualMetrics, + "jvm.gc.major_collection_time", "PS MarkSweep", "counter"); } } @Test public void testDefaultNewGCMetricsUseConcMarkSweepGC() throws IOException { - try (final MisbehavingJMXServer server = new MisbehavingJMXServer( - MisbehavingJMXServer.JDK_11, - "-XX:+UseConcMarkSweepGC -Xmx128M -Xms128M")) { - final List> actualMetrics = startAngGetMetrics(server, true); + try (final MisbehavingJMXServer server = new MisbehavingJMXServer.Builder().withJDKImage( + JDK_11).appendJavaOpts("-XX:+UseConcMarkSweepGC").build()) { + final List> actualMetrics = startAndGetMetrics(server, true); assertThat(actualMetrics, hasSize(13)); - assertGCMetric(actualMetrics, "jvm.gc.minor_collection_count", "ParNew", "counter"); - assertGCMetric(actualMetrics, "jvm.gc.minor_collection_time", "ParNew", "counter"); - assertGCMetric(actualMetrics, "jvm.gc.major_collection_count", "ConcurrentMarkSweep", "counter"); - assertGCMetric(actualMetrics, "jvm.gc.major_collection_time", "ConcurrentMarkSweep", "counter"); + assertGCMetric(actualMetrics, + "jvm.gc.minor_collection_count", "ParNew", "counter"); + assertGCMetric(actualMetrics, + "jvm.gc.minor_collection_time", "ParNew", "counter"); + assertGCMetric(actualMetrics, + "jvm.gc.major_collection_count", "ConcurrentMarkSweep", "counter"); + assertGCMetric(actualMetrics, + "jvm.gc.major_collection_time", "ConcurrentMarkSweep", "counter"); } } @Test public void testDefaultNewGCMetricsUseG1GC() throws IOException { - try (final MisbehavingJMXServer server = new MisbehavingJMXServer( - MisbehavingJMXServer.JDK_17, - "-XX:+UseG1GC -Xmx128M -Xms128M")) { - final List> actualMetrics = startAngGetMetrics(server, true); + try (final MisbehavingJMXServer server = new MisbehavingJMXServer.Builder().withJDKImage( + JDK_17).appendJavaOpts("-XX:+UseG1GC").build()) { + final List> actualMetrics = startAndGetMetrics(server, true); assertThat(actualMetrics, hasSize(13)); - assertGCMetric(actualMetrics, "jvm.gc.minor_collection_count", "G1 Young Generation", "counter"); - assertGCMetric(actualMetrics, "jvm.gc.minor_collection_time", "G1 Young Generation", "counter"); - assertGCMetric(actualMetrics, "jvm.gc.major_collection_count", "G1 Old Generation", "counter"); - assertGCMetric(actualMetrics, "jvm.gc.major_collection_time", "G1 Old Generation", "counter"); + assertGCMetric(actualMetrics, + "jvm.gc.minor_collection_count", "G1 Young Generation", "counter"); + assertGCMetric(actualMetrics, + "jvm.gc.minor_collection_time", "G1 Young Generation", "counter"); + assertGCMetric(actualMetrics, + "jvm.gc.major_collection_count", "G1 Old Generation", "counter"); + assertGCMetric(actualMetrics, + "jvm.gc.major_collection_time", "G1 Old Generation", "counter"); } } @Test public void testDefaultNewGCMetricsUseZGC() throws IOException { - try (final MisbehavingJMXServer server = new MisbehavingJMXServer( - MisbehavingJMXServer.JDK_17, - "-XX:+UseZGC -Xmx128M -Xms128M")) { - final List> actualMetrics = startAngGetMetrics(server, true); + try (final MisbehavingJMXServer server = new MisbehavingJMXServer.Builder().withJDKImage( + JDK_17).appendJavaOpts("-XX:+UseZGC").build()) { + final List> actualMetrics = startAndGetMetrics(server, true); assertThat(actualMetrics, hasSize(13)); - assertGCMetric(actualMetrics, "jvm.gc.zgc_pauses_collection_count", "ZGC Pauses", "counter"); - assertGCMetric(actualMetrics, "jvm.gc.zgc_pauses_collection_time", "ZGC Pauses", "counter"); - assertGCMetric(actualMetrics, "jvm.gc.zgc_cycles_collection_count", "ZGC Cycles", "counter"); - assertGCMetric(actualMetrics, "jvm.gc.zgc_cycles_collection_time", "ZGC Cycles", "counter"); + assertGCMetric(actualMetrics, + "jvm.gc.zgc_pauses_collection_count", "ZGC Pauses", "counter"); + assertGCMetric(actualMetrics, + "jvm.gc.zgc_pauses_collection_time", "ZGC Pauses", "counter"); + assertGCMetric(actualMetrics, + "jvm.gc.zgc_cycles_collection_count", "ZGC Cycles", "counter"); + assertGCMetric(actualMetrics, + "jvm.gc.zgc_cycles_collection_time", "ZGC Cycles", "counter"); } } - private List> startAngGetMetrics(final MisbehavingJMXServer server, final boolean newGCMetrics) throws IOException { + private List> startAndGetMetrics(final MisbehavingJMXServer server, + final boolean newGCMetrics) throws IOException { server.start(); this.initApplicationWithYamlLines( "init_config:", @@ -140,7 +163,10 @@ private static void assertGCMetric(final List> actualMetrics -1, 10.0, Collections.singletonList(String.format("name:%s", gcGeneration)), - Arrays.asList("instance:jmxint_container", "jmx_domain:java.lang", "type:GarbageCollector"), + Arrays.asList( + "instance:jmxint_container", + "jmx_domain:java.lang", + "type:GarbageCollector"), 5, metricType, actualMetrics); diff --git a/src/test/java/org/datadog/jmxfetch/util/server/MisbehavingJMXServer.java b/src/test/java/org/datadog/jmxfetch/util/server/MisbehavingJMXServer.java index f378132c9..eeee3c8ed 100644 --- a/src/test/java/org/datadog/jmxfetch/util/server/MisbehavingJMXServer.java +++ b/src/test/java/org/datadog/jmxfetch/util/server/MisbehavingJMXServer.java @@ -2,25 +2,26 @@ import java.io.IOException; import java.nio.file.Paths; -import lombok.extern.slf4j.Slf4j; -import org.datadog.jmxfetch.JMXServerControlClient; -import org.datadog.jmxfetch.JMXServerSupervisorClient; + import org.testcontainers.containers.GenericContainer; import org.testcontainers.containers.wait.strategy.Wait; import org.testcontainers.images.builder.ImageFromDockerfile; import org.testcontainers.lifecycle.Startable; +import lombok.extern.slf4j.Slf4j; + +import org.datadog.jmxfetch.JMXServerControlClient; +import org.datadog.jmxfetch.JMXServerSupervisorClient; +import org.datadog.jmxfetch.util.server.app.org.datadog.jmxfetch.util.server.JDKImage; + @Slf4j public class MisbehavingJMXServer implements Startable { - public static final int DEFAULT_RMI_PORT = 9090; public static final int DEFAULT_CONTROL_PORT = 9091; public static final int DEFAULT_SUPERVISOR_PORT = 9092; private static final String DEFAULT_JDK_IMAGE = "base"; - public static final String JDK_11 = "eclipse-temurin:11"; - public static final String JDK_17 = "eclipse-temurin:17"; - public static final String JDK_21 = "eclipse-temurin:21"; + private static final String DEFAULT_MISBEHAVING_OPTS = "-Xmx128M -Xms128M"; private static final String RMI_PORT = "RMI_PORT"; private static final String CONTROL_PORT = "CONTROL_PORT"; private static final String SUPERVISOR_PORT = "SUPERVISOR_PORT"; @@ -35,18 +36,6 @@ public class MisbehavingJMXServer implements Startable { private JMXServerControlClient controlClient; private JMXServerSupervisorClient supervisorClient; - public MisbehavingJMXServer( - final String jdkImage, - final String javaOpts) { - this(jdkImage, javaOpts, DEFAULT_RMI_PORT, DEFAULT_CONTROL_PORT, DEFAULT_SUPERVISOR_PORT); - } - public MisbehavingJMXServer( - final int rmiPort, - final int controlPort, - final int supervisorPort) { - this(DEFAULT_JDK_IMAGE, "", rmiPort, controlPort, supervisorPort); - } - public MisbehavingJMXServer( final String jdkImage, final String javaOpts, @@ -112,4 +101,66 @@ public void restoreNetwork() throws IOException { public int getRMIPort() { return this.rmiPort; } + + public static class Builder { + + private String jdkImage; + private String javaOpts; + private int rmiPort; + private int controlPort; + private int supervisorPort; + + public Builder() { + this.jdkImage = MisbehavingJMXServer.DEFAULT_JDK_IMAGE; + this.javaOpts = MisbehavingJMXServer.DEFAULT_MISBEHAVING_OPTS; + this.rmiPort = MisbehavingJMXServer.DEFAULT_RMI_PORT; + this.controlPort = MisbehavingJMXServer.DEFAULT_CONTROL_PORT; + this.supervisorPort = MisbehavingJMXServer.DEFAULT_SUPERVISOR_PORT; + } + + public Builder withJDKImage(final String jdkImage) { + this.jdkImage = jdkImage; + return this; + } + + public Builder withJDKImage(final JDKImage jdkImage) { + this.jdkImage = jdkImage.toString(); + return this; + } + + public Builder withJavaOpts(String javaOpts) { + this.javaOpts = javaOpts; + return this; + } + + public Builder appendJavaOpts(String javaOpts) { + this.javaOpts = String.format("%s %s", javaOpts, this.javaOpts); + return this; + } + + public Builder withRmiPort(int rmiPort) { + this.rmiPort = rmiPort; + return this; + } + + public Builder withControlPort(int controlPort) { + this.controlPort = controlPort; + return this; + } + + public Builder withSupervisorPort(int supervisorPort) { + this.supervisorPort = supervisorPort; + return this; + } + + public MisbehavingJMXServer build() { + return new MisbehavingJMXServer( + this.jdkImage, + this.javaOpts, + this.rmiPort, + this.controlPort, + this.supervisorPort + ); + } + } } diff --git a/src/test/java/org/datadog/jmxfetch/util/server/app/org/datadog/jmxfetch/util/server/JDKImage.java b/src/test/java/org/datadog/jmxfetch/util/server/app/org/datadog/jmxfetch/util/server/JDKImage.java new file mode 100644 index 000000000..9fc2ca3bd --- /dev/null +++ b/src/test/java/org/datadog/jmxfetch/util/server/app/org/datadog/jmxfetch/util/server/JDKImage.java @@ -0,0 +1,19 @@ +package org.datadog.jmxfetch.util.server.app.org.datadog.jmxfetch.util.server; + +public enum JDKImage { + BASE("base"), + JDK_11("eclipse-temurin:11"), + JDK_17("eclipse-temurin:17"), + JDK_21("eclipse-temurin:21"); + + private final String image; + + private JDKImage(final String image) { + this.image = image; + } + + @Override + public String toString() { + return this.image; + } +} From e8a80072560d99d9f6e6cd7d78717e63afcfa7c2 Mon Sep 17 00:00:00 2001 From: Carlos Roman Date: Thu, 21 Dec 2023 13:08:12 +0000 Subject: [PATCH 21/24] Updated docs and comments --- tools/misbehaving-jmx-server/Dockerfile | 5 +++-- tools/misbehaving-jmx-server/README.md | 1 + .../main/java/org/datadog/misbehavingjmxserver/App.java | 2 +- .../src/main/java/org/datadog/supervisor/App.java | 9 ++++++++- 4 files changed, 13 insertions(+), 4 deletions(-) diff --git a/tools/misbehaving-jmx-server/Dockerfile b/tools/misbehaving-jmx-server/Dockerfile index ae02c6e48..a6a9db099 100644 --- a/tools/misbehaving-jmx-server/Dockerfile +++ b/tools/misbehaving-jmx-server/Dockerfile @@ -13,7 +13,8 @@ WORKDIR /app COPY .mvn .mvn/ COPY pom.xml mvnw mvnw.cmd ./ -# Test containers bug doesn't seem to allow mount cache +# TODO: investigate why mount caching does not seem to work Test containers +# Enabling this will speed up tests as the Maven cache can be shared between all builds # RUN --mount=type=cache,id=mavenCache,target=/root/.m2,sharing=locked \ RUN set -eu && \ ./mvnw dependency:resolve; @@ -21,7 +22,7 @@ RUN set -eu && \ # Copy the source code and build the JAR file COPY src/ src/ -# Test containers bug doesn't seem to allow mount cache +# TODO: investigate why mount caching does not seem to work Test containers # RUN --mount=type=cache,id=mavenCache,target=/root/.m2,sharing=locked \ RUN set -eu && \ ./mvnw clean package assembly:single; diff --git a/tools/misbehaving-jmx-server/README.md b/tools/misbehaving-jmx-server/README.md index 93df69e07..73f9f09fd 100644 --- a/tools/misbehaving-jmx-server/README.md +++ b/tools/misbehaving-jmx-server/README.md @@ -25,6 +25,7 @@ a secondary `init` payload that contains the correct RMI Hostname. It is designe - `RMI_HOST` - hostname for JMX to listen on (default localhost) - `CONTROL_PORT` - HTTP control port (default 8080) - `SUPERVISOR_PORT` - HTTP control port for the supervisor process (if using) (default 8088) +- `MISBEHAVING_OPTS` - Used to manage memory, GC configurations, and system properties of the Java process running the JMXServer (default `-Xmx128M -Xms128M`) ## HTTP Control Actions (jmx-server) - POST `/cutNetwork` - Denies any requests to create a new socket (ie, no more connections will be 'accept'ed) and then closes existing TCP sockets diff --git a/tools/misbehaving-jmx-server/src/main/java/org/datadog/misbehavingjmxserver/App.java b/tools/misbehaving-jmx-server/src/main/java/org/datadog/misbehavingjmxserver/App.java index 31cf230a4..ca659dc72 100644 --- a/tools/misbehaving-jmx-server/src/main/java/org/datadog/misbehavingjmxserver/App.java +++ b/tools/misbehaving-jmx-server/src/main/java/org/datadog/misbehavingjmxserver/App.java @@ -257,7 +257,7 @@ public static void main( String[] args ) throws IOException{ try { Thread.currentThread().join(); } catch (InterruptedException e) { - e.printStackTrace(); + log.error("Got an InterruptedException", e); } } } diff --git a/tools/misbehaving-jmx-server/src/main/java/org/datadog/supervisor/App.java b/tools/misbehaving-jmx-server/src/main/java/org/datadog/supervisor/App.java index 907b09d36..312dfc760 100644 --- a/tools/misbehaving-jmx-server/src/main/java/org/datadog/supervisor/App.java +++ b/tools/misbehaving-jmx-server/src/main/java/org/datadog/supervisor/App.java @@ -37,6 +37,8 @@ class SupervisorInitSpec { @Slf4j public class App { + + private static final String MISBEHAVING_OPTS_ENV = "MISBEHAVING_OPTS"; private static Process process = null; // marked when OS process is started private static AtomicBoolean running = new AtomicBoolean(false); @@ -122,7 +124,12 @@ static void stopJMXServer() throws IOException, InterruptedException { } static void startJMXServer() throws IOException { - final String misbehavingOpts = System.getenv("MISBEHAVING_OPTS"); + /* + MISBEHAVING_OPTS_ENV is the environment variable used to pass configuration flags and + system properties to the JVM that runs the JMXServer. This allows you to do such things as + change the garbage collector use by passing "-XX:+UseParallelGC" to it. + */ + final String misbehavingOpts = System.getenv(MISBEHAVING_OPTS_ENV); final String[] extraOpts = misbehavingOpts !=null ? StringUtils.split(misbehavingOpts) : ArrayUtils.EMPTY_STRING_ARRAY; final String[] command = ArrayUtils.addAll( ArrayUtils.insert(0, extraOpts, "java"), "-cp", From 229b018849319d230b9454757695e9a57c314ab4 Mon Sep 17 00:00:00 2001 From: Carlos Date: Thu, 21 Dec 2023 15:55:38 +0000 Subject: [PATCH 22/24] Update tools/misbehaving-jmx-server/README.md Co-authored-by: DeForest Richards <56796055+drichards-87@users.noreply.github.com> --- tools/misbehaving-jmx-server/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/misbehaving-jmx-server/README.md b/tools/misbehaving-jmx-server/README.md index 73f9f09fd..38c100b2d 100644 --- a/tools/misbehaving-jmx-server/README.md +++ b/tools/misbehaving-jmx-server/README.md @@ -25,7 +25,7 @@ a secondary `init` payload that contains the correct RMI Hostname. It is designe - `RMI_HOST` - hostname for JMX to listen on (default localhost) - `CONTROL_PORT` - HTTP control port (default 8080) - `SUPERVISOR_PORT` - HTTP control port for the supervisor process (if using) (default 8088) -- `MISBEHAVING_OPTS` - Used to manage memory, GC configurations, and system properties of the Java process running the JMXServer (default `-Xmx128M -Xms128M`) +- `MISBEHAVING_OPTS` - Manages memory, GC configurations, and system properties of the Java process running the JMXServer (default `-Xmx128M -Xms128M`) ## HTTP Control Actions (jmx-server) - POST `/cutNetwork` - Denies any requests to create a new socket (ie, no more connections will be 'accept'ed) and then closes existing TCP sockets From a902ae1c5834884e308ed836a8f975fb4031483a Mon Sep 17 00:00:00 2001 From: Carlos Roman Date: Thu, 21 Dec 2023 15:56:17 +0000 Subject: [PATCH 23/24] Updated Dockerfile comments --- tools/misbehaving-jmx-server/Dockerfile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/misbehaving-jmx-server/Dockerfile b/tools/misbehaving-jmx-server/Dockerfile index a6a9db099..81fe4e01f 100644 --- a/tools/misbehaving-jmx-server/Dockerfile +++ b/tools/misbehaving-jmx-server/Dockerfile @@ -27,7 +27,7 @@ COPY src/ src/ RUN set -eu && \ ./mvnw clean package assembly:single; -# Use the base image as the the final image +# Use the image specified by FINAL_JRE_IMAGE build arg (default "base") FROM ${FINAL_JRE_IMAGE} AS final # Set the working directory to /app From 87a87505889669f7b455cc77481973f8c505650d Mon Sep 17 00:00:00 2001 From: Carlos Roman Date: Fri, 22 Dec 2023 11:56:52 +0000 Subject: [PATCH 24/24] Updated comments and added correct TODOs --- .../java/org/datadog/jmxfetch/TestGCMetrics.java | 12 +++++++++--- .../datadog/jmxfetch/util/server => }/JDKImage.java | 2 +- .../jmxfetch/util/server/MisbehavingJMXServer.java | 1 - .../org/datadog/jmxfetch/util/server/SimpleApp.java | 1 + 4 files changed, 11 insertions(+), 5 deletions(-) rename src/test/java/org/datadog/jmxfetch/util/server/{app/org/datadog/jmxfetch/util/server => }/JDKImage.java (80%) diff --git a/src/test/java/org/datadog/jmxfetch/TestGCMetrics.java b/src/test/java/org/datadog/jmxfetch/TestGCMetrics.java index 13016b6b5..fdb5f4d45 100644 --- a/src/test/java/org/datadog/jmxfetch/TestGCMetrics.java +++ b/src/test/java/org/datadog/jmxfetch/TestGCMetrics.java @@ -1,8 +1,7 @@ package org.datadog.jmxfetch; -import static org.datadog.jmxfetch.util.MetricsAssert.assertDomainPresent; -import static org.datadog.jmxfetch.util.server.app.org.datadog.jmxfetch.util.server.JDKImage.JDK_11; -import static org.datadog.jmxfetch.util.server.app.org.datadog.jmxfetch.util.server.JDKImage.JDK_17; +import static org.datadog.jmxfetch.util.MetricsAssert.*; +import static org.datadog.jmxfetch.util.server.JDKImage.*; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.empty; import static org.hamcrest.Matchers.hasItems; @@ -144,6 +143,7 @@ private List> startAndGetMetrics(final MisbehavingJMXServer " max_returned_metrics: 300000", " port: " + server.getRMIPort()); // Run one iteration first + // TODO: Investigate why we have to run this twice - AMLII-1353 this.app.doIteration(); // And then pull get the metrics or else reporter does not have correct number of metrics ((ConsoleReporter) appConfig.getReporter()).getMetrics(); @@ -172,6 +172,12 @@ private static void assertGCMetric(final List> actualMetrics actualMetrics); } + /* + This function is needed as the TestGCMetrics.testDefaultOldGC asserts on two metrics that have + different tags. MetricsAssert.assertMetric expects metrics to have unique names so can't be used + to verify correctly G1 Old Generation and G1 Young Generation for the metric jvm.gc.cms.count and + jvm.gc.parnew.time. + */ private static void assertGCMetric(final List> actualMetrics, final String expectedMetric, final List gcGenerations) { diff --git a/src/test/java/org/datadog/jmxfetch/util/server/app/org/datadog/jmxfetch/util/server/JDKImage.java b/src/test/java/org/datadog/jmxfetch/util/server/JDKImage.java similarity index 80% rename from src/test/java/org/datadog/jmxfetch/util/server/app/org/datadog/jmxfetch/util/server/JDKImage.java rename to src/test/java/org/datadog/jmxfetch/util/server/JDKImage.java index 9fc2ca3bd..365f7bd5b 100644 --- a/src/test/java/org/datadog/jmxfetch/util/server/app/org/datadog/jmxfetch/util/server/JDKImage.java +++ b/src/test/java/org/datadog/jmxfetch/util/server/JDKImage.java @@ -1,4 +1,4 @@ -package org.datadog.jmxfetch.util.server.app.org.datadog.jmxfetch.util.server; +package org.datadog.jmxfetch.util.server; public enum JDKImage { BASE("base"), diff --git a/src/test/java/org/datadog/jmxfetch/util/server/MisbehavingJMXServer.java b/src/test/java/org/datadog/jmxfetch/util/server/MisbehavingJMXServer.java index eeee3c8ed..7ebf69dda 100644 --- a/src/test/java/org/datadog/jmxfetch/util/server/MisbehavingJMXServer.java +++ b/src/test/java/org/datadog/jmxfetch/util/server/MisbehavingJMXServer.java @@ -12,7 +12,6 @@ import org.datadog.jmxfetch.JMXServerControlClient; import org.datadog.jmxfetch.JMXServerSupervisorClient; -import org.datadog.jmxfetch.util.server.app.org.datadog.jmxfetch.util.server.JDKImage; @Slf4j public class MisbehavingJMXServer implements Startable { diff --git a/src/test/java/org/datadog/jmxfetch/util/server/SimpleApp.java b/src/test/java/org/datadog/jmxfetch/util/server/SimpleApp.java index 20522d28b..81c0daee2 100644 --- a/src/test/java/org/datadog/jmxfetch/util/server/SimpleApp.java +++ b/src/test/java/org/datadog/jmxfetch/util/server/SimpleApp.java @@ -11,6 +11,7 @@ import javax.management.NotCompliantMBeanException; import javax.management.ObjectName; +// TODO: Create tests to check all supported versions of Java work with this server - AMLII-1354 class SimpleApp { public interface SampleMBean {