Clean up. Fix 404 handling.

Author: ygree

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

@@ -73,7 +73,7 @@ public abstract class HttpServerDecorator<REQUEST, CONNECTION, RESPONSE, REQUEST
 
   private static final UTF8BytesString DEFAULT_RESOURCE_NAME = UTF8BytesString.create("/");
   protected static final UTF8BytesString NOT_FOUND_RESOURCE_NAME = UTF8BytesString.create("404");
-  private static final boolean SHOULD_SET_404_RESOURCE_NAME =
+  protected static final boolean SHOULD_SET_404_RESOURCE_NAME =
       Config.get().isRuleEnabled("URLAsResourceNameRule")
           && Config.get().isRuleEnabled("Status404Rule")
           && Config.get().isRuleEnabled("Status404Decorator");
@@ -358,7 +358,6 @@ public AgentSpan onResponseStatus(final AgentSpan span, final int status) {
     }
 
     if (SHOULD_SET_404_RESOURCE_NAME && status == 404) {
-      // TODO
       span.setResourceName(NOT_FOUND_RESOURCE_NAME, ResourceNamePriorities.HTTP_404);
     }
     return span;

dd-java-agent/instrumentation/jax-rs-annotations-1/src/main/java/datadog/trace/instrumentation/jaxrs1/JaxRsAnnotationsDecorator.java

@@ -57,7 +57,7 @@ public void onJaxRsSpan(
       // This check ensures that we only use the route from the first JAX-RS annotated method that
       // is executed
       if (parent.getLocalRootSpan().getResourceNamePriority()
-          < ResourceNamePriorities.HTTP_FRAMEWORK_ROUTE) { // TODO
+          < ResourceNamePriorities.HTTP_FRAMEWORK_ROUTE) {
         HTTP_RESOURCE_DECORATOR.withRoute(
             parent.getLocalRootSpan(), httpMethodAndRoute.getLeft(), httpMethodAndRoute.getRight());
         parent.getLocalRootSpan().setTag(Tags.COMPONENT, "jax-rs");

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

@@ -64,8 +64,8 @@ repositories {
   }
 }
 
-addTestSuiteForDir('baseTest', 'baseTest')
-addTestSuiteForDir('latestDepTest', 'latestDepTest')
+addTestSuite('baseTest')
+addTestSuite('latestDepTest')
 
 sourceSets {
   main_play27 {

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

@@ -8,6 +8,7 @@
 import datadog.trace.bootstrap.instrumentation.api.AgentPropagation;
 import datadog.trace.bootstrap.instrumentation.api.AgentSpan;
 import datadog.trace.bootstrap.instrumentation.api.AgentSpanContext;
+import datadog.trace.bootstrap.instrumentation.api.ResourceNamePriorities;
 import datadog.trace.bootstrap.instrumentation.api.URIDataAdapter;
 import datadog.trace.bootstrap.instrumentation.api.UTF8BytesString;
 import datadog.trace.bootstrap.instrumentation.decorator.HttpServerDecorator;
@@ -186,4 +187,10 @@ public AgentSpan onError(final AgentSpan span, Throwable throwable) {
     }
     return super.onError(span, throwable);
   }
+
+  public void updateOn404Only(final AgentSpan span, final Result result) {
+    if (SHOULD_SET_404_RESOURCE_NAME && status(result) == 404) {
+      span.setResourceName(NOT_FOUND_RESOURCE_NAME, ResourceNamePriorities.HTTP_404);
+    }
+  }
 }

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

@@ -25,8 +25,11 @@ public Object apply(final Try<Result> result) {
       if (result.isFailure()) {
         DECORATE.onError(span, result.failed().get());
       } else {
+        Result response = result.get();
         if (REPORT_HTTP_STATUS) {
-          DECORATE.onResponse(span, result.get());
+          DECORATE.onResponse(span, response);
+        } else {
+          DECORATE.updateOn404Only(span, response);
         }
       }
       DECORATE.beforeFinish(span);

dd-java-agent/testing/src/main/groovy/datadog/trace/agent/test/base/HttpServerTest.groovy

@@ -191,7 +191,7 @@ abstract class HttpServerTest<SERVER> extends WithHttpServer<SERVER> {
 
   String expectedResourceName(ServerEndpoint endpoint, String method, URI address) {
     if (endpoint.status == 404 && (changesAll404s() || endpoint.path == "/not-found")) {
-      return "404" // TODO
+      return "404"
     } else if (endpoint.hasPathParam) {
       return "$method ${testPathParam()}"
     }

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

@@ -429,11 +429,13 @@ public byte getResourceNamePriority() {
   public void setResourceName(final CharSequence resourceName, byte priority) {
     if (log.isDebugEnabled()) {
       log.debug(
-          "setResourceName `{}`->`{}` with priority {}->{}",
+          "setResourceName `{}`->`{}` with priority {}->{} for traceId={} spanId={}",
           this.resourceName,
           resourceName,
           resourceNamePriority,
-          priority);
+          priority,
+          traceId,
+          spanId);
     }
     if (null == resourceName) {
       return;

gradle/java_no_deps.gradle

@@ -298,16 +298,20 @@ def isJavaVersionAllowedForProperty(JavaVersion version, String propertyPrefix =
   def definedMin = project.hasProperty(minProp)
   def definedMax = project.hasProperty(maxProp)
   if (definedMin && project.getProperty(minProp).compareTo(version) > 0) {
+    logger.info("isJavaVersionAllowedForProperty is false b/o minProp=${minProp} is defined and greater than version=${version}")
     return false
   }
   //default to the general min if defined and specific one is was not defined
   if (!propertyPrefix.isEmpty() && !definedMin && project.hasProperty('minJavaVersionForTests') && project.getProperty('minJavaVersionForTests').compareTo(version) > 0) {
+    logger.info("isJavaVersionAllowedForProperty is false b/o minJavaVersionForTests=${project.getProperty('minJavaVersionForTests')} is defined and greater than version=${version}")
     return false
   }
   if (definedMax && project.getProperty(maxProp).compareTo(version) < 0) {
+    logger.info("isJavaVersionAllowedForProperty is false b/o maxProp=${project.getProperty(maxProp)} is defined and lower than version=${version}")
     return false
   }
   if (!propertyPrefix.isEmpty() && !definedMax && project.hasProperty('maxJavaVersionForTests') && project.getProperty('maxJavaVersionForTests').compareTo(version) < 0) {
+    logger.info("isJavaVersionAllowedForProperty is false b/o maxJavaVersionForTests=${project.getProperty('maxJavaVersionForTests')} is defined and lower than version=${version}")
     return false
   }
   return true