Remove some testNotFound exclusions (#2813)

instrumentation/dropwizard-testing/src/test/groovy/DropwizardTest.groovy

@@ -7,6 +7,7 @@ import static io.opentelemetry.api.trace.SpanKind.INTERNAL
 import static io.opentelemetry.api.trace.SpanKind.SERVER
 import static io.opentelemetry.instrumentation.test.base.HttpServerTest.ServerEndpoint.ERROR
 import static io.opentelemetry.instrumentation.test.base.HttpServerTest.ServerEndpoint.EXCEPTION
+import static io.opentelemetry.instrumentation.test.base.HttpServerTest.ServerEndpoint.NOT_FOUND
 import static io.opentelemetry.instrumentation.test.base.HttpServerTest.ServerEndpoint.PATH_PARAM
 import static io.opentelemetry.instrumentation.test.base.HttpServerTest.ServerEndpoint.QUERY_PARAM
 import static io.opentelemetry.instrumentation.test.base.HttpServerTest.ServerEndpoint.REDIRECT
@@ -58,13 +59,8 @@ class DropwizardTest extends HttpServerTest<DropwizardTestSupport> implements Ag
   }
 
   @Override
-  boolean hasHandlerSpan() {
+  boolean hasHandlerSpan(ServerEndpoint endpoint) {
-    true
+    endpoint != NOT_FOUND
-  }
-
-  @Override
-  boolean testNotFound() {
-    false
   }
 
   @Override
@@ -72,6 +68,18 @@ class DropwizardTest extends HttpServerTest<DropwizardTestSupport> implements Ag
     true
   }
 
+  @Override
+  String expectedServerSpanName(ServerEndpoint endpoint) {
+    switch (endpoint) {
+      case PATH_PARAM:
+        return "/path/{id}/param"
+      case NOT_FOUND:
+        return "/*"
+      default:
+        return endpoint.resolvePath(address).path
+    }
+  }
+
   @Override
   void handlerSpan(TraceAssert trace, int index, Object parent, String method = "GET", ServerEndpoint endpoint = SUCCESS) {
     trace.span(index) {
@@ -89,7 +97,7 @@ class DropwizardTest extends HttpServerTest<DropwizardTestSupport> implements Ag
   @Override
   void serverSpan(TraceAssert trace, int index, String traceID = null, String parentID = null, String method = "GET", Long responseContentLength = null, ServerEndpoint endpoint = SUCCESS) {
     trace.span(index) {
-      name "${endpoint == PATH_PARAM ? "/path/{id}/param" : endpoint.resolvePath(address).path}"
+      name expectedServerSpanName(endpoint)
       kind SERVER
       if (endpoint.errored) {
         status StatusCode.ERROR

instrumentation/finatra-2.9/javaagent/src/latestDepTest/groovy/FinatraServerLatestTest.groovy

@@ -4,6 +4,7 @@
  */
 
 import static io.opentelemetry.api.trace.SpanKind.INTERNAL
+import static io.opentelemetry.instrumentation.test.base.HttpServerTest.ServerEndpoint.NOT_FOUND
 import static io.opentelemetry.instrumentation.test.base.HttpServerTest.ServerEndpoint.SUCCESS
 
 import com.twitter.app.lifecycle.Event
@@ -66,22 +67,21 @@ class FinatraServerLatestTest extends HttpServerTest<HttpServer> implements Agen
     return testServer
   }
 
-  @Override
-  boolean hasHandlerSpan() {
-    return true
-  }
-
-  @Override
-  boolean testNotFound() {
-    // Resource name is set to "GET /notFound"
-    false
-  }
-
   @Override
   void stopServer(HttpServer httpServer) {
     Await.ready(httpServer.close(), TIMEOUT)
   }
 
+  @Override
+  boolean hasHandlerSpan(ServerEndpoint endpoint) {
+    endpoint != NOT_FOUND
+  }
+
+  @Override
+  String expectedServerSpanName(ServerEndpoint endpoint) {
+    return endpoint == NOT_FOUND ? "HTTP GET" : super.expectedServerSpanName(endpoint)
+  }
+
   @Override
   void handlerSpan(TraceAssert trace, int index, Object parent, String method = "GET", ServerEndpoint endpoint = SUCCESS) {
     trace.span(index) {

instrumentation/finatra-2.9/javaagent/src/test/groovy/FinatraServerTest.groovy

@@ -4,6 +4,7 @@
  */
 
 import static io.opentelemetry.api.trace.SpanKind.INTERNAL
+import static io.opentelemetry.instrumentation.test.base.HttpServerTest.ServerEndpoint.NOT_FOUND
 import static io.opentelemetry.instrumentation.test.base.HttpServerTest.ServerEndpoint.SUCCESS
 
 import com.twitter.finatra.http.HttpServer
@@ -47,22 +48,21 @@ class FinatraServerTest extends HttpServerTest<HttpServer> implements AgentTestT
     return testServer
   }
 
-  @Override
-  boolean hasHandlerSpan() {
-    return true
-  }
-
-  @Override
-  boolean testNotFound() {
-    // Resource name is set to "GET /notFound"
-    false
-  }
-
   @Override
   void stopServer(HttpServer httpServer) {
     Await.ready(httpServer.close(), TIMEOUT)
   }
 
+  @Override
+  boolean hasHandlerSpan(ServerEndpoint endpoint) {
+    endpoint != NOT_FOUND
+  }
+
+  @Override
+  String expectedServerSpanName(ServerEndpoint endpoint) {
+    return endpoint == NOT_FOUND ? "HTTP GET" : super.expectedServerSpanName(endpoint)
+  }
+
   @Override
   boolean testPathParam() {
     true

instrumentation/grails-3.0/javaagent/src/test/groovy/test/ErrorController.groovy

@@ -16,9 +16,4 @@ class ErrorController implements Controller {
   def index() {
     render ERROR.body
   }
-
-  @Action
-  def notFound() {
-    response.sendError(404, "Not Found")
-  }
 }

instrumentation/grails-3.0/javaagent/src/test/groovy/test/GrailsTest.groovy

@@ -82,12 +82,12 @@ class GrailsTest extends HttpServerTest<ConfigurableApplicationContext> implemen
   }
 
   @Override
-  boolean hasHandlerSpan() {
+  boolean hasHandlerSpan(ServerEndpoint endpoint) {
     true
   }
 
   @Override
-  boolean hasExceptionOnServerSpan() {
+  boolean hasExceptionOnServerSpan(ServerEndpoint endpoint) {
     true
   }
 
@@ -103,7 +103,7 @@ class GrailsTest extends HttpServerTest<ConfigurableApplicationContext> implemen
 
   @Override
   int getErrorPageSpansCount(ServerEndpoint endpoint) {
-    endpoint == NOT_FOUND ? 3 : 2
+    endpoint == NOT_FOUND ? 1 : 2
   }
 
   @Override
@@ -114,16 +114,9 @@ class GrailsTest extends HttpServerTest<ConfigurableApplicationContext> implemen
   @Override
   void errorPageSpans(TraceAssert trace, int index, Object parent, String method = "GET", ServerEndpoint endpoint) {
     forwardSpan(trace, index, trace.span(0))
-    def errorSpanName = endpoint == NOT_FOUND ? "ErrorController.notFound" : "ErrorController.index"
+    if (endpoint != NOT_FOUND) {
       trace.span(index + 1) {
-      name errorSpanName
+        name "ErrorController.index"
-      kind INTERNAL
-      attributes {
-      }
-    }
-    if (endpoint == NOT_FOUND) {
-      trace.span(index + 2) {
-        name ~/\.sendError$/
         kind INTERNAL
         attributes {
         }

instrumentation/jaxrs/jaxrs-2.0/jaxrs-2.0-testing/src/main/groovy/JaxRsHttpServerTest.groovy

@@ -110,7 +110,7 @@ abstract class JaxRsHttpServerTest<S> extends HttpServerTest<S> implements Agent
   }
 
   @Override
-  boolean hasHandlerSpan() {
+  boolean hasHandlerSpan(ServerEndpoint endpoint) {
     true
   }
 

instrumentation/play/play-2.4/javaagent/src/test/groovy/server/PlayServerTest.groovy

@@ -62,7 +62,7 @@ class PlayServerTest extends HttpServerTest<Server> implements AgentTestTrait {
   }
 
   @Override
-  boolean hasHandlerSpan() {
+  boolean hasHandlerSpan(ServerEndpoint endpoint) {
     true
   }
 

instrumentation/play/play-2.6/javaagent/src/test/groovy/server/PlayServerTest.groovy

@@ -64,7 +64,7 @@ class PlayServerTest extends HttpServerTest<Server> implements AgentTestTrait {
   }
 
   @Override
-  boolean hasHandlerSpan() {
+  boolean hasHandlerSpan(ServerEndpoint endpoint) {
     true
   }
 

instrumentation/ratpack-1.4/javaagent/src/test/groovy/server/RatpackHttpServerTest.groovy

@@ -99,7 +99,7 @@ class RatpackHttpServerTest extends HttpServerTest<EmbeddedApp> implements Agent
   }
 
   @Override
-  boolean hasHandlerSpan() {
+  boolean hasHandlerSpan(ServerEndpoint endpoint) {
     true
   }
 

instrumentation/spring/spring-webmvc-3.1/javaagent/src/test/groovy/test/boot/SpringBootBasedTest.groovy

@@ -50,12 +50,12 @@ class SpringBootBasedTest extends HttpServerTest<ConfigurableApplicationContext>
   }
 
   @Override
-  boolean hasHandlerSpan() {
+  boolean hasHandlerSpan(ServerEndpoint endpoint) {
     true
   }
 
   @Override
-  boolean hasExceptionOnServerSpan() {
+  boolean hasExceptionOnServerSpan(ServerEndpoint endpoint) {
     true
   }
 
@@ -74,25 +74,30 @@ class SpringBootBasedTest extends HttpServerTest<ConfigurableApplicationContext>
     true
   }
 
+  @Override
   boolean hasErrorPageSpans(ServerEndpoint endpoint) {
     endpoint == NOT_FOUND
   }
 
+  @Override
   int getErrorPageSpansCount(ServerEndpoint endpoint) {
     2
   }
 
   @Override
   String expectedServerSpanName(ServerEndpoint endpoint) {
-    if (endpoint == PATH_PARAM) {
+    switch (endpoint) {
+      case PATH_PARAM:
         return getContextPath() + "/path/{id}/param"
-    } else if (endpoint == AUTH_ERROR || endpoint == NOT_FOUND) {
+      case AUTH_ERROR:
+      case NOT_FOUND:
         return getContextPath() + "/error"
-    } else if (endpoint == LOGIN) {
+      case LOGIN:
         return "HTTP POST"
-    }
+      default:
         return super.expectedServerSpanName(endpoint)
     }
+  }
 
   def "test spans with auth error"() {
     setup:

instrumentation/spring/spring-webmvc-3.1/javaagent/src/test/groovy/test/filter/ServletFilterTest.groovy

@@ -8,11 +8,11 @@ package test.filter
 import static io.opentelemetry.api.trace.SpanKind.INTERNAL
 import static io.opentelemetry.instrumentation.test.base.HttpServerTest.ServerEndpoint.ERROR
 import static io.opentelemetry.instrumentation.test.base.HttpServerTest.ServerEndpoint.EXCEPTION
+import static io.opentelemetry.instrumentation.test.base.HttpServerTest.ServerEndpoint.NOT_FOUND
 import static io.opentelemetry.instrumentation.test.base.HttpServerTest.ServerEndpoint.PATH_PARAM
 import static io.opentelemetry.instrumentation.test.base.HttpServerTest.ServerEndpoint.REDIRECT
 import static io.opentelemetry.instrumentation.test.base.HttpServerTest.ServerEndpoint.SUCCESS
 
-import io.opentelemetry.api.trace.StatusCode
 import io.opentelemetry.instrumentation.test.AgentTestTrait
 import io.opentelemetry.instrumentation.test.asserts.TraceAssert
 import io.opentelemetry.instrumentation.test.base.HttpServerTest
@@ -37,13 +37,13 @@ class ServletFilterTest extends HttpServerTest<ConfigurableApplicationContext> i
   }
 
   @Override
-  boolean hasHandlerSpan() {
+  boolean hasHandlerSpan(ServerEndpoint endpoint) {
-    false
+    endpoint == NOT_FOUND
   }
 
   @Override
   boolean hasErrorPageSpans(ServerEndpoint endpoint) {
-    endpoint == ERROR || endpoint == EXCEPTION
+    endpoint == ERROR || endpoint == EXCEPTION || endpoint == NOT_FOUND
   }
 
   @Override
@@ -53,7 +53,7 @@ class ServletFilterTest extends HttpServerTest<ConfigurableApplicationContext> i
 
   @Override
   boolean hasResponseSpan(ServerEndpoint endpoint) {
-    endpoint == REDIRECT || endpoint == ERROR
+    endpoint == REDIRECT || endpoint == ERROR || endpoint == NOT_FOUND
   }
 
   @Override
@@ -63,6 +63,7 @@ class ServletFilterTest extends HttpServerTest<ConfigurableApplicationContext> i
         redirectSpan(trace, index, parent)
         break
       case ERROR:
+      case NOT_FOUND:
         sendErrorSpan(trace, index, parent)
         break
     }
@@ -74,22 +75,11 @@ class ServletFilterTest extends HttpServerTest<ConfigurableApplicationContext> i
   }
 
   @Override
-  boolean testNotFound() {
+  void handlerSpan(TraceAssert trace, int index, Object parent, String method = "GET", ServerEndpoint endpoint) {
-    // FIXME: the instrumentation adds an extra controller span which is not consistent.
-    // Fix tests or remove extra span.
-    false
-  }
-
-  @Override
-  void handlerSpan(TraceAssert trace, int index, Object parent, String method = "GET", ServerEndpoint endpoint = SUCCESS) {
     trace.span(index) {
-      name "TestController.${endpoint.name().toLowerCase()}"
+      name "ResourceHttpRequestHandler.handleRequest"
       kind INTERNAL
       childOf((SpanData) parent)
-      if (endpoint == EXCEPTION) {
-        status StatusCode.ERROR
-        errorEvent(Exception, EXCEPTION.body)
-      }
     }
   }
 
@@ -97,7 +87,7 @@ class ServletFilterTest extends HttpServerTest<ConfigurableApplicationContext> i
   String expectedServerSpanName(ServerEndpoint endpoint) {
     if (endpoint == PATH_PARAM) {
       return "/path/{id}/param"
-    } else if (endpoint == ERROR || endpoint == EXCEPTION) {
+    } else if (endpoint == ERROR || endpoint == EXCEPTION || endpoint == NOT_FOUND) {
       return "/error"
     }
     return endpoint.resolvePath(address).path

instrumentation/struts-2.3/javaagent/src/test/groovy/Struts2ActionSpanTest.groovy

@@ -6,6 +6,7 @@
 import static io.opentelemetry.api.trace.SpanKind.INTERNAL
 import static io.opentelemetry.instrumentation.test.base.HttpServerTest.ServerEndpoint.ERROR
 import static io.opentelemetry.instrumentation.test.base.HttpServerTest.ServerEndpoint.EXCEPTION
+import static io.opentelemetry.instrumentation.test.base.HttpServerTest.ServerEndpoint.NOT_FOUND
 import static io.opentelemetry.instrumentation.test.base.HttpServerTest.ServerEndpoint.PATH_PARAM
 import static io.opentelemetry.instrumentation.test.base.HttpServerTest.ServerEndpoint.REDIRECT
 import static io.opentelemetry.instrumentation.test.utils.TraceUtils.basicServerSpan
@@ -28,11 +29,6 @@ import org.eclipse.jetty.util.resource.FileResource
 
 class Struts2ActionSpanTest extends HttpServerTest<Server> implements AgentTestTrait {
 
-  @Override
-  boolean testNotFound() {
-    return false
-  }
-
   @Override
   boolean testPathParam() {
     return true
@@ -44,13 +40,13 @@ class Struts2ActionSpanTest extends HttpServerTest<Server> implements AgentTestT
   }
 
   @Override
-  boolean hasHandlerSpan() {
+  boolean hasHandlerSpan(ServerEndpoint endpoint) {
-    return true
+    return endpoint != NOT_FOUND
   }
 
   @Override
   boolean hasResponseSpan(ServerEndpoint endpoint) {
-    endpoint == REDIRECT || endpoint == ERROR || endpoint == EXCEPTION
+    endpoint == REDIRECT || endpoint == ERROR || endpoint == EXCEPTION || endpoint == NOT_FOUND
   }
 
   @Override
@@ -61,13 +57,21 @@ class Struts2ActionSpanTest extends HttpServerTest<Server> implements AgentTestT
         break
       case ERROR:
       case EXCEPTION:
+      case NOT_FOUND:
         sendErrorSpan(trace, index, handlerSpan)
         break
     }
   }
 
   String expectedServerSpanName(ServerEndpoint endpoint) {
-    return endpoint == PATH_PARAM ? getContextPath() + "/path/{id}/param" : endpoint.resolvePath(address).path
+    switch (endpoint) {
+      case PATH_PARAM:
+        return getContextPath() + "/path/{id}/param"
+      case NOT_FOUND:
+        return "HTTP GET"
+      default:
+        return endpoint.resolvePath(address).path
+    }
   }
 
   @Override

testing-common/src/main/groovy/io/opentelemetry/instrumentation/test/base/HttpServerTest.groovy

@@ -48,12 +48,12 @@ abstract class HttpServerTest<SERVER> extends InstrumentationSpecification imple
     return ""
   }
 
-  boolean hasHandlerSpan() {
+  boolean hasHandlerSpan(ServerEndpoint endpoint) {
     false
   }
 
-  boolean hasExceptionOnServerSpan() {
+  boolean hasExceptionOnServerSpan(ServerEndpoint endpoint) {
-    !hasHandlerSpan()
+    !hasHandlerSpan(endpoint)
   }
 
   boolean hasRenderSpan(ServerEndpoint endpoint) {
@@ -451,10 +451,10 @@ abstract class HttpServerTest<SERVER> extends InstrumentationSpecification imple
 
   void assertTheTraces(int size, String traceID = null, String parentID = null, String method = "GET", ServerEndpoint endpoint = SUCCESS, String errorMessage = null, Response response = null) {
     def spanCount = 1 // server span
-    if (hasHandlerSpan()) {
+    if (hasResponseSpan(endpoint)) {
       spanCount++
     }
-    if (hasResponseSpan(endpoint)) {
+    if (hasHandlerSpan(endpoint)) {
       spanCount++
     }
     if (endpoint != NOT_FOUND) {
@@ -477,12 +477,12 @@ abstract class HttpServerTest<SERVER> extends InstrumentationSpecification imple
         trace(it, spanCount) {
           def spanIndex = 0
           serverSpan(it, spanIndex++, traceID, parentID, method, response?.body()?.contentLength(), endpoint)
-          if (hasHandlerSpan()) {
+          if (hasHandlerSpan(endpoint)) {
             handlerSpan(it, spanIndex++, span(0), method, endpoint)
           }
           if (endpoint != NOT_FOUND) {
             def controllerSpanIndex = 0
-            if (hasHandlerSpan()) {
+            if (hasHandlerSpan(endpoint)) {
               controllerSpanIndex++
             }
             if (hasForwardSpan()) {