DataDog · cataphract · Nov 25, 2022 · Nov 16, 2022
@@ -1,35 +1,108 @@
+plugins {
+  id 'java-test-fixtures'
+}
 apply from: "$rootDir/gradle/java.gradle"
-apply plugin: 'org.unbroken-dome.test-sets'
 
-testSets {
-  latestDepTest {
-    dirName = 'test'
+String relAppDir = 'openliberty-jars/wlp/usr/servers/defaultServer/dropins/war/testapp'
+sourceSets {
+  webapp {
+    java {
+      destinationDirectory.value project.layout.buildDirectory.dir("$relAppDir/WEB-INF/classes")
+    }
+    output.resourcesDir = project.layout.buildDirectory.dir("$relAppDir/")
   }
 }
 
 configurations {
   zipped
+  testLogging
 }
 
+evaluationDependsOn ':dd-java-agent:instrumentation:servlet:request-3'
+
 dependencies {
-  compileOnly group: 'javax.servlet', name: 'javax.servlet-api', version: '3.0.1'
-  implementation project(':dd-java-agent:instrumentation:servlet-common')
   zipped group: 'io.openliberty', name: 'openliberty-runtime', version: '21.0.0.3', ext: 'zip'
+  testLogging deps.testLogging
+
+  compileOnly group: 'javax.servlet', name: 'javax.servlet-api', version: '3.0.1'
   compileOnly files({ tasks.installOpenLibertyDeps.extractedJars })
+  implementation project(':dd-java-agent:instrumentation:servlet-common')
+
+  testImplementation files({ tasks.installOpenLibertyDeps.wsServerJar })
+  testRuntimeOnly project(':dd-java-agent:instrumentation:osgi-4.3')
+  testRuntimeClasspath files({ tasks.filterLogbackClassic.filteredLogbackDir })
+
+  webappCompileOnly group: 'javax.servlet', name: 'javax.servlet-api', version: '3.0.1'
+  // compileOnly to avoid bringing all the test dependencies to the test app
+  // these are to be provided by the system classloader on test time
+  webappCompileOnly project(':dd-java-agent:instrumentation:servlet:request-3')
+    .tasks['compileTestFixturesJava'].classpath
+  // only the testFixtures jar (not its dependencies) and groovy should be included in the webapp
+  webappImplementation files(
+    project(':dd-java-agent:instrumentation:servlet:request-3')
+    .getTasksByName('testFixturesJar', false).archiveFile
+    )
+  // use the above instead of:
+  //  webappImplementation testFixtures(project(':dd-java-agent:instrumentation:servlet:request-3'))
+  // because using testFixtures() causes some early evaluation of dependencies
+  webappRuntimeOnly deps.groovy
+}
+compileWebappJava.dependsOn ':dd-java-agent:instrumentation:servlet:request-3:testFixturesJar'
+
+configurations.testRuntimeClasspath {
+  exclude group: 'ch.qos.logback', module: 'logback-classic'
+  exclude group: 'org.codehaus.groovy', module: 'groovy-servlet'
+}
+configurations.webappRuntimeClasspath {
+  exclude group: 'ch.qos.logback', module: 'logback-classic'
 }
 
 //unzips the dependencies from the 'zipped' configuration so 'compileOnly' can reference it
-tasks.register('installOpenLibertyDeps', Sync) {
+tasks.register('installOpenLibertyDeps', Copy) {
   def extractDir = "${buildDir}/openliberty-jars"
   ext.extractedJars = fileTree(extractDir) {
-    include "**/*.jar"
+    include "wlp/lib/*.jar"
+    builtBy "installOpenLibertyDeps"
+  }
+  ext.wsServerJar = fileTree("$extractDir") {
+    include "wlp/bin/tools/ws-server.jar"
     builtBy "installOpenLibertyDeps"
   }
   dependsOn configurations.zipped
+  // I didn't manage to get this to work correctly using a Sync task or Sync + outputs.upToDateWhen
+  // (files are updated when not needed, causing intellij to reindex or they are deemed up to date
+  // when the extraction was not done at all).
+  ext.serverXmlFile = file("$extractDir/wlp/usr/servers/defaultServer/server.xml")
+  onlyIf {
+    !ext.serverXmlFile.exists()
+  }
   from {
     configurations.zipped.collect { zipTree(it) }
   }
+  eachFile { fcd ->
+    fcd.path = fcd.path.replaceAll(/\/templates\/(servers\/defaultServer\/.+)/, '/usr/$1')
+  }
   into extractDir
+  outputs.file ext.serverXmlFile
+}
+test.dependsOn webappClasses, installOpenLibertyDeps
+test {
+  jvmArgs += ["-Dserver.xml=${installOpenLibertyDeps.serverXmlFile.absoluteFile}"]
 }
 
+tasks.register('webappCopyJars', Sync) {
+  from configurations.webappRuntimeClasspath.findAll { it.name.endsWith('.jar') }
+  into project.layout.buildDirectory.dir("$relAppDir/WEB-INF/lib")
+  dependsOn ':dd-java-agent:instrumentation:servlet:request-3:testFixturesJar'
+}
+test.dependsOn webappCopyJars
 
+tasks.register('filterLogbackClassic', Sync) {
+  ext.filteredLogbackDir = project.layout.buildDirectory.dir('filteredLogback')
+  from configurations.testLogging
+    .findAll { it.name.contains('logback-') }
+    .collect { zipTree(it) }
+  exclude 'META-INF/**'
+  into ext.filteredLogbackDir
+}
+test.dependsOn filterLogbackClassic
@@ -40,9 +40,9 @@ public static final class Response extends HttpServletExtractAdapter<HttpServlet
     public static final Response GETTER = new Response();
 
     @Override
-    Enumeration<String> getHeaderNames(HttpServletResponse request) {
+    Enumeration<String> getHeaderNames(HttpServletResponse response) {
       try {
-        return Collections.enumeration(request.getHeaderNames());
+        return Collections.enumeration(response.getHeaderNames());
       } catch (NullPointerException e) {
         // SRTServletResponse#getHeaderNames() will throw NPE if called after response close.
         return Collections.emptyEnumeration();

@@ -18,6 +18,7 @@
 import datadog.trace.bootstrap.instrumentation.api.AgentScope;
 import datadog.trace.bootstrap.instrumentation.api.AgentSpan;
 import datadog.trace.instrumentation.servlet.ServletBlockingHelper;
+import java.util.EnumSet;
 import javax.servlet.ServletRequest;
 import javax.servlet.ServletResponse;
 import net.bytebuddy.asm.Advice;
@@ -32,7 +33,7 @@ public LibertyServerInstrumentation() {
 
   @Override
   public String instrumentedType() {
-    return "com.ibm.ws.webcontainer.webapp.WebApp";
+    return "com.ibm.ws.webcontainer.filter.WebAppFilterManager";
   }
 
   @Override
@@ -51,10 +52,13 @@ public String[] helperClassNames() {
   public void adviceTransformations(AdviceTransformation transformation) {
     transformation.applyAdvice(
         isMethod()
-            .and(named("handleRequest"))
+            .and(named("invokeFilters"))
             .and(takesArgument(0, named("javax.servlet.ServletRequest")))
             .and(takesArgument(1, named("javax.servlet.ServletResponse")))
-            .and(takesArgument(2, named("com.ibm.wsspi.http.HttpInboundConnection"))),
+            .and(takesArgument(2, named("com.ibm.wsspi.webcontainer.servlet.IServletContext")))
+            .and(takesArgument(3, named("com.ibm.wsspi.webcontainer.RequestProcessor")))
+            .and(takesArgument(4, EnumSet.class))
+            .and(takesArgument(5, named("com.ibm.wsspi.http.HttpInboundConnection"))),
         LibertyServerInstrumentation.class.getName() + "$HandleRequestAdvice");
   }
 
@@ -110,6 +114,9 @@ public static void closeScope(
       if (scope != null) {
         // we cannot get path at the start because the path/context attributes are not yet
         // initialized
+        // this has the unfortunate consequence that service name (as set via the tag interceptor)
+        // of the top span won't match that of its child spans, because it's instead the original
+        // one that will propagate
         DECORATE.getPath(scope.span(), request);
         scope.close();
       }

@@ -0,0 +1,69 @@
+package datadog.trace.instrumentation.liberty20;
+
+import static datadog.trace.agent.tooling.bytebuddy.matcher.NameMatchers.named;
+import static datadog.trace.api.gateway.Events.EVENTS;
+import static net.bytebuddy.matcher.ElementMatchers.isMethod;
+import static net.bytebuddy.matcher.ElementMatchers.isProtected;
+import static net.bytebuddy.matcher.ElementMatchers.isPublic;
+import static net.bytebuddy.matcher.ElementMatchers.returns;
+import static net.bytebuddy.matcher.ElementMatchers.takesArguments;
+
+import com.google.auto.service.AutoService;
+import datadog.trace.advice.ActiveRequestContext;
+import datadog.trace.advice.RequiresRequestContext;
+import datadog.trace.agent.tooling.Instrumenter;
+import datadog.trace.api.gateway.CallbackProvider;
+import datadog.trace.api.gateway.Flow;
+import datadog.trace.api.gateway.RequestContext;
+import datadog.trace.api.gateway.RequestContextSlot;
+import datadog.trace.bootstrap.instrumentation.api.AgentTracer;
+import java.util.Hashtable;
+import java.util.function.BiFunction;
+import net.bytebuddy.asm.Advice;
+
+@AutoService(Instrumenter.class)
+public class ParsePostDataInstrumentation extends Instrumenter.AppSec
+    implements Instrumenter.ForKnownTypes {
+  public ParsePostDataInstrumentation() {
+    super("liberty");
+  }
+
+  @Override
+  public String[] knownMatchingTypes() {
+    return new String[] {
+      "com.ibm.ws.webcontainer.srt.SRTServletRequest",
+      "com.ibm.ws.webcontainer31.srt.SRTServletRequest31",
+    };
+  }
+
+  @Override
+  public void adviceTransformations(AdviceTransformation transformation) {
+    transformation.applyAdvice(
+        isMethod()
+            .and(named("parsePostData"))
+            .and(isPublic().or(isProtected()))
+            .and(takesArguments(0))
+            .and(returns(Hashtable.class)),
+        ParsePostDataInstrumentation.class.getName() + "$ParsePostDataAdvice");
+  }
+
+  @RequiresRequestContext(RequestContextSlot.APPSEC)
+  static class ParsePostDataAdvice {
+    @Advice.OnMethodExit(suppress = Throwable.class)
+    static void after(
+        @Advice.Return Hashtable<String, String[]> retval,
+        @ActiveRequestContext RequestContext reqCtx) {
+      if (retval == null || retval.isEmpty()) {
+        return;
+      }
+
+      CallbackProvider cbp = AgentTracer.get().getCallbackProvider(RequestContextSlot.APPSEC);
+      BiFunction<RequestContext, Object, Flow<Void>> callback =
+          cbp.getCallback(EVENTS.requestBodyProcessed());
+      if (callback == null) {
+        return;
+      }
+      callback.apply(reqCtx, retval);
+    }
+  }
+}
@@ -44,23 +44,34 @@ public void adviceTransformations(AdviceTransformation transformation) {
   }
 
   public static class ResponseFinishAdvice {
-    @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class)
-    public static void stopSpan(@Advice.This SRTServletResponse resp) {
+    @Advice.OnMethodEnter(suppress = Throwable.class)
+    static AgentSpan onEnter(@Advice.This SRTServletResponse resp) {
+      // this is the last opportunity to have any meaningful
+      // interaction with the response
+      AgentSpan span = null;
       IExtendedRequest req = resp.getRequest();
-      Object spanObj = null;
       try {
-        spanObj = req.getAttribute(DD_SPAN_ATTRIBUTE);
+        Object spanObj = req.getAttribute(DD_SPAN_ATTRIBUTE);
+        if (spanObj instanceof AgentSpan) {
+          span = (AgentSpan) spanObj;
+          req.setAttribute(DD_SPAN_ATTRIBUTE, null);
+          DECORATE.onResponse(span, resp);
+        }
       } catch (NullPointerException e) {
         // OpenLiberty will throw NPE on getAttribute if the response has already been closed.
       }
 
-      if (spanObj instanceof AgentSpan) {
-        req.setAttribute(DD_SPAN_ATTRIBUTE, null);
-        final AgentSpan span = (AgentSpan) spanObj;
-        DECORATE.onResponse(span, resp);
-        DECORATE.beforeFinish(span);
-        span.finish();
+      return span;
+    }
+
+    @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class)
+    public static void stopSpan(
+        @Advice.This SRTServletResponse resp, @Advice.Enter AgentSpan span) {
+      if (span == null) {
+        return;
       }
+      DECORATE.beforeFinish(span);
+      span.finish();
     }
   }
 }