Check logs for errors at smoke tests cleanup (#8111)

Author: smola

dd-smoke-tests/armeria-grpc/src/test/groovy/datadog/smoketest/ArmeriaSmokeTest.groovy

@@ -57,8 +57,6 @@ class ArmeriaSmokeTest extends AbstractServerSmokeTest {
     })
     waitForTraceCount(totalInvocations) >= totalInvocations
     validateLogInjection() == totalInvocations
-    checkLogPostExit()
-    !logHasErrors
   }
 
   void doAndValidateRequest(int id) {

dd-smoke-tests/asm-standalone-billing/src/test/groovy/datadog/smoketest/asmstandalonebilling/AsmStandaloneBillingSmokeTest.groovy

@@ -64,10 +64,8 @@ class AsmStandaloneBillingSmokeTest extends AbstractAsmStandaloneBillingSmokeTes
     def computedStatsHeader = lastTraceRequestHeaders.get('Datadog-Client-Computed-Stats')
     assert computedStatsHeader != null && computedStatsHeader == 'true'
 
-    then:'metrics should be disabled'
-    checkLogPostExit { log ->
-      return log.contains('datadog.trace.agent.common.metrics.MetricsAggregatorFactory - tracer metrics disabled')
-    }
+    then: 'metrics should be disabled'
+    isLogPresent { it.contains('datadog.trace.agent.common.metrics.MetricsAggregatorFactory - tracer metrics disabled') }
   }
 
   void 'test _dd.p.appsec propagation for appsec event'() {

dd-smoke-tests/custom-systemloader/src/test/groovy/datadog/smoketest/CustomSystemLoaderSmokeTest.groovy

@@ -30,20 +30,20 @@ class CustomSystemLoaderSmokeTest extends AbstractSmokeTest {
   def "resource types loaded by custom system class-loader are transformed"() {
     when:
     testedProcess.waitFor(TIMEOUT_SECS, SECONDS)
+
+    then:
+    testedProcess.exitValue() == 0
     int loadedResources = 0
     int transformedResources = 0
-    checkLogPostExit {
+    forEachLogLine { String it ->
       if (it =~ /Loading sample.app.Resource[$]Test[1-3] from TestLoader/) {
         loadedResources++
       }
       if (it =~ /Transformed.*class=sample.app.Resource[$]Test[1-3].*classloader=datadog.smoketest.systemloader.TestLoader/) {
         transformedResources++
       }
     }
-    then:
-    testedProcess.exitValue() == 0
     loadedResources == 3
     transformedResources == 3
-    !logHasErrors
   }
 }

dd-smoke-tests/iast-util/src/main/java/datadog/smoketest/springboot/controller/SsrfController.java

@@ -80,7 +80,10 @@ public String okHttp2(@RequestParam(value = "url") final String url) {
     } catch (final Exception e) {
     }
     client.getDispatcher().getExecutorService().shutdown();
-    client.getConnectionPool().evictAll();
+    com.squareup.okhttp.ConnectionPool pool = client.getConnectionPool();
+    if (pool != null) {
+      pool.evictAll();
+    }
     return "ok";
   }
 

dd-smoke-tests/iast-util/src/testFixtures/groovy/datadog/smoketest/AbstractIastServerSmokeTest.groovy

@@ -54,10 +54,7 @@ abstract class AbstractIastServerSmokeTest extends AbstractServerSmokeTest {
     try {
       processTestLogLines(closure)
     } catch (TimeoutException toe) {
-      checkLogPostExit(closure)
-      if (!found) {
-        throw new AssertionError("No matching tainted found. Tainteds found: ${new JsonBuilder(tainteds).toPrettyString()}")
-      }
+      assert found, "No matching tainted found. Tainteds found: ${new JsonBuilder(tainteds).toPrettyString()}"
     }
   }
 
@@ -83,10 +80,7 @@ abstract class AbstractIastServerSmokeTest extends AbstractServerSmokeTest {
     try {
       processTestLogLines(closure)
     } catch (TimeoutException toe) {
-      checkLogPostExit(closure)
-      if (!found) {
-        throw new AssertionError("No matching vulnerability found. Vulnerabilities found: ${new JsonBuilder(vulnerabilities).toPrettyString()}")
-      }
+      assert found, "No matching vulnerability found. Vulnerabilities found: ${new JsonBuilder(vulnerabilities).toPrettyString()}"
     }
   }
 

dd-smoke-tests/iast-util/src/testFixtures/groovy/datadog/smoketest/AbstractIastSpringBootTest.groovy

@@ -39,33 +39,22 @@ abstract class AbstractIastSpringBootTest extends AbstractIastServerSmokeTest {
     ]
   }
 
-  void 'IAST subsystem starts'() {
-    given: 'an initial request has succeeded'
-    String url = "http://localhost:${httpPort}/greeting"
-    def request = new Request.Builder().url(url).get().build()
-    client.newCall(request).execute()
+  @Override
+  boolean isErrorLog(String log) {
+    if (log.contains('no such algorithm: DES for provider SUN')) {
+      return false
+    }
 
-    when: 'logs are read'
-    String startMsg = null
-    String errorMsg = null
-    checkLogPostExit {
-      if (it.contains('Not starting IAST subsystem')) {
-        errorMsg = it
-      }
-      if (it.contains('IAST is starting')) {
-        startMsg = it
-      }
-      // Check that there's no logged exception about missing classes from Datadog.
-      // We had this problem before with JDK9StackWalker.
-      if (it.contains('java.lang.ClassNotFoundException: datadog/')) {
-        errorMsg = it
-      }
+    if (super.isErrorLog(log) || log.contains('Not starting IAST subsystem')) {
+      return true
+    }
+    // Check that there's no logged exception about missing classes from Datadog.
+    // We had this problem before with JDK9StackWalker.
+    if (log.contains('java.lang.ClassNotFoundException: datadog/')) {
+      return true
     }
 
-    then: 'there are no errors in the log and IAST has started'
-    errorMsg == null
-    startMsg != null
-    !logHasErrors
+    return false
   }
 
   void 'default home page without errors'() {
@@ -82,9 +71,6 @@ abstract class AbstractIastSpringBootTest extends AbstractIastServerSmokeTest {
     responseBodyStr.contains('Sup Dawg')
     response.body().contentType().toString().contains('text/plain')
     response.code() == 200
-
-    checkLogPostExit()
-    !logHasErrors
   }
 
   void 'Multipart Request parameters'() {
@@ -329,13 +315,21 @@ abstract class AbstractIastSpringBootTest extends AbstractIastServerSmokeTest {
     def request = new Request.Builder().url(url).get().build()
 
     when: 'ensure the controller is loaded'
-    client.newCall(request).execute()
+    def resp = client.newCall(request).execute()
 
-    then: 'a vulnerability pops in the logs (startup traces might not always be available)'
-    hasVulnerabilityInLogs { vul ->
-      vul.type == 'WEAK_HASH' &&
-        vul.evidence.value == 'SHA1' &&
-        vul.location.spanId > 0
+    then:
+    resp.code() == 200
+    resp.close()
+
+    and: 'a vulnerability pops in the logs (startup traces might not always be available)'
+    boolean found = false
+    isLogPresent { String log ->
+      def vulns = parseVulnerabilitiesLog(log)
+      vulns.any { vul ->
+        vul.type == 'WEAK_HASH' &&
+          vul.evidence.value == 'SHA1' &&
+          vul.location.spanId > 0
+      }
     }
   }
 
@@ -1060,8 +1054,10 @@ abstract class AbstractIastSpringBootTest extends AbstractIastServerSmokeTest {
 
     then:
     response.successful
-    hasVulnerabilityInLogs { vul ->
-      vul.type == 'SESSION_REWRITING'
+    // Vulnerability may have been detected in a previous request instead, check the full logs.
+    isLogPresent { String log ->
+      def vulns = parseVulnerabilitiesLog(log)
+      vulns.any { it.type == 'SESSION_REWRITING' }
     }
   }
 

dd-smoke-tests/java9-modules/src/test/groovy/datadog/smoketest/Java9ModulesSmokeTest.groovy

@@ -23,6 +23,12 @@ class Java9ModulesSmokeTest extends AbstractSmokeTest {
     processBuilder.directory(new File(buildDirectory))
   }
 
+  @Override
+  boolean isErrorLog(String line) {
+    // FIXME: Too many bootstrap errors.
+    return false
+  }
+
   def "Module application runs correctly"() {
     expect:
     assert testedProcess.waitFor(TIMEOUT_SECS, SECONDS)

dd-smoke-tests/jboss-modules/src/test/groovy/datadog/smoketest/AbstractModulesSmokeTest.groovy

@@ -28,27 +28,20 @@ abstract class AbstractModulesSmokeTest extends AbstractSmokeTest {
     return processBuilder
   }
 
+  @Override
+  boolean isErrorLog(String log) {
+    super.isErrorLog(log) || log.contains("Cannot resolve type description") || log.contains("Instrumentation muzzled")
+  }
+
   def "example application runs without errors"() {
     when:
     testedProcess.waitFor()
-    boolean instrumentedMessageClient = false
-    checkLogPostExit {
-      // check for additional OSGi class-loader issues
-      if (it.contains("Cannot resolve type description") ||
-        it.contains("Instrumentation muzzled")) {
-        println it
-        logHasErrors = true
-      }
-      if (it.contains("Transformed - instrumentation.target.class=datadog.smoketest.jbossmodules.client.MessageClient")) {
-        println it
-        instrumentedMessageClient = true
-      }
-    }
 
-    then:
+    then: 'MessageClient is transformed'
     testedProcess.exitValue() == 0
-    instrumentedMessageClient
-    !logHasErrors
+    processTestLogLines {
+      it.contains("Transformed - instrumentation.target.class=datadog.smoketest.jbossmodules.client.MessageClient")
+    }
   }
 
   @Override

dd-smoke-tests/log-injection/src/test/groovy/datadog/smoketest/LogInjectionSmokeTest.groovy

@@ -104,6 +104,21 @@ abstract class LogInjectionSmokeTest extends AbstractSmokeTest {
     return processBuilder
   }
 
+  @Override
+  boolean isErrorLog(String log) {
+    // Exclude some errors that we consistently get because of the logging setups used here:
+    if (log.contains('no applicable action for [immediateFlush]')) {
+      return false
+    }
+    if (log.contains('JSONLayout contains an invalid element or attribute')) {
+      return false
+    }
+    if (log.contains('JSONLayout has no parameter that matches element')) {
+      return false
+    }
+    return super.isErrorLog(log)
+  }
+
   @Override
   def logLevel() {
     return "debug"

dd-smoke-tests/osgi/src/test/groovy/datadog/smoketest/AbstractOSGiSmokeTest.groovy

@@ -37,27 +37,20 @@ abstract class AbstractOSGiSmokeTest extends AbstractSmokeTest {
 
   abstract List<String> frameworkArguments()
 
+  @Override
+  boolean isErrorLog(String log) {
+    super.isErrorLog(log) || log.contains("Cannot resolve type description") || log.contains("Instrumentation muzzled")
+  }
+
   def "example application runs without errors"() {
     when:
     testedProcess.waitFor()
-    boolean instrumentedMessageClient = false
-    checkLogPostExit {
-      // check for additional OSGi class-loader issues
-      if (it.contains("Cannot resolve type description") ||
-        it.contains("Instrumentation muzzled")) {
-        println it
-        logHasErrors = true
-      }
-      if (it.contains("Transformed - instrumentation.target.class=datadog.smoketest.osgi.client.MessageClient")) {
-        println it
-        instrumentedMessageClient = true
-      }
-    }
 
     then:
     testedProcess.exitValue() == 0
-    instrumentedMessageClient
-    !logHasErrors
+    processTestLogLines {
+      it.contains("Transformed - instrumentation.target.class=datadog.smoketest.osgi.client.MessageClient")
+    }
   }
 
   @Override

dd-smoke-tests/quarkus-native/src/test/groovy/datadog/smoketest/QuarkusNativeSmokeTest.groovy

@@ -55,8 +55,6 @@ abstract class QuarkusNativeSmokeTest extends AbstractServerSmokeTest {
     })
     waitForTraceCount(totalInvocations) == totalInvocations
     validateLogInjection(resourceName()) == totalInvocations
-    checkLogPostExit()
-    !logHasErrors
   }
 
   void doAndValidateRequest(int id) {

dd-smoke-tests/quarkus/src/test/groovy/datadog/smoketest/QuarkusSmokeTest.groovy

@@ -57,8 +57,6 @@ abstract class QuarkusSmokeTest extends AbstractServerSmokeTest {
     })
     waitForTraceCount(totalInvocations) == totalInvocations
     validateLogInjection(resourceName()) == totalInvocations
-    checkLogPostExit()
-    !logHasErrors
   }
 
   void doAndValidateRequest(int id) {

dd-smoke-tests/spring-boot-3.0-native/src/test/groovy/SpringBootNativeInstrumentationTest.groovy

@@ -60,6 +60,12 @@ class SpringBootNativeInstrumentationTest extends AbstractServerSmokeTest {
     false
   }
 
+  @Override
+  boolean isErrorLog(String log) {
+    // Check that there are no ClassNotFound errors printed from bad reflect-config.json
+    super.isErrorLog(log) || log.contains("ClassNotFoundException")
+  }
+
   def "check native instrumentation"() {
     setup:
     String url = "http://localhost:${httpPort}/hello"
@@ -81,18 +87,6 @@ class SpringBootNativeInstrumentationTest extends AbstractServerSmokeTest {
       LockSupport.parkNanos(1_000_000)
     }
     countJfrs() > 0
-
-    when:
-    checkLogPostExit {
-      // Check that there are no ClassNotFound errors printed from bad reflect-config.json
-      if (it.contains("ClassNotFoundException")) {
-        println "Found ClassNotFoundException in log: ${it}"
-        logHasErrors = true
-      }
-    }
-
-    then:
-    !logHasErrors
   }
 
   int countJfrs() {

dd-smoke-tests/spring-boot-rabbit/src/test/groovy/datadog/smoketest/SpringBootRabbitIntegrationTest.groovy

@@ -89,6 +89,14 @@ class SpringBootRabbitIntegrationTest extends AbstractServerSmokeTest {
     return expected
   }
 
+  @Override
+  boolean isErrorLog(String log) {
+    if (log.contains('org.springframework.amqp.rabbit.listener.SimpleMessageListenerContainer - Failed to check/redeclare auto-delete queue(s).')) {
+      return false
+    }
+    return super.isErrorLog(log)
+  }
+
   def "check message #message roundtrip"() {
     setup:
     String url = "http://localhost:${httpPort}/roundtrip/${message}"

dd-smoke-tests/springboot/src/test/groovy/datadog/smoketest/IastSpringBootSmokeTest.groovy

@@ -40,13 +40,17 @@ class IastSpringBootSmokeTest extends AbstractIastSpringBootTest {
 
     then:
     response.successful
-    hasVulnerabilityInLogs {
-      vul ->
-      vul.type == 'HARDCODED_SECRET'
-      && vul.location.method == 'hardcodedSecret'
-      && vul.location.path == 'datadog.smoketest.springboot.controller.HardcodedSecretController'
-      && vul.location.line == 11
-      && vul.evidence.value == 'age-secret-key'
+    isLogPresent {
+      String log ->
+      def vulns = parseVulnerabilitiesLog(log)
+      vulns.any {
+        vul ->
+        vul.type == 'HARDCODED_SECRET'
+        && vul.location.method == 'hardcodedSecret'
+        && vul.location.path == 'datadog.smoketest.springboot.controller.HardcodedSecretController'
+        && vul.location.line == 11
+        && vul.evidence.value == 'age-secret-key'
+      }
     }
   }
 

dd-smoke-tests/src/main/groovy/datadog/smoketest/AbstractSmokeTest.groovy

@@ -243,6 +243,7 @@ abstract class AbstractSmokeTest extends ProcessManager {
 
   def cleanupSpec() {
     stopServer()
+    assertNoErrorLogs()
   }
 
   def startServer() {