Skip to content

Commit 2bf5e75

Browse files
jbachorikjbachorik
authored
Fix the profiler stackdepth setting propagation in recent (22+) Java versions (#9130)
* Fix the stackdepth setting propagation to JFR * Emit the dd-trace-java version as profiler setting event * Add profiler integration test assertions * Attempt to fix the JvmOptions in native-image * Fix the JvmOptions parsing of procfs cmdline
1 parent b94ca59 commit 2bf5e75

File tree

17 files changed

+341
-101
lines changed

17 files changed

+341
-101
lines changed

‎components/environment/src/main/java/datadog/environment/JvmOptions.java

Lines changed: 33 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@
1515
import java.nio.file.Paths;
1616
import java.util.ArrayList;
1717
import java.util.List;
18-
import java.util.ListIterator;
1918
import java.util.StringTokenizer;
2019

2120
/** Fetches and captures the JVM options. */
@@ -41,28 +40,44 @@ private String[] readProcFsCmdLine() {
4140
return null;
4241
}
4342

44-
@SuppressForbidden // Class.forName() as backup
4543
private List<String> findVmOptions() {
44+
return findVmOptions(PROCFS_CMDLINE);
45+
}
46+
47+
@SuppressForbidden // Class.forName() as backup
48+
// Visible for testing
49+
List<String> findVmOptions(String[] procfsCmdline) {
4650
// Try ProcFS on Linux
47-
if (PROCFS_CMDLINE != null) {
51+
// Be aware that when running a native image, the command line in /proc/self/cmdline is just the
52+
// executable
53+
if (procfsCmdline != null) {
54+
// Create list of VM options
55+
List<String> vmOptions = new ArrayList<>();
4856
// Start at 1 to skip "java" command itself
4957
int index = 1;
50-
// Look for main class or "-jar", end of VM options
51-
for (; index < PROCFS_CMDLINE.length; index++) {
52-
if (!PROCFS_CMDLINE[index].startsWith("-") || "-jar".equals(PROCFS_CMDLINE[index])) {
53-
break;
54-
}
55-
}
56-
// Create list of VM options
57-
List<String> vmOptions = new ArrayList<>(asList(PROCFS_CMDLINE).subList(1, index + 1));
58-
ListIterator<String> iterator = vmOptions.listIterator();
59-
while (iterator.hasNext()) {
60-
String vmOption = iterator.next();
61-
if (vmOption.startsWith("@")) {
62-
iterator.remove();
63-
for (String argument : getArgumentsFromFile(vmOption)) {
64-
iterator.add(argument);
58+
// Look for first self-standing argument that is not prefixed with "-" or end of VM options
59+
// Skip "-jar" and the jar file
60+
// Simultaneously, collect all arguments in the VM options
61+
for (; index < procfsCmdline.length; index++) {
62+
String argument = procfsCmdline[index];
63+
if (argument.startsWith("@")) {
64+
vmOptions.addAll(getArgumentsFromFile(argument));
65+
} else {
66+
if ("-jar".equals(argument)) {
67+
// skip "-jar" and the jar file
68+
index++;
69+
continue;
70+
} else if ("-cp".equals(argument)) {
71+
// slurp '-cp' and the classpath
72+
vmOptions.add(argument);
73+
if (index + 1 < procfsCmdline.length) {
74+
argument = procfsCmdline[++index];
75+
}
76+
} else if (!argument.startsWith("-")) {
77+
// end of VM options
78+
break;
6579
}
80+
vmOptions.add(argument);
6681
}
6782
}
6883
// Insert JDK_JAVA_OPTIONS at the start if present and supported

‎components/environment/src/test/java/datadog/environment/JvmOptionsTest.java

Lines changed: 101 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,15 +8,23 @@
88
import static java.util.Collections.emptyList;
99
import static java.util.Collections.emptyMap;
1010
import static java.util.Collections.singletonList;
11+
import static java.util.Objects.requireNonNull;
1112
import static org.junit.jupiter.api.Assertions.assertEquals;
1213
import static org.junit.jupiter.api.Assumptions.assumeTrue;
1314
import static org.junit.jupiter.params.provider.Arguments.arguments;
1415

1516
import datadog.environment.CommandLineHelper.Result;
17+
import java.io.BufferedReader;
18+
import java.io.IOException;
19+
import java.io.InputStream;
20+
import java.io.InputStreamReader;
21+
import java.util.ArrayList;
22+
import java.util.Collection;
1623
import java.util.HashMap;
1724
import java.util.List;
1825
import java.util.Map;
1926
import java.util.stream.Stream;
27+
import org.junit.jupiter.api.Assertions;
2028
import org.junit.jupiter.params.ParameterizedTest;
2129
import org.junit.jupiter.params.provider.Arguments;
2230
import org.junit.jupiter.params.provider.MethodSource;
@@ -105,6 +113,67 @@ void testFindVmOptions(
105113
assertEquals(expectedArguments.jvmOptions, result.jvmOptions, "Failed to get JVM options");
106114
}
107115

116+
@MethodSource
117+
private static Stream<Arguments> procFsCmdLine() {
118+
// spotless:off
119+
return Stream.of(
120+
arguments(
121+
"No arguments",
122+
new String[0],
123+
emptyList()
124+
),
125+
arguments(
126+
"Native image launcher",
127+
new String[]{"native-image-launcher", "-Xmx512m"},
128+
singletonList("-Xmx512m")
129+
),
130+
arguments(
131+
"Java with JAR and options",
132+
new String[]{"java", "-Xmx512m", "-Xms256m", "-jar", "app.jar"},
133+
asList("-Xmx512m", "-Xms256m")
134+
),
135+
arguments(
136+
"Java from class and options",
137+
new String[]{"java", "-Xmx512m", "-Xms256m", "-cp", "app.jar", "Main"},
138+
asList("-Xmx512m", "-Xms256m", "-cp", "app.jar")
139+
),
140+
arguments(
141+
"Java from class and options, mixed",
142+
new String[]{"java", "-Xms256m", "-cp", "app.jar", "-Xmx512m", "Main"},
143+
asList("-Xms256m", "-cp", "app.jar", "-Xmx512m")
144+
),
145+
arguments(
146+
"Args from file",
147+
new String[]{"java", "-Dargfile.prop=test", "-verbose:class", argFile("carriage-return-separated"), "-jar", "app.jar"},
148+
flatten("-Dargfile.prop=test", "-verbose:class", expectedArsFromArgFile("carriage-return-separated"))
149+
),
150+
arguments(
151+
"Args from file",
152+
new String[]{"java", "-Dargfile.prop=test", "-verbose:class", argFile("new-line-separated"), "-jar", "app.jar"},
153+
flatten("-Dargfile.prop=test", "-verbose:class", expectedArsFromArgFile("new-line-separated"))
154+
),
155+
arguments(
156+
"Args from file",
157+
new String[]{"java", "-Dargfile.prop=test", "-verbose:class", argFile("space-separated"), "-jar", "app.jar"},
158+
flatten("-Dargfile.prop=test", "-verbose:class", expectedArsFromArgFile("space-separated"))
159+
),
160+
arguments(
161+
"Args from file",
162+
new String[]{"java", "-Dargfile.prop=test", "-verbose:class", argFile("tab-separated"), "-jar", "app.jar"},
163+
flatten("-Dargfile.prop=test", "-verbose:class", expectedArsFromArgFile("tab-separated"))
164+
));
165+
// spotless:on
166+
}
167+
168+
@ParameterizedTest(name = "[{index}] {0}")
169+
@MethodSource("procFsCmdLine")
170+
void testFindVmOptionsWithProcFsCmdLine(
171+
String useCase, String[] procfsCmdline, List<String> expected) throws Exception {
172+
JvmOptions vmOptions = new JvmOptions();
173+
List<String> found = vmOptions.findVmOptions(procfsCmdline);
174+
assertEquals(expected, found);
175+
}
176+
108177
private void skipJdkJavaOptionsOnJava8(Map<String, String> environmentVariables) {
109178
assumeTrue(
110179
JavaVirtualMachine.isJavaVersionAtLeast(9)
@@ -121,4 +190,36 @@ private static Map<String, String> env(String... keysAndValues) {
121190
}
122191
return env;
123192
}
193+
194+
private static String argFile(String name) {
195+
return "@src/test/resources/argfiles/" + name + ".txt";
196+
}
197+
198+
private static List<String> expectedArsFromArgFile(String name) {
199+
List<String> arguments = new ArrayList<>();
200+
try (InputStream stream =
201+
requireNonNull(
202+
CommandLineTest.class.getResourceAsStream("/argfiles/" + name + "-expected.txt"));
203+
BufferedReader reader = new BufferedReader(new InputStreamReader(stream))) {
204+
String line;
205+
while ((line = reader.readLine()) != null) {
206+
arguments.add(line);
207+
}
208+
} catch (IOException e) {
209+
Assertions.fail("Failed to read expected args from " + name + "argfile", e);
210+
}
211+
return arguments;
212+
}
213+
214+
private static List<String> flatten(Object... values) {
215+
List<String> result = new ArrayList<>();
216+
for (Object value : values) {
217+
if (value instanceof Collection) {
218+
result.addAll((Collection<? extends String>) value);
219+
} else {
220+
result.add(value.toString());
221+
}
222+
}
223+
return result;
224+
}
124225
}

‎dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/Agent.java

Lines changed: 41 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,6 @@
6464
import java.net.URISyntaxException;
6565
import java.net.URL;
6666
import java.security.CodeSource;
67-
import java.util.Arrays;
6867
import java.util.EnumSet;
6968
import java.util.concurrent.TimeUnit;
7069
import java.util.concurrent.atomic.AtomicBoolean;
@@ -268,19 +267,20 @@ public static void start(
268267
}
269268
}
270269

270+
boolean retryProfilerStart = false;
271271
if (profilingEnabled) {
272272
if (!isOracleJDK8()) {
273273
// Profiling agent startup code is written in a way to allow `startProfilingAgent` be called
274274
// multiple times
275275
// If early profiling is enabled then this call will start profiling.
276276
// If early profiling is disabled then later call will do this.
277-
startProfilingAgent(true, inst);
277+
retryProfilerStart = startProfilingAgent(true, true, inst);
278278
} else {
279279
log.debug("Oracle JDK 8 detected. Delaying profiler initialization.");
280280
// Profiling can not run early on Oracle JDK 8 because it will cause JFR initialization
281281
// deadlock.
282282
// Oracle JDK 8 JFR controller requires JMX so register an 'after-jmx-initialized' callback.
283-
PROFILER_INIT_AFTER_JMX = () -> startProfilingAgent(false, inst);
283+
PROFILER_INIT_AFTER_JMX = () -> startProfilingAgent(false, true, inst);
284284
}
285285
}
286286

@@ -376,7 +376,7 @@ public static void start(
376376
log.debug("Custom logger detected. Delaying Profiling initialization.");
377377
registerLogManagerCallback(new StartProfilingAgentCallback(inst));
378378
} else {
379-
startProfilingAgent(false, inst);
379+
startProfilingAgent(false, retryProfilerStart, inst);
380380
// only enable instrumentation based profilers when we know JFR is ready
381381
InstrumentationBasedProfiling.enableInstrumentationBasedProfiling();
382382
}
@@ -671,7 +671,7 @@ public AgentThread agentThread() {
671671

672672
@Override
673673
public void execute() {
674-
startProfilingAgent(false, inst);
674+
startProfilingAgent(false, true, inst);
675675
// only enable instrumentation based profilers when we know JFR is ready
676676
InstrumentationBasedProfiling.enableInstrumentationBasedProfiling();
677677
}
@@ -1229,48 +1229,51 @@ private static ProfilingContextIntegration createProfilingContextIntegration() {
12291229
return ProfilingContextIntegration.NoOp.INSTANCE;
12301230
}
12311231

1232-
private static void startProfilingAgent(final boolean isStartingFirst, Instrumentation inst) {
1233-
StaticEventLogger.begin("ProfilingAgent");
1234-
1232+
private static boolean startProfilingAgent(
1233+
final boolean earlyStart, final boolean firstAttempt, Instrumentation inst) {
12351234
if (isAwsLambdaRuntime()) {
1236-
log.info("Profiling not supported in AWS Lambda runtimes");
1237-
return;
1235+
if (firstAttempt) {
1236+
log.info("Profiling not supported in AWS Lambda runtimes");
1237+
}
1238+
return false;
12381239
}
12391240

1240-
final ClassLoader contextLoader = Thread.currentThread().getContextClassLoader();
1241-
try {
1242-
Thread.currentThread().setContextClassLoader(AGENT_CLASSLOADER);
1243-
final Class<?> profilingAgentClass =
1244-
AGENT_CLASSLOADER.loadClass("com.datadog.profiling.agent.ProfilingAgent");
1245-
final Method profilingInstallerMethod =
1246-
profilingAgentClass.getMethod(
1247-
"run", Boolean.TYPE, ClassLoader.class, Instrumentation.class);
1248-
profilingInstallerMethod.invoke(null, isStartingFirst, AGENT_CLASSLOADER, inst);
1241+
boolean requestRetry = false;
1242+
1243+
if (firstAttempt) {
1244+
StaticEventLogger.begin("ProfilingAgent");
1245+
final ClassLoader contextLoader = Thread.currentThread().getContextClassLoader();
1246+
try {
1247+
Thread.currentThread().setContextClassLoader(AGENT_CLASSLOADER);
1248+
final Class<?> profilingAgentClass =
1249+
AGENT_CLASSLOADER.loadClass("com.datadog.profiling.agent.ProfilingAgent");
1250+
final Method profilingInstallerMethod =
1251+
profilingAgentClass.getMethod("run", Boolean.TYPE, Instrumentation.class);
1252+
requestRetry = (boolean) profilingInstallerMethod.invoke(null, earlyStart, inst);
1253+
} catch (final Throwable ex) {
1254+
log.error(SEND_TELEMETRY, "Throwable thrown while starting profiling agent", ex);
1255+
} finally {
1256+
Thread.currentThread().setContextClassLoader(contextLoader);
1257+
}
1258+
StaticEventLogger.end("ProfilingAgent");
1259+
}
1260+
if (!earlyStart) {
12491261
/*
12501262
* Install the tracer hooks only when not using 'early start'.
12511263
* The 'early start' is happening so early that most of the infrastructure has not been set up yet.
12521264
*/
1253-
if (!isStartingFirst) {
1254-
log.debug("Scheduling scope event factory registration");
1255-
WithGlobalTracer.registerOrExecute(
1256-
new WithGlobalTracer.Callback() {
1257-
@Override
1258-
public void withTracer(TracerAPI tracer) {
1259-
log.debug("Initializing profiler tracer integrations");
1260-
tracer.getProfilingContext().onStart();
1261-
}
1262-
});
1263-
}
1264-
} catch (final Throwable t) {
1265-
log.error(
1266-
SEND_TELEMETRY,
1267-
"Throwable thrown while starting profiling agent "
1268-
+ Arrays.toString(t.getCause().getStackTrace()));
1269-
} finally {
1270-
Thread.currentThread().setContextClassLoader(contextLoader);
1265+
initProfilerContext();
12711266
}
1267+
return requestRetry;
1268+
}
12721269

1273-
StaticEventLogger.end("ProfilingAgent");
1270+
private static void initProfilerContext() {
1271+
log.debug("Scheduling profiler context initialization");
1272+
WithGlobalTracer.registerOrExecute(
1273+
tracer -> {
1274+
log.debug("Initializing profiler context integration");
1275+
tracer.getProfilingContext().onStart();
1276+
});
12741277
}
12751278

12761279
private static boolean isAwsLambdaRuntime() {

‎dd-java-agent/agent-profiling/profiling-controller-ddprof/build.gradle

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ excludedClassesCoverage += [
2323
dependencies {
2424
api libs.slf4j
2525
api project(':internal-api')
26+
api project(':utils:version-utils')
2627
api project(':dd-java-agent:agent-profiling:profiling-ddprof')
2728
api project(':dd-java-agent:agent-profiling:profiling-controller')
2829
api project(':dd-java-agent:agent-profiling:profiling-utils')

‎dd-java-agent/agent-profiling/profiling-controller-ddprof/src/main/java/com/datadog/profiling/controller/ddprof/DatadogProfilerSettings.java

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
import com.datadog.profiling.controller.ProfilerSettingsSupport;
44
import com.datadog.profiling.ddprof.DatadogProfiler;
5+
import datadog.common.version.VersionInfo;
56
import datadog.trace.bootstrap.config.provider.ConfigProvider;
67

78
public class DatadogProfilerSettings extends ProfilerSettingsSupport {
@@ -14,6 +15,7 @@ public DatadogProfilerSettings(DatadogProfiler datadogProfiler) {
1415
}
1516

1617
public void publish() {
18+
datadogProfiler.recordSetting(VERSION_KEY, VersionInfo.VERSION);
1719
datadogProfiler.recordSetting(UPLOAD_PERIOD_KEY, String.valueOf(uploadPeriod), "seconds");
1820
datadogProfiler.recordSetting(UPLOAD_TIMEOUT_KEY, String.valueOf(uploadTimeout), "seconds");
1921
datadogProfiler.recordSetting(UPLOAD_COMPRESSION_KEY, uploadCompression);
@@ -27,7 +29,9 @@ public void publish() {
2729
datadogProfiler.recordSetting(PERF_EVENTS_PARANOID_KEY, perfEventsParanoid);
2830
datadogProfiler.recordSetting(NATIVE_STACKS_KEY, String.valueOf(hasNativeStacks));
2931
datadogProfiler.recordSetting(JFR_IMPLEMENTATION_KEY, "ddprof");
30-
datadogProfiler.recordSetting(STACK_DEPTH_KEY, String.valueOf(stackDepth));
32+
datadogProfiler.recordSetting(
33+
"ddprof " + STACK_DEPTH_KEY,
34+
String.valueOf(requestedStackDepth)); // ddprof-java will accept the requested stack depth
3135
datadogProfiler.recordSetting(SELINUX_STATUS_KEY, seLinuxStatus);
3236
if (serviceInstrumentationType != null) {
3337
datadogProfiler.recordSetting(SERVICE_INSTRUMENTATION_TYPE, serviceInstrumentationType);

‎dd-java-agent/agent-profiling/profiling-controller-jfr/implementation/build.gradle

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,3 +53,13 @@ idea {
5353
jdkName = '11'
5454
}
5555
}
56+
57+
project.afterEvaluate {
58+
tasks.withType(Test).configureEach {
59+
if (javaLauncher.get().metadata.languageVersion.asInt() >= 9) {
60+
jvmArgs += [
61+
'--add-opens',
62+
'jdk.jfr/jdk.jfr.internal=ALL-UNNAMED'] // JPMSJFRAccess needs access to jdk.jfr.internal package
63+
}
64+
}
65+
}

0 commit comments

Comments
 (0)