From 364af30d777e141760a7d2b36b1679d4bee6af39 Mon Sep 17 00:00:00 2001 From: Jaroslav Bachorik Date: Thu, 2 Jan 2025 13:48:04 +0100 Subject: [PATCH 1/2] Fix failing NativeLibrariesTest --- .github/workflows/test_workflow.yml | 61 +++++++++++++------ ddprof-test/build.gradle | 3 +- .../profiler/AbstractProfilerTest.java | 8 ++- .../nativelibs/NativeLibrariesTest.java | 8 ++- .../profiler/shutdown/ShutdownTest.java | 5 +- 5 files changed, 60 insertions(+), 25 deletions(-) diff --git a/.github/workflows/test_workflow.yml b/.github/workflows/test_workflow.yml index b6eb94c8..c98ad473 100644 --- a/.github/workflows/test_workflow.yml +++ b/.github/workflows/test_workflow.yml @@ -68,6 +68,7 @@ jobs: sudo sysctl vm.mmap_rnd_bits=28 set +e + export KEEP_JFRS=true export TEST_COMMIT=${{ github.sha }} export TEST_CONFIGURATION=glibc/${{ matrix.java_version }}-amd64 export LIBC=glibc @@ -84,8 +85,9 @@ jobs: } }') ./gradlew -PkeepJFRs :ddprof-test:test${{ matrix.config }} --no-daemon --parallel --build-cache --no-watch-fs - ls -la .gradle - if [ $? -ne 0 ]; then + EXIT_CODE=$? + rm -rf $JAVA_TEST_HOME + if [ $EXIT_CODE -ne 0 ]; then echo "glibc-${{ matrix.java_version }}-${{ matrix.config }}-amd64" >> failures_glibc-${{ matrix.java_version }}-${{ matrix.config }}-amd64.txt exit 1 fi @@ -114,7 +116,7 @@ jobs: if: failure() with: name: recordings - path: /tmp/*.jfr + path: /tmp/recordings test-linux-glibc-aarch64: strategy: @@ -170,22 +172,28 @@ jobs: wget -nv https://download.bell-sw.com/java/${{ matrix.java_version }}/bellsoft-jdk${{ matrix.java_version }}-linux-aarch64.tar.gz -O jdk.tar.gz tar xzf *.tar.gz find . -type d -name 'jdk*' -maxdepth 1| xargs -I {} mv {} ${TEST_JDK_DIR} + + ls -la ${TEST_JDK_DIR} fi - name: Test run: | sudo sysctl vm.mmap_rnd_bits=28 set +e + export KEEP_JFRS=true export TEST_COMMIT=${{ github.sha }} export TEST_CONFIGURATION=glibc/${{ matrix.java_version }}-aarch64 export LIBC=glibc export JAVA_TEST_HOME=$(pwd)/test_${{ env.JAVA_VERSION }}_jdk export SANITIZER=${{ matrix.config }} ./gradlew -PkeepJFRs :ddprof-test:test${{ matrix.config }} --no-daemon --parallel --build-cache --no-watch-fs - if [ $? -ne 0 ]; then + EXIT_CODE=$? + rm -rf $JAVA_TEST_HOME + if [ $EXIT_CODE -ne 0 ]; then echo "glibc-${{ matrix.java_version }}-${{ matrix.config }}-aarch64" >> failures_glibc-${{ matrix.java_version }}-${{ matrix.config }}-aarch64.txt exit 1 fi + ls -la /tmp - name: Upload logs uses: actions/upload-artifact@v3 if: always() @@ -211,7 +219,7 @@ jobs: if: failure() with: name: recordings - path: /tmp/*.jfr + path: /tmp/recordings test-linux-glibc-j9-amd64: strategy: @@ -257,6 +265,7 @@ jobs: - name: Test run: | set +e + export KEEP_JFRS=true export TEST_COMMIT=${{ github.sha }} export TEST_CONFIGURATION=glibc/j9/${{ matrix.java_version }}-amd64 export LIBC=glibc @@ -273,7 +282,9 @@ jobs: }') chmod a+x gradlew ./gradlew -PkeepJFRs :ddprof-test:test${{ matrix.config }} --no-daemon --parallel --build-cache --no-watch-fs - if [ $? -ne 0 ]; then + EXIT_CODE=$? + rm -rf $JAVA_TEST_HOME + if [ $EXIT_CODE -ne 0 ]; then echo "glibc-j9-${{ matrix.java_version }}-${{ matrix.config }}-amd64" >> failures_glibc-j9-${{ matrix.java_version }}-${{ matrix.config }}-amd64.txt exit 1 fi @@ -302,7 +313,7 @@ jobs: if: failure() with: name: recordings - path: /tmp/*.jfr + path: /tmp/recordings test-linux-glibc-j9-aarch64: strategy: @@ -351,6 +362,7 @@ jobs: - name: Test run: | set +e + export KEEP_JFRS=true export TEST_COMMIT=${{ github.sha }} export TEST_CONFIGURATION=glibc/j9/${{ matrix.java_version }}-aarch64 export LIBC=glibc @@ -367,7 +379,9 @@ jobs: }') chmod a+x gradlew ./gradlew -PkeepJFRs :ddprof-test:test${{ matrix.config }} --no-daemon --parallel --build-cache --no-watch-fs - if [ $? -ne 0 ]; then + EXIT_CODE=$? + rm -rf $JAVA_TEST_HOME + if [ $EXIT_CODE -ne 0 ]; then echo "glibc-j9-${{ matrix.java_version }}-${{ matrix.config }}-aarch64" >> failures_glibc-j9-${{ matrix.java_version }}-${{ matrix.config }}-aarch64.txt exit 1 fi @@ -396,7 +410,7 @@ jobs: if: failure() with: name: recordings - path: /tmp/*.jfr + path: /tmp/recordings test-linux-glibc-oracle8: strategy: @@ -452,6 +466,7 @@ jobs: - name: Test run: | set +e + export KEEP_JFRS=true export TEST_COMMIT=${{ github.sha }} export TEST_CONFIGURATION=glibc/oracle/8-amd64 export LIBC=glibc @@ -470,7 +485,9 @@ jobs: } }') ./gradlew -PkeepJFRs :ddprof-test:test${{ matrix.config }} --no-daemon --parallel --build-cache --no-watch-fs - if [ $? -ne 0 ]; then + EXIT_CODE=$? + rm -rf $JAVA_TEST_HOME + if [ $EXIT_CODE -ne 0 ]; then echo "glibc-oracle8-${{ matrix.config }}" >> failures_glibc-oracle8-${{ matrix.config }}.txt exit 1 fi @@ -498,7 +515,7 @@ jobs: if: failure() with: name: recordings - path: /tmp/*.jfr + path: /tmp/recordings test-linux-musl-amd64: strategy: @@ -568,6 +585,7 @@ jobs: - name: Test run: | set +e + export KEEP_JFRS=true export JAVA_HOME=$(pwd)/test_11.0.25_jdk export PATH=$JAVA_HOME/bin:$PATH export TEST_COMMIT=${{ github.sha }} @@ -586,7 +604,10 @@ jobs: } }') ./gradlew -PkeepJFRs :ddprof-test:test${{ matrix.config }} --no-daemon --parallel --build-cache --no-watch-fs - if [ $? -ne 0 ]; then + EXIT_CODE=$? + rm -rf $JAVA_TEST_HOME + rm -rf $JAVA_HOME + if [ $EXIT_CODE -ne 0 ]; then echo "musl-${{ matrix.java_version }}-${{ matrix.config }}-amd64" >> failures_musl-${{ matrix.java_version }}-${{ matrix.config }}-amd64.txt exit 1 fi @@ -615,7 +636,7 @@ jobs: if: failure() with: name: recordings - path: /tmp/*.jfr + path: /tmp/recordings test-linux-glibc-zing-amd64: strategy: @@ -694,6 +715,7 @@ jobs: if: steps.set_config.outputs.config == 'release' || steps.set_config.outputs.config == 'debug' run: | set +e + export KEEP_JFRS=true export TEST_COMMIT=${{ github.sha }} export TEST_CONFIGURATION=glibc/zing/${{ matrix.java_version }}-amd64 export LIBC=glibc @@ -712,7 +734,9 @@ jobs: } }') ./gradlew -PkeepJFRs :ddprof-test:test${{ matrix.config }} --no-daemon --parallel --build-cache --no-watch-fs - if [ $? -ne 0 ]; then + EXIT_CODE=$? + rm -rf $JAVA_TEST_HOME + if [ $EXIT_CODE -ne 0 ]; then echo "glibc-zing-${{ matrix.java_version }}-${{ matrix.config }}-amd64" >> failures_zing-${{ matrix.java_version }}-${{ matrix.config }}-amd64.txt exit 1 fi @@ -740,7 +764,7 @@ jobs: if: failure() with: name: recordings - path: /tmp/*.jfr + path: /tmp/recordings test-linux-glibc-zing-aarch64: strategy: @@ -821,6 +845,7 @@ jobs: if: steps.set_config.outputs.config == 'release' || steps.set_config.outputs.config == 'debug' run: | set +e + export KEEP_JFRS=true export TEST_COMMIT=${{ github.sha }} export TEST_CONFIGURATION=glibc/zing/${{ matrix.java_version }}-aarch64 export LIBC=glibc @@ -839,7 +864,9 @@ jobs: } }') ./gradlew -PkeepJFRs :ddprof-test:test${{ matrix.config }} --no-daemon --parallel --build-cache --no-watch-fs - if [ $? -ne 0 ]; then + EXIT_CODE=$? + rm -rf $JAVA_TEST_HOME + if [ $EXIT_CODE -ne 0 ]; then echo "glibc-zing-${{ matrix.java_version }}-${{ matrix.config }}-aarch64" >> failures_zing-${{ matrix.java_version }}-${{ matrix.config }}-aarch64.txt exit 1 fi @@ -867,4 +894,4 @@ jobs: if: failure() with: name: recordings - path: /tmp/*.jfr \ No newline at end of file + path: /tmp/recordings \ No newline at end of file diff --git a/ddprof-test/build.gradle b/ddprof-test/build.gradle index d0f4a131..66e9c548 100644 --- a/ddprof-test/build.gradle +++ b/ddprof-test/build.gradle @@ -78,8 +78,9 @@ tasks.withType(Test).configureEach { def config = it.name.replace("test", "") - def keepRecordings = hasProperty("keepJFRs") + def keepRecordings = project.hasProperty("keepJFRs") || Boolean.parseBoolean(System.getenv("KEEP_JFRS")) + println("===> keepRecordings: ${keepRecordings}") jvmArgs "-Dddprof_test.keep_jfrs=${keepRecordings}", '-Djdk.attach.allowAttachSelf', '-Djol.tryWithSudo=true', "-Dddprof_test.config=${config}", '-XX:ErrorFile=build/hs_err_pid%p.log', '-XX:+ResizeTLAB', '-Xmx512m' diff --git a/ddprof-test/src/test/java/com/datadoghq/profiler/AbstractProfilerTest.java b/ddprof-test/src/test/java/com/datadoghq/profiler/AbstractProfilerTest.java index 9d20e134..1ffa9c94 100644 --- a/ddprof-test/src/test/java/com/datadoghq/profiler/AbstractProfilerTest.java +++ b/ddprof-test/src/test/java/com/datadoghq/profiler/AbstractProfilerTest.java @@ -153,7 +153,10 @@ public void setupProfiler(TestInfo testInfo) throws Exception { String testConfig = System.getenv("TEST_CONFIGURATION"); testConfig = testConfig == null ? "" : testConfig; - jfrDump = Files.createTempFile(Paths.get("/tmp"), testConfig.replace('/', '_') + "-" + testInfo.getTestMethod().map(m -> m.getDeclaringClass().getSimpleName() + "_" + m.getName()).orElse("unknown"), ".jfr"); + Path rootDir = Paths.get("/tmp/recordings"); + Files.createDirectories(rootDir); + + jfrDump = Files.createTempFile(rootDir, testConfig.replace('/', '_') + "-" + testInfo.getTestMethod().map(m -> m.getDeclaringClass().getSimpleName() + "_" + m.getName()).orElse("unknown"), ".jfr"); profiler = JavaProfiler.getInstance(); String command = "start," + getAmendedProfilerCommand() + ",jfr,file=" + jfrDump.toAbsolutePath(); cpuInterval = command.contains("cpu") ? parseInterval(command, "cpu") : (command.contains("interval") ? parseInterval(command, "interval") : Duration.ZERO); @@ -168,6 +171,7 @@ public void setupProfiler(TestInfo testInfo) throws Exception { public void cleanup() throws Exception { after(); stopProfiler(); + System.out.println("===> keep_jfrs: " + Boolean.getBoolean("ddprof_test.keep_jfrs")); if (jfrDump != null && !Boolean.getBoolean("ddprof_test.keep_jfrs")) { Files.deleteIfExists(jfrDump); } @@ -183,13 +187,11 @@ private void checkConfig() { try { IItemCollection profilerConfig = verifyEvents("datadog.DatadogProfilerConfig"); for (IItemIterable items : profilerConfig) { - IMemberAccessor cpuEngineAccessor = CPU_ENGINE.getAccessor(items.getType()); IMemberAccessor cpuIntervalAccessor = CPU_INTERVAL.getAccessor(items.getType()); IMemberAccessor wallIntervalAccessor = WALL_INTERVAL.getAccessor(items.getType()); for (IItem item : items) { long cpuIntervalMillis = cpuIntervalAccessor.getMember(item).longValueIn(MILLISECOND); long wallIntervalMillis = wallIntervalAccessor.getMember(item).longValueIn(MILLISECOND); - System.out.println("===> cpu interval: " + cpuIntervalMillis); if (!Platform.isJ9() && Platform.isJavaVersionAtLeast(11)) { // fixme J9 engine have weird defaults and need fixing assertEquals(cpuInterval.toMillis(), cpuIntervalMillis); diff --git a/ddprof-test/src/test/java/com/datadoghq/profiler/nativelibs/NativeLibrariesTest.java b/ddprof-test/src/test/java/com/datadoghq/profiler/nativelibs/NativeLibrariesTest.java index 9419ae8b..046fe3e9 100644 --- a/ddprof-test/src/test/java/com/datadoghq/profiler/nativelibs/NativeLibrariesTest.java +++ b/ddprof-test/src/test/java/com/datadoghq/profiler/nativelibs/NativeLibrariesTest.java @@ -45,16 +45,18 @@ protected String getProfilerCommand() { public void test() { Assumptions.assumeFalse(Platform.isZing() || Platform.isJ9()); boolean isMusl = Optional.ofNullable(System.getenv("TEST_CONFIGURATION")).orElse("").startsWith("musl"); + boolean isAsan = Optional.ofNullable(System.getenv("TEST_CONFIGURATION")).orElse("").contains("asan"); + int iterations = Platform.isAarch64() && isAsan ? 200 : 100; int blackhole = 0; - for (int i = 0; i < 100; i++) { + for (int i = 0; i < iterations; i++) { blackhole ^= lz4Java(); } - for (int i = 0; i < 100; i++) { + for (int i = 0; i < iterations; i++) { blackhole ^= zstdJni(); } // snappy-java may not load under musl if (!isMusl) { - for (int i = 0; i < 100; i++) { + for (int i = 0; i < iterations; i++) { blackhole ^= snappyJava(); } } diff --git a/ddprof-test/src/test/java/com/datadoghq/profiler/shutdown/ShutdownTest.java b/ddprof-test/src/test/java/com/datadoghq/profiler/shutdown/ShutdownTest.java index 1ed92233..a7d2cbf3 100644 --- a/ddprof-test/src/test/java/com/datadoghq/profiler/shutdown/ShutdownTest.java +++ b/ddprof-test/src/test/java/com/datadoghq/profiler/shutdown/ShutdownTest.java @@ -5,6 +5,7 @@ import java.io.IOException; import java.nio.file.Files; import java.nio.file.Path; +import java.nio.file.Paths; import java.util.Queue; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; @@ -41,7 +42,9 @@ public void testShutdownCpuAndWall() throws IOException { } private static void runTest(JavaProfiler profiler, String command) throws IOException { - Path jfrDump = Files.createTempFile("filter-test", ".jfr"); + Path rootDir = Paths.get("/tmp/recordings"); + Files.createDirectories(rootDir); + Path jfrDump = Files.createTempFile(rootDir, "shutdown-test", ".jfr"); String commandWithDump = command + ",jfr,file=" + jfrDump.toAbsolutePath(); ExecutorService executor = Executors.newSingleThreadExecutor(); Queue errors = new LinkedBlockingQueue<>(); From 5203cc6b9fa502721b00dd668d56fee49c8a1745 Mon Sep 17 00:00:00 2001 From: Jaroslav Bachorik Date: Fri, 3 Jan 2025 09:34:10 +0100 Subject: [PATCH 2/2] Adjust assertions --- .../profiler/nativelibs/NativeLibrariesTest.java | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/ddprof-test/src/test/java/com/datadoghq/profiler/nativelibs/NativeLibrariesTest.java b/ddprof-test/src/test/java/com/datadoghq/profiler/nativelibs/NativeLibrariesTest.java index 046fe3e9..25cb05e0 100644 --- a/ddprof-test/src/test/java/com/datadoghq/profiler/nativelibs/NativeLibrariesTest.java +++ b/ddprof-test/src/test/java/com/datadoghq/profiler/nativelibs/NativeLibrariesTest.java @@ -73,11 +73,11 @@ public void test() { modeCounters.computeIfAbsent(mode, x -> new AtomicInteger()).incrementAndGet(); if ("NATIVE".equals(mode)) { String library = ""; - if (stacktrace.contains("LZ4JNI")) { + if (stacktrace.contains("LZ4JNI") || stacktrace.contains(".LZ4HC_")) { library = "LZ4"; - } else if (stacktrace.contains("Java_org_xerial_snappy_SnappyNative")) { + } else if (stacktrace.contains("Java_org_xerial_snappy_SnappyNative") || stacktrace.contains("libsnappyjava")) { library = "SNAPPY"; - } else if (stacktrace.contains("Java_com_github_luben_zstd")) { + } else if (stacktrace.contains("Java_com_github_luben_zstd") || stacktrace.contains(".ZSTD_")) { library = "ZSTD"; } else if (stacktrace.contains("Compile")) { library = "JIT"; @@ -89,9 +89,8 @@ public void test() { assertTrue(modeCounters.containsKey("JVM"), "no JVM samples"); assertTrue(modeCounters.containsKey("NATIVE"), "no NATIVE samples"); assertTrue(libraryCounters.containsKey("LZ4"), "no lz4-java samples"); - // looks like we might drop these samples with FP unwinding (which we have to use on MacOS) - // flaky - // assertTrue(isMusl || Platform.isMac() || libraryCounters.containsKey("SNAPPY"), "no snappy-java samples"); + // snappy is problematic on musl; we are not running it + assertTrue(isMusl || libraryCounters.containsKey("SNAPPY"), "no snappy-java samples"); assertTrue(libraryCounters.containsKey("ZSTD"), "no zstd-jni samples"); modeCounters.forEach((mode, count) -> System.err.println(mode + ": " + count.get())); libraryCounters.forEach((lib, count) -> System.err.println(lib + ": " + count.get()));