Test undertow and armeria http2 server (#11361)

instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/internal/HttpProtocolUtil.java

@@ -0,0 +1,39 @@
+/*
+ * Copyright The OpenTelemetry Authors
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+package io.opentelemetry.instrumentation.api.internal;
+
+import javax.annotation.Nullable;
+
+/**
+ * This class is internal and is hence not for public use. Its APIs are unstable and can change at
+ * any time.
+ */
+public final class HttpProtocolUtil {
+
+  public static String getProtocol(@Nullable String protocol) {
+    if (protocol != null && protocol.startsWith("HTTP/")) {
+      return "http";
+    }
+    return null;
+  }
+
+  public static String getVersion(@Nullable String protocol) {
+    if (protocol != null && protocol.startsWith("HTTP/")) {
+      return normalizeHttpVersion(protocol.substring("HTTP/".length()));
+    }
+    return null;
+  }
+
+  public static String normalizeHttpVersion(String version) {
+    if ("2.0".equals(version)) {
+      return "2";
+    }
+
+    return version;
+  }
+
+  private HttpProtocolUtil() {}
+}

instrumentation/armeria-1.3/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/armeria/v1_3/ArmeriaHttp2ServerTest.java

@@ -0,0 +1,17 @@
+/*
+ * Copyright The OpenTelemetry Authors
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+package io.opentelemetry.javaagent.instrumentation.armeria.v1_3;
+
+import io.opentelemetry.instrumentation.testing.junit.http.HttpServerTestOptions;
+
+class ArmeriaHttp2ServerTest extends ArmeriaHttpServerTest {
+
+  @Override
+  protected void configure(HttpServerTestOptions options) {
+    super.configure(options);
+    options.useHttp2();
+  }
+}

instrumentation/armeria-1.3/library/src/test/java/io/opentelemetry/instrumentation/armeria/v1_3/ArmeriaHttp2ServerTest.java

@@ -0,0 +1,17 @@
+/*
+ * Copyright The OpenTelemetry Authors
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+package io.opentelemetry.instrumentation.armeria.v1_3;
+
+import io.opentelemetry.instrumentation.testing.junit.http.HttpServerTestOptions;
+
+class ArmeriaHttp2ServerTest extends ArmeriaHttpServerTest {
+
+  @Override
+  protected void configure(HttpServerTestOptions options) {
+    super.configure(options);
+    options.useHttp2();
+  }
+}

instrumentation/undertow-1.4/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/undertow/HttpTransferEncodingInstrumentation.java → instrumentation/undertow-1.4/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/undertow/HttpServerConnectionInstrumentation.java

@@ -7,6 +7,7 @@ package io.opentelemetry.javaagent.instrumentation.undertow;
 
 import static io.opentelemetry.javaagent.instrumentation.undertow.UndertowSingletons.helper;
 import static net.bytebuddy.matcher.ElementMatchers.named;
+import static net.bytebuddy.matcher.ElementMatchers.namedOneOf;
 import static net.bytebuddy.matcher.ElementMatchers.takesArgument;
 
 import io.opentelemetry.context.Context;
@@ -18,16 +19,18 @@ import net.bytebuddy.asm.Advice;
 import net.bytebuddy.description.type.TypeDescription;
 import net.bytebuddy.matcher.ElementMatcher;
 
-public class HttpTransferEncodingInstrumentation implements TypeInstrumentation {
+public class HttpServerConnectionInstrumentation implements TypeInstrumentation {
   @Override
   public ElementMatcher<TypeDescription> typeMatcher() {
-    return named("io.undertow.server.protocol.http.HttpTransferEncoding");
+    return namedOneOf(
+        "io.undertow.server.protocol.http.HttpServerConnection",
+        "io.undertow.server.protocol.http2.Http2ServerConnection");
   }
 
   @Override
   public void transform(TypeTransformer transformer) {
     transformer.applyAdviceToMethod(
-        named("createSinkConduit")
+        named("getSinkConduit")
             .and(takesArgument(0, named("io.undertow.server.HttpServerExchange"))),
         this.getClass().getName() + "$ResponseAdvice");
   }

instrumentation/undertow-1.4/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/undertow/UndertowHttpAttributesGetter.java

@@ -5,6 +5,7 @@
 
 package io.opentelemetry.javaagent.instrumentation.undertow;
 
+import io.opentelemetry.instrumentation.api.internal.HttpProtocolUtil;
 import io.opentelemetry.instrumentation.api.semconv.http.HttpServerAttributesGetter;
 import io.undertow.server.HttpServerExchange;
 import io.undertow.util.HeaderValues;
@@ -65,22 +66,14 @@ public class UndertowHttpAttributesGetter
   @Override
   public String getNetworkProtocolName(
       HttpServerExchange exchange, @Nullable HttpServerExchange unused) {
-    String protocol = exchange.getProtocol().toString();
-    if (protocol.startsWith("HTTP/")) {
-      return "http";
-    }
-    return null;
+    return HttpProtocolUtil.getProtocol(exchange.getProtocol().toString());
   }
 
   @Nullable
   @Override
   public String getNetworkProtocolVersion(
       HttpServerExchange exchange, @Nullable HttpServerExchange unused) {
-    String protocol = exchange.getProtocol().toString();
-    if (protocol.startsWith("HTTP/")) {
-      return protocol.substring("HTTP/".length());
-    }
-    return null;
+    return HttpProtocolUtil.getVersion(exchange.getProtocol().toString());
   }
 
   @Override

instrumentation/undertow-1.4/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/undertow/UndertowInstrumentationModule.java

@@ -32,6 +32,6 @@ public class UndertowInstrumentationModule extends InstrumentationModule {
     return asList(
         new HandlerInstrumentation(),
         new HttpServerExchangeInstrumentation(),
-        new HttpTransferEncodingInstrumentation());
+        new HttpServerConnectionInstrumentation());
   }
 }

instrumentation/undertow-1.4/javaagent/src/test/groovy/UndertowHttp2ServerTest.groovy

@@ -0,0 +1,18 @@
+/*
+ * Copyright The OpenTelemetry Authors
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+import io.undertow.Undertow
+import io.undertow.UndertowOptions
+
+class UndertowHttp2ServerTest extends UndertowServerTest {
+
+  void configureUndertow(Undertow.Builder builder) {
+    builder.setServerOption(UndertowOptions.ENABLE_HTTP2, true)
+  }
+
+  boolean useHttp2() {
+    true
+  }
+}

instrumentation/undertow-1.4/javaagent/src/test/groovy/UndertowServerTest.groovy

@@ -36,72 +36,74 @@ class UndertowServerTest extends HttpServerTest<Undertow> implements AgentTestTr
 
   @Override
   Undertow startServer(int port) {
-    Undertow server = Undertow.builder()
-      .addHttpListener(port, "localhost")
-      .setHandler(Handlers.path()
-        .addExactPath(SUCCESS.rawPath()) { exchange ->
-          controller(SUCCESS) {
-            exchange.getResponseSender().send(SUCCESS.body)
-          }
-        }
-        .addExactPath(QUERY_PARAM.rawPath()) { exchange ->
-          controller(QUERY_PARAM) {
-            exchange.getResponseSender().send(exchange.getQueryString())
-          }
-        }
-        .addExactPath(REDIRECT.rawPath()) { exchange ->
-          controller(REDIRECT) {
-            exchange.setStatusCode(StatusCodes.FOUND)
-            exchange.getResponseHeaders().put(Headers.LOCATION, REDIRECT.body)
-            exchange.endExchange()
-          }
-        }
-        .addExactPath(CAPTURE_HEADERS.rawPath()) { exchange ->
-          controller(CAPTURE_HEADERS) {
-            exchange.setStatusCode(StatusCodes.OK)
-            exchange.getResponseHeaders().put(new HttpString("X-Test-Response"), exchange.getRequestHeaders().getFirst("X-Test-Request"))
-            exchange.getResponseSender().send(CAPTURE_HEADERS.body)
-          }
-        }
-        .addExactPath(ERROR.rawPath()) { exchange ->
-          controller(ERROR) {
-            exchange.setStatusCode(ERROR.status)
-            exchange.getResponseSender().send(ERROR.body)
-          }
-        }
-        .addExactPath(EXCEPTION.rawPath()) { exchange ->
-          controller(EXCEPTION) {
-            throw new Exception(EXCEPTION.body)
-          }
-        }
-        .addExactPath(INDEXED_CHILD.rawPath()) { exchange ->
-          controller(INDEXED_CHILD) {
-            INDEXED_CHILD.collectSpanAttributes { name -> exchange.getQueryParameters().get(name).peekFirst() }
-            exchange.getResponseSender().send(INDEXED_CHILD.body)
-          }
-        }
-        .addExactPath("sendResponse") { exchange ->
-          Span.current().addEvent("before-event")
-          runWithSpan("sendResponse") {
-            exchange.setStatusCode(StatusCodes.OK)
-            exchange.getResponseSender().send("sendResponse")
-          }
-          // event is added only when server span has not been ended
-          // we need to make sure that sending response does not end server span
-          Span.current().addEvent("after-event")
-        }
-        .addExactPath("sendResponseWithException") { exchange ->
-          Span.current().addEvent("before-event")
-          runWithSpan("sendResponseWithException") {
-            exchange.setStatusCode(StatusCodes.OK)
-            exchange.getResponseSender().send("sendResponseWithException")
-          }
-          // event is added only when server span has not been ended
-          // we need to make sure that sending response does not end server span
-          Span.current().addEvent("after-event")
-          throw new Exception("exception after sending response")
-        }
-      ).build()
+    Undertow.Builder builder = Undertow.builder()
+        .addHttpListener(port, "localhost")
+        .setHandler(Handlers.path()
+            .addExactPath(SUCCESS.rawPath()) { exchange ->
+              controller(SUCCESS) {
+                exchange.getResponseSender().send(SUCCESS.body)
+              }
+            }
+            .addExactPath(QUERY_PARAM.rawPath()) { exchange ->
+              controller(QUERY_PARAM) {
+                exchange.getResponseSender().send(exchange.getQueryString())
+              }
+            }
+            .addExactPath(REDIRECT.rawPath()) { exchange ->
+              controller(REDIRECT) {
+                exchange.setStatusCode(StatusCodes.FOUND)
+                exchange.getResponseHeaders().put(Headers.LOCATION, REDIRECT.body)
+                exchange.endExchange()
+              }
+            }
+            .addExactPath(CAPTURE_HEADERS.rawPath()) { exchange ->
+              controller(CAPTURE_HEADERS) {
+                exchange.setStatusCode(StatusCodes.OK)
+                exchange.getResponseHeaders().put(new HttpString("X-Test-Response"), exchange.getRequestHeaders().getFirst("X-Test-Request"))
+                exchange.getResponseSender().send(CAPTURE_HEADERS.body)
+              }
+            }
+            .addExactPath(ERROR.rawPath()) { exchange ->
+              controller(ERROR) {
+                exchange.setStatusCode(ERROR.status)
+                exchange.getResponseSender().send(ERROR.body)
+              }
+            }
+            .addExactPath(EXCEPTION.rawPath()) { exchange ->
+              controller(EXCEPTION) {
+                throw new Exception(EXCEPTION.body)
+              }
+            }
+            .addExactPath(INDEXED_CHILD.rawPath()) { exchange ->
+              controller(INDEXED_CHILD) {
+                INDEXED_CHILD.collectSpanAttributes { name -> exchange.getQueryParameters().get(name).peekFirst() }
+                exchange.getResponseSender().send(INDEXED_CHILD.body)
+              }
+            }
+            .addExactPath("sendResponse") { exchange ->
+              Span.current().addEvent("before-event")
+              runWithSpan("sendResponse") {
+                exchange.setStatusCode(StatusCodes.OK)
+                exchange.getResponseSender().send("sendResponse")
+              }
+              // event is added only when server span has not been ended
+              // we need to make sure that sending response does not end server span
+              Span.current().addEvent("after-event")
+            }
+            .addExactPath("sendResponseWithException") { exchange ->
+              Span.current().addEvent("before-event")
+              runWithSpan("sendResponseWithException") {
+                exchange.setStatusCode(StatusCodes.OK)
+                exchange.getResponseSender().send("sendResponseWithException")
+              }
+              // event is added only when server span has not been ended
+              // we need to make sure that sending response does not end server span
+              Span.current().addEvent("after-event")
+              throw new Exception("exception after sending response")
+            }
+        )
+    configureUndertow(builder)
+    Undertow server = builder.build()
     server.start()
     return server
   }
@@ -111,6 +113,9 @@ class UndertowServerTest extends HttpServerTest<Undertow> implements AgentTestTr
     undertow.stop()
   }
 
+  void configureUndertow(Undertow.Builder builder) {
+  }
+
   @Override
   Set<AttributeKey<?>> httpAttributes(ServerEndpoint endpoint) {
     def attributes = super.httpAttributes(endpoint)
@@ -147,6 +152,7 @@ class UndertowServerTest extends HttpServerTest<Undertow> implements AgentTestTr
             eventName "after-event"
           }
 
+          def protocolVersion = useHttp2() ? "2" : "1.1"
           attributes {
             "$ClientAttributes.CLIENT_ADDRESS" TEST_CLIENT_IP
             "$UrlAttributes.URL_SCHEME" uri.getScheme()
@@ -154,7 +160,7 @@ class UndertowServerTest extends HttpServerTest<Undertow> implements AgentTestTr
             "$HttpAttributes.HTTP_REQUEST_METHOD" "GET"
             "$HttpAttributes.HTTP_RESPONSE_STATUS_CODE" 200
             "$UserAgentAttributes.USER_AGENT_ORIGINAL" TEST_USER_AGENT
-            "$NetworkAttributes.NETWORK_PROTOCOL_VERSION" "1.1"
+            "$NetworkAttributes.NETWORK_PROTOCOL_VERSION" protocolVersion
             "$ServerAttributes.SERVER_ADDRESS" uri.host
             "$ServerAttributes.SERVER_PORT" uri.port
             "$NetworkAttributes.NETWORK_PEER_ADDRESS" "127.0.0.1"
@@ -196,6 +202,7 @@ class UndertowServerTest extends HttpServerTest<Undertow> implements AgentTestTr
           }
           errorEvent(Exception, "exception after sending response", 2)
 
+          def protocolVersion = useHttp2() ? "2" : "1.1"
           attributes {
             "$ClientAttributes.CLIENT_ADDRESS" TEST_CLIENT_IP
             "$UrlAttributes.URL_SCHEME" uri.getScheme()
@@ -203,7 +210,7 @@ class UndertowServerTest extends HttpServerTest<Undertow> implements AgentTestTr
             "$HttpAttributes.HTTP_REQUEST_METHOD" "GET"
             "$HttpAttributes.HTTP_RESPONSE_STATUS_CODE" 200
             "$UserAgentAttributes.USER_AGENT_ORIGINAL" TEST_USER_AGENT
-            "$NetworkAttributes.NETWORK_PROTOCOL_VERSION" "1.1"
+            "$NetworkAttributes.NETWORK_PROTOCOL_VERSION" protocolVersion
             "$ServerAttributes.SERVER_ADDRESS" uri.host
             "$ServerAttributes.SERVER_PORT" uri.port
             "$NetworkAttributes.NETWORK_PEER_ADDRESS" "127.0.0.1"

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

@@ -34,6 +34,7 @@ import static io.opentelemetry.instrumentation.testing.junit.http.ServerEndpoint
 import static io.opentelemetry.instrumentation.testing.junit.http.ServerEndpoint.PATH_PARAM
 import static io.opentelemetry.instrumentation.testing.junit.http.ServerEndpoint.QUERY_PARAM
 import static io.opentelemetry.instrumentation.testing.junit.http.ServerEndpoint.SUCCESS
+import static org.junit.jupiter.api.Assumptions.assumeFalse
 import static org.junit.jupiter.api.Assumptions.assumeTrue
 
 @Unroll
@@ -80,6 +81,10 @@ abstract class HttpServerTest<SERVER> extends InstrumentationSpecification imple
     return ""
   }
 
+  boolean useHttp2() {
+    false
+  }
+
   boolean hasHandlerSpan(ServerEndpoint endpoint) {
     false
   }
@@ -249,6 +254,7 @@ abstract class HttpServerTest<SERVER> extends InstrumentationSpecification imple
       if (!testNonStandardHttpMethod()) {
         options.disableTestNonStandardHttpMethod()
       }
+      options.useHttp2 = useHttp2()
     }
 
     // Override trace assertion method. We can call java assertions from groovy but not the other
@@ -352,12 +358,16 @@ abstract class HttpServerTest<SERVER> extends InstrumentationSpecification imple
 
   def "http pipelining test"() {
     assumeTrue(testHttpPipelining())
+    // test uses http 1.1
+    assumeFalse(useHttp2())
     expect:
     junitTest.httpPipelining()
   }
 
   def "non standard http method"() {
     assumeTrue(testNonStandardHttpMethod())
+    // test uses http 1.1
+    assumeFalse(useHttp2())
     expect:
     junitTest.requestWithNonStandardHttpMethod()
   }

testing-common/src/main/java/io/opentelemetry/instrumentation/testing/junit/http/AbstractHttpServerTest.java

@@ -16,6 +16,7 @@ import static io.opentelemetry.instrumentation.testing.junit.http.ServerEndpoint
 import static io.opentelemetry.instrumentation.testing.junit.http.ServerEndpoint.SUCCESS;
 import static io.opentelemetry.sdk.testing.assertj.OpenTelemetryAssertions.assertThat;
 import static io.opentelemetry.sdk.testing.assertj.OpenTelemetryAssertions.equalTo;
+import static org.junit.jupiter.api.Assumptions.assumeFalse;
 import static org.junit.jupiter.api.Assumptions.assumeTrue;
 
 import com.google.errorprone.annotations.CanIgnoreReturnValue;
@@ -123,7 +124,12 @@ public abstract class AbstractHttpServerTest<SERVER> extends AbstractHttpServerU
   }
 
   protected AggregatedHttpRequest request(ServerEndpoint uri, String method) {
-    return AggregatedHttpRequest.of(HttpMethod.valueOf(method), resolveAddress(uri));
+    return AggregatedHttpRequest.of(
+        HttpMethod.valueOf(method), resolveAddress(uri, getProtocolPrefix()));
+  }
+
+  private String getProtocolPrefix() {
+    return options.useHttp2 ? "h2c://" : "h1c://";
   }
 
   @ParameterizedTest
@@ -316,7 +322,8 @@ public abstract class AbstractHttpServerTest<SERVER> extends AbstractHttpServerU
     QueryParams formBody = QueryParams.builder().add("test-parameter", "test value õäöü").build();
     AggregatedHttpRequest request =
         AggregatedHttpRequest.of(
-            RequestHeaders.builder(HttpMethod.POST, resolveAddress(CAPTURE_PARAMETERS))
+            RequestHeaders.builder(
+                    HttpMethod.POST, resolveAddress(CAPTURE_PARAMETERS, getProtocolPrefix()))
                 .contentType(MediaType.FORM_DATA)
                 .build(),
             HttpData.ofUtf8(formBody.toQueryString()));
@@ -393,7 +400,7 @@ public abstract class AbstractHttpServerTest<SERVER> extends AbstractHttpServerU
       HttpRequestBuilder request =
           HttpRequest.builder()
               // Force HTTP/1 via h1c so upgrade requests don't show up as traces
-              .get(endpoint.resolvePath(address).toString().replace("http://", "h1c://"))
+              .get(endpoint.resolvePath(address).toString().replace("http://", getProtocolPrefix()))
               .queryParam(ServerEndpoint.ID_PARAMETER_NAME, index);
 
       testing.runWithSpan(
@@ -415,6 +422,8 @@ public abstract class AbstractHttpServerTest<SERVER> extends AbstractHttpServerU
   @Test
   void httpPipelining() throws InterruptedException {
     assumeTrue(options.testHttpPipelining);
+    // test uses http 1.1
+    assumeFalse(options.useHttp2);
 
     int count = 10;
     CountDownLatch countDownLatch = new CountDownLatch(count);
@@ -480,6 +489,8 @@ public abstract class AbstractHttpServerTest<SERVER> extends AbstractHttpServerU
   @Test
   void requestWithNonStandardHttpMethod() throws InterruptedException {
     assumeTrue(options.testNonStandardHttpMethod);
+    // test uses http 1.1
+    assumeFalse(options.useHttp2);
 
     EventLoopGroup eventLoopGroup = new NioEventLoopGroup();
     try {
@@ -751,9 +762,8 @@ public abstract class AbstractHttpServerTest<SERVER> extends AbstractHttpServerU
 
           if (attrs.get(NetworkAttributes.NETWORK_PROTOCOL_VERSION) != null) {
             assertThat(attrs)
-                .hasEntrySatisfying(
-                    NetworkAttributes.NETWORK_PROTOCOL_VERSION,
-                    entry -> assertThat(entry).isIn("1.1", "2.0"));
+                .containsEntry(
+                    NetworkAttributes.NETWORK_PROTOCOL_VERSION, options.useHttp2 ? "2" : "1.1");
           }
 
           assertThat(attrs).containsEntry(ServerAttributes.SERVER_ADDRESS, "localhost");

testing-common/src/main/java/io/opentelemetry/instrumentation/testing/junit/http/AbstractHttpServerUsingTest.java

@@ -87,9 +87,13 @@ public abstract class AbstractHttpServerUsingTest<SERVER> {
   }
 
   protected String resolveAddress(ServerEndpoint uri) {
+    return resolveAddress(uri, "h1c://");
+  }
+
+  protected String resolveAddress(ServerEndpoint uri, String protocolPrefix) {
     String url = uri.resolvePath(address).toString();
     // Force HTTP/1 via h1c so upgrade requests don't show up as traces
-    url = url.replace("http://", "h1c://");
+    url = url.replace("http://", protocolPrefix);
     if (uri.getQuery() != null) {
       url += "?" + uri.getQuery();
     }

testing-common/src/main/java/io/opentelemetry/instrumentation/testing/junit/http/HttpServerTestOptions.java

@@ -65,6 +65,7 @@ public final class HttpServerTestOptions {
   boolean testHttpPipelining = true;
   boolean testNonStandardHttpMethod = true;
   boolean verifyServerSpanEndTime = true;
+  boolean useHttp2 = false;
 
   HttpServerTestOptions() {}
 
@@ -227,6 +228,17 @@ public final class HttpServerTestOptions {
     return this;
   }
 
+  @CanIgnoreReturnValue
+  public HttpServerTestOptions setUseHttp2(boolean useHttp2) {
+    this.useHttp2 = useHttp2;
+    return this;
+  }
+
+  @CanIgnoreReturnValue
+  public HttpServerTestOptions useHttp2() {
+    return setUseHttp2(true);
+  }
+
   @FunctionalInterface
   public interface SpanNameMapper {