Improve diagnostics for hung-test dump collection

buildSrc/src/main/kotlin/datadog/gradle/plugin/dump/DumpHangedTestPlugin.kt

@@ -3,6 +3,7 @@ package datadog.gradle.plugin.dump
 import org.gradle.api.Plugin
 import org.gradle.api.Project
 import org.gradle.api.Task
+import org.gradle.api.logging.Logger
 import org.gradle.api.model.ObjectFactory
 import org.gradle.api.provider.Property
 import org.gradle.api.provider.Provider
@@ -118,23 +119,35 @@ class DumpHangedTestPlugin : Plugin<Project> {
         }
         .get()
 
-      dumpsDir.mkdirs()
+      if (!dumpsDir.isDirectory && !dumpsDir.mkdirs()) {
+        throw IOException("Could not create dump directory $dumpsDir")
+      }
 
-      ProcessHandle.current().children()
-        .filter { it.info().commandLine().getOrElse { "" }.contains("Gradle Test Executor") }
-        .forEach { process ->
-          collectDump(dumpsDir, process)
+      var executorCount = 0
+      ProcessHandle.current().children().use { children ->
+        children.filter { it.info().commandLine().getOrElse { "" }.contains("Gradle Test Executor") }
+          .forEach { process ->
+            executorCount++
+            collectDump(t, dumpsDir, process)
 
-          process.children().forEach { child ->
-            collectDump(dumpsDir, child)
+            process.children().use { descendants ->
+              descendants.forEach { child -> collectDump(t, dumpsDir, child) }
+            }
           }
-        }
+      }
+      if (executorCount == 0) {
+        t.logger.warn("No Gradle test executors found for ${t.path}; attempting all-JVM thread dumps")
+      }
 
       // Just in case collect all thread dumps by using special PID `0`.
       val allThreadsFile = file(dumpsDir, "all-thread-dumps")
-      runCmd(Redirect.to(allThreadsFile), "jcmd", "0", "Thread.print", "-l")
+      runCmd(t.logger, t.path, Redirect.to(allThreadsFile), "jcmd", "0", "Thread.print", "-l")
+      t.logger.quiet("Finished dump collection for ${t.path}; output directory: $dumpsDir")
+    } catch (e: InterruptedException) {
+      Thread.currentThread().interrupt()
+      t.logger.warn("Dump collection interrupted for ${t.path}", e)
     } catch (e: Throwable) {
-      t.logger.warn("Taking dumps failed with error: ${e.message ?: e.javaClass.name}, for ${t.path}")
+      t.logger.warn("Taking dumps failed for ${t.path}", e)
     }
   }
 
@@ -153,21 +166,38 @@ class DumpHangedTestPlugin : Plugin<Project> {
   }
 
   private fun runCmd(
+    logger: Logger,
+    taskPath: String,
     redirectTo: Redirect,
     vararg args: String
   ) {
-    val exitCode = ProcessBuilder(*args)
-      .redirectErrorStream(true)
-      .redirectOutput(redirectTo)
-      .start()
-      .waitFor()
-
-    if (exitCode != 0) {
-      throw IOException("Process failed: ${args.joinToString(" ")}, exit code: $exitCode")
+    val command = args.joinToString(" ")
+    val output = redirectTo.file()?.absolutePath ?: "daemon output"
+    val start = System.nanoTime()
+    logger.quiet("Starting dump command for $taskPath: $command; output: $output")
+    var process: Process? = null
+    try {
+      process = ProcessBuilder(*args)
+        .redirectErrorStream(true)
+        .redirectOutput(redirectTo)
+        .start()
+      val exitCode = process.waitFor()
+
+      if (exitCode != 0) {
+        throw IOException("Process failed with exit code $exitCode")
+      }
+      logger.quiet("Completed dump command for $taskPath in ${TimeUnit.NANOSECONDS.toMillis(System.nanoTime() - start)} ms: $command")
+    } catch (e: InterruptedException) {
+      process?.destroyForcibly()
+      logger.warn("Dump command interrupted for $taskPath: $command; output: $output", e)
+      throw e
+    } catch (e: Exception) {
+      logger.warn("Dump command failed for $taskPath: $command; output: $output", e)
     }
   }
 
   private fun collectDump(
+    t: Task,
     baseDir: File,
     process: ProcessHandle
   ) {
@@ -176,15 +206,15 @@ class DumpHangedTestPlugin : Plugin<Project> {
     if (process.info().command().getOrElse { "" }.contains("/ibm8")) {
       // On IBM JDK thread dump can be collected by signaling process with `kill -3`.
       // It will be writen into `/tmp/javacore.YYYYMMDD.HHMMSS.PID.SEQ.txt
-      runCmd(Redirect.INHERIT, "kill", "-3", pid)
+      runCmd(t.logger, t.path, Redirect.INHERIT, "kill", "-3", pid)
     } else {
       // Collect heap dump by pid.
       val heapDumpPath = file(baseDir, "$pid-heap-dump", "hprof").absolutePath
-      runCmd(Redirect.INHERIT, "jcmd", pid, "GC.heap_dump", heapDumpPath)
+      runCmd(t.logger, t.path, Redirect.INHERIT, "jcmd", pid, "GC.heap_dump", heapDumpPath)
 
       // Collect thread dump by pid.
       val threadDumpFile = file(baseDir, "$pid-thread-dump", "log")
-      runCmd(Redirect.to(threadDumpFile), "jcmd", pid, "Thread.print", "-l")
+      runCmd(t.logger, t.path, Redirect.to(threadDumpFile), "jcmd", pid, "Thread.print", "-l")
     }
   }
 }

buildSrc/src/test/kotlin/datadog/gradle/plugin/dump/DumpHangedTestIntegrationTest.kt

@@ -5,6 +5,9 @@ import org.junit.jupiter.api.Assertions.assertFalse
 import org.junit.jupiter.api.Assertions.assertTrue
 import org.junit.jupiter.api.Test
 import org.junit.jupiter.api.assertNotNull
+import org.junit.jupiter.api.condition.EnabledOnOs
+import org.junit.jupiter.api.condition.OS
+import java.io.File
 
 class DumpHangedTestIntegrationTest : GradleFixture() {
   @Test
@@ -30,14 +33,59 @@ class DumpHangedTestIntegrationTest : GradleFixture() {
 
     val dumps = buildFile("dumps")
     assertTrue(dumps.exists()) // Assert dumps created.
+    assertTrue(output.any { it.startsWith("Starting dump command for :test:") && it.contains("GC.heap_dump") })
+    assertTrue(output.any { it.startsWith("Completed dump command for :test") && it.contains("Thread.print -l") })
 
     // Assert actual dumps created.
     val dumpFiles = dumps.list()
     assertNotNull(dumpFiles.find { it.endsWith(".hprof") })
     assertNotNull(dumpFiles.find { it.startsWith("all-thread-dumps") })
   }
 
-  private fun runGradleTest(testSleepMillis: Long): List<String> {
+  @Test
+  fun `should report directory failures with stack trace`() {
+    writeFile("build/dumps", "This file prevents creating the dump directory")
+
+    val output = runGradleTest(testSleepMillis = 25_000)
+
+    assertTrue(output.any { it.contains("Taking dumps failed for :test") })
+    assertTrue(output.any { it.contains("java.io.IOException: Could not create dump directory") })
+    assertTrue(output.any { it.contains("DumpHangedTestPlugin.takeDump(") })
+  }
+
+  @Test
+  @EnabledOnOs(OS.LINUX, OS.MAC)
+  fun `should attempt thread dumps after heap dump failure`() {
+    val jcmd = writeFile(
+      "bin/jcmd",
+      """
+      #!/bin/sh
+      if [ "${'$'}2" = "GC.heap_dump" ]; then
+        echo "Synthetic heap dump failure" >&2
+        exit 7
+      fi
+      echo "Synthetic thread dump for PID ${'$'}1"
+      """
+    )
+    assertTrue(jcmd.setExecutable(true))
+    // Start a separate daemon so subprocess lookup uses this test's PATH.
+    writeGradleProperties("org.gradle.jvmargs=-DdumpFailureTest=true")
+
+    val output = runGradleTest(
+      testSleepMillis = 25_000,
+      env = mapOf("PATH" to "${jcmd.parent}${File.pathSeparator}${System.getenv("PATH")}")
+    )
+
+    assertTrue(output.any { it.startsWith("Dump command failed for :test:") && it.contains("GC.heap_dump") })
+    assertTrue(output.any { it.contains("java.io.IOException: Process failed with exit code 7") })
+    assertTrue(output.any { it.contains("DumpHangedTestPlugin.runCmd(") })
+    val dumps = buildFile("dumps").listFiles().orEmpty()
+    assertTrue(dumps.any { it.name.contains("-thread-dump-") && it.readText().startsWith("Synthetic thread dump") })
+    assertTrue(dumps.any { it.name.startsWith("all-thread-dumps-") && it.readText().contains("PID 0") })
+    assertTrue(output.any { it.startsWith("Finished dump collection for :test;") })
+  }
+
+  private fun runGradleTest(testSleepMillis: Long, env: Map<String, String> = emptyMap()): List<String> {
     writeSettings("""rootProject.name = "test-project"""")
 
     writeRootProject(
@@ -91,6 +139,6 @@ class DumpHangedTestIntegrationTest : GradleFixture() {
       sourceSet = "test"
     )
 
-    return run("test", forwardOutput = true).output.lines()
+    return run("test", env = env, forwardOutput = true).output.lines()
   }
 }