Use the local root span for the http.route tag in Play

Author: manuel-alvarez-alvarez

dd-java-agent/instrumentation/play-2.3/src/main/java/datadog/trace/instrumentation/play23/PlayHttpServerDecorator.java

@@ -87,12 +87,17 @@ public AgentSpan onRequest(
       final Option pathOption = request.tags().get(Routes.ROUTE_PATTERN());
       if (!pathOption.isEmpty()) {
         final String path = (String) pathOption.get();
-        HTTP_RESOURCE_DECORATOR.withRoute(span, request.method(), path);
+        HTTP_RESOURCE_DECORATOR.withRoute(spanForHttpRoute(span), request.method(), path);
       }
     }
     return span;
   }
 
+  private AgentSpan spanForHttpRoute(final AgentSpan span) {
+    final AgentSpan rootSpan = span.getLocalRootSpan();
+    return rootSpan == null ? span : rootSpan;
+  }
+
   @Override
   public AgentSpan onError(final AgentSpan span, Throwable throwable) {
     if (REPORT_HTTP_STATUS) {

dd-java-agent/instrumentation/play-2.4/src/main/java/datadog/trace/instrumentation/play24/PlayHttpServerDecorator.java

@@ -87,12 +87,17 @@ public AgentSpan onRequest(
       final Option pathOption = request.tags().get("ROUTE_PATTERN");
       if (!pathOption.isEmpty()) {
         final String path = (String) pathOption.get();
-        HTTP_RESOURCE_DECORATOR.withRoute(span, request.method(), path);
+        HTTP_RESOURCE_DECORATOR.withRoute(spanForHttpRoute(span), request.method(), path);
       }
     }
     return span;
   }
 
+  private AgentSpan spanForHttpRoute(final AgentSpan span) {
+    final AgentSpan rootSpan = span.getLocalRootSpan();
+    return rootSpan == null ? span : rootSpan;
+  }
+
   @Override
   public AgentSpan onError(final AgentSpan span, Throwable throwable) {
     if (REPORT_HTTP_STATUS) {

dd-java-agent/instrumentation/play-2.6/src/main/java/datadog/trace/instrumentation/play26/PlayHttpServerDecorator.java

@@ -141,12 +141,17 @@ public AgentSpan onRequest(
         CharSequence path =
             PATH_CACHE.computeIfAbsent(
                 defOption.get().path(), p -> addMissingSlash(p, request.path()));
-        HTTP_RESOURCE_DECORATOR.withRoute(span, request.method(), path, true);
+        HTTP_RESOURCE_DECORATOR.withRoute(spanForHttpRoute(span), request.method(), path, true);
       }
     }
     return span;
   }
 
+  private AgentSpan spanForHttpRoute(final AgentSpan span) {
+    final AgentSpan rootSpan = span.getLocalRootSpan();
+    return rootSpan == null ? span : rootSpan;
+  }
+
   /*
      This is a workaround to add a `/` if it is missing when using split routes.
 

dd-smoke-tests/play-2.4/app/controllers/AppSecController.scala

@@ -0,0 +1,11 @@
+package controllers
+
+import play.api.mvc.{Action, AnyContent, Controller}
+
+class AppSecController extends Controller {
+
+  def apiSecuritySampling(statusCode: Int, test: String): Action[AnyContent] = Action {
+    Status(statusCode)("EXECUTED")
+  }
+
+}

dd-smoke-tests/play-2.4/build.gradle

@@ -63,6 +63,7 @@ dependencies {
   implementation group: 'io.opentracing', name: 'opentracing-util', version: '0.32.0'
 
   testImplementation project(':dd-smoke-tests')
+  testImplementation project(':dd-smoke-tests:appsec')
 }
 
 configurations.testImplementation {

dd-smoke-tests/play-2.4/conf/routes

@@ -6,3 +6,6 @@
 # An example controller showing a sample home page
 GET     /welcomej                           controllers.JController.doGet(id: Int ?= 0)
 GET     /welcomes                           controllers.SController.doGet(id: Option[Int])
+
+# AppSec endpoints for testing
+GET     /api_security/sampling/:statusCode  controllers.AppSecController.apiSecuritySampling(statusCode: Int, test: String)

dd-smoke-tests/play-2.4/src/test/groovy/datadog/smoketest/AppSecPlayNettySmokeTest.groovy

@@ -0,0 +1,92 @@
+package datadog.smoketest
+
+import datadog.smoketest.appsec.AbstractAppSecServerSmokeTest
+import datadog.trace.agent.test.utils.OkHttpUtils
+import okhttp3.Request
+import okhttp3.Response
+import spock.lang.Shared
+
+import java.nio.file.Files
+
+import static java.util.concurrent.TimeUnit.SECONDS
+
+class AppSecPlayNettySmokeTest  extends AbstractAppSecServerSmokeTest {
+
+  @Shared
+  File playDirectory = new File("${buildDirectory}/stage/main")
+
+  @Override
+  ProcessBuilder createProcessBuilder() {
+    // If the server is not shut down correctly, this file can be left there and will block
+    // the start of a new test
+    def runningPid = new File(playDirectory.getPath(), "RUNNING_PID")
+    if (runningPid.exists()) {
+      runningPid.delete()
+    }
+    def command = isWindows() ? 'main.bat' : 'main'
+    ProcessBuilder processBuilder = new ProcessBuilder("${playDirectory}/bin/${command}")
+    processBuilder.directory(playDirectory)
+    processBuilder.environment().put("JAVA_OPTS",
+      (defaultAppSecProperties + defaultJavaProperties).collect({ it.replace(' ', '\\ ')}).join(" ")
+      + " -Dconfig.file=${playDirectory}/conf/application.conf"
+      + " -Dhttp.port=${httpPort}"
+      + " -Dhttp.address=127.0.0.1"
+      + " -Dplay.server.provider=play.core.server.NettyServerProvider"
+      + " -Ddd.writer.type=MultiWriter:TraceStructureWriter:${output.getAbsolutePath()},DDAgentWriter")
+    return processBuilder
+  }
+
+  @Override
+  File createTemporaryFile() {
+    return new File("${buildDirectory}/tmp/trace-structure-play-2.4-appsec-netty.out")
+  }
+
+  void 'API Security samples only one request per endpoint'() {
+    given:
+    def url = "http://localhost:${httpPort}/api_security/sampling/200?test=value"
+    def client = OkHttpUtils.clientBuilder().build()
+    def request = new Request.Builder()
+      .url(url)
+      .addHeader('X-My-Header', "value")
+      .get()
+      .build()
+
+    when:
+    List<Response> responses = (1..3).collect {
+      client.newCall(request).execute()
+    }
+
+    then:
+    responses.each {
+      assert it.code() == 200
+    }
+    waitForTraceCount(3)
+    def spans = rootSpans.toList().toSorted { it.span.duration }
+    spans.size() == 3
+    def sampledSpans = spans.findAll { it.meta.keySet().any { it.startsWith('_dd.appsec.s.req.') } }
+    sampledSpans.size() == 1
+    def span = sampledSpans[0]
+    span.meta.containsKey('_dd.appsec.s.req.query')
+    span.meta.containsKey('_dd.appsec.s.req.headers')
+  }
+
+  // Ensure to clean up server and not only the shell script that starts it
+  def cleanupSpec() {
+    def pid = runningServerPid()
+    if (pid) {
+      def commands = isWindows() ? ['taskkill', '/PID', pid, '/T', '/F'] : ['kill', '-9', pid]
+      new ProcessBuilder(commands).start().waitFor(10, SECONDS)
+    }
+  }
+
+  def runningServerPid() {
+    def runningPid = new File(playDirectory.getPath(), 'RUNNING_PID')
+    if (runningPid.exists()) {
+      return Files.lines(runningPid.toPath()).findAny().orElse(null)
+    }
+  }
+
+  static isWindows() {
+    return System.getProperty('os.name').toLowerCase().contains('win')
+  }
+}

dd-smoke-tests/play-2.5/app/controllers/AppSecController.scala

@@ -0,0 +1,11 @@
+package controllers
+
+import play.api.mvc.{Action, AnyContent, Controller}
+
+class AppSecController extends Controller {
+
+  def apiSecuritySampling(statusCode: Int, test: String): Action[AnyContent] = Action {
+    Status(statusCode)("EXECUTED")
+  }
+
+}

dd-smoke-tests/play-2.5/build.gradle

@@ -65,6 +65,7 @@ dependencies {
   implementation group: 'io.opentracing', name: 'opentracing-util', version: '0.32.0'
 
   testImplementation project(':dd-smoke-tests')
+  testImplementation project(':dd-smoke-tests:appsec')
 }
 
 configurations.testImplementation {

dd-smoke-tests/play-2.5/conf/routes

@@ -6,3 +6,6 @@
 # An example controller showing a sample home page
 GET     /welcomej                           controllers.JController.doGet(id: Int ?= 0)
 GET     /welcomes                           controllers.SController.doGet(id: Option[Int])
+
+# AppSec endpoints for testing
+GET     /api_security/sampling/:statusCode  controllers.AppSecController.apiSecuritySampling(statusCode: Int, test: String)