Test undertow and armeria http2 server (#11361)

open-telemetry · May 15, 2024 · defd7cb · defd7cb
1 parent bae4c3b
commit defd7cb
Show file tree

Hide file tree

Showing 12 changed files with 219 additions and 89 deletions.
diff --git a/...ion-api/src/main/java/io/opentelemetry/instrumentation/api/internal/HttpProtocolUtil.java b/...ion-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() {}
+}
diff --git a/.../java/io/opentelemetry/javaagent/instrumentation/armeria/v1_3/ArmeriaHttp2ServerTest.java b/.../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();
+  }
+}
diff --git a/...y/src/test/java/io/opentelemetry/instrumentation/armeria/v1_3/ArmeriaHttp2ServerTest.java b/...y/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();
+  }
+}
diff --git a/.../HttpTransferEncodingInstrumentation.java → .../HttpServerConnectionInstrumentation.java b/.../HttpTransferEncodingInstrumentation.java → .../HttpServerConnectionInstrumentation.java
@@ -7,6 +7,7 @@
 
 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.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");
   }

diff --git a/...ava/io/opentelemetry/javaagent/instrumentation/undertow/UndertowHttpAttributesGetter.java b/...ava/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 String getUrlQuery(HttpServerExchange exchange) {
   @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

diff --git a/...va/io/opentelemetry/javaagent/instrumentation/undertow/UndertowInstrumentationModule.java b/...va/io/opentelemetry/javaagent/instrumentation/undertow/UndertowInstrumentationModule.java
@@ -32,6 +32,6 @@ public List<TypeInstrumentation> typeInstrumentations() {
     return asList(
         new HandlerInstrumentation(),
         new HttpServerExchangeInstrumentation(),
-        new HttpTransferEncodingInstrumentation());
+        new HttpServerConnectionInstrumentation());
   }
 }
diff --git a/instrumentation/undertow-1.4/javaagent/src/test/groovy/UndertowHttp2ServerTest.groovy b/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
+  }
+}
diff --git a/instrumentation/undertow-1.4/javaagent/src/test/groovy/UndertowServerTest.groovy b/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,14 +152,15 @@ 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()
             "$UrlAttributes.URL_PATH" uri.getPath()
             "$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,14 +202,15 @@ 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()
             "$UrlAttributes.URL_PATH" uri.getPath()
             "$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"

diff --git a/...g-common/src/main/groovy/io/opentelemetry/instrumentation/test/base/HttpServerTest.groovy b/...g-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()
   }