Skip to content

Commit 5ae3d48

Browse files
manuel-alvarez-alvarezmanuel-alvarez-alvarez
committed
Fix gateway test and improve code
1 parent ad782b6 commit 5ae3d48

File tree

4 files changed

+73
-36
lines changed

4 files changed

+73
-36
lines changed

‎dd-java-agent/instrumentation/play-2.3/src/main/java/datadog/trace/instrumentation/play23/PlayHttpServerDecorator.java‎

Lines changed: 23 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -4,26 +4,29 @@
44
import static datadog.trace.bootstrap.instrumentation.decorator.http.HttpResourceDecorator.HTTP_RESOURCE_DECORATOR;
55

66
import datadog.trace.api.Config;
7+
import datadog.trace.api.gateway.CallbackProvider;
78
import datadog.trace.api.gateway.RequestContext;
89
import datadog.trace.api.gateway.RequestContextSlot;
910
import datadog.trace.bootstrap.instrumentation.api.AgentPropagation;
1011
import datadog.trace.bootstrap.instrumentation.api.AgentSpan;
1112
import datadog.trace.bootstrap.instrumentation.api.AgentSpanContext;
12-
import datadog.trace.bootstrap.instrumentation.api.AgentTracer;
1313
import datadog.trace.bootstrap.instrumentation.api.URIDataAdapter;
1414
import datadog.trace.bootstrap.instrumentation.api.UTF8BytesString;
1515
import datadog.trace.bootstrap.instrumentation.decorator.HttpServerDecorator;
1616
import java.lang.reflect.InvocationTargetException;
1717
import java.lang.reflect.UndeclaredThrowableException;
1818
import java.util.concurrent.CompletionException;
1919
import java.util.function.BiConsumer;
20+
import org.slf4j.Logger;
21+
import org.slf4j.LoggerFactory;
2022
import play.api.Routes;
2123
import play.api.mvc.Headers;
2224
import play.api.mvc.Request;
2325
import scala.Option;
2426

2527
public class PlayHttpServerDecorator
2628
extends HttpServerDecorator<Request, Request, play.api.mvc.Result, Headers> {
29+
private static final Logger LOG = LoggerFactory.getLogger(PlayHttpServerDecorator.class);
2730
public static final boolean REPORT_HTTP_STATUS = Config.get().getPlayReportHttpStatus();
2831
public static final CharSequence PLAY_REQUEST = UTF8BytesString.create("play.request");
2932
public static final CharSequence PLAY_ACTION = UTF8BytesString.create("play-action");
@@ -92,25 +95,33 @@ public AgentSpan onRequest(
9295
final Option pathOption = request.tags().get(Routes.ROUTE_PATTERN());
9396
if (!pathOption.isEmpty()) {
9497
final String path = (String) pathOption.get();
95-
handleRoute(span, request.method(), path);
98+
HTTP_RESOURCE_DECORATOR.withRoute(span, request.method(), path);
99+
dispatchRoute(span, path);
96100
}
97101
}
98102
return span;
99103
}
100104

101-
private void handleRoute(final AgentSpan span, final String method, final String route) {
102-
HTTP_RESOURCE_DECORATOR.withRoute(span, method, route);
103-
// play does not set the http.route in the local root span so we need to store it in the context
104-
// for API security
105-
final RequestContext ctx = span.getRequestContext();
106-
if (ctx != null) {
107-
final BiConsumer<RequestContext, String> cb =
108-
AgentTracer.get()
109-
.getCallbackProvider(RequestContextSlot.APPSEC)
110-
.getCallback(EVENTS.httpRoute());
105+
/**
106+
* Play does not set the http.route in the local root span so we need to store it in the context
107+
* for API security
108+
*/
109+
private void dispatchRoute(final AgentSpan span, final String route) {
110+
try {
111+
final RequestContext ctx = span.getRequestContext();
112+
if (ctx == null) {
113+
return;
114+
}
115+
final CallbackProvider cbp = tracer().getCallbackProvider(RequestContextSlot.APPSEC);
116+
if (cbp == null) {
117+
return;
118+
}
119+
final BiConsumer<RequestContext, String> cb = cbp.getCallback(EVENTS.httpRoute());
111120
if (cb != null) {
112121
cb.accept(ctx, route);
113122
}
123+
} catch (final Throwable t) {
124+
LOG.debug("Failed to dispatch route", t);
114125
}
115126
}
116127

‎dd-java-agent/instrumentation/play-2.4/src/main/java/datadog/trace/instrumentation/play24/PlayHttpServerDecorator.java‎

Lines changed: 23 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -4,26 +4,29 @@
44
import static datadog.trace.bootstrap.instrumentation.decorator.http.HttpResourceDecorator.HTTP_RESOURCE_DECORATOR;
55

66
import datadog.trace.api.Config;
7+
import datadog.trace.api.gateway.CallbackProvider;
78
import datadog.trace.api.gateway.RequestContext;
89
import datadog.trace.api.gateway.RequestContextSlot;
910
import datadog.trace.bootstrap.instrumentation.api.AgentPropagation;
1011
import datadog.trace.bootstrap.instrumentation.api.AgentSpan;
1112
import datadog.trace.bootstrap.instrumentation.api.AgentSpanContext;
12-
import datadog.trace.bootstrap.instrumentation.api.AgentTracer;
1313
import datadog.trace.bootstrap.instrumentation.api.URIDataAdapter;
1414
import datadog.trace.bootstrap.instrumentation.api.UTF8BytesString;
1515
import datadog.trace.bootstrap.instrumentation.decorator.HttpServerDecorator;
1616
import java.lang.reflect.InvocationTargetException;
1717
import java.lang.reflect.UndeclaredThrowableException;
1818
import java.util.concurrent.CompletionException;
1919
import java.util.function.BiConsumer;
20+
import org.slf4j.Logger;
21+
import org.slf4j.LoggerFactory;
2022
import play.api.mvc.Headers;
2123
import play.api.mvc.Request;
2224
import play.api.mvc.Result;
2325
import scala.Option;
2426

2527
public class PlayHttpServerDecorator
2628
extends HttpServerDecorator<Request, Request, Result, Headers> {
29+
private static final Logger LOG = LoggerFactory.getLogger(PlayHttpServerDecorator.class);
2730
public static final boolean REPORT_HTTP_STATUS = Config.get().getPlayReportHttpStatus();
2831
public static final CharSequence PLAY_REQUEST = UTF8BytesString.create("play.request");
2932
public static final CharSequence PLAY_ACTION = UTF8BytesString.create("play-action");
@@ -92,25 +95,33 @@ public AgentSpan onRequest(
9295
final Option pathOption = request.tags().get("ROUTE_PATTERN");
9396
if (!pathOption.isEmpty()) {
9497
final String path = (String) pathOption.get();
95-
handleRoute(span, request.method(), path);
98+
HTTP_RESOURCE_DECORATOR.withRoute(span, request.method(), path);
99+
dispatchRoute(span, path);
96100
}
97101
}
98102
return span;
99103
}
100104

101-
private void handleRoute(final AgentSpan span, final String method, final String route) {
102-
HTTP_RESOURCE_DECORATOR.withRoute(span, method, route);
103-
// play does not set the http.route in the local root span so we need to store it in the context
104-
// for API security
105-
final RequestContext ctx = span.getRequestContext();
106-
if (ctx != null) {
107-
final BiConsumer<RequestContext, String> cb =
108-
AgentTracer.get()
109-
.getCallbackProvider(RequestContextSlot.APPSEC)
110-
.getCallback(EVENTS.httpRoute());
105+
/**
106+
* Play does not set the http.route in the local root span so we need to store it in the context
107+
* for API security
108+
*/
109+
private void dispatchRoute(final AgentSpan span, final String route) {
110+
try {
111+
final RequestContext ctx = span.getRequestContext();
112+
if (ctx == null) {
113+
return;
114+
}
115+
final CallbackProvider cbp = tracer().getCallbackProvider(RequestContextSlot.APPSEC);
116+
if (cbp == null) {
117+
return;
118+
}
119+
final BiConsumer<RequestContext, String> cb = cbp.getCallback(EVENTS.httpRoute());
111120
if (cb != null) {
112121
cb.accept(ctx, route);
113122
}
123+
} catch (final Throwable t) {
124+
LOG.debug("Failed to dispatch route", t);
114125
}
115126
}
116127

‎dd-java-agent/instrumentation/play-2.6/src/main/java/datadog/trace/instrumentation/play26/PlayHttpServerDecorator.java‎

Lines changed: 23 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -6,12 +6,12 @@
66
import datadog.trace.api.Config;
77
import datadog.trace.api.cache.DDCache;
88
import datadog.trace.api.cache.DDCaches;
9+
import datadog.trace.api.gateway.CallbackProvider;
910
import datadog.trace.api.gateway.RequestContext;
1011
import datadog.trace.api.gateway.RequestContextSlot;
1112
import datadog.trace.bootstrap.instrumentation.api.AgentPropagation;
1213
import datadog.trace.bootstrap.instrumentation.api.AgentSpan;
1314
import datadog.trace.bootstrap.instrumentation.api.AgentSpanContext;
14-
import datadog.trace.bootstrap.instrumentation.api.AgentTracer;
1515
import datadog.trace.bootstrap.instrumentation.api.ResourceNamePriorities;
1616
import datadog.trace.bootstrap.instrumentation.api.URIDataAdapter;
1717
import datadog.trace.bootstrap.instrumentation.api.URIUtils;
@@ -24,6 +24,8 @@
2424
import java.lang.reflect.UndeclaredThrowableException;
2525
import java.util.concurrent.CompletionException;
2626
import java.util.function.BiConsumer;
27+
import org.slf4j.Logger;
28+
import org.slf4j.LoggerFactory;
2729
import play.api.mvc.Headers;
2830
import play.api.mvc.Request;
2931
import play.api.mvc.Result;
@@ -35,6 +37,7 @@
3537

3638
public class PlayHttpServerDecorator
3739
extends HttpServerDecorator<Request, Request, Result, Headers> {
40+
private static final Logger LOG = LoggerFactory.getLogger(PlayHttpServerDecorator.class);
3841
public static final boolean REPORT_HTTP_STATUS = Config.get().getPlayReportHttpStatus();
3942
public static final CharSequence PLAY_REQUEST = UTF8BytesString.create("play.request");
4043
public static final CharSequence PLAY_ACTION = UTF8BytesString.create("play-action");
@@ -148,25 +151,33 @@ public AgentSpan onRequest(
148151
CharSequence path =
149152
PATH_CACHE.computeIfAbsent(
150153
defOption.get().path(), p -> addMissingSlash(p, request.path()));
151-
handleRoute(span, request.method(), path);
154+
HTTP_RESOURCE_DECORATOR.withRoute(span, request.method(), path, true);
155+
dispatchRoute(span, path);
152156
}
153157
}
154158
return span;
155159
}
156160

157-
private void handleRoute(final AgentSpan span, final String method, final CharSequence route) {
158-
HTTP_RESOURCE_DECORATOR.withRoute(span, method, route, true);
159-
// play does not set the http.route in the local root span so we need to store it in the context
160-
// for API security
161-
final RequestContext ctx = span.getRequestContext();
162-
if (ctx != null) {
163-
final BiConsumer<RequestContext, String> cb =
164-
AgentTracer.get()
165-
.getCallbackProvider(RequestContextSlot.APPSEC)
166-
.getCallback(EVENTS.httpRoute());
161+
/**
162+
* Play does not set the http.route in the local root span so we need to store it in the context
163+
* for API security
164+
*/
165+
private void dispatchRoute(final AgentSpan span, final CharSequence route) {
166+
try {
167+
final RequestContext ctx = span.getRequestContext();
168+
if (ctx == null) {
169+
return;
170+
}
171+
final CallbackProvider cbp = tracer().getCallbackProvider(RequestContextSlot.APPSEC);
172+
if (cbp == null) {
173+
return;
174+
}
175+
final BiConsumer<RequestContext, String> cb = cbp.getCallback(EVENTS.httpRoute());
167176
if (cb != null) {
168177
cb.accept(ctx, URIUtils.decode(route.toString()));
169178
}
179+
} catch (final Throwable t) {
180+
LOG.debug("Failed to dispatch route", t);
170181
}
171182
}
172183

‎internal-api/src/test/java/datadog/trace/api/gateway/InstrumentationGatewayTest.java‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -223,6 +223,8 @@ public void testNormalCalls() {
223223
cbp.getCallback(events.execCmd()).apply(null, null);
224224
ss.registerCallback(events.shellCmd(), callback);
225225
cbp.getCallback(events.shellCmd()).apply(null, null);
226+
ss.registerCallback(events.httpRoute(), callback);
227+
cbp.getCallback(events.httpRoute()).accept(null, null);
226228
assertThat(callback.count).isEqualTo(Events.MAX_EVENTS);
227229
}
228230

@@ -293,6 +295,8 @@ public void testThrowableBlocking() {
293295
cbp.getCallback(events.execCmd()).apply(null, null);
294296
ss.registerCallback(events.shellCmd(), throwback);
295297
cbp.getCallback(events.shellCmd()).apply(null, null);
298+
ss.registerCallback(events.httpRoute(), throwback);
299+
cbp.getCallback(events.httpRoute()).accept(null, null);
296300
assertThat(throwback.count).isEqualTo(Events.MAX_EVENTS);
297301
}
298302

0 commit comments

Comments
 (0)