From 5934ae8fd28b2a7b2b9eb892c2192428c2a113d1 Mon Sep 17 00:00:00 2001 From: Scott Mitchell Date: Tue, 20 Jun 2017 17:22:56 -0700 Subject: [PATCH] Http2FrameLogger Updates Motivation: The Http2FrameLogger uses a custom format when logging events. We should use the more familiar format of 'channel event type: details' and single line logging for more consistent debugging. Modifications: - Http2FrameLogger should not use a StringBuilder and instead should directly use the Logger - Http2FrameLogger should use the more consistent format defined above Result: Http2FrameLogger's logging formate is more consistent with other log events. --- .../handler/codec/http2/Http2FrameLogger.java | 95 ++++++------------- 1 file changed, 27 insertions(+), 68 deletions(-) diff --git a/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2FrameLogger.java b/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2FrameLogger.java index c8c1cd865d..5a29d7f461 100644 --- a/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2FrameLogger.java +++ b/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2FrameLogger.java @@ -15,8 +15,6 @@ */ package io.netty.handler.codec.http2; -import static io.netty.util.internal.ObjectUtil.checkNotNull; - import io.netty.buffer.ByteBuf; import io.netty.buffer.ByteBufUtil; import io.netty.channel.ChannelHandlerAdapter; @@ -27,6 +25,8 @@ import io.netty.util.internal.logging.InternalLogLevel; import io.netty.util.internal.logging.InternalLogger; import io.netty.util.internal.logging.InternalLoggerFactory; +import static io.netty.util.internal.ObjectUtil.checkNotNull; + /** * Logs HTTP2 frames for debugging purposes. */ @@ -61,105 +61,74 @@ public class Http2FrameLogger extends ChannelHandlerAdapter { public void logData(Direction direction, ChannelHandlerContext ctx, int streamId, ByteBuf data, int padding, boolean endStream) { - if (enabled()) { - log(direction, - "%s DATA: streamId=%d, padding=%d, endStream=%b, length=%d, bytes=%s", - ctx.channel(), streamId, padding, endStream, data.readableBytes(), toString(data)); - } + logger.log(level, "{} {} PRIORITY: streamId={} padding={} endStream={} length={} bytes={}", ctx.channel(), + direction.name(), streamId, padding, endStream, data.readableBytes(), toString(data)); } public void logHeaders(Direction direction, ChannelHandlerContext ctx, int streamId, Http2Headers headers, int padding, boolean endStream) { - if (enabled()) { - log(direction, "%s HEADERS: streamId=%d, headers=%s, padding=%d, endStream=%b", - ctx.channel(), streamId, headers, padding, endStream); - } + logger.log(level, "{} {} PRIORITY: streamId={} headers={} padding={} endStream={}", ctx.channel(), + direction.name(), streamId, headers, padding, endStream); } public void logHeaders(Direction direction, ChannelHandlerContext ctx, int streamId, Http2Headers headers, int streamDependency, short weight, boolean exclusive, int padding, boolean endStream) { - if (enabled()) { - log(direction, - "%s HEADERS: streamId=%d, headers=%s, streamDependency=%d, weight=%d, " - + "exclusive=%b, padding=%d, endStream=%b", - ctx.channel(), streamId, headers, streamDependency, weight, exclusive, padding, endStream); - } + logger.log(level, "{} {} PRIORITY: streamId={} headers={} streamDependency={} weight={} exclusive={} " + + "padding={} endStream={}", ctx.channel(), + direction.name(), streamId, headers, streamDependency, weight, exclusive, padding, endStream); } public void logPriority(Direction direction, ChannelHandlerContext ctx, int streamId, int streamDependency, short weight, boolean exclusive) { - if (enabled()) { - log(direction, "%s PRIORITY: streamId=%d, streamDependency=%d, weight=%d, exclusive=%b", - ctx.channel(), streamId, streamDependency, weight, exclusive); - } + logger.log(level, "{} {} PRIORITY: streamId={} streamDependency={} weight={} exclusive={}", ctx.channel(), + direction.name(), streamId, streamDependency, weight, exclusive); } public void logRstStream(Direction direction, ChannelHandlerContext ctx, int streamId, long errorCode) { - if (enabled()) { - log(direction, "%s RST_STREAM: streamId=%d, errorCode=%d", ctx.channel(), streamId, errorCode); - } + logger.log(level, "{} {} RST_STREAM: streamId={} errorCode={}", ctx.channel(), + direction.name(), streamId, errorCode); } public void logSettingsAck(Direction direction, ChannelHandlerContext ctx) { - if (enabled()) { - log(direction, "%s SETTINGS: ack=true", ctx.channel()); - } + logger.log(level, "{} {} SETTINGS: ack=true", ctx.channel(), direction.name()); } public void logSettings(Direction direction, ChannelHandlerContext ctx, Http2Settings settings) { - if (enabled()) { - log(direction, "%s SETTINGS: ack=false, settings=%s", ctx.channel(), settings); - } + logger.log(level, "{} {} SETTINGS: ack=false settings={}", ctx.channel(), direction.name(), settings); } public void logPing(Direction direction, ChannelHandlerContext ctx, ByteBuf data) { - if (enabled()) { - log(direction, "%s PING: ack=false, length=%d, bytes=%s", ctx.channel(), - data.readableBytes(), toString(data)); - } + logger.log(level, "{} {} PING: ack=false length={} bytes={}", ctx.channel(), + direction.name(), data.readableBytes(), toString(data)); } public void logPingAck(Direction direction, ChannelHandlerContext ctx, ByteBuf data) { - if (enabled()) { - log(direction, "%s PING: ack=true, length=%d, bytes=%s", - ctx.channel(), data.readableBytes(), toString(data)); - } + logger.log(level, "{} {} PING: ack=true length={} bytes={}", ctx.channel(), + direction.name(), data.readableBytes(), toString(data)); } public void logPushPromise(Direction direction, ChannelHandlerContext ctx, int streamId, int promisedStreamId, Http2Headers headers, int padding) { - if (enabled()) { - log(direction, "%s PUSH_PROMISE: streamId=%d, promisedStreamId=%d, headers=%s, padding=%d", - ctx.channel(), streamId, promisedStreamId, headers, padding); - } + logger.log(level, "{} {} PUSH_PROMISE: streamId={} promisedStreamId={} headers={} padding={}", ctx.channel(), + direction.name(), streamId, promisedStreamId, headers, padding); } public void logGoAway(Direction direction, ChannelHandlerContext ctx, int lastStreamId, long errorCode, ByteBuf debugData) { - if (enabled()) { - log(direction, "%s GO_AWAY: lastStreamId=%d, errorCode=%d, length=%d, bytes=%s", - ctx.channel(), lastStreamId, errorCode, debugData.readableBytes(), toString(debugData)); - } + logger.log(level, "{} {} GO_AWAY: lastStreamId={} errorCode={} length={} bytes={}", ctx.channel(), + direction.name(), lastStreamId, errorCode, debugData.readableBytes(), toString(debugData)); } public void logWindowsUpdate(Direction direction, ChannelHandlerContext ctx, int streamId, int windowSizeIncrement) { - if (enabled()) { - log(direction, "%s WINDOW_UPDATE: streamId=%d, windowSizeIncrement=%d", - ctx.channel(), streamId, windowSizeIncrement); - } + logger.log(level, "{} {} WINDOW_UPDATE: streamId={} windowSizeIncrement={}", ctx.channel(), + direction.name(), streamId, windowSizeIncrement); } public void logUnknownFrame(Direction direction, ChannelHandlerContext ctx, byte frameType, int streamId, Http2Flags flags, ByteBuf data) { - if (enabled()) { - log(direction, "%s UNKNOWN: frameType=%d, streamId=%d, flags=%d, length=%d, bytes=%s", - ctx.channel(), frameType & 0xFF, streamId, flags.value(), data.readableBytes(), toString(data)); - } - } - - private boolean enabled() { - return logger.isEnabled(level); + logger.log(level, "{} {} UNKNOWN: frameType={} streamId={} flags={} length={} bytes={}", ctx.channel(), + direction.name(), frameType & 0xFF, streamId, flags.value(), data.readableBytes(), toString(data)); } private String toString(ByteBuf buf) { @@ -172,14 +141,4 @@ public class Http2FrameLogger extends ChannelHandlerAdapter { int length = Math.min(buf.readableBytes(), BUFFER_LENGTH_THRESHOLD); return ByteBufUtil.hexDump(buf, buf.readerIndex(), length) + "..."; } - - private void log(Direction direction, String format, Object... args) { - StringBuilder b = new StringBuilder(200); - b.append("\n----------------") - .append(direction.name()) - .append("--------------------\n") - .append(String.format(format, args)) - .append("\n------------------------------------"); - logger.log(level, b.toString()); - } }