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

@@ -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'() {

@@ -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
   }
 }
@@ -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";
   }
 

@@ -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()}")
+      }
     }
   }
 

@@ -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'
     }
   }
 

@@ -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)

@@ -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

@@ -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"

@@ -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

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

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

@@ -60,12 +60,6 @@ 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"
@@ -87,6 +81,18 @@ 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() {

@@ -89,14 +89,6 @@ 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}"

@@ -40,17 +40,13 @@ class IastSpringBootSmokeTest extends AbstractIastSpringBootTest {
 
     then:
     response.successful
-    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'
-      }
+    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'
     }
   }
 

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