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

This reverts commit ab205f6.

Author: smola

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

@@ -57,6 +57,8 @@ 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,8 +64,10 @@ class AsmStandaloneBillingSmokeTest extends AbstractAsmStandaloneBillingSmokeTes
     def computedStatsHeader = lastTraceRequestHeaders.get('Datadog-Client-Computed-Stats')
     assert computedStatsHeader != null && computedStatsHeader == 'true'
 
-    then: 'metrics should be disabled'
-    isLogPresent { it.contains('datadog.trace.agent.common.metrics.MetricsAggregatorFactory - tracer metrics disabled') }
+    then:'metrics should be disabled'
+    checkLogPostExit { log ->
+      return log.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
-    forEachLogLine { String it ->
+    checkLogPostExit {
       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,10 +80,7 @@ public String okHttp2(@RequestParam(value = "url") final String url) {
     } catch (final Exception e) {
     }
     client.getDispatcher().getExecutorService().shutdown();
-    com.squareup.okhttp.ConnectionPool pool = client.getConnectionPool();
-    if (pool != null) {
-      pool.evictAll();
-    }
+    client.getConnectionPool().evictAll();
     return "ok";
   }
 

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

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

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

@@ -41,22 +41,33 @@ abstract class AbstractIastSpringBootTest extends AbstractIastServerSmokeTest {
     ]
   }
 
-  @Override
-  boolean isErrorLog(String log) {
-    if (log.contains('no such algorithm: DES for provider SUN')) {
-      return false
-    }
+  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()
 
-    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
+    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
+      }
     }
 
-    return false
+    then: 'there are no errors in the log and IAST has started'
+    errorMsg == null
+    startMsg != null
+    !logHasErrors
   }
 
   void 'default home page without errors'() {
@@ -73,6 +84,9 @@ 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'() {
@@ -317,21 +331,13 @@ abstract class AbstractIastSpringBootTest extends AbstractIastServerSmokeTest {
     def request = new Request.Builder().url(url).get().build()
 
     when: 'ensure the controller is loaded'
-    def resp = client.newCall(request).execute()
-
-    then:
-    resp.code() == 200
-    resp.close()
+    client.newCall(request).execute()
 
-    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
-      }
+    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
     }
   }
 
@@ -1056,10 +1062,8 @@ abstract class AbstractIastSpringBootTest extends AbstractIastServerSmokeTest {
 
     then:
     response.successful
-    // 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' }
+    hasVulnerabilityInLogs { vul ->
+      vul.type == 'SESSION_REWRITING'
     }
   }
 

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

@@ -23,12 +23,6 @@ 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,20 +28,27 @@ 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: 'MessageClient is transformed'
+    then:
     testedProcess.exitValue() == 0
-    processTestLogLines {
-      it.contains("Transformed - instrumentation.target.class=datadog.smoketest.jbossmodules.client.MessageClient")
-    }
+    instrumentedMessageClient
+    !logHasErrors
   }
 
   @Override

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

@@ -104,21 +104,6 @@ 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,20 +37,27 @@ 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
-    processTestLogLines {
-      it.contains("Transformed - instrumentation.target.class=datadog.smoketest.osgi.client.MessageClient")
-    }
+    instrumentedMessageClient
+    !logHasErrors
   }
 
   @Override