Extract HttpServerTest INDEXED_CHILD span attribute collection logic (#3095)

* Extrawct HttpServerTest INDEXED_CHILD span attribute collection logic

* Adjust whitespace

* Fix context issue with JAX-RS

* Rerun tests

instrumentation/jaxrs/jaxrs-2.0/jaxrs-2.0-testing/src/main/groovy/test/JaxRsTestResource.groovy

@@ -14,7 +14,6 @@ import static io.opentelemetry.instrumentation.test.base.HttpServerTest.ServerEn
 import static io.opentelemetry.instrumentation.test.base.HttpServerTest.ServerEndpoint.SUCCESS
 import static java.util.concurrent.TimeUnit.SECONDS
 
-import io.opentelemetry.api.trace.Span
 import io.opentelemetry.instrumentation.test.base.HttpServerTest
 import java.util.concurrent.CompletableFuture
 import java.util.concurrent.CompletionStage
@@ -88,10 +87,12 @@ class JaxRsTestResource {
 
   @Path("/child")
   @GET
-  void indexed_child(@Suspended AsyncResponse response, @QueryParam("id") long id) {
+  void indexed_child(@Context UriInfo uriInfo, @Suspended AsyncResponse response) {
+    def parameters = uriInfo.queryParameters
+
     CompletableFuture.runAsync({
       HttpServerTest.controller(INDEXED_CHILD) {
-        Span.current().setAttribute("test.request.id", id)
+        INDEXED_CHILD.collectSpanAttributes { parameters.getFirst(it) }
         response.resume("")
       }
     })

instrumentation/netty/netty-3.8/javaagent/src/test/groovy/Netty38ServerTest.groovy

@@ -16,7 +16,6 @@ import static org.jboss.netty.handler.codec.http.HttpHeaders.Names.CONTENT_TYPE
 import static org.jboss.netty.handler.codec.http.HttpHeaders.Names.LOCATION
 import static org.jboss.netty.handler.codec.http.HttpVersion.HTTP_1_1
 
-import io.opentelemetry.api.trace.Span
 import io.opentelemetry.instrumentation.test.AgentTestTrait
 import io.opentelemetry.instrumentation.test.base.HttpServerTest
 import org.jboss.netty.bootstrap.ServerBootstrap
@@ -75,8 +74,7 @@ class Netty38ServerTest extends HttpServerTest<ServerBootstrap> implements Agent
                 response.setContent(responseContent)
                 break
               case INDEXED_CHILD:
-                QueryStringDecoder queryStringDecoder = new QueryStringDecoder(uri)
+                endpoint.collectSpanAttributes { new QueryStringDecoder(uri).getParameters().get(it).find() }
-                Span.current().setAttribute("test.request.id", queryStringDecoder.getParameters().get("id").find() as long)
                 response = new DefaultHttpResponse(HTTP_1_1, HttpResponseStatus.valueOf(endpoint.status))
                 break
               case QUERY_PARAM:

instrumentation/netty/netty-4.0/javaagent/src/test/groovy/Netty40ServerTest.groovy

@@ -36,7 +36,6 @@ import io.netty.handler.codec.http.QueryStringDecoder
 import io.netty.handler.logging.LogLevel
 import io.netty.handler.logging.LoggingHandler
 import io.netty.util.CharsetUtil
-import io.opentelemetry.api.trace.Span
 import io.opentelemetry.instrumentation.test.AgentTestTrait
 import io.opentelemetry.instrumentation.test.base.HttpServerTest
 
@@ -73,8 +72,7 @@ class Netty40ServerTest extends HttpServerTest<EventLoopGroup> implements AgentT
                       response = new DefaultFullHttpResponse(HTTP_1_1, HttpResponseStatus.valueOf(endpoint.status), content)
                       break
                     case INDEXED_CHILD:
-                      QueryStringDecoder queryStringDecoder = new QueryStringDecoder(uri)
+                      endpoint.collectSpanAttributes { new QueryStringDecoder(uri).parameters().get(it).find() }
-                      Span.current().setAttribute("test.request.id", queryStringDecoder.parameters().get("id").find() as long)
                       response = new DefaultFullHttpResponse(HTTP_1_1, HttpResponseStatus.valueOf(endpoint.status))
                       break
                     case QUERY_PARAM:

instrumentation/netty/netty-4.1/javaagent/src/test/groovy/Netty41ServerTest.groovy

@@ -35,7 +35,6 @@ import io.netty.handler.codec.http.QueryStringDecoder
 import io.netty.handler.logging.LogLevel
 import io.netty.handler.logging.LoggingHandler
 import io.netty.util.CharsetUtil
-import io.opentelemetry.api.trace.Span
 import io.opentelemetry.instrumentation.test.AgentTestTrait
 import io.opentelemetry.instrumentation.test.base.HttpServerTest
 
@@ -72,8 +71,7 @@ class Netty41ServerTest extends HttpServerTest<EventLoopGroup> implements AgentT
                       response = new DefaultFullHttpResponse(HTTP_1_1, HttpResponseStatus.valueOf(endpoint.status), content)
                       break
                     case INDEXED_CHILD:
-                      QueryStringDecoder queryStringDecoder = new QueryStringDecoder(uri)
+                      endpoint.collectSpanAttributes { new QueryStringDecoder(uri).parameters().get(it).find() }
-                      Span.current().setAttribute("test.request.id", queryStringDecoder.parameters().get("id").find() as long)
                       response = new DefaultFullHttpResponse(HTTP_1_1, HttpResponseStatus.valueOf(endpoint.status))
                       break
                     case QUERY_PARAM:

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

@@ -13,7 +13,6 @@ import static io.opentelemetry.instrumentation.test.base.HttpServerTest.ServerEn
 import static io.opentelemetry.instrumentation.test.base.HttpServerTest.ServerEndpoint.SUCCESS
 import static play.mvc.Http.Context.Implicit.request
 
-import io.opentelemetry.api.trace.Span
 import java.util.concurrent.CompletableFuture
 import java.util.function.Supplier
 import play.libs.concurrent.HttpExecution
@@ -36,7 +35,7 @@ class PlayAsyncServerTest extends PlayServerTest {
         .GET(INDEXED_CHILD.getPath()).routeTo({
         CompletableFuture.supplyAsync({
           controller(INDEXED_CHILD) {
-            Span.current().setAttribute("test.request.id", request().getQueryString("id") as long)
+            INDEXED_CHILD.collectSpanAttributes { request().getQueryString(it) }
             Results.status(INDEXED_CHILD.getStatus())
           }
         }, HttpExecution.defaultContext())

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

@@ -14,7 +14,6 @@ import static io.opentelemetry.instrumentation.test.base.HttpServerTest.ServerEn
 import static io.opentelemetry.instrumentation.test.base.HttpServerTest.ServerEndpoint.SUCCESS
 import static play.mvc.Http.Context.Implicit.request
 
-import io.opentelemetry.api.trace.Span
 import io.opentelemetry.api.trace.StatusCode
 import io.opentelemetry.instrumentation.test.AgentTestTrait
 import io.opentelemetry.instrumentation.test.asserts.TraceAssert
@@ -37,7 +36,7 @@ class PlayServerTest extends HttpServerTest<Server> implements AgentTestTrait {
       } as Supplier)
         .GET(INDEXED_CHILD.getPath()).routeTo({
         controller(INDEXED_CHILD) {
-          Span.current().setAttribute("test.request.id", request().getQueryString("id") as long)
+          INDEXED_CHILD.collectSpanAttributes { request().getQueryString(it) }
           Results.status(INDEXED_CHILD.getStatus())
         }
       } as Supplier)

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

@@ -13,7 +13,6 @@ import static io.opentelemetry.instrumentation.test.base.HttpServerTest.ServerEn
 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.Span
 import ratpack.exec.Promise
 import ratpack.groovy.test.embed.GroovyEmbeddedApp
 import ratpack.test.embed.EmbeddedApp
@@ -48,7 +47,7 @@ class RatpackAsyncHttpServerTest extends RatpackHttpServerTest {
               INDEXED_CHILD
             } then {
               controller(INDEXED_CHILD) {
-                Span.current().setAttribute("test.request.id", request.queryParams.get("id") as long)
+                INDEXED_CHILD.collectSpanAttributes { request.queryParams.get(it) }
                 context.response.status(INDEXED_CHILD.status).send()
               }
             }

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

@@ -13,7 +13,6 @@ import static io.opentelemetry.instrumentation.test.base.HttpServerTest.ServerEn
 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.Span
 import ratpack.exec.Promise
 import ratpack.groovy.test.embed.GroovyEmbeddedApp
 import ratpack.test.embed.EmbeddedApp
@@ -48,7 +47,7 @@ class RatpackForkedHttpServerTest extends RatpackHttpServerTest {
               INDEXED_CHILD
             }.fork().then {
               controller(INDEXED_CHILD) {
-                Span.current().setAttribute("test.request.id", request.queryParams.get("id") as long)
+                INDEXED_CHILD.collectSpanAttributes { request.queryParams.get(it) }
                 context.response.status(INDEXED_CHILD.status).send()
               }
             }

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

@@ -14,7 +14,6 @@ import static io.opentelemetry.instrumentation.test.base.HttpServerTest.ServerEn
 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.Span
 import io.opentelemetry.api.trace.StatusCode
 import io.opentelemetry.instrumentation.test.AgentTestTrait
 import io.opentelemetry.instrumentation.test.asserts.TraceAssert
@@ -48,7 +47,7 @@ class RatpackHttpServerTest extends HttpServerTest<EmbeddedApp> implements Agent
         prefix(INDEXED_CHILD.rawPath()) {
           all {
             controller(INDEXED_CHILD) {
-              Span.current().setAttribute("test.request.id", request.queryParams.get("id") as long)
+              INDEXED_CHILD.collectSpanAttributes { request.queryParams.get(it) }
               context.response.status(INDEXED_CHILD.status).send()
             }
           }

instrumentation/servlet/servlet-3.0/javaagent/src/test/groovy/TestServlet3.groovy

@@ -11,7 +11,6 @@ import static io.opentelemetry.instrumentation.test.base.HttpServerTest.ServerEn
 import static io.opentelemetry.instrumentation.test.base.HttpServerTest.ServerEndpoint.SUCCESS
 
 import groovy.servlet.AbstractHttpServlet
-import io.opentelemetry.api.trace.Span
 import io.opentelemetry.instrumentation.test.base.HttpServerTest
 import java.util.concurrent.Phaser
 import javax.servlet.RequestDispatcher
@@ -40,7 +39,7 @@ class TestServlet3 {
             break
           case INDEXED_CHILD:
             resp.status = endpoint.status
-            Span.current().setAttribute("test.request.id", req.getParameter("id") as long)
+            endpoint.collectSpanAttributes { req.getParameter(it) }
             break
           case QUERY_PARAM:
             resp.status = endpoint.status
@@ -78,7 +77,7 @@ class TestServlet3 {
                 context.complete()
                 break
               case INDEXED_CHILD:
-                Span.current().setAttribute("test.request.id", req.getParameter("id") as long)
+                endpoint.collectSpanAttributes { req.getParameter(it) }
                 resp.status = endpoint.status
                 context.complete()
                 break
@@ -128,7 +127,7 @@ class TestServlet3 {
               resp.writer.print(endpoint.body)
               break
             case INDEXED_CHILD:
-              Span.current().setAttribute("test.request.id", req.getParameter("id") as long)
+              endpoint.collectSpanAttributes { req.getParameter(it) }
               resp.status = endpoint.status
               break
             case QUERY_PARAM:

instrumentation/servlet/servlet-5.0/javaagent/src/test/groovy/TestServlet5.groovy

@@ -10,7 +10,6 @@ import static io.opentelemetry.instrumentation.test.base.HttpServerTest.ServerEn
 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.Span
 import io.opentelemetry.instrumentation.test.base.HttpServerTest
 import jakarta.servlet.RequestDispatcher
 import jakarta.servlet.ServletException
@@ -40,7 +39,7 @@ class TestServlet5 {
             break
           case INDEXED_CHILD:
             resp.status = endpoint.status
-            Span.current().setAttribute("test.request.id", req.getParameter("id") as long)
+            endpoint.collectSpanAttributes { req.getParameter(it) }
             break
           case QUERY_PARAM:
             resp.status = endpoint.status
@@ -78,7 +77,7 @@ class TestServlet5 {
                 context.complete()
                 break
               case INDEXED_CHILD:
-                Span.current().setAttribute("test.request.id", req.getParameter("id") as long)
+                endpoint.collectSpanAttributes { req.getParameter(it) }
                 resp.status = endpoint.status
                 context.complete()
                 break
@@ -128,7 +127,7 @@ class TestServlet5 {
               resp.writer.print(endpoint.body)
               break
             case INDEXED_CHILD:
-              Span.current().setAttribute("test.request.id", req.getParameter("id") as long)
+              endpoint.collectSpanAttributes { req.getParameter(it) }
               resp.status = endpoint.status
               break
             case QUERY_PARAM:

instrumentation/spring/spring-webflux-5.0/javaagent/src/test/java/server/base/ServerTestController.java

@@ -5,7 +5,6 @@
 
 package server.base;
 
-import io.opentelemetry.api.trace.Span;
 import io.opentelemetry.instrumentation.test.base.HttpServerTest.ServerEndpoint;
 import java.net.URI;
 import java.util.concurrent.Callable;
@@ -96,9 +95,7 @@ public abstract class ServerTestController {
     return wrapControllerMethod(
         endpoint,
         () -> {
-          Span.current()
+          endpoint.collectSpanAttributes(it -> request.getQueryParams().getFirst(it));
-              .setAttribute(
-                  "test.request.id", Long.parseLong(request.getQueryParams().getFirst("id")));
           setStatus(response, endpoint);
           return "";
         });

instrumentation/vertx-reactive-3.5/javaagent/src/test/groovy/server/VertxRxCircuitBreakerHttpServerTest.groovy

@@ -13,7 +13,6 @@ import static io.opentelemetry.instrumentation.test.base.HttpServerTest.ServerEn
 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.Span
 import io.opentelemetry.instrumentation.test.base.HttpServerTest
 import io.vertx.circuitbreaker.CircuitBreakerOptions
 import io.vertx.core.Future
@@ -64,7 +63,7 @@ class VertxRxCircuitBreakerHttpServerTest extends VertxRxHttpServerTest {
           }
           HttpServerTest.ServerEndpoint endpoint = it.result()
           controller(endpoint) {
-            Span.current().setAttribute("test.request.id", ctx.request().params().get("id") as long)
+            endpoint.collectSpanAttributes { ctx.request().params().get(it) }
             ctx.response().setStatusCode(endpoint.status).end()
           }
         })

instrumentation/vertx-reactive-3.5/javaagent/src/test/groovy/server/VertxRxHttpServerTest.groovy

@@ -14,7 +14,6 @@ import static io.opentelemetry.instrumentation.test.base.HttpServerTest.ServerEn
 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.Span
 import io.opentelemetry.instrumentation.test.AgentTestTrait
 import io.opentelemetry.instrumentation.test.base.HttpServerTest
 import io.vertx.core.DeploymentOptions
@@ -95,8 +94,8 @@ class VertxRxHttpServerTest extends HttpServerTest<Vertx> implements AgentTestTr
         }
       }
       router.route(INDEXED_CHILD.path).handler { ctx ->
-        controller(QUERY_PARAM) {
+        controller(INDEXED_CHILD) {
-          Span.current().setAttribute("test.request.id", ctx.request().params().get("id") as long)
+          INDEXED_CHILD.collectSpanAttributes { ctx.request().params().get(it) }
           ctx.response().setStatusCode(INDEXED_CHILD.status).end()
         }
       }

instrumentation/vertx-web-3.0/javaagent/src/test/java/server/VertxWebServer.java

@@ -13,7 +13,6 @@ import static io.opentelemetry.instrumentation.test.base.HttpServerTest.ServerEn
 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.Span;
 import io.opentelemetry.instrumentation.test.base.HttpServerTest;
 import io.vertx.core.AbstractVerticle;
 import io.vertx.core.Future;
@@ -53,9 +52,7 @@ public class VertxWebServer extends AbstractVerticle {
                 HttpServerTest.controller(
                     INDEXED_CHILD,
                     () -> {
-                      Span.current()
+                      INDEXED_CHILD.collectSpanAttributes(it -> ctx.request().getParam(it));
-                          .setAttribute(
-                              "test.request.id", Long.parseLong(ctx.request().getParam("id")));
                       ctx.response().setStatusCode(INDEXED_CHILD.getStatus()).end();
                       return null;
                     }));

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

@@ -138,6 +138,9 @@ abstract class HttpServerTest<SERVER> extends InstrumentationSpecification imple
     AUTH_ERROR("basicsecured/endpoint", 401, null),
     INDEXED_CHILD("child", 200, null),
 
+    public static final String ID_ATTRIBUTE_NAME = "test.request.id"
+    public static final String ID_PARAMETER_NAME = "id"
+
     private final URI uriObj
     private final String path
     final String query
@@ -177,11 +180,31 @@ abstract class HttpServerTest<SERVER> extends InstrumentationSpecification imple
       return new URI(uri.scheme, null, uri.host, uri.port, uri.path, uri.query, null)
     }
 
+    /**
+     * Populates custom test attributes for the {@link HttpServerTest#controller} span (which must
+     * be the current span when this is called) based on URL parameters. Required for
+     * {@link #INDEXED_CHILD}.
+     */
+    void collectSpanAttributes(UrlParameterProvider parameterProvider) {
+      if (this == INDEXED_CHILD) {
+        String value = parameterProvider.getParameter(ID_PARAMETER_NAME)
+
+        if (value != null) {
+          Span.current().setAttribute(ID_ATTRIBUTE_NAME, value as long)
+        }
+      }
+    }
+
     private static final Map<String, ServerEndpoint> PATH_MAP = values().collectEntries { [it.path, it] }
 
     static ServerEndpoint forPath(String path) {
       return PATH_MAP.get(path)
     }
+
+    // Static keyword required for Scala interop
+    static interface UrlParameterProvider {
+      String getParameter(String name)
+    }
   }
 
   Request.Builder request(ServerEndpoint uri, String method, RequestBody body) {
@@ -416,11 +439,11 @@ abstract class HttpServerTest<SERVER> extends InstrumentationSpecification imple
       def job = {
         latch.await()
         def url = HttpUrl.get(endpoint.resolvePath(address)).newBuilder()
-          .query("id=$index")
+          .query("${ServerEndpoint.ID_PARAMETER_NAME}=$index")
           .build()
         Request.Builder builder = request(url, "GET", null)
         runUnderTrace("client " + index) {
-          Span.current().setAttribute("test.request.id", index)
+          Span.current().setAttribute(ServerEndpoint.ID_ATTRIBUTE_NAME, index)
           propagator.inject(Context.current(), builder, setter)
           client.newCall(builder.build()).execute()
         }
@@ -439,7 +462,7 @@ abstract class HttpServerTest<SERVER> extends InstrumentationSpecification imple
           def requestId = Integer.parseInt(rootSpan.name.substring("client ".length()))
 
           basicSpan(it, 0, "client " + requestId, null, null) {
-            it."test.request.id" requestId
+            "${ServerEndpoint.ID_ATTRIBUTE_NAME}" requestId
           }
           indexedServerSpan(it, span(0), requestId)
 
@@ -648,7 +671,7 @@ abstract class HttpServerTest<SERVER> extends InstrumentationSpecification imple
       name "controller"
       childOf((SpanData) parent)
       attributes {
-        it."test.request.id" requestId
+        "${ServerEndpoint.ID_ATTRIBUTE_NAME}" requestId
       }
     }
   }