From 5ca171998369eda1eb56263b24158d4ebc8521ed Mon Sep 17 00:00:00 2001 From: Carlos Roman Date: Thu, 21 Dec 2023 10:44:31 +0000 Subject: [PATCH] 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; + } +}