Skip to content

Commit

Permalink
Use super to make netty instrumentation clearer (#7736)
Browse files Browse the repository at this point in the history
I think it makes it clearer that these classes are decorating the super
methods.
  • Loading branch information
trask authored Feb 6, 2023
1 parent 9979d8f commit e7820f8
Show file tree
Hide file tree
Showing 12 changed files with 53 additions and 51 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,10 @@ public class HttpClientRequestTracingHandler extends SimpleChannelDownstreamHand
VirtualField.find(Channel.class, NettyClientRequestAndContexts.class);

@Override
public void writeRequested(ChannelHandlerContext ctx, MessageEvent event) {
public void writeRequested(ChannelHandlerContext ctx, MessageEvent event) throws Exception {
Object message = event.getMessage();
if (!(message instanceof HttpRequest)) {
ctx.sendDownstream(event);
super.writeRequested(ctx, event);
return;
}

Expand All @@ -45,7 +45,7 @@ public void writeRequested(ChannelHandlerContext ctx, MessageEvent event) {
HttpRequestAndChannel request =
HttpRequestAndChannel.create((HttpRequest) message, ctx.getChannel());
if (!instrumenter().shouldStart(parentContext, request)) {
ctx.sendDownstream(event);
super.writeRequested(ctx, event);
return;
}

Expand All @@ -54,7 +54,7 @@ public void writeRequested(ChannelHandlerContext ctx, MessageEvent event) {
ctx.getChannel(), NettyClientRequestAndContexts.create(parentContext, context, request));

try (Scope ignored = context.makeCurrent()) {
ctx.sendDownstream(event);
super.writeRequested(ctx, event);
// span is ended normally in HttpClientResponseTracingHandler
} catch (Throwable throwable) {
instrumenter().end(context, request, null, throwable);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,11 @@ public class HttpClientResponseTracingHandler extends SimpleChannelUpstreamHandl
VirtualField.find(Channel.class, NettyClientRequestAndContexts.class);

@Override
public void messageReceived(ChannelHandlerContext ctx, MessageEvent msg) {
public void messageReceived(ChannelHandlerContext ctx, MessageEvent msg) throws Exception {
NettyClientRequestAndContexts requestAndContexts = requestContextsField.get(ctx.getChannel());

if (requestAndContexts == null) {
ctx.sendUpstream(msg);
super.messageReceived(ctx, msg);
return;
}

Expand All @@ -42,7 +42,7 @@ public void messageReceived(ChannelHandlerContext ctx, MessageEvent msg) {

// We want the callback in the scope of the parent, not the client span
try (Scope ignored = requestAndContexts.parentContext().makeCurrent()) {
ctx.sendUpstream(msg);
super.messageReceived(ctx, msg);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -23,15 +23,15 @@ public class HttpServerRequestTracingHandler extends SimpleChannelUpstreamHandle
VirtualField.find(Channel.class, NettyServerRequestAndContext.class);

@Override
public void messageReceived(ChannelHandlerContext ctx, MessageEvent event) {
public void messageReceived(ChannelHandlerContext ctx, MessageEvent event) throws Exception {
Object message = event.getMessage();
if (!(message instanceof HttpRequest)) {
NettyServerRequestAndContext requestAndContext = requestAndContextField.get(ctx.getChannel());
if (requestAndContext == null) {
ctx.sendUpstream(event);
super.messageReceived(ctx, event);
} else {
try (Scope ignored = requestAndContext.context().makeCurrent()) {
ctx.sendUpstream(event);
super.messageReceived(ctx, event);
}
}
return;
Expand All @@ -41,15 +41,16 @@ public void messageReceived(ChannelHandlerContext ctx, MessageEvent event) {
HttpRequestAndChannel request =
HttpRequestAndChannel.create((HttpRequest) message, ctx.getChannel());
if (!instrumenter().shouldStart(parentContext, request)) {
ctx.sendUpstream(event);
super.messageReceived(ctx, event);
return;
}

Context context = instrumenter().start(parentContext, request);
requestAndContextField.set(
ctx.getChannel(), NettyServerRequestAndContext.create(request, context));

try (Scope ignored = context.makeCurrent()) {
ctx.sendUpstream(event);
super.messageReceived(ctx, event);
// the span is ended normally in HttpServerResponseTracingHandler
} catch (Throwable throwable) {
instrumenter().end(context, request, null, throwable);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,11 +24,11 @@ public class HttpServerResponseTracingHandler extends SimpleChannelDownstreamHan
VirtualField.find(Channel.class, NettyServerRequestAndContext.class);

@Override
public void writeRequested(ChannelHandlerContext ctx, MessageEvent msg) {
public void writeRequested(ChannelHandlerContext ctx, MessageEvent msg) throws Exception {
NettyServerRequestAndContext requestAndContext = requestAndContextField.get(ctx.getChannel());

if (requestAndContext == null || !(msg.getMessage() instanceof HttpResponse)) {
ctx.sendDownstream(msg);
super.writeRequested(ctx, msg);
return;
}

Expand All @@ -38,7 +38,7 @@ public void writeRequested(ChannelHandlerContext ctx, MessageEvent msg) {

Throwable error = null;
try (Scope ignored = context.makeCurrent()) {
ctx.sendDownstream(msg);
super.writeRequested(ctx, msg);
} catch (Throwable t) {
error = t;
throw t;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,28 +64,29 @@ public NettySslInstrumentationHandler(
}

@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
// are on classpath); checking just to be extra safe
if (SSL_HANDSHAKE_COMPLETION_EVENT == null) {
ctx.pipeline().remove(this);
instrumentationHandlerField.set(realHandler, null);
ctx.fireChannelRegistered();
super.channelRegistered(ctx);
return;
}

// remember the parent context from the time of channel registration;
// this happens inside Bootstrap#connect()
parentContext = Context.current();
ctx.fireChannelRegistered();
super.channelRegistered(ctx);
}

@Override
public void connect(
ChannelHandlerContext ctx,
SocketAddress remoteAddress,
SocketAddress localAddress,
ChannelPromise promise) {
ChannelPromise promise)
throws Exception {

// netty SslHandler starts the handshake after it receives the channelActive() signal; this
// happens just after the connection is established
Expand All @@ -101,11 +102,11 @@ public void connect(
context = instrumenter.start(parentContext, request);
}
});
ctx.connect(remoteAddress, localAddress, promise);
super.connect(ctx, remoteAddress, localAddress, promise);
}

@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 (ctx.pipeline().context(this) != null) {
ctx.pipeline().remove(this);
Expand All @@ -117,7 +118,7 @@ public void userEventTriggered(ChannelHandlerContext ctx, Object evt) {
}
}

ctx.fireUserEventTriggered(evt);
super.userEventTriggered(ctx, evt);
}

private static Throwable getCause(Object evt) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,9 @@
public class HttpClientRequestTracingHandler extends ChannelOutboundHandlerAdapter {

@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)) {
ctx.write(msg, prm);
super.write(ctx, msg, prm);
return;
}

Expand All @@ -33,7 +33,7 @@ public void write(ChannelHandlerContext ctx, Object msg, ChannelPromise prm) {

HttpRequestAndChannel request = HttpRequestAndChannel.create((HttpRequest) msg, ctx.channel());
if (!instrumenter().shouldStart(parentContext, request)) {
ctx.write(msg, prm);
super.write(ctx, msg, prm);
return;
}

Expand All @@ -47,7 +47,7 @@ public void write(ChannelHandlerContext ctx, Object msg, ChannelPromise prm) {
requestAttr.set(request);

try (Scope ignored = context.makeCurrent()) {
ctx.write(msg, prm);
super.write(ctx, msg, prm);
// span is ended normally in HttpClientResponseTracingHandler
} catch (Throwable throwable) {
instrumenter().end(contextAttr.getAndRemove(), requestAttr.getAndRemove(), null, throwable);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,11 @@
public class HttpClientResponseTracingHandler extends ChannelInboundHandlerAdapter {

@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);
Context context = contextAttr.get();
if (context == null) {
ctx.fireChannelRead(msg);
super.channelRead(ctx, msg);
return;
}

Expand Down Expand Up @@ -53,10 +53,10 @@ public void channelRead(ChannelHandlerContext ctx, Object msg) {
// We want the callback in the scope of the parent, not the client span
if (parentContext != null) {
try (Scope ignored = parentContext.makeCurrent()) {
ctx.fireChannelRead(msg);
super.channelRead(ctx, msg);
}
} else {
ctx.fireChannelRead(msg);
super.channelRead(ctx, msg);
}

if (msg instanceof FullHttpResponse) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,18 +20,18 @@
public class HttpServerRequestTracingHandler extends ChannelInboundHandlerAdapter {

@Override
public void channelRead(ChannelHandlerContext ctx, Object msg) {
public void channelRead(ChannelHandlerContext ctx, Object msg) throws Exception {
Channel channel = ctx.channel();
Attribute<Context> contextAttr = channel.attr(AttributeKeys.SERVER_CONTEXT);
Attribute<HttpRequestAndChannel> requestAttr = channel.attr(AttributeKeys.SERVER_REQUEST);

if (!(msg instanceof HttpRequest)) {
Context serverContext = contextAttr.get();
if (serverContext == null) {
ctx.fireChannelRead(msg);
super.channelRead(ctx, msg);
} else {
try (Scope ignored = serverContext.makeCurrent()) {
ctx.fireChannelRead(msg);
super.channelRead(ctx, msg);
}
}
return;
Expand All @@ -44,7 +44,7 @@ public void channelRead(ChannelHandlerContext ctx, Object msg) {
HttpRequestAndChannel request = HttpRequestAndChannel.create((HttpRequest) msg, channel);

if (!instrumenter().shouldStart(parentContext, request)) {
ctx.fireChannelRead(msg);
super.channelRead(ctx, msg);
return;
}

Expand All @@ -53,7 +53,7 @@ public void channelRead(ChannelHandlerContext ctx, Object msg) {
requestAttr.set(request);

try (Scope ignored = context.makeCurrent()) {
ctx.fireChannelRead(msg);
super.channelRead(ctx, msg);
// the span is ended normally in HttpServerResponseTracingHandler
} catch (Throwable throwable) {
// make sure to remove the server context on end() call
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,9 @@ public HttpClientRequestTracingHandler(
}

@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)) {
ctx.write(msg, prm);
super.write(ctx, msg, prm);
return;
}

Expand All @@ -48,7 +48,7 @@ public void write(ChannelHandlerContext ctx, Object msg, ChannelPromise prm) {

HttpRequestAndChannel request = HttpRequestAndChannel.create((HttpRequest) msg, ctx.channel());
if (!instrumenter.shouldStart(parentContext, request) || isAwsRequest(request)) {
ctx.write(msg, prm);
super.write(ctx, msg, prm);
return;
}

Expand All @@ -62,7 +62,7 @@ public void write(ChannelHandlerContext ctx, Object msg, ChannelPromise prm) {
requestAttr.set(request);

try (Scope ignored = context.makeCurrent()) {
ctx.write(msg, prm);
super.write(ctx, msg, prm);
// span is ended normally in HttpClientResponseTracingHandler
} catch (Throwable throwable) {
instrumenter.end(contextAttr.getAndSet(null), requestAttr.getAndSet(null), null, throwable);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,11 +37,11 @@ public HttpClientResponseTracingHandler(
}

@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);
Context context = contextAttr.get();
if (context == null) {
ctx.fireChannelRead(msg);
super.channelRead(ctx, msg);
return;
}

Expand Down Expand Up @@ -69,10 +69,10 @@ public void channelRead(ChannelHandlerContext ctx, Object msg) {
// We want the callback in the scope of the parent, not the client span
if (parentContext != null) {
try (Scope ignored = parentContext.makeCurrent()) {
ctx.fireChannelRead(msg);
super.channelRead(ctx, msg);
}
} else {
ctx.fireChannelRead(msg);
super.channelRead(ctx, msg);
}

if (msg instanceof FullHttpResponse) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,18 +35,18 @@ public HttpServerRequestTracingHandler(
}

@Override
public void channelRead(ChannelHandlerContext ctx, Object msg) {
public void channelRead(ChannelHandlerContext ctx, Object msg) throws Exception {
Channel channel = ctx.channel();
Attribute<Context> contextAttr = channel.attr(AttributeKeys.SERVER_CONTEXT);
Attribute<HttpRequestAndChannel> requestAttr = channel.attr(HTTP_REQUEST);

if (!(msg instanceof HttpRequest)) {
Context serverContext = contextAttr.get();
if (serverContext == null) {
ctx.fireChannelRead(msg);
super.channelRead(ctx, msg);
} else {
try (Scope ignored = serverContext.makeCurrent()) {
ctx.fireChannelRead(msg);
super.channelRead(ctx, msg);
}
}
return;
Expand All @@ -59,7 +59,7 @@ public void channelRead(ChannelHandlerContext ctx, Object msg) {
HttpRequestAndChannel request = HttpRequestAndChannel.create((HttpRequest) msg, channel);

if (!instrumenter.shouldStart(parentContext, request)) {
ctx.fireChannelRead(msg);
super.channelRead(ctx, msg);
return;
}

Expand All @@ -68,7 +68,7 @@ public void channelRead(ChannelHandlerContext ctx, Object msg) {
requestAttr.set(request);

try (Scope ignored = context.makeCurrent()) {
ctx.fireChannelRead(msg);
super.channelRead(ctx, msg);
// the span is ended normally in HttpServerResponseTracingHandler
} catch (Throwable throwable) {
// make sure to remove the server context on end() call
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,11 +42,11 @@ public HttpServerResponseTracingHandler(
}

@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);
Context context = contextAttr.get();
if (context == null) {
ctx.write(msg, prm);
super.write(ctx, msg, prm);
return;
}

Expand Down Expand Up @@ -87,7 +87,7 @@ public void write(ChannelHandlerContext ctx, Object msg, ChannelPromise prm) {
}

try (Scope ignored = context.makeCurrent()) {
ctx.write(msg, writePromise);
super.write(ctx, msg, writePromise);
} catch (Throwable throwable) {
end(ctx.channel(), null, throwable);
throw throwable;
Expand Down

0 comments on commit e7820f8

Please sign in to comment.