Skip to content

Commit ec1aabd

Browse files
authored
Merge pull request DataDog#2406 from DataDog/mcculls/fixJmxTest
Update JMXFetchTest to test a real process rather than use flaky reflection hacks
2 parents 98922a1 + 8f0864f commit ec1aabd

3 files changed

Lines changed: 111 additions & 71 deletions

File tree

dd-java-agent/src/test/groovy/datadog/trace/agent/JMXFetchTest.groovy

Lines changed: 55 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -1,93 +1,94 @@
11
package datadog.trace.agent
22

33
import datadog.trace.agent.test.IntegrationTestUtils
4-
import datadog.trace.api.Config
54
import jvmbootstraptest.AgentLoadedChecker
6-
import org.junit.Rule
7-
import org.junit.contrib.java.lang.system.RestoreSystemProperties
5+
import jvmbootstraptest.JmxStartedChecker
6+
import spock.lang.Shared
87
import spock.lang.Specification
98
import spock.lang.Timeout
109

11-
import java.lang.reflect.Method
12-
1310
@Timeout(30)
1411
class JMXFetchTest extends Specification {
12+
@Shared
13+
DatagramSocket jmxStatsSocket
14+
15+
def setupSpec() {
16+
jmxStatsSocket = new DatagramSocket(0)
17+
}
1518

16-
@Rule
17-
public final RestoreSystemProperties restoreSystemProperties = new RestoreSystemProperties()
19+
def cleanupSpec() {
20+
jmxStatsSocket.close()
21+
}
1822

1923
def "test jmxfetch"() {
2024
setup:
21-
def currentContextLoader = Thread.currentThread().getContextClassLoader()
22-
DatagramSocket socket = new DatagramSocket(0)
23-
24-
System.setProperty("dd.jmxfetch.enabled", "true")
25-
System.setProperty("dd.jmxfetch.start-delay", "0")
26-
System.setProperty("dd.jmxfetch.statsd.port", Integer.toString(socket.localPort))
27-
// Overwrite writer type to disable console jmxfetch reporter
28-
System.setProperty("dd.writer.type", "DDAgentWriter")
29-
30-
def classLoader = IntegrationTestUtils.getJmxFetchClassLoader()
31-
// Have to set this so JMXFetch knows where to find resources
32-
Thread.currentThread().setContextClassLoader(classLoader)
33-
final Class<?> jmxFetchAgentClass =
34-
classLoader.loadClass("datadog.trace.agent.jmxfetch.JMXFetch")
35-
final Method jmxFetchInstallerMethod = jmxFetchAgentClass.getDeclaredMethod("run", Config)
36-
jmxFetchInstallerMethod.setAccessible(true)
37-
jmxFetchInstallerMethod.invoke(null, new Config())
25+
// verify that JMX starts and reports metrics through the given socket.
26+
def returnCode = IntegrationTestUtils.runOnSeparateJvm(JmxStartedChecker.getName()
27+
, ["-Ddd.jmxfetch.enabled=true",
28+
"-Ddd.jmxfetch.start-delay=0",
29+
"-Ddd.jmxfetch.statsd.port=${jmxStatsSocket.localPort}",
30+
"-Ddd.writer.type=DDAgentWriter"] as String[]
31+
, "" as String[]
32+
, [:]
33+
, true)
3834

3935
byte[] buf = new byte[1500]
4036
DatagramPacket packet = new DatagramPacket(buf, buf.length)
41-
socket.receive(packet)
37+
jmxStatsSocket.receive(packet)
4238
String received = new String(packet.getData(), 0, packet.getLength())
4339

44-
Set<String> threads = Thread.getAllStackTraces().keySet().collect { it.name }
45-
4640
expect:
47-
threads.contains("dd-jmx-collector")
48-
received.contains("jvm.")
49-
50-
cleanup:
51-
jmxFetchInstallerMethod.setAccessible(false)
52-
socket.close()
53-
Thread.currentThread().setContextClassLoader(currentContextLoader)
41+
returnCode == 0
42+
received.contains("#service:${JmxStartedChecker.getName()}")
5443
}
5544

5645
def "Agent loads when JmxFetch is misconfigured"() {
46+
setup:
5747
// verify the agent starts up correctly with a bogus address.
58-
expect:
59-
IntegrationTestUtils.runOnSeparateJvm(AgentLoadedChecker.getName()
48+
def returnCode = IntegrationTestUtils.runOnSeparateJvm(AgentLoadedChecker.getName()
6049
, ["-Ddd.jmxfetch.enabled=true",
6150
"-Ddd.jmxfetch.start-delay=0",
62-
"-Ddd.jmxfetch.statsd.host=example.local"] as String[]
51+
"-Ddd.jmxfetch.statsd.host=example.local",
52+
"-Ddd.writer.type=DDAgentWriter"] as String[]
6353
, "" as String[]
6454
, [:]
65-
, true) == 0
55+
, true)
56+
57+
expect:
58+
returnCode == 0
6659
}
6760

6861
def "test jmxfetch config"() {
6962
setup:
70-
names.each {
71-
System.setProperty("dd.jmxfetch.${it}.enabled", "$enable")
63+
def configSettings = names.collect {
64+
"-Ddd.jmxfetch.${it}.enabled=${enable}"
65+
}
66+
def testOutput = new ByteArrayOutputStream()
67+
def returnCode = IntegrationTestUtils.runOnSeparateJvm(JmxStartedChecker.getName()
68+
, ["-Ddd.jmxfetch.enabled=true",
69+
"-Ddd.jmxfetch.start-delay=0",
70+
"-Ddd.jmxfetch.statsd.port=${jmxStatsSocket.localPort}",
71+
"-Ddd.trace.debug=true",
72+
"-Ddd.writer.type=DDAgentWriter"] + configSettings as String[]
73+
, "" as String[]
74+
, [:]
75+
, new PrintStream(testOutput))
76+
77+
def actualConfig = []
78+
new ByteArrayInputStream((testOutput.toByteArray())).eachLine {
79+
System.out.println(it)
80+
def match = (it =~ 'Reading metric config resource (.*)')
81+
if (match) {
82+
actualConfig += match[0][1]
83+
}
7284
}
73-
def classLoader = IntegrationTestUtils.getJmxFetchClassLoader()
74-
// Have to set this so JMXFetch knows where to find resources
75-
Thread.currentThread().setContextClassLoader(classLoader)
76-
final Class<?> jmxFetchAgentClass =
77-
classLoader.loadClass("datadog.trace.agent.jmxfetch.JMXFetch")
78-
final Method jmxFetchInstallerMethod = jmxFetchAgentClass.getDeclaredMethod("getInternalMetricFiles")
79-
jmxFetchInstallerMethod.setAccessible(true)
8085

8186
expect:
82-
jmxFetchInstallerMethod.invoke(null).sort() == result.sort()
83-
84-
cleanup:
85-
names.each {
86-
System.clearProperty("dd.jmxfetch.${it}.enabled")
87-
}
87+
returnCode == 0
88+
actualConfig as Set == expectedConfig as Set
8889

8990
where:
90-
names | enable | result
91+
names | enable | expectedConfig
9192
[] | true | []
9293
["tomcat"] | false | []
9394
["tomcat"] | true | ["datadog/trace/agent/jmxfetch/metricconfigs/tomcat.yaml"]

dd-java-agent/src/test/java/datadog/trace/agent/test/IntegrationTestUtils.java

Lines changed: 38 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
import java.io.IOException;
1010
import java.io.InputStream;
1111
import java.io.InputStreamReader;
12+
import java.io.PrintStream;
1213
import java.lang.management.ManagementFactory;
1314
import java.lang.management.RuntimeMXBean;
1415
import java.lang.reflect.Field;
@@ -33,11 +34,6 @@ public static ClassLoader getAgentClassLoader() {
3334
return getAgentFieldClassloader("AGENT_CLASSLOADER");
3435
}
3536

36-
/** Returns the classloader the jmxfetch is running on. */
37-
public static ClassLoader getJmxFetchClassLoader() {
38-
return getAgentFieldClassloader("JMXFETCH_CLASSLOADER");
39-
}
40-
4137
private static ClassLoader getAgentFieldClassloader(final String fieldName) {
4238
Field classloaderField = null;
4339
try {
@@ -157,16 +153,43 @@ public static int runOnSeparateJvm(
157153
final Map<String, String> envVars,
158154
final boolean printOutputStreams)
159155
throws Exception {
156+
return runOnSeparateJvm(
157+
mainClassName, jvmArgs, mainMethodArgs, envVars, printOutputStreams ? System.out : null);
158+
}
159+
160+
public static int runOnSeparateJvm(
161+
final String mainClassName,
162+
final String[] jvmArgs,
163+
final String[] mainMethodArgs,
164+
final Map<String, String> envVars,
165+
final PrintStream out)
166+
throws Exception {
160167
final String classPath = System.getProperty("java.class.path");
168+
return runOnSeparateJvm(mainClassName, jvmArgs, mainMethodArgs, envVars, classPath, out);
169+
}
170+
171+
public static int runOnSeparateJvm(
172+
final String mainClassName,
173+
final String[] jvmArgs,
174+
final String[] mainMethodArgs,
175+
final Map<String, String> envVars,
176+
final String classpath,
177+
final boolean printOutputStreams)
178+
throws Exception {
161179
return runOnSeparateJvm(
162-
mainClassName, jvmArgs, mainMethodArgs, envVars, classPath, printOutputStreams);
180+
mainClassName,
181+
jvmArgs,
182+
mainMethodArgs,
183+
envVars,
184+
classpath,
185+
printOutputStreams ? System.out : null);
163186
}
164187

165188
/**
166189
* On a separate JVM, run the main method for a given class.
167190
*
168191
* @param mainClassName The name of the entry point class. Must declare a main method.
169-
* @param printOutputStreams if true, print stdout and stderr of the child jvm
192+
* @param out Optional stream to print the stdout and stderr of the child jvm
170193
* @return the return code of the child jvm
171194
* @throws Exception
172195
*/
@@ -176,7 +199,7 @@ public static int runOnSeparateJvm(
176199
final String[] mainMethodArgs,
177200
final Map<String, String> envVars,
178201
final String classpath,
179-
final boolean printOutputStreams)
202+
final PrintStream out)
180203
throws Exception {
181204

182205
final String separator = System.getProperty("file.separator");
@@ -201,10 +224,8 @@ public static int runOnSeparateJvm(
201224

202225
final Process process = processBuilder.start();
203226

204-
final StreamGobbler errorGobbler =
205-
new StreamGobbler(process.getErrorStream(), "ERROR", printOutputStreams);
206-
final StreamGobbler outputGobbler =
207-
new StreamGobbler(process.getInputStream(), "OUTPUT", printOutputStreams);
227+
final StreamGobbler errorGobbler = new StreamGobbler(process.getErrorStream(), "ERROR", out);
228+
final StreamGobbler outputGobbler = new StreamGobbler(process.getInputStream(), "OUTPUT", out);
208229
outputGobbler.start();
209230
errorGobbler.start();
210231

@@ -238,12 +259,12 @@ private static void waitFor(final Process process, final long timeout, final Tim
238259
private static class StreamGobbler extends Thread {
239260
InputStream stream;
240261
String type;
241-
boolean print;
262+
PrintStream out;
242263

243-
private StreamGobbler(final InputStream stream, final String type, final boolean print) {
264+
private StreamGobbler(final InputStream stream, final String type, final PrintStream out) {
244265
this.stream = stream;
245266
this.type = type;
246-
this.print = print;
267+
this.out = out;
247268
}
248269

249270
@Override
@@ -252,8 +273,8 @@ public void run() {
252273
final BufferedReader reader = new BufferedReader(new InputStreamReader(stream));
253274
String line = null;
254275
while ((line = reader.readLine()) != null) {
255-
if (print) {
256-
System.out.println(type + "> " + line);
276+
if (null != out) {
277+
out.println(type + "> " + line);
257278
}
258279
}
259280
} catch (final IOException e) {
Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
package jvmbootstraptest;
2+
3+
public class JmxStartedChecker {
4+
public static void main(final String[] args) throws Exception {
5+
AgentLoadedChecker.main(args);
6+
7+
boolean jmxStarted = false;
8+
for (Thread t : Thread.getAllStackTraces().keySet()) {
9+
if ("dd-jmx-collector".equals(t.getName())) {
10+
jmxStarted = true;
11+
}
12+
}
13+
14+
if (!jmxStarted) {
15+
throw new IllegalStateException("JMXFetch did not start");
16+
}
17+
}
18+
}

0 commit comments

Comments
 (0)