Skip to content

Commit d56d38c

Browse files
authored
Fix the Play Framework's span resource name priority so that the client JAX-RS 404 cannot override it. (#8591)
Add debug for setResourceName
1 parent 06e93e8 commit d56d38c

File tree

7 files changed

+29
-5
lines changed

7 files changed

+29
-5
lines changed

dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/decorator/HttpServerDecorator.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@ public abstract class HttpServerDecorator<REQUEST, CONNECTION, RESPONSE, REQUEST
7373

7474
private static final UTF8BytesString DEFAULT_RESOURCE_NAME = UTF8BytesString.create("/");
7575
protected static final UTF8BytesString NOT_FOUND_RESOURCE_NAME = UTF8BytesString.create("404");
76-
private static final boolean SHOULD_SET_404_RESOURCE_NAME =
76+
protected static final boolean SHOULD_SET_404_RESOURCE_NAME =
7777
Config.get().isRuleEnabled("URLAsResourceNameRule")
7878
&& Config.get().isRuleEnabled("Status404Rule")
7979
&& Config.get().isRuleEnabled("Status404Decorator");

dd-java-agent/instrumentation/play-2.6/build.gradle

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -64,8 +64,8 @@ repositories {
6464
}
6565
}
6666

67-
addTestSuiteForDir('baseTest', 'baseTest')
68-
addTestSuiteForDir('latestDepTest', 'latestDepTest')
67+
addTestSuite('baseTest')
68+
addTestSuite('latestDepTest')
6969

7070
sourceSets {
7171
main_play27 {

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@ public static void stopTraceOnResponse(
8484
// set the resource name on the upstream akka/netty span if there is one
8585
if (rootSpan != null && playControllerSpan.getResourceName() != null) {
8686
rootSpan.setResourceName(
87-
playControllerSpan.getResourceName(), ResourceNamePriorities.HTTP_PATH_NORMALIZER);
87+
playControllerSpan.getResourceName(), ResourceNamePriorities.HTTP_FRAMEWORK_ROUTE);
8888
}
8989
}
9090
}

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

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
import datadog.trace.bootstrap.instrumentation.api.AgentPropagation;
99
import datadog.trace.bootstrap.instrumentation.api.AgentSpan;
1010
import datadog.trace.bootstrap.instrumentation.api.AgentSpanContext;
11+
import datadog.trace.bootstrap.instrumentation.api.ResourceNamePriorities;
1112
import datadog.trace.bootstrap.instrumentation.api.URIDataAdapter;
1213
import datadog.trace.bootstrap.instrumentation.api.UTF8BytesString;
1314
import datadog.trace.bootstrap.instrumentation.decorator.HttpServerDecorator;
@@ -186,4 +187,10 @@ public AgentSpan onError(final AgentSpan span, Throwable throwable) {
186187
}
187188
return super.onError(span, throwable);
188189
}
190+
191+
public void updateOn404Only(final AgentSpan span, final Result result) {
192+
if (SHOULD_SET_404_RESOURCE_NAME && status(result) == 404) {
193+
span.setResourceName(NOT_FOUND_RESOURCE_NAME, ResourceNamePriorities.HTTP_404);
194+
}
195+
}
189196
}

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

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,8 +25,11 @@ public Object apply(final Try<Result> result) {
2525
if (result.isFailure()) {
2626
DECORATE.onError(span, result.failed().get());
2727
} else {
28+
Result response = result.get();
2829
if (REPORT_HTTP_STATUS) {
29-
DECORATE.onResponse(span, result.get());
30+
DECORATE.onResponse(span, response);
31+
} else {
32+
DECORATE.updateOn404Only(span, response);
3033
}
3134
}
3235
DECORATE.beforeFinish(span);

dd-trace-core/src/main/java/datadog/trace/core/DDSpanContext.java

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -428,6 +428,16 @@ public byte getResourceNamePriority() {
428428
}
429429

430430
public void setResourceName(final CharSequence resourceName, byte priority) {
431+
if (log.isDebugEnabled()) {
432+
log.debug(
433+
"setResourceName `{}`->`{}` with priority {}->{} for traceId={} spanId={}",
434+
this.resourceName,
435+
resourceName,
436+
resourceNamePriority,
437+
priority,
438+
traceId,
439+
spanId);
440+
}
431441
if (null == resourceName) {
432442
return;
433443
}

gradle/java_no_deps.gradle

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -307,16 +307,20 @@ def isJavaVersionAllowedForProperty(JavaVersion version, String propertyPrefix =
307307
def definedMin = project.hasProperty(minProp)
308308
def definedMax = project.hasProperty(maxProp)
309309
if (definedMin && project.getProperty(minProp).compareTo(version) > 0) {
310+
logger.info("isJavaVersionAllowedForProperty is false b/o minProp=${minProp} is defined and greater than version=${version}")
310311
return false
311312
}
312313
//default to the general min if defined and specific one is was not defined
313314
if (!propertyPrefix.isEmpty() && !definedMin && project.hasProperty('minJavaVersionForTests') && project.getProperty('minJavaVersionForTests').compareTo(version) > 0) {
315+
logger.info("isJavaVersionAllowedForProperty is false b/o minJavaVersionForTests=${project.getProperty('minJavaVersionForTests')} is defined and greater than version=${version}")
314316
return false
315317
}
316318
if (definedMax && project.getProperty(maxProp).compareTo(version) < 0) {
319+
logger.info("isJavaVersionAllowedForProperty is false b/o maxProp=${project.getProperty(maxProp)} is defined and lower than version=${version}")
317320
return false
318321
}
319322
if (!propertyPrefix.isEmpty() && !definedMax && project.hasProperty('maxJavaVersionForTests') && project.getProperty('maxJavaVersionForTests').compareTo(version) < 0) {
323+
logger.info("isJavaVersionAllowedForProperty is false b/o maxJavaVersionForTests=${project.getProperty('maxJavaVersionForTests')} is defined and lower than version=${version}")
320324
return false
321325
}
322326
return true

0 commit comments

Comments
 (0)