From 079e0faa2d6da8a528ee6ed7eea0f19b8b47b157 Mon Sep 17 00:00:00 2001 From: Ago Allikmaa Date: Thu, 23 Mar 2023 12:11:24 +0200 Subject: [PATCH] Add HttpServerResponseCustomizer support for Netty (#8094) Add `HttpServerResponseCustomizer` support for Netty 3.8, 4.0 and 4.1 instrumentations and enable testing for it in their respective `HttpServerTest` tests. --- .../HttpServerResponseTracingHandler.java | 11 ++++++++ .../v3_8/server/NettyHttpResponseMutator.java | 20 +++++++++++++ .../netty/v3_8/server/Netty38ServerTest.java | 1 + .../HttpServerResponseTracingHandler.java | 11 ++++++++ .../v4_0/server/NettyHttpResponseMutator.java | 20 +++++++++++++ .../src/test/groovy/Netty40ServerTest.groovy | 5 ++++ .../NettyChannelPipelineInstrumentation.java | 10 +++++-- ...HttpServerResponseBeforeCommitHandler.java | 22 +++++++++++++++ .../v4_1/NettyHttpServerResponseMutator.java | 18 ++++++++++++ .../netty/v4_1/Netty41ServerTest.java | 7 +++++ .../netty/v4_1/NettyServerTelemetry.java | 7 +++-- ...HttpServerResponseBeforeCommitHandler.java | 28 +++++++++++++++++++ .../HttpServerResponseTracingHandler.java | 7 ++++- .../server/HttpServerTracingHandler.java | 6 ++-- 14 files changed, 166 insertions(+), 7 deletions(-) create mode 100644 instrumentation/netty/netty-3.8/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/v3_8/server/NettyHttpResponseMutator.java create mode 100644 instrumentation/netty/netty-4.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/v4_0/server/NettyHttpResponseMutator.java create mode 100644 instrumentation/netty/netty-4.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/v4_1/NettyHttpServerResponseBeforeCommitHandler.java create mode 100644 instrumentation/netty/netty-4.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/v4_1/NettyHttpServerResponseMutator.java create mode 100644 instrumentation/netty/netty-4.1/library/src/main/java/io/opentelemetry/instrumentation/netty/v4_1/internal/server/HttpServerResponseBeforeCommitHandler.java diff --git a/instrumentation/netty/netty-3.8/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/v3_8/server/HttpServerResponseTracingHandler.java b/instrumentation/netty/netty-3.8/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/v3_8/server/HttpServerResponseTracingHandler.java index cd371a53e9..7de9910e9d 100644 --- a/instrumentation/netty/netty-3.8/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/v3_8/server/HttpServerResponseTracingHandler.java +++ b/instrumentation/netty/netty-3.8/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/v3_8/server/HttpServerResponseTracingHandler.java @@ -11,6 +11,7 @@ import io.opentelemetry.context.Context; import io.opentelemetry.context.Scope; import io.opentelemetry.instrumentation.api.util.VirtualField; import io.opentelemetry.instrumentation.netty.common.internal.NettyErrorHolder; +import io.opentelemetry.javaagent.bootstrap.http.HttpServerResponseCustomizerHolder; import io.opentelemetry.javaagent.instrumentation.netty.v3_8.HttpRequestAndChannel; import org.jboss.netty.channel.Channel; import org.jboss.netty.channel.ChannelHandlerContext; @@ -38,6 +39,7 @@ public class HttpServerResponseTracingHandler extends SimpleChannelDownstreamHan Throwable error = null; try (Scope ignored = context.makeCurrent()) { + customizeResponse(context, response); super.writeRequested(ctx, msg); } catch (Throwable t) { error = t; @@ -47,4 +49,13 @@ public class HttpServerResponseTracingHandler extends SimpleChannelDownstreamHan instrumenter().end(context, request, response, error); } } + + private static void customizeResponse(Context context, HttpResponse response) { + try { + HttpServerResponseCustomizerHolder.getCustomizer() + .customize(context, response, NettyHttpResponseMutator.INSTANCE); + } catch (Throwable ignore) { + // Ignore. + } + } } diff --git a/instrumentation/netty/netty-3.8/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/v3_8/server/NettyHttpResponseMutator.java b/instrumentation/netty/netty-3.8/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/v3_8/server/NettyHttpResponseMutator.java new file mode 100644 index 0000000000..760b11cc8c --- /dev/null +++ b/instrumentation/netty/netty-3.8/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/v3_8/server/NettyHttpResponseMutator.java @@ -0,0 +1,20 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.javaagent.instrumentation.netty.v3_8.server; + +import io.opentelemetry.javaagent.bootstrap.http.HttpServerResponseMutator; +import org.jboss.netty.handler.codec.http.HttpResponse; + +public enum NettyHttpResponseMutator implements HttpServerResponseMutator { + INSTANCE; + + NettyHttpResponseMutator() {} + + @Override + public void appendHeader(HttpResponse response, String name, String value) { + response.headers().add(name, value); + } +} diff --git a/instrumentation/netty/netty-3.8/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/netty/v3_8/server/Netty38ServerTest.java b/instrumentation/netty/netty-3.8/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/netty/v3_8/server/Netty38ServerTest.java index 5e6a43d711..21241d4ba2 100644 --- a/instrumentation/netty/netty-3.8/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/netty/v3_8/server/Netty38ServerTest.java +++ b/instrumentation/netty/netty-3.8/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/netty/v3_8/server/Netty38ServerTest.java @@ -89,6 +89,7 @@ class Netty38ServerTest extends AbstractHttpServerTest { }); options.setExpectedException(new IllegalArgumentException(ServerEndpoint.EXCEPTION.getBody())); + options.setHasResponseCustomizer(serverEndpoint -> true); } private static ChannelPipeline channelPipeline() { diff --git a/instrumentation/netty/netty-4.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/v4_0/server/HttpServerResponseTracingHandler.java b/instrumentation/netty/netty-4.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/v4_0/server/HttpServerResponseTracingHandler.java index 4921df0d86..9388c98a25 100644 --- a/instrumentation/netty/netty-4.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/v4_0/server/HttpServerResponseTracingHandler.java +++ b/instrumentation/netty/netty-4.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/v4_0/server/HttpServerResponseTracingHandler.java @@ -17,6 +17,7 @@ import io.opentelemetry.context.Context; import io.opentelemetry.context.Scope; import io.opentelemetry.instrumentation.netty.common.internal.NettyErrorHolder; import io.opentelemetry.instrumentation.netty.v4.common.HttpRequestAndChannel; +import io.opentelemetry.javaagent.bootstrap.http.HttpServerResponseCustomizerHolder; import io.opentelemetry.javaagent.instrumentation.netty.v4_0.AttributeKeys; import javax.annotation.Nullable; @@ -31,6 +32,7 @@ public class HttpServerResponseTracingHandler extends ChannelOutboundHandlerAdap } try (Scope ignored = context.makeCurrent()) { + customizeResponse(context, (HttpResponse) msg); ctx.write(msg, prm); end(ctx.channel(), (HttpResponse) msg, null); } catch (Throwable throwable) { @@ -46,4 +48,13 @@ public class HttpServerResponseTracingHandler extends ChannelOutboundHandlerAdap error = NettyErrorHolder.getOrDefault(context, error); instrumenter().end(context, request, response, error); } + + private static void customizeResponse(Context context, HttpResponse response) { + try { + HttpServerResponseCustomizerHolder.getCustomizer() + .customize(context, response, NettyHttpResponseMutator.INSTANCE); + } catch (Throwable ignore) { + // Ignore. + } + } } diff --git a/instrumentation/netty/netty-4.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/v4_0/server/NettyHttpResponseMutator.java b/instrumentation/netty/netty-4.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/v4_0/server/NettyHttpResponseMutator.java new file mode 100644 index 0000000000..e1f3c4993b --- /dev/null +++ b/instrumentation/netty/netty-4.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/v4_0/server/NettyHttpResponseMutator.java @@ -0,0 +1,20 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.javaagent.instrumentation.netty.v4_0.server; + +import io.netty.handler.codec.http.HttpResponse; +import io.opentelemetry.javaagent.bootstrap.http.HttpServerResponseMutator; + +public enum NettyHttpResponseMutator implements HttpServerResponseMutator { + INSTANCE; + + NettyHttpResponseMutator() {} + + @Override + public void appendHeader(HttpResponse response, String name, String value) { + response.headers().add(name, value); + } +} diff --git a/instrumentation/netty/netty-4.0/javaagent/src/test/groovy/Netty40ServerTest.groovy b/instrumentation/netty/netty-4.0/javaagent/src/test/groovy/Netty40ServerTest.groovy index af147f5415..25e112391a 100644 --- a/instrumentation/netty/netty-4.0/javaagent/src/test/groovy/Netty40ServerTest.groovy +++ b/instrumentation/netty/netty-4.0/javaagent/src/test/groovy/Netty40ServerTest.groovy @@ -47,6 +47,11 @@ class Netty40ServerTest extends HttpServerTest implements AgentT static final LoggingHandler LOGGING_HANDLER = new LoggingHandler(SERVER_LOGGER.name, LogLevel.DEBUG) + @Override + boolean hasResponseCustomizer(ServerEndpoint endpoint) { + true + } + @Override EventLoopGroup startServer(int port) { def eventLoopGroup = new NioEventLoopGroup() diff --git a/instrumentation/netty/netty-4.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/v4_1/NettyChannelPipelineInstrumentation.java b/instrumentation/netty/netty-4.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/v4_1/NettyChannelPipelineInstrumentation.java index 73ff517ddc..e410a9a351 100644 --- a/instrumentation/netty/netty-4.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/v4_1/NettyChannelPipelineInstrumentation.java +++ b/instrumentation/netty/netty-4.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/v4_1/NettyChannelPipelineInstrumentation.java @@ -105,11 +105,17 @@ public class NettyChannelPipelineInstrumentation ChannelHandler ourHandler = null; // Server pipeline handlers if (handler instanceof HttpServerCodec) { - ourHandler = new HttpServerTracingHandler(NettyServerSingletons.instrumenter()); + ourHandler = + new HttpServerTracingHandler( + NettyServerSingletons.instrumenter(), + NettyHttpServerResponseBeforeCommitHandler.INSTANCE); } else if (handler instanceof HttpRequestDecoder) { ourHandler = new HttpServerRequestTracingHandler(NettyServerSingletons.instrumenter()); } else if (handler instanceof HttpResponseEncoder) { - ourHandler = new HttpServerResponseTracingHandler(NettyServerSingletons.instrumenter()); + ourHandler = + new HttpServerResponseTracingHandler( + NettyServerSingletons.instrumenter(), + NettyHttpServerResponseBeforeCommitHandler.INSTANCE); // Client pipeline handlers } else if (handler instanceof HttpClientCodec) { ourHandler = new HttpClientTracingHandler(NettyClientSingletons.instrumenter()); diff --git a/instrumentation/netty/netty-4.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/v4_1/NettyHttpServerResponseBeforeCommitHandler.java b/instrumentation/netty/netty-4.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/v4_1/NettyHttpServerResponseBeforeCommitHandler.java new file mode 100644 index 0000000000..db1e4c9ecb --- /dev/null +++ b/instrumentation/netty/netty-4.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/v4_1/NettyHttpServerResponseBeforeCommitHandler.java @@ -0,0 +1,22 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.javaagent.instrumentation.netty.v4_1; + +import io.netty.handler.codec.http.HttpResponse; +import io.opentelemetry.context.Context; +import io.opentelemetry.instrumentation.netty.v4_1.internal.server.HttpServerResponseBeforeCommitHandler; +import io.opentelemetry.javaagent.bootstrap.http.HttpServerResponseCustomizerHolder; + +public enum NettyHttpServerResponseBeforeCommitHandler + implements HttpServerResponseBeforeCommitHandler { + INSTANCE; + + @Override + public void handle(Context context, HttpResponse response) { + HttpServerResponseCustomizerHolder.getCustomizer() + .customize(context, response, NettyHttpServerResponseMutator.INSTANCE); + } +} diff --git a/instrumentation/netty/netty-4.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/v4_1/NettyHttpServerResponseMutator.java b/instrumentation/netty/netty-4.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/v4_1/NettyHttpServerResponseMutator.java new file mode 100644 index 0000000000..4631a864c1 --- /dev/null +++ b/instrumentation/netty/netty-4.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/v4_1/NettyHttpServerResponseMutator.java @@ -0,0 +1,18 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.javaagent.instrumentation.netty.v4_1; + +import io.netty.handler.codec.http.HttpResponse; +import io.opentelemetry.javaagent.bootstrap.http.HttpServerResponseMutator; + +public enum NettyHttpServerResponseMutator implements HttpServerResponseMutator { + INSTANCE; + + @Override + public void appendHeader(HttpResponse response, String name, String value) { + response.headers().add(name, value); + } +} diff --git a/instrumentation/netty/netty-4.1/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/netty/v4_1/Netty41ServerTest.java b/instrumentation/netty/netty-4.1/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/netty/v4_1/Netty41ServerTest.java index 0bed30e1fc..a800f9ebc9 100644 --- a/instrumentation/netty/netty-4.1/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/netty/v4_1/Netty41ServerTest.java +++ b/instrumentation/netty/netty-4.1/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/netty/v4_1/Netty41ServerTest.java @@ -9,6 +9,7 @@ import io.netty.channel.ChannelPipeline; import io.opentelemetry.instrumentation.netty.v4_1.AbstractNetty41ServerTest; import io.opentelemetry.instrumentation.testing.junit.InstrumentationExtension; import io.opentelemetry.instrumentation.testing.junit.http.HttpServerInstrumentationExtension; +import io.opentelemetry.instrumentation.testing.junit.http.HttpServerTestOptions; import org.junit.jupiter.api.extension.RegisterExtension; public class Netty41ServerTest extends AbstractNetty41ServerTest { @@ -16,6 +17,12 @@ public class Netty41ServerTest extends AbstractNetty41ServerTest { @RegisterExtension static final InstrumentationExtension testing = HttpServerInstrumentationExtension.forAgent(); + @Override + protected void configure(HttpServerTestOptions options) { + super.configure(options); + options.setHasResponseCustomizer(serverEndpoint -> true); + } + @Override protected void configurePipeline(ChannelPipeline channelPipeline) {} } diff --git a/instrumentation/netty/netty-4.1/library/src/main/java/io/opentelemetry/instrumentation/netty/v4_1/NettyServerTelemetry.java b/instrumentation/netty/netty-4.1/library/src/main/java/io/opentelemetry/instrumentation/netty/v4_1/NettyServerTelemetry.java index f78abcb67a..1160ff1ca4 100644 --- a/instrumentation/netty/netty-4.1/library/src/main/java/io/opentelemetry/instrumentation/netty/v4_1/NettyServerTelemetry.java +++ b/instrumentation/netty/netty-4.1/library/src/main/java/io/opentelemetry/instrumentation/netty/v4_1/NettyServerTelemetry.java @@ -13,6 +13,7 @@ import io.opentelemetry.api.OpenTelemetry; import io.opentelemetry.instrumentation.api.instrumenter.Instrumenter; import io.opentelemetry.instrumentation.netty.v4.common.HttpRequestAndChannel; import io.opentelemetry.instrumentation.netty.v4_1.internal.server.HttpServerRequestTracingHandler; +import io.opentelemetry.instrumentation.netty.v4_1.internal.server.HttpServerResponseBeforeCommitHandler; import io.opentelemetry.instrumentation.netty.v4_1.internal.server.HttpServerResponseTracingHandler; import io.opentelemetry.instrumentation.netty.v4_1.internal.server.HttpServerTracingHandler; @@ -51,7 +52,8 @@ public final class NettyServerTelemetry { * responses. Must be paired with {@link #createRequestHandler()}. */ public ChannelOutboundHandlerAdapter createResponseHandler() { - return new HttpServerResponseTracingHandler(instrumenter); + return new HttpServerResponseTracingHandler( + instrumenter, HttpServerResponseBeforeCommitHandler.Noop.INSTANCE); } /** @@ -61,6 +63,7 @@ public final class NettyServerTelemetry { public CombinedChannelDuplexHandler< ? extends ChannelInboundHandlerAdapter, ? extends ChannelOutboundHandlerAdapter> createCombinedHandler() { - return new HttpServerTracingHandler(instrumenter); + return new HttpServerTracingHandler( + instrumenter, HttpServerResponseBeforeCommitHandler.Noop.INSTANCE); } } diff --git a/instrumentation/netty/netty-4.1/library/src/main/java/io/opentelemetry/instrumentation/netty/v4_1/internal/server/HttpServerResponseBeforeCommitHandler.java b/instrumentation/netty/netty-4.1/library/src/main/java/io/opentelemetry/instrumentation/netty/v4_1/internal/server/HttpServerResponseBeforeCommitHandler.java new file mode 100644 index 0000000000..2225bb732b --- /dev/null +++ b/instrumentation/netty/netty-4.1/library/src/main/java/io/opentelemetry/instrumentation/netty/v4_1/internal/server/HttpServerResponseBeforeCommitHandler.java @@ -0,0 +1,28 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.instrumentation.netty.v4_1.internal.server; + +import io.netty.handler.codec.http.HttpResponse; +import io.opentelemetry.context.Context; + +/** + * This class is internal and is hence not for public use. Its APIs are unstable and can change at + * any time. + */ +public interface HttpServerResponseBeforeCommitHandler { + void handle(Context context, HttpResponse response); + + /** + * This class is internal and is hence not for public use. Its APIs are unstable and can change at + * any time. + */ + enum Noop implements HttpServerResponseBeforeCommitHandler { + INSTANCE; + + @Override + public void handle(Context context, HttpResponse response) {} + } +} diff --git a/instrumentation/netty/netty-4.1/library/src/main/java/io/opentelemetry/instrumentation/netty/v4_1/internal/server/HttpServerResponseTracingHandler.java b/instrumentation/netty/netty-4.1/library/src/main/java/io/opentelemetry/instrumentation/netty/v4_1/internal/server/HttpServerResponseTracingHandler.java index 3805ac8878..c9a0860434 100644 --- a/instrumentation/netty/netty-4.1/library/src/main/java/io/opentelemetry/instrumentation/netty/v4_1/internal/server/HttpServerResponseTracingHandler.java +++ b/instrumentation/netty/netty-4.1/library/src/main/java/io/opentelemetry/instrumentation/netty/v4_1/internal/server/HttpServerResponseTracingHandler.java @@ -35,10 +35,13 @@ public class HttpServerResponseTracingHandler extends ChannelOutboundHandlerAdap AttributeKey.valueOf(HttpServerResponseTracingHandler.class, "http-server-response"); private final Instrumenter instrumenter; + private final HttpServerResponseBeforeCommitHandler beforeCommitHandler; public HttpServerResponseTracingHandler( - Instrumenter instrumenter) { + Instrumenter instrumenter, + HttpServerResponseBeforeCommitHandler beforeCommitHandler) { this.instrumenter = instrumenter; + this.beforeCommitHandler = beforeCommitHandler; } @Override @@ -65,6 +68,7 @@ public class HttpServerResponseTracingHandler extends ChannelOutboundHandlerAdap // Going to finish the span after the write of the last content finishes. if (msg instanceof FullHttpResponse) { // Headers and body all sent together, we have the response information in the msg. + beforeCommitHandler.handle(context, (HttpResponse) msg); writePromise.addListener( future -> end(ctx.channel(), (FullHttpResponse) msg, writePromise)); } else { @@ -82,6 +86,7 @@ public class HttpServerResponseTracingHandler extends ChannelOutboundHandlerAdap writePromise = prm; if (msg instanceof HttpResponse) { // Headers before body has been sent, store them to use when finishing the span. + beforeCommitHandler.handle(context, (HttpResponse) msg); ctx.channel().attr(HTTP_SERVER_RESPONSE).set((HttpResponse) msg); } } diff --git a/instrumentation/netty/netty-4.1/library/src/main/java/io/opentelemetry/instrumentation/netty/v4_1/internal/server/HttpServerTracingHandler.java b/instrumentation/netty/netty-4.1/library/src/main/java/io/opentelemetry/instrumentation/netty/v4_1/internal/server/HttpServerTracingHandler.java index 817e3b08c6..0cf9484baf 100644 --- a/instrumentation/netty/netty-4.1/library/src/main/java/io/opentelemetry/instrumentation/netty/v4_1/internal/server/HttpServerTracingHandler.java +++ b/instrumentation/netty/netty-4.1/library/src/main/java/io/opentelemetry/instrumentation/netty/v4_1/internal/server/HttpServerTracingHandler.java @@ -18,9 +18,11 @@ public class HttpServerTracingHandler extends CombinedChannelDuplexHandler< HttpServerRequestTracingHandler, HttpServerResponseTracingHandler> { - public HttpServerTracingHandler(Instrumenter instrumenter) { + public HttpServerTracingHandler( + Instrumenter instrumenter, + HttpServerResponseBeforeCommitHandler responseBeforeCommitHandler) { super( new HttpServerRequestTracingHandler(instrumenter), - new HttpServerResponseTracingHandler(instrumenter)); + new HttpServerResponseTracingHandler(instrumenter, responseBeforeCommitHandler)); } }