diff --git a/dd-java-agent/instrumentation/vertx-web-3.4/src/main/java/datadog/trace/instrumentation/vertx_3_4/server/EndHandlerWrapper.java b/dd-java-agent/instrumentation/vertx-web-3.4/src/main/java/datadog/trace/instrumentation/vertx_3_4/server/EndHandlerWrapper.java index 22b0e5b4fb4..a8cb1ebb079 100644 --- a/dd-java-agent/instrumentation/vertx-web-3.4/src/main/java/datadog/trace/instrumentation/vertx_3_4/server/EndHandlerWrapper.java +++ b/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 { @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(); diff --git a/dd-java-agent/instrumentation/vertx-web-3.4/src/main/java/datadog/trace/instrumentation/vertx_3_4/server/RouteHandlerWrapper.java b/dd-java-agent/instrumentation/vertx-web-3.4/src/main/java/datadog/trace/instrumentation/vertx_3_4/server/RouteHandlerWrapper.java index 11d7b1634df..1c0d35a0191 100644 --- a/dd-java-agent/instrumentation/vertx-web-3.4/src/main/java/datadog/trace/instrumentation/vertx_3_4/server/RouteHandlerWrapper.java +++ b/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 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; } } diff --git a/dd-java-agent/instrumentation/vertx-web-4.0/src/main/java/datadog/trace/instrumentation/vertx_4_0/server/EndHandlerWrapper.java b/dd-java-agent/instrumentation/vertx-web-4.0/src/main/java/datadog/trace/instrumentation/vertx_4_0/server/EndHandlerWrapper.java index b4e35ce9070..a71584b11a7 100644 --- a/dd-java-agent/instrumentation/vertx-web-4.0/src/main/java/datadog/trace/instrumentation/vertx_4_0/server/EndHandlerWrapper.java +++ b/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 { @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(); diff --git a/dd-java-agent/instrumentation/vertx-web-4.0/src/main/java/datadog/trace/instrumentation/vertx_4_0/server/RouteHandlerWrapper.java b/dd-java-agent/instrumentation/vertx-web-4.0/src/main/java/datadog/trace/instrumentation/vertx_4_0/server/RouteHandlerWrapper.java index 94d056fb57e..6366ca163c6 100644 --- a/dd-java-agent/instrumentation/vertx-web-4.0/src/main/java/datadog/trace/instrumentation/vertx_4_0/server/RouteHandlerWrapper.java +++ b/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; } } diff --git a/dd-smoke-tests/vertx-3.4/application/src/main/java/datadog/vertx_3_4/MainVerticle.java b/dd-smoke-tests/vertx-3.4/application/src/main/java/datadog/vertx_3_4/MainVerticle.java index a681137ff9d..3b0b4d04761 100644 --- a/dd-smoke-tests/vertx-3.4/application/src/main/java/datadog/vertx_3_4/MainVerticle.java +++ b/dd-smoke-tests/vertx-3.4/application/src/main/java/datadog/vertx_3_4/MainVerticle.java @@ -42,12 +42,19 @@ public void start(Future 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() diff --git a/dd-smoke-tests/vertx-3.4/build.gradle b/dd-smoke-tests/vertx-3.4/build.gradle index 2ee69edf538..2d23603b9f1 100644 --- a/dd-smoke-tests/vertx-3.4/build.gradle +++ b/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" diff --git a/dd-smoke-tests/vertx-3.4/src/test/groovy/datadog/smoketest/AppSecVertxSmokeTest.groovy b/dd-smoke-tests/vertx-3.4/src/test/groovy/datadog/smoketest/AppSecVertxSmokeTest.groovy new file mode 100644 index 00000000000..d73b56adfc0 --- /dev/null +++ b/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 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 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') + } +} diff --git a/dd-smoke-tests/vertx-4.2/application/src/main/java/datadog/vertx_4_2/MainVerticle.java b/dd-smoke-tests/vertx-4.2/application/src/main/java/datadog/vertx_4_2/MainVerticle.java index 7afb9d965ce..64f795433bf 100644 --- a/dd-smoke-tests/vertx-4.2/application/src/main/java/datadog/vertx_4_2/MainVerticle.java +++ b/dd-smoke-tests/vertx-4.2/application/src/main/java/datadog/vertx_4_2/MainVerticle.java @@ -43,11 +43,19 @@ public void start(Promise 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() diff --git a/dd-smoke-tests/vertx-4.2/build.gradle b/dd-smoke-tests/vertx-4.2/build.gradle index c5f54819823..0d1ca0cd001 100644 --- a/dd-smoke-tests/vertx-4.2/build.gradle +++ b/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" diff --git a/dd-smoke-tests/vertx-4.2/src/test/groovy/AppSecVertxSmokeTest.groovy b/dd-smoke-tests/vertx-4.2/src/test/groovy/AppSecVertxSmokeTest.groovy new file mode 100644 index 00000000000..1ccf20d6fd1 --- /dev/null +++ b/dd-smoke-tests/vertx-4.2/src/test/groovy/AppSecVertxSmokeTest.groovy @@ -0,0 +1,73 @@ +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 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 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') + } +}