Write http.route tag as soon as possible in vert.x

dd-java-agent/instrumentation/vertx-web-3.4/src/main/java/datadog/trace/instrumentation/vertx_3_4/server/EndHandlerWrapper.java

@@ -1,13 +1,9 @@
 package datadog.trace.instrumentation.vertx_3_4.server;
 
-import static datadog.trace.bootstrap.instrumentation.decorator.http.HttpResourceDecorator.HTTP_RESOURCE_DECORATOR;
 import static datadog.trace.instrumentation.vertx_3_4.server.RouteHandlerWrapper.HANDLER_SPAN_CONTEXT_KEY;
-import static datadog.trace.instrumentation.vertx_3_4.server.RouteHandlerWrapper.PARENT_SPAN_CONTEXT_KEY;
-import static datadog.trace.instrumentation.vertx_3_4.server.RouteHandlerWrapper.ROUTE_CONTEXT_KEY;
 import static datadog.trace.instrumentation.vertx_3_4.server.VertxDecorator.DECORATE;
 
 import datadog.trace.bootstrap.instrumentation.api.AgentSpan;
-import datadog.trace.bootstrap.instrumentation.api.Tags;
 import io.vertx.core.Handler;
 import io.vertx.ext.web.RoutingContext;
 
@@ -23,20 +19,11 @@ public class EndHandlerWrapper implements Handler<Void> {
   @Override
   public void handle(final Void event) {
     AgentSpan span = routingContext.get(HANDLER_SPAN_CONTEXT_KEY);
-    AgentSpan parentSpan = routingContext.get(PARENT_SPAN_CONTEXT_KEY);
-    String path = routingContext.get(ROUTE_CONTEXT_KEY);
     try {
       if (actual != null) {
         actual.handle(event);
       }
     } finally {
-      if (path != null
-          && parentSpan != null
-          // do not override route with a "/" if it's already set (it's probably more meaningful)
-          && !(path.equals("/") && parentSpan.getTag(Tags.HTTP_ROUTE) != null)) {
-        HTTP_RESOURCE_DECORATOR.withRoute(
-            parentSpan, routingContext.request().rawMethod(), path, true);
-      }
       if (span != null) {
         DECORATE.onResponse(span, routingContext.response());
         span.finish();

dd-java-agent/instrumentation/vertx-web-3.4/src/main/java/datadog/trace/instrumentation/vertx_3_4/server/RouteHandlerWrapper.java

@@ -4,10 +4,10 @@
 import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.activeSpan;
 import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.noopScope;
 import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.startSpan;
+import static datadog.trace.bootstrap.instrumentation.decorator.http.HttpResourceDecorator.HTTP_RESOURCE_DECORATOR;
 import static datadog.trace.instrumentation.vertx_3_4.server.VertxDecorator.DECORATE;
 import static datadog.trace.instrumentation.vertx_3_4.server.VertxDecorator.INSTRUMENTATION_NAME;
 
-import datadog.trace.api.gateway.Flow;
 import datadog.trace.bootstrap.instrumentation.api.AgentScope;
 import datadog.trace.bootstrap.instrumentation.api.AgentSpan;
 import datadog.trace.bootstrap.instrumentation.api.Tags;
@@ -39,7 +39,6 @@ public RouteHandlerWrapper(final Handler<RoutingContext> handler) {
   @Override
   public void handle(final RoutingContext routingContext) {
     AgentSpan span = routingContext.get(HANDLER_SPAN_CONTEXT_KEY);
-    Flow.Action.RequestBlockingAction rba = null;
     if (spanStarter) {
       if (span == null) {
         AgentSpan parentSpan = activeSpan();
@@ -52,8 +51,7 @@ public void handle(final RoutingContext routingContext) {
         DECORATE.afterStart(span);
         span.setResourceName(DECORATE.className(actual.getClass()));
       }
-
-      updateRoutingContextWithRoute(routingContext);
+      setRoute(routingContext);
     }
     try (final AgentScope scope = span != null ? activateSpan(span) : noopScope()) {
       try {
@@ -65,7 +63,12 @@ public void handle(final RoutingContext routingContext) {
     }
   }
 
-  private void updateRoutingContextWithRoute(RoutingContext routingContext) {
+  private void setRoute(RoutingContext routingContext) {
+    final AgentSpan parentSpan = routingContext.get(PARENT_SPAN_CONTEXT_KEY);
+    if (parentSpan == null) {
+      return;
+    }
+
     final String method = routingContext.request().rawMethod();
     String mountPoint = routingContext.mountPoint();
     String path = routingContext.currentRoute().getPath();
@@ -78,8 +81,22 @@ private void updateRoutingContextWithRoute(RoutingContext routingContext) {
       }
       path = mountPoint + path;
     }
-    if (method != null && path != null) {
+    if (method != null && path != null && shouldUpdateRoute(routingContext, parentSpan, path)) {
       routingContext.put(ROUTE_CONTEXT_KEY, path);
+      HTTP_RESOURCE_DECORATOR.withRoute(parentSpan, method, path, true);
+    }
+  }
+
+  static boolean shouldUpdateRoute(
+      final RoutingContext routingContext, final AgentSpan span, final String path) {
+    if (span == null) {
+      return false;
+    }
+    final String currentRoute = routingContext.get(ROUTE_CONTEXT_KEY);
+    if (currentRoute != null && currentRoute.equals(path)) {
+      return false;
     }
+    // do not override route with a "/" if it's already set (it's probably more meaningful)
+    return !path.equals("/") || span.getTag(Tags.HTTP_ROUTE) == null;
   }
 }

dd-java-agent/instrumentation/vertx-web-4.0/src/main/java/datadog/trace/instrumentation/vertx_4_0/server/EndHandlerWrapper.java

@@ -1,13 +1,9 @@
 package datadog.trace.instrumentation.vertx_4_0.server;
 
-import static datadog.trace.bootstrap.instrumentation.decorator.http.HttpResourceDecorator.HTTP_RESOURCE_DECORATOR;
 import static datadog.trace.instrumentation.vertx_4_0.server.RouteHandlerWrapper.HANDLER_SPAN_CONTEXT_KEY;
-import static datadog.trace.instrumentation.vertx_4_0.server.RouteHandlerWrapper.PARENT_SPAN_CONTEXT_KEY;
-import static datadog.trace.instrumentation.vertx_4_0.server.RouteHandlerWrapper.ROUTE_CONTEXT_KEY;
 import static datadog.trace.instrumentation.vertx_4_0.server.VertxDecorator.DECORATE;
 
 import datadog.trace.bootstrap.instrumentation.api.AgentSpan;
-import datadog.trace.bootstrap.instrumentation.api.Tags;
 import io.vertx.core.Handler;
 import io.vertx.ext.web.RoutingContext;
 
@@ -23,20 +19,11 @@ public class EndHandlerWrapper implements Handler<Void> {
   @Override
   public void handle(final Void event) {
     AgentSpan span = routingContext.get(HANDLER_SPAN_CONTEXT_KEY);
-    AgentSpan parentSpan = routingContext.get(PARENT_SPAN_CONTEXT_KEY);
-    String path = routingContext.get(ROUTE_CONTEXT_KEY);
     try {
       if (actual != null) {
         actual.handle(event);
       }
     } finally {
-      if (path != null
-          && parentSpan != null
-          // do not override route with a "/" if it's already set (it's probably more meaningful)
-          && !(path.equals("/") && parentSpan.getTag(Tags.HTTP_ROUTE) != null)) {
-        HTTP_RESOURCE_DECORATOR.withRoute(
-            parentSpan, routingContext.request().method().name(), path, true);
-      }
       if (span != null) {
         DECORATE.onResponse(span, routingContext.response());
         span.finish();

dd-java-agent/instrumentation/vertx-web-4.0/src/main/java/datadog/trace/instrumentation/vertx_4_0/server/RouteHandlerWrapper.java

@@ -4,6 +4,7 @@
 import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.activeSpan;
 import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.noopScope;
 import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.startSpan;
+import static datadog.trace.bootstrap.instrumentation.decorator.http.HttpResourceDecorator.HTTP_RESOURCE_DECORATOR;
 import static datadog.trace.instrumentation.vertx_4_0.server.VertxDecorator.DECORATE;
 import static datadog.trace.instrumentation.vertx_4_0.server.VertxDecorator.INSTRUMENTATION_NAME;
 
@@ -46,7 +47,7 @@ public void handle(final RoutingContext routingContext) {
         DECORATE.afterStart(span);
         span.setResourceName(DECORATE.className(actual.getClass()));
       }
-      updateRoutingContextWithRoute(routingContext);
+      setRoute(routingContext);
     }
 
     try (final AgentScope scope = span != null ? activateSpan(span) : noopScope()) {
@@ -59,7 +60,12 @@ public void handle(final RoutingContext routingContext) {
     }
   }
 
-  private void updateRoutingContextWithRoute(RoutingContext routingContext) {
+  private void setRoute(RoutingContext routingContext) {
+    final AgentSpan parentSpan = routingContext.get(PARENT_SPAN_CONTEXT_KEY);
+    if (parentSpan == null) {
+      return;
+    }
+
     final String method = routingContext.request().method().name();
     final String mountPoint = routingContext.mountPoint();
 
@@ -73,8 +79,22 @@ private void updateRoutingContextWithRoute(RoutingContext routingContext) {
               : mountPoint;
       path = noBackslashhMountPoint + path;
     }
-    if (method != null && path != null) {
+    if (method != null && path != null && shouldUpdateRoute(routingContext, parentSpan, path)) {
       routingContext.put(ROUTE_CONTEXT_KEY, path);
+      HTTP_RESOURCE_DECORATOR.withRoute(parentSpan, method, path, true);
+    }
+  }
+
+  static boolean shouldUpdateRoute(
+      final RoutingContext routingContext, final AgentSpan span, final String path) {
+    if (span == null) {
+      return false;
+    }
+    final String currentRoute = routingContext.get(ROUTE_CONTEXT_KEY);
+    if (currentRoute != null && currentRoute.equals(path)) {
+      return false;
     }
+    // do not override route with a "/" if it's already set (it's probably more meaningful)
+    return !path.equals("/") || span.getTag(Tags.HTTP_ROUTE) == null;
   }
 }

dd-smoke-tests/vertx-3.4/application/src/main/java/datadog/vertx_3_4/MainVerticle.java

@@ -42,12 +42,19 @@ public void start(Future<Void> startPromise) throws Exception {
                     .setStatusCode(200)
                     .putHeader("content-type", "text/plain")
                     .end(randomFactorial().toString()));
+    router
+        .route("/api_security/sampling/:status_code")
+        .handler(
+            ctx ->
+                ctx.response()
+                    .setStatusCode(Integer.parseInt(ctx.request().getParam("status_code")))
+                    .end("EXECUTED"));
 
     vertx
         .createHttpServer(new HttpServerOptions().setHandle100ContinueAutomatically(true))
         .requestHandler(
             req -> {
-              if (req.path().startsWith("/routes")) {
+              if (req.path().startsWith("/routes") || req.path().startsWith("/api_security")) {
                 router.accept(req);
               } else {
                 req.response()

dd-smoke-tests/vertx-3.4/build.gradle

@@ -8,6 +8,7 @@ apply from: "$rootDir/gradle/java.gradle"
 dependencies {
   testImplementation project(':dd-smoke-tests')
   testImplementation(testFixtures(project(":dd-smoke-tests:iast-util")))
+  testImplementation project(':dd-smoke-tests:appsec')
 }
 
 def appDir = "$projectDir/application"

dd-smoke-tests/vertx-3.4/src/test/groovy/datadog/smoketest/AppSecVertxSmokeTest.groovy

@@ -0,0 +1,75 @@
+package datadog.smoketest
+
+import datadog.smoketest.appsec.AbstractAppSecServerSmokeTest
+import datadog.trace.agent.test.utils.OkHttpUtils
+import okhttp3.Request
+import okhttp3.Response
+import spock.lang.IgnoreIf
+
+@IgnoreIf({
+  // TODO https://github.com/eclipse-vertx/vert.x/issues/2172
+  new BigDecimal(System.getProperty("java.specification.version")).isAtLeast(17.0) })
+class AppSecVertxSmokeTest extends AbstractAppSecServerSmokeTest {
+
+  @Override
+  def logLevel() {
+    'DEBUG'
+  }
+
+  @Override
+  ProcessBuilder createProcessBuilder() {
+    String vertxUberJar = System.getProperty("datadog.smoketest.vertx.uberJar.path")
+
+    List<String> command = new ArrayList<>()
+    command.add(javaPath())
+    command.addAll(defaultJavaProperties)
+    command.addAll(defaultAppSecProperties)
+    command.addAll((String[]) [
+      "-Ddd.writer.type=MultiWriter:TraceStructureWriter:${output.getAbsolutePath()},DDAgentWriter",
+      "-Dvertx.http.port=${httpPort}",
+      "-jar",
+      vertxUberJar
+    ])
+    ProcessBuilder processBuilder = new ProcessBuilder(command)
+    processBuilder.directory(new File(buildDirectory))
+  }
+
+  @Override
+  File createTemporaryFile() {
+    return new File("${buildDirectory}/tmp/trace-structure-vertx.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.params')
+    span.meta.containsKey('_dd.appsec.s.req.headers')
+  }
+}

dd-smoke-tests/vertx-4.2/application/src/main/java/datadog/vertx_4_2/MainVerticle.java

@@ -43,11 +43,19 @@ public void start(Promise<Void> startPromise) throws Exception {
                     .putHeader("content-type", "text/plain")
                     .end(randomFactorial().toString()));
 
+    router
+        .route("/api_security/sampling/:status_code")
+        .handler(
+            ctx ->
+                ctx.response()
+                    .setStatusCode(Integer.parseInt(ctx.request().getParam("status_code")))
+                    .end("EXECUTED"));
+
     vertx
         .createHttpServer(new HttpServerOptions().setHandle100ContinueAutomatically(true))
         .requestHandler(
             req -> {
-              if (req.path().startsWith("/routes")) {
+              if (req.path().startsWith("/routes") || req.path().startsWith("/api_security")) {
                 router.handle(req);
               } else {
                 req.response()

dd-smoke-tests/vertx-4.2/build.gradle

@@ -7,6 +7,7 @@ apply from: "$rootDir/gradle/java.gradle"
 dependencies {
   testImplementation project(':dd-smoke-tests')
   testImplementation(testFixtures(project(":dd-smoke-tests:iast-util")))
+  testImplementation project(':dd-smoke-tests:appsec')
 }
 
 def appDir = "$projectDir/application"