Use super to make netty instrumentation clearer (#7736)

I think it makes it clearer that these classes are decorating the super
methods.
This commit is contained in:
Trask Stalnaker 2023-02-06 08:15:53 -08:00 committed by GitHub
parent 9979d8f620
commit e7820f8e9e
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
12 changed files with 53 additions and 51 deletions

View File

@ -26,10 +26,10 @@ public class HttpClientRequestTracingHandler extends SimpleChannelDownstreamHand
VirtualField.find(Channel.class, NettyClientRequestAndContexts.class); VirtualField.find(Channel.class, NettyClientRequestAndContexts.class);
@Override @Override
public void writeRequested(ChannelHandlerContext ctx, MessageEvent event) { public void writeRequested(ChannelHandlerContext ctx, MessageEvent event) throws Exception {
Object message = event.getMessage(); Object message = event.getMessage();
if (!(message instanceof HttpRequest)) { if (!(message instanceof HttpRequest)) {
ctx.sendDownstream(event); super.writeRequested(ctx, event);
return; return;
} }
@ -45,7 +45,7 @@ public class HttpClientRequestTracingHandler extends SimpleChannelDownstreamHand
HttpRequestAndChannel request = HttpRequestAndChannel request =
HttpRequestAndChannel.create((HttpRequest) message, ctx.getChannel()); HttpRequestAndChannel.create((HttpRequest) message, ctx.getChannel());
if (!instrumenter().shouldStart(parentContext, request)) { if (!instrumenter().shouldStart(parentContext, request)) {
ctx.sendDownstream(event); super.writeRequested(ctx, event);
return; return;
} }
@ -54,7 +54,7 @@ public class HttpClientRequestTracingHandler extends SimpleChannelDownstreamHand
ctx.getChannel(), NettyClientRequestAndContexts.create(parentContext, context, request)); ctx.getChannel(), NettyClientRequestAndContexts.create(parentContext, context, request));
try (Scope ignored = context.makeCurrent()) { try (Scope ignored = context.makeCurrent()) {
ctx.sendDownstream(event); super.writeRequested(ctx, event);
// span is ended normally in HttpClientResponseTracingHandler // span is ended normally in HttpClientResponseTracingHandler
} catch (Throwable throwable) { } catch (Throwable throwable) {
instrumenter().end(context, request, null, throwable); instrumenter().end(context, request, null, throwable);

View File

@ -22,11 +22,11 @@ public class HttpClientResponseTracingHandler extends SimpleChannelUpstreamHandl
VirtualField.find(Channel.class, NettyClientRequestAndContexts.class); VirtualField.find(Channel.class, NettyClientRequestAndContexts.class);
@Override @Override
public void messageReceived(ChannelHandlerContext ctx, MessageEvent msg) { public void messageReceived(ChannelHandlerContext ctx, MessageEvent msg) throws Exception {
NettyClientRequestAndContexts requestAndContexts = requestContextsField.get(ctx.getChannel()); NettyClientRequestAndContexts requestAndContexts = requestContextsField.get(ctx.getChannel());
if (requestAndContexts == null) { if (requestAndContexts == null) {
ctx.sendUpstream(msg); super.messageReceived(ctx, msg);
return; return;
} }
@ -42,7 +42,7 @@ public class HttpClientResponseTracingHandler extends SimpleChannelUpstreamHandl
// We want the callback in the scope of the parent, not the client span // We want the callback in the scope of the parent, not the client span
try (Scope ignored = requestAndContexts.parentContext().makeCurrent()) { try (Scope ignored = requestAndContexts.parentContext().makeCurrent()) {
ctx.sendUpstream(msg); super.messageReceived(ctx, msg);
} }
} }
} }

View File

@ -23,15 +23,15 @@ public class HttpServerRequestTracingHandler extends SimpleChannelUpstreamHandle
VirtualField.find(Channel.class, NettyServerRequestAndContext.class); VirtualField.find(Channel.class, NettyServerRequestAndContext.class);
@Override @Override
public void messageReceived(ChannelHandlerContext ctx, MessageEvent event) { public void messageReceived(ChannelHandlerContext ctx, MessageEvent event) throws Exception {
Object message = event.getMessage(); Object message = event.getMessage();
if (!(message instanceof HttpRequest)) { if (!(message instanceof HttpRequest)) {
NettyServerRequestAndContext requestAndContext = requestAndContextField.get(ctx.getChannel()); NettyServerRequestAndContext requestAndContext = requestAndContextField.get(ctx.getChannel());
if (requestAndContext == null) { if (requestAndContext == null) {
ctx.sendUpstream(event); super.messageReceived(ctx, event);
} else { } else {
try (Scope ignored = requestAndContext.context().makeCurrent()) { try (Scope ignored = requestAndContext.context().makeCurrent()) {
ctx.sendUpstream(event); super.messageReceived(ctx, event);
} }
} }
return; return;
@ -41,7 +41,8 @@ public class HttpServerRequestTracingHandler extends SimpleChannelUpstreamHandle
HttpRequestAndChannel request = HttpRequestAndChannel request =
HttpRequestAndChannel.create((HttpRequest) message, ctx.getChannel()); HttpRequestAndChannel.create((HttpRequest) message, ctx.getChannel());
if (!instrumenter().shouldStart(parentContext, request)) { if (!instrumenter().shouldStart(parentContext, request)) {
ctx.sendUpstream(event); super.messageReceived(ctx, event);
return;
} }
Context context = instrumenter().start(parentContext, request); Context context = instrumenter().start(parentContext, request);
@ -49,7 +50,7 @@ public class HttpServerRequestTracingHandler extends SimpleChannelUpstreamHandle
ctx.getChannel(), NettyServerRequestAndContext.create(request, context)); ctx.getChannel(), NettyServerRequestAndContext.create(request, context));
try (Scope ignored = context.makeCurrent()) { try (Scope ignored = context.makeCurrent()) {
ctx.sendUpstream(event); super.messageReceived(ctx, event);
// the span is ended normally in HttpServerResponseTracingHandler // the span is ended normally in HttpServerResponseTracingHandler
} catch (Throwable throwable) { } catch (Throwable throwable) {
instrumenter().end(context, request, null, throwable); instrumenter().end(context, request, null, throwable);

View File

@ -24,11 +24,11 @@ public class HttpServerResponseTracingHandler extends SimpleChannelDownstreamHan
VirtualField.find(Channel.class, NettyServerRequestAndContext.class); VirtualField.find(Channel.class, NettyServerRequestAndContext.class);
@Override @Override
public void writeRequested(ChannelHandlerContext ctx, MessageEvent msg) { public void writeRequested(ChannelHandlerContext ctx, MessageEvent msg) throws Exception {
NettyServerRequestAndContext requestAndContext = requestAndContextField.get(ctx.getChannel()); NettyServerRequestAndContext requestAndContext = requestAndContextField.get(ctx.getChannel());
if (requestAndContext == null || !(msg.getMessage() instanceof HttpResponse)) { if (requestAndContext == null || !(msg.getMessage() instanceof HttpResponse)) {
ctx.sendDownstream(msg); super.writeRequested(ctx, msg);
return; return;
} }
@ -38,7 +38,7 @@ public class HttpServerResponseTracingHandler extends SimpleChannelDownstreamHan
Throwable error = null; Throwable error = null;
try (Scope ignored = context.makeCurrent()) { try (Scope ignored = context.makeCurrent()) {
ctx.sendDownstream(msg); super.writeRequested(ctx, msg);
} catch (Throwable t) { } catch (Throwable t) {
error = t; error = t;
throw t; throw t;

View File

@ -64,20 +64,20 @@ public final class NettySslInstrumentationHandler extends ChannelDuplexHandler {
} }
@Override @Override
public void channelRegistered(ChannelHandlerContext ctx) { public void channelRegistered(ChannelHandlerContext ctx) throws Exception {
// this should never happen at this point (since the handler is only registered when SSL classes // this should never happen at this point (since the handler is only registered when SSL classes
// are on classpath); checking just to be extra safe // are on classpath); checking just to be extra safe
if (SSL_HANDSHAKE_COMPLETION_EVENT == null) { if (SSL_HANDSHAKE_COMPLETION_EVENT == null) {
ctx.pipeline().remove(this); ctx.pipeline().remove(this);
instrumentationHandlerField.set(realHandler, null); instrumentationHandlerField.set(realHandler, null);
ctx.fireChannelRegistered(); super.channelRegistered(ctx);
return; return;
} }
// remember the parent context from the time of channel registration; // remember the parent context from the time of channel registration;
// this happens inside Bootstrap#connect() // this happens inside Bootstrap#connect()
parentContext = Context.current(); parentContext = Context.current();
ctx.fireChannelRegistered(); super.channelRegistered(ctx);
} }
@Override @Override
@ -85,7 +85,8 @@ public final class NettySslInstrumentationHandler extends ChannelDuplexHandler {
ChannelHandlerContext ctx, ChannelHandlerContext ctx,
SocketAddress remoteAddress, SocketAddress remoteAddress,
SocketAddress localAddress, SocketAddress localAddress,
ChannelPromise promise) { ChannelPromise promise)
throws Exception {
// netty SslHandler starts the handshake after it receives the channelActive() signal; this // netty SslHandler starts the handshake after it receives the channelActive() signal; this
// happens just after the connection is established // happens just after the connection is established
@ -101,11 +102,11 @@ public final class NettySslInstrumentationHandler extends ChannelDuplexHandler {
context = instrumenter.start(parentContext, request); context = instrumenter.start(parentContext, request);
} }
}); });
ctx.connect(remoteAddress, localAddress, promise); super.connect(ctx, remoteAddress, localAddress, promise);
} }
@Override @Override
public void userEventTriggered(ChannelHandlerContext ctx, Object evt) { public void userEventTriggered(ChannelHandlerContext ctx, Object evt) throws Exception {
if (SSL_HANDSHAKE_COMPLETION_EVENT.isInstance(evt)) { if (SSL_HANDSHAKE_COMPLETION_EVENT.isInstance(evt)) {
if (ctx.pipeline().context(this) != null) { if (ctx.pipeline().context(this) != null) {
ctx.pipeline().remove(this); ctx.pipeline().remove(this);
@ -117,7 +118,7 @@ public final class NettySslInstrumentationHandler extends ChannelDuplexHandler {
} }
} }
ctx.fireUserEventTriggered(evt); super.userEventTriggered(ctx, evt);
} }
private static Throwable getCause(Object evt) { private static Throwable getCause(Object evt) {

View File

@ -20,9 +20,9 @@ import io.opentelemetry.javaagent.instrumentation.netty.v4_0.AttributeKeys;
public class HttpClientRequestTracingHandler extends ChannelOutboundHandlerAdapter { public class HttpClientRequestTracingHandler extends ChannelOutboundHandlerAdapter {
@Override @Override
public void write(ChannelHandlerContext ctx, Object msg, ChannelPromise prm) { public void write(ChannelHandlerContext ctx, Object msg, ChannelPromise prm) throws Exception {
if (!(msg instanceof HttpRequest)) { if (!(msg instanceof HttpRequest)) {
ctx.write(msg, prm); super.write(ctx, msg, prm);
return; return;
} }
@ -33,7 +33,7 @@ public class HttpClientRequestTracingHandler extends ChannelOutboundHandlerAdapt
HttpRequestAndChannel request = HttpRequestAndChannel.create((HttpRequest) msg, ctx.channel()); HttpRequestAndChannel request = HttpRequestAndChannel.create((HttpRequest) msg, ctx.channel());
if (!instrumenter().shouldStart(parentContext, request)) { if (!instrumenter().shouldStart(parentContext, request)) {
ctx.write(msg, prm); super.write(ctx, msg, prm);
return; return;
} }
@ -47,7 +47,7 @@ public class HttpClientRequestTracingHandler extends ChannelOutboundHandlerAdapt
requestAttr.set(request); requestAttr.set(request);
try (Scope ignored = context.makeCurrent()) { try (Scope ignored = context.makeCurrent()) {
ctx.write(msg, prm); super.write(ctx, msg, prm);
// span is ended normally in HttpClientResponseTracingHandler // span is ended normally in HttpClientResponseTracingHandler
} catch (Throwable throwable) { } catch (Throwable throwable) {
instrumenter().end(contextAttr.getAndRemove(), requestAttr.getAndRemove(), null, throwable); instrumenter().end(contextAttr.getAndRemove(), requestAttr.getAndRemove(), null, throwable);

View File

@ -21,11 +21,11 @@ import io.opentelemetry.javaagent.instrumentation.netty.v4_0.AttributeKeys;
public class HttpClientResponseTracingHandler extends ChannelInboundHandlerAdapter { public class HttpClientResponseTracingHandler extends ChannelInboundHandlerAdapter {
@Override @Override
public void channelRead(ChannelHandlerContext ctx, Object msg) { public void channelRead(ChannelHandlerContext ctx, Object msg) throws Exception {
Attribute<Context> contextAttr = ctx.channel().attr(AttributeKeys.CLIENT_CONTEXT); Attribute<Context> contextAttr = ctx.channel().attr(AttributeKeys.CLIENT_CONTEXT);
Context context = contextAttr.get(); Context context = contextAttr.get();
if (context == null) { if (context == null) {
ctx.fireChannelRead(msg); super.channelRead(ctx, msg);
return; return;
} }
@ -53,10 +53,10 @@ public class HttpClientResponseTracingHandler extends ChannelInboundHandlerAdapt
// We want the callback in the scope of the parent, not the client span // We want the callback in the scope of the parent, not the client span
if (parentContext != null) { if (parentContext != null) {
try (Scope ignored = parentContext.makeCurrent()) { try (Scope ignored = parentContext.makeCurrent()) {
ctx.fireChannelRead(msg); super.channelRead(ctx, msg);
} }
} else { } else {
ctx.fireChannelRead(msg); super.channelRead(ctx, msg);
} }
if (msg instanceof FullHttpResponse) { if (msg instanceof FullHttpResponse) {

View File

@ -20,7 +20,7 @@ import io.opentelemetry.javaagent.instrumentation.netty.v4_0.AttributeKeys;
public class HttpServerRequestTracingHandler extends ChannelInboundHandlerAdapter { public class HttpServerRequestTracingHandler extends ChannelInboundHandlerAdapter {
@Override @Override
public void channelRead(ChannelHandlerContext ctx, Object msg) { public void channelRead(ChannelHandlerContext ctx, Object msg) throws Exception {
Channel channel = ctx.channel(); Channel channel = ctx.channel();
Attribute<Context> contextAttr = channel.attr(AttributeKeys.SERVER_CONTEXT); Attribute<Context> contextAttr = channel.attr(AttributeKeys.SERVER_CONTEXT);
Attribute<HttpRequestAndChannel> requestAttr = channel.attr(AttributeKeys.SERVER_REQUEST); Attribute<HttpRequestAndChannel> requestAttr = channel.attr(AttributeKeys.SERVER_REQUEST);
@ -28,10 +28,10 @@ public class HttpServerRequestTracingHandler extends ChannelInboundHandlerAdapte
if (!(msg instanceof HttpRequest)) { if (!(msg instanceof HttpRequest)) {
Context serverContext = contextAttr.get(); Context serverContext = contextAttr.get();
if (serverContext == null) { if (serverContext == null) {
ctx.fireChannelRead(msg); super.channelRead(ctx, msg);
} else { } else {
try (Scope ignored = serverContext.makeCurrent()) { try (Scope ignored = serverContext.makeCurrent()) {
ctx.fireChannelRead(msg); super.channelRead(ctx, msg);
} }
} }
return; return;
@ -44,7 +44,7 @@ public class HttpServerRequestTracingHandler extends ChannelInboundHandlerAdapte
HttpRequestAndChannel request = HttpRequestAndChannel.create((HttpRequest) msg, channel); HttpRequestAndChannel request = HttpRequestAndChannel.create((HttpRequest) msg, channel);
if (!instrumenter().shouldStart(parentContext, request)) { if (!instrumenter().shouldStart(parentContext, request)) {
ctx.fireChannelRead(msg); super.channelRead(ctx, msg);
return; return;
} }
@ -53,7 +53,7 @@ public class HttpServerRequestTracingHandler extends ChannelInboundHandlerAdapte
requestAttr.set(request); requestAttr.set(request);
try (Scope ignored = context.makeCurrent()) { try (Scope ignored = context.makeCurrent()) {
ctx.fireChannelRead(msg); super.channelRead(ctx, msg);
// the span is ended normally in HttpServerResponseTracingHandler // the span is ended normally in HttpServerResponseTracingHandler
} catch (Throwable throwable) { } catch (Throwable throwable) {
// make sure to remove the server context on end() call // make sure to remove the server context on end() call

View File

@ -35,9 +35,9 @@ public class HttpClientRequestTracingHandler extends ChannelOutboundHandlerAdapt
} }
@Override @Override
public void write(ChannelHandlerContext ctx, Object msg, ChannelPromise prm) { public void write(ChannelHandlerContext ctx, Object msg, ChannelPromise prm) throws Exception {
if (!(msg instanceof HttpRequest)) { if (!(msg instanceof HttpRequest)) {
ctx.write(msg, prm); super.write(ctx, msg, prm);
return; return;
} }
@ -48,7 +48,7 @@ public class HttpClientRequestTracingHandler extends ChannelOutboundHandlerAdapt
HttpRequestAndChannel request = HttpRequestAndChannel.create((HttpRequest) msg, ctx.channel()); HttpRequestAndChannel request = HttpRequestAndChannel.create((HttpRequest) msg, ctx.channel());
if (!instrumenter.shouldStart(parentContext, request) || isAwsRequest(request)) { if (!instrumenter.shouldStart(parentContext, request) || isAwsRequest(request)) {
ctx.write(msg, prm); super.write(ctx, msg, prm);
return; return;
} }
@ -62,7 +62,7 @@ public class HttpClientRequestTracingHandler extends ChannelOutboundHandlerAdapt
requestAttr.set(request); requestAttr.set(request);
try (Scope ignored = context.makeCurrent()) { try (Scope ignored = context.makeCurrent()) {
ctx.write(msg, prm); super.write(ctx, msg, prm);
// span is ended normally in HttpClientResponseTracingHandler // span is ended normally in HttpClientResponseTracingHandler
} catch (Throwable throwable) { } catch (Throwable throwable) {
instrumenter.end(contextAttr.getAndSet(null), requestAttr.getAndSet(null), null, throwable); instrumenter.end(contextAttr.getAndSet(null), requestAttr.getAndSet(null), null, throwable);

View File

@ -37,11 +37,11 @@ public class HttpClientResponseTracingHandler extends ChannelInboundHandlerAdapt
} }
@Override @Override
public void channelRead(ChannelHandlerContext ctx, Object msg) { public void channelRead(ChannelHandlerContext ctx, Object msg) throws Exception {
Attribute<Context> contextAttr = ctx.channel().attr(AttributeKeys.CLIENT_CONTEXT); Attribute<Context> contextAttr = ctx.channel().attr(AttributeKeys.CLIENT_CONTEXT);
Context context = contextAttr.get(); Context context = contextAttr.get();
if (context == null) { if (context == null) {
ctx.fireChannelRead(msg); super.channelRead(ctx, msg);
return; return;
} }
@ -69,10 +69,10 @@ public class HttpClientResponseTracingHandler extends ChannelInboundHandlerAdapt
// We want the callback in the scope of the parent, not the client span // We want the callback in the scope of the parent, not the client span
if (parentContext != null) { if (parentContext != null) {
try (Scope ignored = parentContext.makeCurrent()) { try (Scope ignored = parentContext.makeCurrent()) {
ctx.fireChannelRead(msg); super.channelRead(ctx, msg);
} }
} else { } else {
ctx.fireChannelRead(msg); super.channelRead(ctx, msg);
} }
if (msg instanceof FullHttpResponse) { if (msg instanceof FullHttpResponse) {

View File

@ -35,7 +35,7 @@ public class HttpServerRequestTracingHandler extends ChannelInboundHandlerAdapte
} }
@Override @Override
public void channelRead(ChannelHandlerContext ctx, Object msg) { public void channelRead(ChannelHandlerContext ctx, Object msg) throws Exception {
Channel channel = ctx.channel(); Channel channel = ctx.channel();
Attribute<Context> contextAttr = channel.attr(AttributeKeys.SERVER_CONTEXT); Attribute<Context> contextAttr = channel.attr(AttributeKeys.SERVER_CONTEXT);
Attribute<HttpRequestAndChannel> requestAttr = channel.attr(HTTP_REQUEST); Attribute<HttpRequestAndChannel> requestAttr = channel.attr(HTTP_REQUEST);
@ -43,10 +43,10 @@ public class HttpServerRequestTracingHandler extends ChannelInboundHandlerAdapte
if (!(msg instanceof HttpRequest)) { if (!(msg instanceof HttpRequest)) {
Context serverContext = contextAttr.get(); Context serverContext = contextAttr.get();
if (serverContext == null) { if (serverContext == null) {
ctx.fireChannelRead(msg); super.channelRead(ctx, msg);
} else { } else {
try (Scope ignored = serverContext.makeCurrent()) { try (Scope ignored = serverContext.makeCurrent()) {
ctx.fireChannelRead(msg); super.channelRead(ctx, msg);
} }
} }
return; return;
@ -59,7 +59,7 @@ public class HttpServerRequestTracingHandler extends ChannelInboundHandlerAdapte
HttpRequestAndChannel request = HttpRequestAndChannel.create((HttpRequest) msg, channel); HttpRequestAndChannel request = HttpRequestAndChannel.create((HttpRequest) msg, channel);
if (!instrumenter.shouldStart(parentContext, request)) { if (!instrumenter.shouldStart(parentContext, request)) {
ctx.fireChannelRead(msg); super.channelRead(ctx, msg);
return; return;
} }
@ -68,7 +68,7 @@ public class HttpServerRequestTracingHandler extends ChannelInboundHandlerAdapte
requestAttr.set(request); requestAttr.set(request);
try (Scope ignored = context.makeCurrent()) { try (Scope ignored = context.makeCurrent()) {
ctx.fireChannelRead(msg); super.channelRead(ctx, msg);
// the span is ended normally in HttpServerResponseTracingHandler // the span is ended normally in HttpServerResponseTracingHandler
} catch (Throwable throwable) { } catch (Throwable throwable) {
// make sure to remove the server context on end() call // make sure to remove the server context on end() call

View File

@ -42,11 +42,11 @@ public class HttpServerResponseTracingHandler extends ChannelOutboundHandlerAdap
} }
@Override @Override
public void write(ChannelHandlerContext ctx, Object msg, ChannelPromise prm) { public void write(ChannelHandlerContext ctx, Object msg, ChannelPromise prm) throws Exception {
Attribute<Context> contextAttr = ctx.channel().attr(AttributeKeys.SERVER_CONTEXT); Attribute<Context> contextAttr = ctx.channel().attr(AttributeKeys.SERVER_CONTEXT);
Context context = contextAttr.get(); Context context = contextAttr.get();
if (context == null) { if (context == null) {
ctx.write(msg, prm); super.write(ctx, msg, prm);
return; return;
} }
@ -87,7 +87,7 @@ public class HttpServerResponseTracingHandler extends ChannelOutboundHandlerAdap
} }
try (Scope ignored = context.makeCurrent()) { try (Scope ignored = context.makeCurrent()) {
ctx.write(msg, writePromise); super.write(ctx, msg, writePromise);
} catch (Throwable throwable) { } catch (Throwable throwable) {
end(ctx.channel(), null, throwable); end(ctx.channel(), null, throwable);
throw throwable; throw throwable;