Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Test undertow and armeria http2 server #11361

Merged
merged 2 commits into from
May 15, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -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/")) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I extracted an util class because this same pattern is used in many instrumentations and all of them probably report http2 version as 2.0 instead of 2.

return normalizeHttpVersion(protocol.substring("HTTP/".length()));
}
return null;
}

public static String normalizeHttpVersion(String version) {
if ("2.0".equals(version)) {
return "2";
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

}

return version;
}

private HttpProtocolUtil() {}
}
Original file line number Diff line number Diff line change
@@ -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();
}
}
Original file line number Diff line number Diff line change
@@ -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();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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");
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,6 @@ public List<TypeInstrumentation> typeInstrumentations() {
return asList(
new HandlerInstrumentation(),
new HttpServerExchangeInstrumentation(),
new HttpTransferEncodingInstrumentation());
new HttpServerConnectionInstrumentation());
}
}
Original file line number Diff line number Diff line change
@@ -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
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand All @@ -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)
Expand Down Expand Up @@ -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"
Expand Down Expand Up @@ -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"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -80,6 +81,10 @@ abstract class HttpServerTest<SERVER> extends InstrumentationSpecification imple
return ""
}

boolean useHttp2() {
false
}

boolean hasHandlerSpan(ServerEndpoint endpoint) {
false
}
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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())
trask marked this conversation as resolved.
Show resolved Hide resolved
expect:
junitTest.requestWithNonStandardHttpMethod()
}
Expand Down
Loading
Loading