Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,8 @@ Never use 'gradle' or 'gradlew' directly. Instead, use the '/build-and-summarize
./gradlew testDebug
./gradlew testAsan
./gradlew testTsan
./gradlew -Pprofiler.options=${user options} testProcessRelease
./gradlew -Pprofiler.options=${user options} testProcessDebug

# Run C++ unit tests only (via GtestPlugin)
./gradlew :ddprof-lib:gtestDebug # All debug tests
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -429,6 +429,98 @@ class ProfilerTestPlugin : Plugin<Project> {
}
}

/**
* Create Exec-based test task that always runs in a separate process.
* Available on all platforms. Supports -Pprofiler.options for overriding profiler settings.
* Task name: testProcess<Config> (e.g., testProcessDebug, testProcessRelease)
*/
private fun createProcessTestTask(
project: Project,
extension: ProfilerTestExtension,
testConfig: TestTaskConfiguration,
testCfg: Configuration,
sourceSets: SourceSetContainer
) {
val taskName = "testProcess${testConfig.configName.replaceFirstChar { it.uppercase() }}"
project.tasks.register(taskName, Exec::class.java) {
val execTask = this
execTask.description = "Runs tests in separate process with ${testConfig.configName} library (supports -Pprofiler.options)"
execTask.group = "verification"
execTask.onlyIf { testConfig.isActive && !project.hasProperty("skip-tests") }

// Dependencies
execTask.dependsOn(project.tasks.named("compileTestJava"))
execTask.dependsOn(testCfg)
execTask.dependsOn(sourceSets.getByName("test").output)

// Configure at execution time to capture properties
execTask.doFirst {
execTask.executable = PlatformUtils.testJavaExecutable()

val allArgs = mutableListOf<String>()

// JVM args
allArgs.addAll(testConfig.standardJvmArgs)
// Version-gated at execution time, when the real test JVM (JAVA_TEST_HOME) is resolvable.
allArgs.addAll(carrierExportJvmArgs(project))
if (extension.nativeLibDir.isPresent) {
allArgs.add("-Djava.library.path=${extension.nativeLibDir.get().asFile.absolutePath}")
}
allArgs.addAll(testConfig.extraJvmArgs)
Comment on lines +463 to +469

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Mirror the test JVM args in process tasks

When the new process task launches ProfilerTestRunner, it forwards only the standard and extra JVM args. In the existing test* tasks, this spot also adds the execution-time carrierExportJvmArgs(project) and testConfig.configJvmArgs; without those, testProcessDebug on JDK 21+ does not export jdk.internal.misc so the carrier-scoped context tests skip/fall back, and testProcessAsan omits the ASAN heap-placement flags from ConfigurationPresets. Please include the same JVM-arg sources before running the tests.

Useful? React with 👍 / 👎.

allArgs.addAll(testConfig.configJvmArgs)

// System properties
testConfig.systemProperties.forEach { (key, value) ->
allArgs.add("-D$key=$value")
}

// Test filter from -Ptests property
val testsFilter = project.findProperty("tests") as String?
if (testsFilter != null) {
allArgs.add("-Dtest.filter=$testsFilter")
}

// Profiler options from -Pprofiler.options property
val profilerOptions = project.findProperty("profiler.options") as String?
if (profilerOptions != null) {
allArgs.add("-Dddprof.test.options=$profilerOptions")
}

// Classpath
allArgs.add("-cp")
allArgs.add(testConfig.testClasspath.asPath)

// Use custom test runner
allArgs.add("com.datadoghq.profiler.test.ProfilerTestRunner")

execTask.args = allArgs
}

// Environment variables
testConfig.environmentVariables.forEach { (key, value) ->
execTask.environment(key, value)
}

// Remove LD_LIBRARY_PATH to let RPATH work correctly
execTask.doFirst {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe it's cleaner do not add LD_LIBRARY_PATH in the loop just above?

val currentLdLibPath = (execTask.environment["LD_LIBRARY_PATH"] as? String) ?: System.getenv("LD_LIBRARY_PATH")
if (!currentLdLibPath.isNullOrEmpty()) {
project.logger.info("Removing LD_LIBRARY_PATH to prevent cross-JDK library conflicts (was: $currentLdLibPath)")
execTask.environment.remove("LD_LIBRARY_PATH")
}
}

// Sanitizer conditions
when (testConfig.configName) {
"asan" -> execTask.onlyIf {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Avoid single-JVM ASAN process runs

When configName is asan, this predicate enables testProcessAsan, but that task is a single Exec that launches ProfilerTestRunner once for the entire suite. The regular ASAN Test task explicitly uses forkEvery(25) because one JVM accumulates per-test JFR/JMC allocations until it OOMs under -Xmx512m; this new path has no equivalent, so full testProcessAsan runs reintroduce the late ASAN-suite OOM that the existing task avoids.

Useful? React with 👍 / 👎.

PlatformUtils.locateLibasan() != null &&
!PlatformUtils.isTestJvmJ9()
}
"tsan" -> execTask.onlyIf { false }
}
}
}

private fun generateMultiConfigTasks(project: Project, extension: ProfilerTestExtension) {
val nativeBuildExt = project.rootProject.extensions.findByType(NativeBuildExtension::class.java)
?: return // No native build extension, nothing to generate
Expand Down Expand Up @@ -496,6 +588,10 @@ class ProfilerTestPlugin : Plugin<Project> {
createTestTask(project, extension, testConfig, testCfg, sourceSets)
}

// Create process-based test task (always uses Exec, available on all platforms)
// Supports -Pprofiler.options for overriding profiler settings
createProcessTestTask(project, extension, testConfig, testCfg, sourceSets)

// Create application tasks for specified configs
if (configName in applicationConfigs && appMainClass.isNotEmpty()) {
// Create main configuration
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -376,10 +376,81 @@ public final void registerCurrentThreadForWallClockProfiling() {
profiler.addThread();
}

/**
* Merges two profiler command strings. Options in overrides take precedence
* over options in base for matching keys.
*
* @param base The base profiler command (from test's getProfilerCommand())
* @param overrides The override options (from -Pprofiler.options)
* @return Merged command string with overrides taking precedence
*/
private static String mergeProfilerOptions(String base, String overrides) {
Comment thread
zhengyu123 marked this conversation as resolved.
if (overrides == null || overrides.isEmpty()) {
return base;
}

// Parse base options into ordered map
Map<String, String> options = new java.util.LinkedHashMap<>();
for (String part : base.split(",")) {
int eq = part.indexOf('=');
if (eq > 0) {
options.put(part.substring(0, eq), part.substring(eq + 1));
} else if (!part.isEmpty()) {
options.put(part, "");
}
}

// Apply overrides
for (String part : overrides.split(",")) {
Comment thread
zhengyu123 marked this conversation as resolved.
int eq = part.indexOf('=');
if (eq > 0) {
options.put(part.substring(0, eq), part.substring(eq + 1));
} else if (!part.isEmpty()) {
options.put(part, "");
}
}

// Rebuild command, preserving keys with empty values (e.g. "filter=")
// so untouched base options round-trip exactly.
StringBuilder result = new StringBuilder();
for (Map.Entry<String, String> entry : options.entrySet()) {
if (result.length() > 0) {
result.append(",");
}
Comment thread
zhengyu123 marked this conversation as resolved.
result.append(entry.getKey());
result.append("=").append(entry.getValue());

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Preserve no-value profiler flags

When either the base command or -Pprofiler.options contains a flag without an equals sign, this rebuild path stores it as an empty string and then always emits key=. The native parser distinguishes those forms: for example memory uses the default allocation interval while memory= parses to 0, and jvmtistacks enables with no value but jvmtistacks= disables it. As a result, testProcessDebug -Pprofiler.options=memory does not run the requested profiler configuration and can turn on every-allocation sampling instead.

Useful? React with 👍 / 👎.

}
return result.toString();
}

/**
* Applies user-provided options from -Pprofiler.options (via the
* "ddprof.test.options" system property) to a profiler command string,
* overriding any matching keys already present in it.
*
* <p>Tests that build their profiler command directly (rather than through
* {@link #getProfilerCommand()}/{@link #getAmendedProfilerCommand()}) must
* call this before {@code JavaProfiler.execute(...)} so that

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Cover the remaining direct profiler restart

Fresh evidence after the latest patch: a repo-wide rg "execute\\(" ddprof-test/src/test/java still finds PrecheckTest.suppressionCounterIsZeroWhenPrecheckDisabled restarting the profiler with profiler.execute("start," + getPrecheckDisabledProfilerCommand()...) and not calling this helper. When testProcessDebug -Pprofiler.options=fjmethodid=false -Ptests=PrecheckTest runs, the @BeforeEach recording uses the override but that second recording silently reverts to the hard-coded options, so the new task still cannot validate a user option across the whole selected test.

Useful? React with 👍 / 👎.

* -Pprofiler.options reliably applies across the whole test set, not just
* to subclasses of this class.
*
* @param profilerCommand the command to apply overrides to
* @return the command with overrides applied, or unchanged if none were requested
*/
public static String applyProfilerOptionOverrides(String profilerCommand) {
String userOptions = System.getProperty("ddprof.test.options");
if (userOptions != null && !userOptions.isEmpty()) {
profilerCommand = mergeProfilerOptions(profilerCommand, userOptions);
Comment on lines +441 to +443

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Apply profiler options to all profiler starts

When testProcessDebug is run without -Ptests, this system property is only consumed by AbstractProfilerTest; tests that call JavaProfiler.execute(...) directly still start the profiler with their hard-coded commands (for example ShutdownTest.runTest and OtelContextStorageModeTest). That means a process run requested with -Pprofiler.options=fjmethodid=false still exercises part of the suite with the default profiler options, so it is not a reliable way to validate that option across the task's full test set.

Useful? React with 👍 / 👎.

System.out.println("[TEST] Applied profiler.options: " + userOptions);
}
return profilerCommand;
}

private String getAmendedProfilerCommand() {
String profilerCommand = getProfilerCommand();
String profilerCommand = applyProfilerOptionOverrides(getProfilerCommand());

String testCstack = (String)testParams.get("cstack");
if (testCstack != null) {
if (testCstack != null && !profilerCommand.contains("cstack=")) {
profilerCommand += ",cstack=" + testCstack;
Comment thread
zhengyu123 marked this conversation as resolved.
} else if(!(ALLOW_NATIVE_CSTACKS || profilerCommand.contains("cstack="))) {
profilerCommand += ",cstack=fp";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
*/
package com.datadoghq.profiler.context;

import com.datadoghq.profiler.AbstractProfilerTest;
import com.datadoghq.profiler.JavaProfiler;
import com.datadoghq.profiler.ThreadContext;
import org.junit.jupiter.api.AfterEach;
Expand Down Expand Up @@ -58,7 +59,8 @@ public void cleanup() {
public void testOtelStorageModeContext() throws Exception {
Path jfrFile = Files.createTempFile("otel-ctx-otel", ".jfr");

profiler.execute(String.format("start,cpu=1ms,attributes=tag1;tag2;tag3,jfr,file=%s", jfrFile.toAbsolutePath()));
profiler.execute(String.format("start,%s,jfr,file=%s",
AbstractProfilerTest.applyProfilerOptionOverrides("cpu=1ms,attributes=tag1;tag2;tag3"), jfrFile.toAbsolutePath()));
profilerStarted = true;

long localRootSpanId = 0x1111222233334444L;
Expand All @@ -84,7 +86,8 @@ public void testOtelStorageModeContext() throws Exception {
public void testOtelModeCustomAttributes() throws Exception {
Path jfrFile = Files.createTempFile("otel-ctx-attrs", ".jfr");

profiler.execute(String.format("start,cpu=1ms,attributes=http.route;db.system,jfr,file=%s", jfrFile.toAbsolutePath()));
profiler.execute(String.format("start,%s,jfr,file=%s",
AbstractProfilerTest.applyProfilerOptionOverrides("cpu=1ms,attributes=http.route;db.system"), jfrFile.toAbsolutePath()));
profilerStarted = true;

long localRootSpanId = 0x1111222233334444L;
Expand Down Expand Up @@ -114,7 +117,8 @@ public void testOtelModeCustomAttributes() throws Exception {
public void testOtelModeAttributeOverflow() throws Exception {
Path jfrFile = Files.createTempFile("otel-ctx-overflow", ".jfr");

profiler.execute(String.format("start,cpu=1ms,attributes=k0;k1;k2;k3;k4,jfr,file=%s", jfrFile.toAbsolutePath()));
profiler.execute(String.format("start,%s,jfr,file=%s",
AbstractProfilerTest.applyProfilerOptionOverrides("cpu=1ms,attributes=k0;k1;k2;k3;k4"), jfrFile.toAbsolutePath()));
profilerStarted = true;

profiler.setContext(0x2L, 0x1L, 0L, 0x3L);
Expand Down Expand Up @@ -203,7 +207,8 @@ public void testThreadIsolation() throws InterruptedException {
@Test
public void testSpanTransitionClearsAttributes() throws Exception {
Path jfrFile = Files.createTempFile("otel-ctx-transition", ".jfr");
profiler.execute(String.format("start,cpu=1ms,attributes=http.route,jfr,file=%s", jfrFile.toAbsolutePath()));
profiler.execute(String.format("start,%s,jfr,file=%s",
AbstractProfilerTest.applyProfilerOptionOverrides("cpu=1ms,attributes=http.route"), jfrFile.toAbsolutePath()));
profilerStarted = true;

// Span A: set a custom attribute
Expand Down Expand Up @@ -246,7 +251,8 @@ public void testRepeatedContextWrites() {
@Test
public void testAttributeCacheIsolation() throws Exception {
Path jfrFile = Files.createTempFile("otel-attr-cache-iso", ".jfr");
profiler.execute(String.format("start,cpu=1ms,attributes=attr0,jfr,file=%s", jfrFile.toAbsolutePath()));
profiler.execute(String.format("start,%s,jfr,file=%s",
AbstractProfilerTest.applyProfilerOptionOverrides("cpu=1ms,attributes=attr0"), jfrFile.toAbsolutePath()));
profilerStarted = true;

final String valueA = "FB"; // hashCode = 2236, slot 188
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

package com.datadoghq.profiler.lifecycle;

import com.datadoghq.profiler.AbstractProfilerTest;
import com.datadoghq.profiler.JavaProfiler;
import com.datadoghq.profiler.Platform;

Expand Down Expand Up @@ -48,7 +49,7 @@ public class StartWithChurningThreadsTest {
private static final int CYCLES = 500;
// filter= disables thread filtering so all threads receive SIGVTALRM,
// maximising the chance of racing execute() with active thread churn.
private static final String PROFILER_CMD = "start,wall=1ms,filter=,jfr,file=";
private static final String PROFILER_OPTIONS = "wall=1ms,filter=";

@Timeout(120)
@Test
Expand All @@ -60,6 +61,7 @@ public void startRacesThreadStartEnd() throws Exception {
JavaProfiler profiler = JavaProfiler.getInstance();
Path recordings = Files.createTempDirectory("lifecycle-cell2-");
Queue<Throwable> errors = new LinkedBlockingQueue<>();
String profilerCmd = "start," + AbstractProfilerTest.applyProfilerOptionOverrides(PROFILER_OPTIONS) + ",jfr,file=";

AtomicBoolean running = new AtomicBoolean(true);
// Latch: churn threads signal they are running before execute() is called,
Expand All @@ -84,7 +86,7 @@ public void startRacesThreadStartEnd() throws Exception {
Path jfr = Files.createTempFile(recordings, "c2-" + cycle + "-", ".jfr");
try {
// execute() races with active thread churn
profiler.execute(PROFILER_CMD + jfr.toAbsolutePath());
profiler.execute(profilerCmd + jfr.toAbsolutePath());
Thread.sleep(cycle % 3);
profiler.stop();
} catch (Throwable t) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

package com.datadoghq.profiler.lifecycle;

import com.datadoghq.profiler.AbstractProfilerTest;
import com.datadoghq.profiler.JavaProfiler;
import com.datadoghq.profiler.Platform;

Expand Down Expand Up @@ -49,7 +50,7 @@ public class StopWithChurningThreadsTest {
// wall=1ms maximises signals in-flight during teardown; filter= disables
// thread filtering so all threads receive SIGVTALRM, increasing collision
// probability with ThreadEnd.
private static final String PROFILER_CMD = "start,wall=1ms,filter=,jfr,file=";
private static final String PROFILER_OPTIONS = "wall=1ms,filter=";

@Timeout(120)
@Test
Expand All @@ -61,6 +62,7 @@ public void stopRacesThreadEnd() throws Exception {
JavaProfiler profiler = JavaProfiler.getInstance();
Path recordings = Files.createTempDirectory("lifecycle-cell1-");
Queue<Throwable> errors = new LinkedBlockingQueue<>();
String profilerCmd = "start," + AbstractProfilerTest.applyProfilerOptionOverrides(PROFILER_OPTIONS) + ",jfr,file=";

AtomicBoolean running = new AtomicBoolean(true);
CountDownLatch churnRunning = new CountDownLatch(CHURN_THREADS);
Expand All @@ -83,7 +85,7 @@ public void stopRacesThreadEnd() throws Exception {
for (int cycle = 0; cycle < CYCLES; cycle++) {
Path jfr = Files.createTempFile(recordings, "c1-" + cycle + "-", ".jfr");
try {
profiler.execute(PROFILER_CMD + jfr.toAbsolutePath());
profiler.execute(profilerCmd + jfr.toAbsolutePath());
// Minimal delay: hit stop() while signals are still in-flight
// and threads are actively dying. Varies per cycle to avoid
// always landing in the same phase of the signal period.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ public void testNoSigsegvAfterClassUnloadAndMethodCleanup() throws Exception {

try {
profiler.execute(
"start,cpu=1ms,jfr,mcleanup=true,file=" + baseFile.toAbsolutePath());
"start," + applyProfilerOptionOverrides(getProfilerCommand()) + ",jfr,mcleanup=true,file=" + baseFile.toAbsolutePath());
Thread.sleep(200); // Let the profiler stabilize

// 1. Load a class, execute its methods to appear in CPU profiling stack traces,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ public void testCleanupEffectivenessComparison() throws Exception {

profiler.execute(
"start,"
+ getProfilerCommand()
+ applyProfilerOptionOverrides(getProfilerCommand())
+ ",jfr,mcleanup=false,file="
+ noCleanupBaseFile);

Expand Down Expand Up @@ -182,7 +182,7 @@ public void testCleanupEffectivenessComparison() throws Exception {
classCounter = 0;

profiler.execute(
"start," + getProfilerCommand() + ",jfr,mcleanup=true,file=" + withCleanupBaseFile);
"start," + applyProfilerOptionOverrides(getProfilerCommand()) + ",jfr,mcleanup=true,file=" + withCleanupBaseFile);

Thread.sleep(300); // Stabilize

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ public void testNoSigsegvInWriteStackTracesAfterClassUnload() throws Exception {
try {
// mcleanup=false isolates the test to the writeStackTraces read path; the
// SharedLineNumberTable destructor path is covered by CleanupAfterClassUnloadTest.
profiler.execute("start,cpu=1ms,jfr,mcleanup=false,file=" + baseFile.toAbsolutePath());
profiler.execute("start," + applyProfilerOptionOverrides(getProfilerCommand()) + ",jfr,mcleanup=false,file=" + baseFile.toAbsolutePath());
try {
Thread.sleep(200); // let the profiler stabilize

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ public void testNativeSocketProfilerRestart() throws Exception {
Files.createDirectories(Paths.get("/tmp/recordings"));
Path jfr2 = Files.createTempFile(Paths.get("/tmp/recordings"), "NativeSocketRestartTest_restart", ".jfr");
try {
profiler.execute("start,natsock=100us,jfr,file=" + jfr2.toAbsolutePath());
profiler.execute("start," + applyProfilerOptionOverrides("natsock=100us") + ",jfr,file=" + jfr2.toAbsolutePath());

doTcpTransfer(4096, 64);

Expand Down
Loading
Loading