Skip to content

Commit 1f2b612

Browse files
manuel-alvarez-alvarezmanuel-alvarez-alvarez
authored
Write http.route tag as soon as possible in vert.x (#8952)
1 parent 03e7d3e commit 1f2b612

File tree

10 files changed

+213
-37
lines changed

10 files changed

+213
-37
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: 23 additions & 6 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;
@@ -39,7 +39,6 @@ public RouteHandlerWrapper(final Handler<RoutingContext> handler) {
3939
@Override
4040
public void handle(final RoutingContext routingContext) {
4141
AgentSpan span = routingContext.get(HANDLER_SPAN_CONTEXT_KEY);
42-
Flow.Action.RequestBlockingAction rba = null;
4342
if (spanStarter) {
4443
if (span == null) {
4544
AgentSpan parentSpan = activeSpan();
@@ -52,8 +51,7 @@ public void handle(final RoutingContext routingContext) {
5251
DECORATE.afterStart(span);
5352
span.setResourceName(DECORATE.className(actual.getClass()));
5453
}
55-
56-
updateRoutingContextWithRoute(routingContext);
54+
setRoute(routingContext);
5755
}
5856
try (final AgentScope scope = span != null ? activateSpan(span) : noopScope()) {
5957
try {
@@ -65,7 +63,12 @@ public void handle(final RoutingContext routingContext) {
6563
}
6664
}
6765

68-
private void updateRoutingContextWithRoute(RoutingContext routingContext) {
66+
private void setRoute(RoutingContext routingContext) {
67+
final AgentSpan parentSpan = routingContext.get(PARENT_SPAN_CONTEXT_KEY);
68+
if (parentSpan == null) {
69+
return;
70+
}
71+
6972
final String method = routingContext.request().rawMethod();
7073
String mountPoint = routingContext.mountPoint();
7174
String path = routingContext.currentRoute().getPath();
@@ -78,8 +81,22 @@ private void updateRoutingContextWithRoute(RoutingContext routingContext) {
7881
}
7982
path = mountPoint + path;
8083
}
81-
if (method != null && path != null) {
84+
if (method != null && path != null && shouldUpdateRoute(routingContext, parentSpan, path)) {
8285
routingContext.put(ROUTE_CONTEXT_KEY, path);
86+
HTTP_RESOURCE_DECORATOR.withRoute(parentSpan, method, path, true);
87+
}
88+
}
89+
90+
static boolean shouldUpdateRoute(
91+
final RoutingContext routingContext, final AgentSpan span, final String path) {
92+
if (span == null) {
93+
return false;
94+
}
95+
final String currentRoute = routingContext.get(ROUTE_CONTEXT_KEY);
96+
if (currentRoute != null && currentRoute.equals(path)) {
97+
return false;
8398
}
99+
// do not override route with a "/" if it's already set (it's probably more meaningful)
100+
return !path.equals("/") || span.getTag(Tags.HTTP_ROUTE) == null;
84101
}
85102
}

‎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: 23 additions & 3 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

@@ -46,7 +47,7 @@ public void handle(final RoutingContext routingContext) {
4647
DECORATE.afterStart(span);
4748
span.setResourceName(DECORATE.className(actual.getClass()));
4849
}
49-
updateRoutingContextWithRoute(routingContext);
50+
setRoute(routingContext);
5051
}
5152

5253
try (final AgentScope scope = span != null ? activateSpan(span) : noopScope()) {
@@ -59,7 +60,12 @@ public void handle(final RoutingContext routingContext) {
5960
}
6061
}
6162

62-
private void updateRoutingContextWithRoute(RoutingContext routingContext) {
63+
private void setRoute(RoutingContext routingContext) {
64+
final AgentSpan parentSpan = routingContext.get(PARENT_SPAN_CONTEXT_KEY);
65+
if (parentSpan == null) {
66+
return;
67+
}
68+
6369
final String method = routingContext.request().method().name();
6470
final String mountPoint = routingContext.mountPoint();
6571

@@ -73,8 +79,22 @@ private void updateRoutingContextWithRoute(RoutingContext routingContext) {
7379
: mountPoint;
7480
path = noBackslashhMountPoint + path;
7581
}
76-
if (method != null && path != null) {
82+
if (method != null && path != null && shouldUpdateRoute(routingContext, parentSpan, path)) {
7783
routingContext.put(ROUTE_CONTEXT_KEY, path);
84+
HTTP_RESOURCE_DECORATOR.withRoute(parentSpan, method, path, true);
85+
}
86+
}
87+
88+
static boolean shouldUpdateRoute(
89+
final RoutingContext routingContext, final AgentSpan span, final String path) {
90+
if (span == null) {
91+
return false;
92+
}
93+
final String currentRoute = routingContext.get(ROUTE_CONTEXT_KEY);
94+
if (currentRoute != null && currentRoute.equals(path)) {
95+
return false;
7896
}
97+
// do not override route with a "/" if it's already set (it's probably more meaningful)
98+
return !path.equals("/") || span.getTag(Tags.HTTP_ROUTE) == null;
7999
}
80100
}

‎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"

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

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

‎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)