Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,9 @@ public String[] helperClassNames() {
@Override
public String[] knownMatchingTypes() {
return new String[] {
"io.vertx.core.http.impl.Http1xServerResponse", "io.vertx.core.http.impl.Http2ServerResponse "
"io.vertx.core.http.impl.Http1xServerResponse",
"io.vertx.core.http.impl.Http2ServerResponse",
"io.vertx.core.http.impl.HttpServerResponseImpl" // when v >= 5.1
};
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import static datadog.trace.instrumentation.vertx_4_0.server.VertxDecorator.DECORATE;
import static datadog.trace.instrumentation.vertx_4_0.server.VertxDecorator.INSTRUMENTATION_NAME;

import datadog.appsec.api.blocking.BlockingException;
import datadog.context.ContextScope;
import datadog.trace.bootstrap.instrumentation.api.AgentSpan;
import datadog.trace.bootstrap.instrumentation.api.Tags;
Expand Down Expand Up @@ -65,6 +66,12 @@ public void handle(final RoutingContext routingContext) {
actual.handle(routingContext);
} catch (final Throwable t) {
DECORATE.onError(span, t);
if (t instanceof BlockingException) {
// AppSec uses BlockingException as control flow after committing a blocking response
// from advice such as WafPublishingBodyHandler and RoutingContextJsonAdvice. Finish
// immediately because that abort path may bypass Vert.x response/routing end callbacks.
finishHandlerSpan(routingContext);
}
throw t;
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package server

import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.BODY_JSON
import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.ERROR
import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.EXCEPTION
import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.LOGIN
Expand All @@ -16,6 +17,8 @@ import datadog.trace.instrumentation.netty41.server.NettyHttpServerDecorator
import datadog.trace.instrumentation.vertx_4_0.server.VertxDecorator
import io.vertx.core.AbstractVerticle
import io.vertx.core.Vertx
import okhttp3.MediaType
import okhttp3.RequestBody

class VertxHttpServerForkedTest extends HttpServerTest<Vertx> {
@Override
Expand Down Expand Up @@ -181,4 +184,28 @@ class VertxHttpServerWorkerForkedTest extends VertxHttpServerForkedTest {
HttpServer server() {
return new VertxServer(verticle(), routerBasePath(), true)
}

def 'test blocking of JSON request body finishes route handler span'() {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Replace the new Spock test with JUnit 5

The repo instructions in /workspace/dd-trace-java/AGENTS.md explicitly say not to write new Groovy/Spock tests and to migrate existing Groovy tests to JUnit 5. This adds a new Spock feature method to the Vert.x 5 test suite, so the new regression coverage should be implemented as JUnit 5 coverage instead of extending the deprecated Groovy suite.

Useful? React with 👍 / 👎.

setup:
// VertxTestServer handles BODY_JSON by calling ctx.body().asJsonObject().
// The IG_BODY_CONVERTED_HEADER is consumed by HttpServerTest's AppSec test callback, which
// returns a RequestBlockingAction from requestBodyProcessed() when that JSON body is converted.
def request = request(
BODY_JSON, 'POST',
RequestBody.create(MediaType.get('application/json'), '{"a": "x"}'))
.header(IG_BODY_CONVERTED_HEADER, 'true')
.build()

when:
def response = client.newCall(request).execute()

then:
response.code() == 413
response.body().charStream().text.contains('"title":"You\'ve been blocked"')
!handlerRan
// The client receiving a 413 only proves the blocking response was committed.
// We want to make sure that a BlockingException does now abort the worker route handler
// before the vertx.route-handler span has been finished (which would leave it dangling)
TEST_WRITER.waitForTraces(1)
}
}
Loading