Repository: netty/netty
Issue: Hide InternalLogger from Http2FrameLogger
Labels: defect

InternalLogger is intended for internal use only.  We should hide it from users.
- Instead of `InternalLogLevel`, we can use io.netty.handler.logging.LogLevel`.
- Instead of accepting `InternalLogger`, we can accept `String` and `Class<?>` to create an `InternalLogger` in the constructor of `Http2FrameLogger`.


Issue comments:
I see a similar problem also in the spdy example.

@trustin for this we will need to make the toInternLogger() method public... wdyt ?

@trustin @normanmaurer is there any action that I can take on this currently? And if so, will it be backward compatible with 4.1?

@nmittler The thing is `InternalLogger` and `InternalLogLevel` are internal API and thus a user or an example should not use and should not be exposed to them.  My suggestion was to use `LogLevel` in the `io.netty.handler.logging` package instead and let user pass the name of the logger instead of `InternalLogger`, but I'm fine if there's a better idea.


Pull request review comments:
Could we maybe add a javadoc comment telling this is for internal usage only ?

Done.


Patch that resolved the issue:
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 4a9d0834534..81b6024cb18 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
@@ -16,9 +16,11 @@
 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;
+import io.netty.handler.logging.LogLevel;
 import io.netty.util.internal.logging.InternalLogLevel;
 import io.netty.util.internal.logging.InternalLogger;
 import io.netty.util.internal.logging.InternalLoggerFactory;
@@ -37,11 +39,19 @@ public enum Direction {
     private final InternalLogger logger;
     private final InternalLogLevel level;
 
-    public Http2FrameLogger(InternalLogLevel level) {
-        this(level, InternalLoggerFactory.getInstance(Http2FrameLogger.class));
+    public Http2FrameLogger(LogLevel level) {
+        this(level.toInternalLevel(), InternalLoggerFactory.getInstance(Http2FrameLogger.class));
+    }
+
+    public Http2FrameLogger(LogLevel level, String name) {
+        this(level.toInternalLevel(), InternalLoggerFactory.getInstance(name));
+    }
+
+    public Http2FrameLogger(LogLevel level, Class<?> clazz) {
+        this(level.toInternalLevel(), InternalLoggerFactory.getInstance(clazz));
     }
 
-    public Http2FrameLogger(InternalLogLevel level, InternalLogger logger) {
+    private Http2FrameLogger(InternalLogLevel level, InternalLogger logger) {
         this.level = checkNotNull(level, "level");
         this.logger = checkNotNull(logger, "logger");
     }
diff --git a/example/src/main/java/io/netty/example/http2/client/Http2ClientInitializer.java b/example/src/main/java/io/netty/example/http2/client/Http2ClientInitializer.java
index c7c35d1cb20..842ea7184b7 100644
--- a/example/src/main/java/io/netty/example/http2/client/Http2ClientInitializer.java
+++ b/example/src/main/java/io/netty/example/http2/client/Http2ClientInitializer.java
@@ -14,7 +14,8 @@
  */
 package io.netty.example.http2.client;
 
-import static io.netty.util.internal.logging.InternalLogLevel.INFO;
+import static io.netty.handler.logging.LogLevel.INFO;
+
 import io.netty.channel.ChannelHandlerAdapter;
 import io.netty.channel.ChannelHandlerContext;
 import io.netty.channel.ChannelInitializer;
@@ -39,14 +40,12 @@
 import io.netty.handler.codec.http2.HttpToHttp2ConnectionHandler;
 import io.netty.handler.codec.http2.InboundHttp2ToHttpAdapter;
 import io.netty.handler.ssl.SslContext;
-import io.netty.util.internal.logging.InternalLoggerFactory;
 
 /**
  * Configures the client pipeline to support HTTP/2 frames.
  */
 public class Http2ClientInitializer extends ChannelInitializer<SocketChannel> {
-    private static final Http2FrameLogger logger =
-                    new Http2FrameLogger(INFO, InternalLoggerFactory.getInstance(Http2ClientInitializer.class));
+    private static final Http2FrameLogger logger = new Http2FrameLogger(INFO, Http2ClientInitializer.class);
 
     private final SslContext sslCtx;
     private final int maxContentLength;
diff --git a/example/src/main/java/io/netty/example/http2/server/HelloWorldHttp2Handler.java b/example/src/main/java/io/netty/example/http2/server/HelloWorldHttp2Handler.java
index caf8142f1cf..9c9129b03d1 100644
--- a/example/src/main/java/io/netty/example/http2/server/HelloWorldHttp2Handler.java
+++ b/example/src/main/java/io/netty/example/http2/server/HelloWorldHttp2Handler.java
@@ -19,7 +19,8 @@
 import static io.netty.buffer.Unpooled.unreleasableBuffer;
 import static io.netty.example.http2.Http2ExampleUtil.UPGRADE_RESPONSE_HEADER;
 import static io.netty.handler.codec.http.HttpResponseStatus.OK;
-import static io.netty.util.internal.logging.InternalLogLevel.INFO;
+import static io.netty.handler.logging.LogLevel.INFO;
+
 import io.netty.buffer.ByteBuf;
 import io.netty.channel.ChannelHandlerContext;
 import io.netty.handler.codec.AsciiString;
@@ -40,15 +41,13 @@
 import io.netty.handler.codec.http2.Http2InboundFrameLogger;
 import io.netty.handler.codec.http2.Http2OutboundFrameLogger;
 import io.netty.util.CharsetUtil;
-import io.netty.util.internal.logging.InternalLoggerFactory;
 
 /**
  * A simple handler that responds with the message "Hello World!".
  */
 public class HelloWorldHttp2Handler extends Http2ConnectionHandler {
 
-    private static final Http2FrameLogger logger = new Http2FrameLogger(INFO,
-            InternalLoggerFactory.getInstance(HelloWorldHttp2Handler.class));
+    private static final Http2FrameLogger logger = new Http2FrameLogger(INFO, HelloWorldHttp2Handler.class);
     static final ByteBuf RESPONSE_BYTES = unreleasableBuffer(copiedBuffer("Hello World", CharsetUtil.UTF_8));
 
     public HelloWorldHttp2Handler() {
diff --git a/handler/src/main/java/io/netty/handler/logging/LogLevel.java b/handler/src/main/java/io/netty/handler/logging/LogLevel.java
index 688c21264c4..f05002238e2 100644
--- a/handler/src/main/java/io/netty/handler/logging/LogLevel.java
+++ b/handler/src/main/java/io/netty/handler/logging/LogLevel.java
@@ -34,11 +34,13 @@ public enum LogLevel {
     }
 
     /**
-     * Converts the specified {@link LogLevel} to its {@link InternalLogLevel} variant.
+     * For internal use only.
+     *
+     * <p/>Converts the specified {@link LogLevel} to its {@link InternalLogLevel} variant.
      *
      * @return the converted level.
      */
-    InternalLogLevel toInternalLevel() {
+    public InternalLogLevel toInternalLevel() {
         return internalLevel;
     }
 }


Test patch: