Fix http pipelining test on Netty 4.1 (#8412)

This commit is contained in:
Lauri Tulmin 2023-05-05 13:45:46 +03:00 committed by GitHub
parent 83a4054fcc
commit 0ecd1468f3
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 18 additions and 33 deletions

View File

@ -55,7 +55,6 @@ class FinatraServerLatestTest extends AbstractHttpServerTest[HttpServer] {
override def test(endpoint: ServerEndpoint): Boolean = override def test(endpoint: ServerEndpoint): Boolean =
endpoint != ServerEndpoint.NOT_FOUND endpoint != ServerEndpoint.NOT_FOUND
}) })
options.setTestHttpPipelining(false)
} }
override protected def assertHandlerSpan( override protected def assertHandlerSpan(

View File

@ -56,7 +56,6 @@ class FinatraServerTest extends AbstractHttpServerTest[HttpServer] {
override def test(endpoint: ServerEndpoint): Boolean = override def test(endpoint: ServerEndpoint): Boolean =
endpoint != ServerEndpoint.NOT_FOUND endpoint != ServerEndpoint.NOT_FOUND
}) })
options.setTestHttpPipelining(false)
} }
override protected def assertHandlerSpan( override protected def assertHandlerSpan(

View File

@ -7,7 +7,6 @@ package io.opentelemetry.instrumentation.netty.v4_1.internal.server;
import static io.opentelemetry.instrumentation.netty.v4_1.internal.server.HttpServerRequestTracingHandler.HTTP_SERVER_REQUEST; import static io.opentelemetry.instrumentation.netty.v4_1.internal.server.HttpServerRequestTracingHandler.HTTP_SERVER_REQUEST;
import io.netty.channel.Channel;
import io.netty.channel.ChannelFuture; import io.netty.channel.ChannelFuture;
import io.netty.channel.ChannelHandlerContext; import io.netty.channel.ChannelHandlerContext;
import io.netty.channel.ChannelOutboundHandlerAdapter; import io.netty.channel.ChannelOutboundHandlerAdapter;
@ -69,18 +68,18 @@ public class HttpServerResponseTracingHandler extends ChannelOutboundHandlerAdap
if (msg instanceof FullHttpResponse) { if (msg instanceof FullHttpResponse) {
// Headers and body all sent together, we have the response information in the msg. // Headers and body all sent together, we have the response information in the msg.
beforeCommitHandler.handle(context, (HttpResponse) msg); beforeCommitHandler.handle(context, (HttpResponse) msg);
contextAttr.set(null);
HttpRequestAndChannel request = ctx.channel().attr(HTTP_SERVER_REQUEST).getAndSet(null);
writePromise.addListener( writePromise.addListener(
future -> end(ctx.channel(), (FullHttpResponse) msg, writePromise)); future -> end(context, request, (FullHttpResponse) msg, writePromise));
} else { } else {
// Body sent after headers. We stored the response information in the context when // Body sent after headers. We stored the response information in the context when
// encountering HttpResponse (which was not FullHttpResponse since it's not // encountering HttpResponse (which was not FullHttpResponse since it's not
// LastHttpContent). // LastHttpContent).
writePromise.addListener( contextAttr.set(null);
future -> HttpRequestAndChannel request = ctx.channel().attr(HTTP_SERVER_REQUEST).getAndSet(null);
end( HttpResponse response = ctx.channel().attr(HTTP_SERVER_RESPONSE).getAndSet(null);
ctx.channel(), writePromise.addListener(future -> end(context, request, response, writePromise));
ctx.channel().attr(HTTP_SERVER_RESPONSE).getAndSet(null),
writePromise));
} }
} else { } else {
writePromise = prm; writePromise = prm;
@ -94,20 +93,24 @@ public class HttpServerResponseTracingHandler extends ChannelOutboundHandlerAdap
try (Scope ignored = context.makeCurrent()) { try (Scope ignored = context.makeCurrent()) {
super.write(ctx, msg, writePromise); super.write(ctx, msg, writePromise);
} catch (Throwable throwable) { } catch (Throwable throwable) {
end(ctx.channel(), null, throwable); contextAttr.set(null);
HttpRequestAndChannel request = ctx.channel().attr(HTTP_SERVER_REQUEST).getAndSet(null);
end(context, request, null, throwable);
throw throwable; throw throwable;
} }
} }
private void end(Channel channel, HttpResponse response, ChannelFuture future) { private void end(
Context context, HttpRequestAndChannel request, HttpResponse response, ChannelFuture future) {
Throwable error = future.isSuccess() ? null : future.cause(); Throwable error = future.isSuccess() ? null : future.cause();
end(channel, response, error); end(context, request, response, error);
} }
// make sure to remove the server context on end() call private void end(
private void end(Channel channel, @Nullable HttpResponse response, @Nullable Throwable error) { Context context,
Context context = channel.attr(AttributeKeys.SERVER_CONTEXT).getAndSet(null); HttpRequestAndChannel request,
HttpRequestAndChannel request = channel.attr(HTTP_SERVER_REQUEST).getAndSet(null); @Nullable HttpResponse response,
@Nullable Throwable error) {
error = NettyErrorHolder.getOrDefault(context, error); error = NettyErrorHolder.getOrDefault(context, error);
instrumenter.end(context, request, response, error); instrumenter.end(context, request, response, error);
} }

View File

@ -62,7 +62,6 @@ public abstract class AbstractNetty41ServerTest extends AbstractHttpServerTest<E
unused -> unused ->
Sets.difference( Sets.difference(
DEFAULT_HTTP_ATTRIBUTES, Collections.singleton(SemanticAttributes.HTTP_ROUTE))); DEFAULT_HTTP_ATTRIBUTES, Collections.singleton(SemanticAttributes.HTTP_ROUTE)));
options.setTestHttpPipelining(false);
} }
@Override @Override

View File

@ -68,11 +68,6 @@ class VertxRxHttpServerTest extends HttpServerTest<Vertx> implements AgentTestTr
return false return false
} }
@Override
boolean testHttpPipelining() {
false
}
protected Class<AbstractVerticle> verticle() { protected Class<AbstractVerticle> verticle() {
return VertxReactiveWebServer return VertxReactiveWebServer
} }

View File

@ -62,11 +62,6 @@ class VertxRxHttpServerTest extends HttpServerTest<Vertx> implements AgentTestTr
return true return true
} }
@Override
boolean testHttpPipelining() {
false
}
@Override @Override
boolean verifyServerSpanEndTime() { boolean verifyServerSpanEndTime() {
// server spans are ended inside of the controller spans // server spans are ended inside of the controller spans

View File

@ -13,9 +13,4 @@ class VertxLatestHttpServerTest extends AbstractVertxHttpServerTest {
protected Class<? extends AbstractVerticle> verticle() { protected Class<? extends AbstractVerticle> verticle() {
return VertxLatestWebServer return VertxLatestWebServer
} }
@Override
boolean testHttpPipelining() {
false
}
} }