Skip to content

Commit f6fc3e6

Browse files
Write http.route tag as soon as possible in vert.x
1 parent c4e17f1 commit f6fc3e6

File tree

10 files changed

+188
-45
lines changed

10 files changed

+188
-45
lines changed

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

Lines changed: 0 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,9 @@
11
package datadog.trace.instrumentation.vertx_3_4.server;
22

3-
import static datadog.trace.bootstrap.instrumentation.decorator.http.HttpResourceDecorator.HTTP_RESOURCE_DECORATOR;
43
import static datadog.trace.instrumentation.vertx_3_4.server.RouteHandlerWrapper.HANDLER_SPAN_CONTEXT_KEY;
5-
import static datadog.trace.instrumentation.vertx_3_4.server.RouteHandlerWrapper.PARENT_SPAN_CONTEXT_KEY;
6-
import static datadog.trace.instrumentation.vertx_3_4.server.RouteHandlerWrapper.ROUTE_CONTEXT_KEY;
74
import static datadog.trace.instrumentation.vertx_3_4.server.VertxDecorator.DECORATE;
85

96
import datadog.trace.bootstrap.instrumentation.api.AgentSpan;
10-
import datadog.trace.bootstrap.instrumentation.api.Tags;
117
import io.vertx.core.Handler;
128
import io.vertx.ext.web.RoutingContext;
139

@@ -23,20 +19,11 @@ public class EndHandlerWrapper implements Handler<Void> {
2319
@Override
2420
public void handle(final Void event) {
2521
AgentSpan span = routingContext.get(HANDLER_SPAN_CONTEXT_KEY);
26-
AgentSpan parentSpan = routingContext.get(PARENT_SPAN_CONTEXT_KEY);
27-
String path = routingContext.get(ROUTE_CONTEXT_KEY);
2822
try {
2923
if (actual != null) {
3024
actual.handle(event);
3125
}
3226
} finally {
33-
if (path != null
34-
&& parentSpan != null
35-
// do not override route with a "/" if it's already set (it's probably more meaningful)
36-
&& !(path.equals("/") && parentSpan.getTag(Tags.HTTP_ROUTE) != null)) {
37-
HTTP_RESOURCE_DECORATOR.withRoute(
38-
parentSpan, routingContext.request().rawMethod(), path, true);
39-
}
4027
if (span != null) {
4128
DECORATE.onResponse(span, routingContext.response());
4229
span.finish();

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

Lines changed: 14 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -4,10 +4,10 @@
44
import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.activeSpan;
55
import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.noopScope;
66
import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.startSpan;
7+
import static datadog.trace.bootstrap.instrumentation.decorator.http.HttpResourceDecorator.HTTP_RESOURCE_DECORATOR;
78
import static datadog.trace.instrumentation.vertx_3_4.server.VertxDecorator.DECORATE;
89
import static datadog.trace.instrumentation.vertx_3_4.server.VertxDecorator.INSTRUMENTATION_NAME;
910

10-
import datadog.trace.api.gateway.Flow;
1111
import datadog.trace.bootstrap.instrumentation.api.AgentScope;
1212
import datadog.trace.bootstrap.instrumentation.api.AgentSpan;
1313
import datadog.trace.bootstrap.instrumentation.api.Tags;
@@ -17,9 +17,7 @@
1717
import io.vertx.ext.web.impl.RouterImpl;
1818

1919
public class RouteHandlerWrapper implements Handler<RoutingContext> {
20-
static final String PARENT_SPAN_CONTEXT_KEY = AgentSpan.class.getName() + ".parent";
2120
static final String HANDLER_SPAN_CONTEXT_KEY = AgentSpan.class.getName() + ".handler";
22-
static final String ROUTE_CONTEXT_KEY = "dd." + Tags.HTTP_ROUTE;
2321

2422
private final Handler<RoutingContext> actual;
2523
private final boolean spanStarter;
@@ -39,21 +37,19 @@ public RouteHandlerWrapper(final Handler<RoutingContext> handler) {
3937
@Override
4038
public void handle(final RoutingContext routingContext) {
4139
AgentSpan span = routingContext.get(HANDLER_SPAN_CONTEXT_KEY);
42-
Flow.Action.RequestBlockingAction rba = null;
4340
if (spanStarter) {
4441
if (span == null) {
4542
AgentSpan parentSpan = activeSpan();
46-
routingContext.put(PARENT_SPAN_CONTEXT_KEY, parentSpan);
4743

4844
span = startSpan(INSTRUMENTATION_NAME);
4945
routingContext.put(HANDLER_SPAN_CONTEXT_KEY, span);
5046

5147
routingContext.response().endHandler(new EndHandlerWrapper(routingContext));
5248
DECORATE.afterStart(span);
5349
span.setResourceName(DECORATE.className(actual.getClass()));
54-
}
5550

56-
updateRoutingContextWithRoute(routingContext);
51+
setRoute(parentSpan, routingContext);
52+
}
5753
}
5854
try (final AgentScope scope = span != null ? activateSpan(span) : noopScope()) {
5955
try {
@@ -65,7 +61,7 @@ public void handle(final RoutingContext routingContext) {
6561
}
6662
}
6763

68-
private void updateRoutingContextWithRoute(RoutingContext routingContext) {
64+
private void setRoute(AgentSpan parentSpan, RoutingContext routingContext) {
6965
final String method = routingContext.request().rawMethod();
7066
String mountPoint = routingContext.mountPoint();
7167
String path = routingContext.currentRoute().getPath();
@@ -78,8 +74,16 @@ private void updateRoutingContextWithRoute(RoutingContext routingContext) {
7874
}
7975
path = mountPoint + path;
8076
}
81-
if (method != null && path != null) {
82-
routingContext.put(ROUTE_CONTEXT_KEY, path);
77+
if (method != null && path != null && shouldUpdateRoute(parentSpan, path)) {
78+
HTTP_RESOURCE_DECORATOR.withRoute(parentSpan, method, path, true);
79+
}
80+
}
81+
82+
static boolean shouldUpdateRoute(final AgentSpan span, final String path) {
83+
if (span == null) {
84+
return false;
8385
}
86+
// do not override route with a "/" if it's already set (it's probably more meaningful)
87+
return !path.equals("/") || span.getTag(Tags.HTTP_ROUTE) == null;
8488
}
8589
}

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

Lines changed: 0 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,9 @@
11
package datadog.trace.instrumentation.vertx_4_0.server;
22

3-
import static datadog.trace.bootstrap.instrumentation.decorator.http.HttpResourceDecorator.HTTP_RESOURCE_DECORATOR;
43
import static datadog.trace.instrumentation.vertx_4_0.server.RouteHandlerWrapper.HANDLER_SPAN_CONTEXT_KEY;
5-
import static datadog.trace.instrumentation.vertx_4_0.server.RouteHandlerWrapper.PARENT_SPAN_CONTEXT_KEY;
6-
import static datadog.trace.instrumentation.vertx_4_0.server.RouteHandlerWrapper.ROUTE_CONTEXT_KEY;
74
import static datadog.trace.instrumentation.vertx_4_0.server.VertxDecorator.DECORATE;
85

96
import datadog.trace.bootstrap.instrumentation.api.AgentSpan;
10-
import datadog.trace.bootstrap.instrumentation.api.Tags;
117
import io.vertx.core.Handler;
128
import io.vertx.ext.web.RoutingContext;
139

@@ -23,20 +19,11 @@ public class EndHandlerWrapper implements Handler<Void> {
2319
@Override
2420
public void handle(final Void event) {
2521
AgentSpan span = routingContext.get(HANDLER_SPAN_CONTEXT_KEY);
26-
AgentSpan parentSpan = routingContext.get(PARENT_SPAN_CONTEXT_KEY);
27-
String path = routingContext.get(ROUTE_CONTEXT_KEY);
2822
try {
2923
if (actual != null) {
3024
actual.handle(event);
3125
}
3226
} finally {
33-
if (path != null
34-
&& parentSpan != null
35-
// do not override route with a "/" if it's already set (it's probably more meaningful)
36-
&& !(path.equals("/") && parentSpan.getTag(Tags.HTTP_ROUTE) != null)) {
37-
HTTP_RESOURCE_DECORATOR.withRoute(
38-
parentSpan, routingContext.request().method().name(), path, true);
39-
}
4027
if (span != null) {
4128
DECORATE.onResponse(span, routingContext.response());
4229
span.finish();

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

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.activeSpan;
55
import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.noopScope;
66
import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.startSpan;
7+
import static datadog.trace.bootstrap.instrumentation.decorator.http.HttpResourceDecorator.HTTP_RESOURCE_DECORATOR;
78
import static datadog.trace.instrumentation.vertx_4_0.server.VertxDecorator.DECORATE;
89
import static datadog.trace.instrumentation.vertx_4_0.server.VertxDecorator.INSTRUMENTATION_NAME;
910

@@ -15,9 +16,7 @@
1516
import io.vertx.ext.web.impl.RouteImpl;
1617

1718
public class RouteHandlerWrapper implements Handler<RoutingContext> {
18-
static final String PARENT_SPAN_CONTEXT_KEY = AgentSpan.class.getName() + ".parent";
1919
static final String HANDLER_SPAN_CONTEXT_KEY = AgentSpan.class.getName() + ".handler";
20-
static final String ROUTE_CONTEXT_KEY = "dd." + Tags.HTTP_ROUTE;
2120

2221
private final Handler<RoutingContext> actual;
2322
private final boolean spanStarter;
@@ -37,16 +36,16 @@ public void handle(final RoutingContext routingContext) {
3736
if (spanStarter) {
3837
if (span == null) {
3938
AgentSpan parentSpan = activeSpan();
40-
routingContext.put(PARENT_SPAN_CONTEXT_KEY, parentSpan);
4139

4240
span = startSpan(INSTRUMENTATION_NAME);
4341
routingContext.put(HANDLER_SPAN_CONTEXT_KEY, span);
4442

4543
routingContext.response().endHandler(new EndHandlerWrapper(routingContext));
4644
DECORATE.afterStart(span);
4745
span.setResourceName(DECORATE.className(actual.getClass()));
46+
47+
setRoute(parentSpan, routingContext);
4848
}
49-
updateRoutingContextWithRoute(routingContext);
5049
}
5150

5251
try (final AgentScope scope = span != null ? activateSpan(span) : noopScope()) {
@@ -59,7 +58,7 @@ public void handle(final RoutingContext routingContext) {
5958
}
6059
}
6160

62-
private void updateRoutingContextWithRoute(RoutingContext routingContext) {
61+
private void setRoute(AgentSpan parentSpan, RoutingContext routingContext) {
6362
final String method = routingContext.request().method().name();
6463
final String mountPoint = routingContext.mountPoint();
6564

@@ -73,8 +72,16 @@ private void updateRoutingContextWithRoute(RoutingContext routingContext) {
7372
: mountPoint;
7473
path = noBackslashhMountPoint + path;
7574
}
76-
if (method != null && path != null) {
77-
routingContext.put(ROUTE_CONTEXT_KEY, path);
75+
if (method != null && path != null && shouldUpdateRoute(parentSpan, path)) {
76+
HTTP_RESOURCE_DECORATOR.withRoute(parentSpan, method, path, true);
77+
}
78+
}
79+
80+
static boolean shouldUpdateRoute(final AgentSpan span, final String path) {
81+
if (span == null) {
82+
return false;
7883
}
84+
// do not override route with a "/" if it's already set (it's probably more meaningful)
85+
return !path.equals("/") || span.getTag(Tags.HTTP_ROUTE) == null;
7986
}
8087
}

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

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,12 +42,19 @@ public void start(Future<Void> startPromise) throws Exception {
4242
.setStatusCode(200)
4343
.putHeader("content-type", "text/plain")
4444
.end(randomFactorial().toString()));
45+
router
46+
.route("/api_security/sampling/:status_code")
47+
.handler(
48+
ctx ->
49+
ctx.response()
50+
.setStatusCode(Integer.parseInt(ctx.request().getParam("status_code")))
51+
.end("EXECUTED"));
4552

4653
vertx
4754
.createHttpServer(new HttpServerOptions().setHandle100ContinueAutomatically(true))
4855
.requestHandler(
4956
req -> {
50-
if (req.path().startsWith("/routes")) {
57+
if (req.path().startsWith("/routes") || req.path().startsWith("/api_security")) {
5158
router.accept(req);
5259
} else {
5360
req.response()

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ apply from: "$rootDir/gradle/java.gradle"
88
dependencies {
99
testImplementation project(':dd-smoke-tests')
1010
testImplementation(testFixtures(project(":dd-smoke-tests:iast-util")))
11+
testImplementation project(':dd-smoke-tests:appsec')
1112
}
1213

1314
def appDir = "$projectDir/application"
Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,71 @@
1+
package datadog.smoketest
2+
3+
import datadog.smoketest.appsec.AbstractAppSecServerSmokeTest
4+
import datadog.trace.agent.test.utils.OkHttpUtils
5+
import okhttp3.Request
6+
import okhttp3.Response
7+
8+
class AppSecVertxSmokeTest extends AbstractAppSecServerSmokeTest {
9+
10+
@Override
11+
def logLevel() {
12+
'DEBUG'
13+
}
14+
15+
@Override
16+
ProcessBuilder createProcessBuilder() {
17+
String vertxUberJar = System.getProperty("datadog.smoketest.vertx.uberJar.path")
18+
19+
List<String> command = new ArrayList<>()
20+
command.add(javaPath())
21+
command.addAll(defaultJavaProperties)
22+
command.addAll(defaultAppSecProperties)
23+
command.addAll((String[]) [
24+
"-Ddd.writer.type=MultiWriter:TraceStructureWriter:${output.getAbsolutePath()},DDAgentWriter",
25+
"-Dvertx.http.port=${httpPort}",
26+
"-jar",
27+
vertxUberJar
28+
])
29+
ProcessBuilder processBuilder = new ProcessBuilder(command)
30+
processBuilder.directory(new File(buildDirectory))
31+
}
32+
33+
@Override
34+
File createTemporaryFile() {
35+
return new File("${buildDirectory}/tmp/trace-structure-vertx.out")
36+
}
37+
38+
void 'API Security samples only one request per endpoint'() {
39+
given:
40+
def url = "http://localhost:${httpPort}/api_security/sampling/200?test=value"
41+
def client = OkHttpUtils.clientBuilder().build()
42+
def request = new Request.Builder()
43+
.url(url)
44+
.addHeader('X-My-Header', "value")
45+
.get()
46+
.build()
47+
48+
when:
49+
List<Response> responses = (1..3).collect {
50+
client.newCall(request).execute()
51+
}
52+
53+
then:
54+
responses.each {
55+
assert it.code() == 200
56+
}
57+
waitForTraceCount(3)
58+
def spans = rootSpans.toList().toSorted { it.span.duration }
59+
spans.size() == 3
60+
def sampledSpans = spans.findAll {
61+
it.meta.keySet().any {
62+
it.startsWith('_dd.appsec.s.req.')
63+
}
64+
}
65+
sampledSpans.size() == 1
66+
def span = sampledSpans[0]
67+
span.meta.containsKey('_dd.appsec.s.req.query')
68+
span.meta.containsKey('_dd.appsec.s.req.params')
69+
span.meta.containsKey('_dd.appsec.s.req.headers')
70+
}
71+
}

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

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,11 +43,19 @@ public void start(Promise<Void> startPromise) throws Exception {
4343
.putHeader("content-type", "text/plain")
4444
.end(randomFactorial().toString()));
4545

46+
router
47+
.route("/api_security/sampling/:status_code")
48+
.handler(
49+
ctx ->
50+
ctx.response()
51+
.setStatusCode(Integer.parseInt(ctx.request().getParam("status_code")))
52+
.end("EXECUTED"));
53+
4654
vertx
4755
.createHttpServer(new HttpServerOptions().setHandle100ContinueAutomatically(true))
4856
.requestHandler(
4957
req -> {
50-
if (req.path().startsWith("/routes")) {
58+
if (req.path().startsWith("/routes") || req.path().startsWith("/api_security")) {
5159
router.handle(req);
5260
} else {
5361
req.response()

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ apply from: "$rootDir/gradle/java.gradle"
77
dependencies {
88
testImplementation project(':dd-smoke-tests')
99
testImplementation(testFixtures(project(":dd-smoke-tests:iast-util")))
10+
testImplementation project(':dd-smoke-tests:appsec')
1011
}
1112

1213
def appDir = "$projectDir/application"

0 commit comments

Comments
 (0)