Repository: netty/netty
Issue: Ability to change the logger and level used by DnsNameResolverBuilder#traceEnabled

### Expected behavior
Ability to change the logger and level used by `DnsNameResolverBuilder#traceEnabled`

### Actual behavior
When `DnsNameResolverBuilder#traceEnabled` is enabled, it uses always the logger and level below
https://github.com/netty/netty/blob/46cb4015ff25ff2dd3d0dede5c3185ac498e56b0/resolver-dns/src/main/java/io/netty/resolver/dns/TraceDnsQueryLifeCycleObserverFactory.java#L26-L28

It would be nice if those can be configured.

Possible solutions:
- `DnsNameResolverBuilder#traceEnabled` where one can specify logger and level

Or

- `TraceDnsQueryLifeCycleObserverFactory` can be made public and also it can expose construction similar to what `LoggingHandler` provides i.e.

```
    public TraceDnsQueryLifeCycleObserverFactory(Class<?> clazz, LogLevel level) {
        ObjectUtil.checkNotNull(clazz, "clazz");
        ObjectUtil.checkNotNull(level, "level");
        this.logger = InternalLoggerFactory.getInstance(clazz);
        this.level = level.toInternalLevel();
    }
```

With this approach one will be able to add such observer with `DnsNameResolverBuilder#dnsQueryLifecycleObserverFactory`

this second approach will require a dependency to `netty-handler` module

Wdyt?


### Netty version
current snapshot

### JVM version (e.g. `java -version`)
jdk 8

### OS version (e.g. `uname -a`)
Mac OS 

Issue comments:
It's not clear to me why you want to change the log level, and if you want to change it to TRACE or INFO, or some other level? Log levels are normally hard-coded on the usage site, and the current code follows that convention.

This setting is about whether or not logging is enabled. Detail configuration of how logging works is something we'd prefer to leave to the logging framework.
@chrisvest The use case is the following: Reactor Netty wants to expose this as a configuration so that the user can decide which logger and what level to use. With the current approach the user sees that the tracing is enabled but needs to know the exact logger and level that are needed (which are not from Reactor Netty but from a library that Reactor Netty uses)
But that's the case with all logging in Netty, isn't it? Well, almost I suppose. The logging implementation that is used everywhere is discovered at runtime, and the level is always hard coded, with the exception of `LoggingHandler` and `Http2FrameLogger`. With that in mind I think the second approach makes the most sense: expose some clean-up version of `TraceDnsQueryLifeCycleObserverFactory`, and remove `traceEnabled`.
> But that's the case with all logging in Netty, isn't it? Well, almost I suppose. The logging implementation that is used everywhere is discovered at runtime, and the level is always hard coded, with the exception of `LoggingHandler` and `Http2FrameLogger`. With that in mind I think the second approach makes the most sense: expose some clean-up version of `TraceDnsQueryLifeCycleObserverFactory`, and remove `traceEnabled`.

Yes I agree, from my point of view it is like `LoggingHandler`, I can add this handler to the pipeline. The same is with `TraceDnsQueryLifeCycleObserverFactory` I can add it to the configuration.

Pull request review comments:
Is there any particular concern to use `checkNotNull ` instead of `requireNonNull`. The latter is used in the version for Netty 5.
Because it not depends on java8!
ops ... true 
Netty 4.1 needs to support Java 6 at runtime where `requireNonNull` is not available.

Patch that resolved the issue:
diff --git a/resolver-dns/pom.xml b/resolver-dns/pom.xml
index 0925c9f47de..84e1733a79b 100644
--- a/resolver-dns/pom.xml
+++ b/resolver-dns/pom.xml
@@ -63,6 +63,11 @@
       <artifactId>netty-codec-dns</artifactId>
       <version>${project.version}</version>
     </dependency>
+    <dependency>
+      <groupId>${project.groupId}</groupId>
+      <artifactId>netty-handler</artifactId>
+      <version>${project.version}</version>
+    </dependency>
     <dependency>
       <groupId>org.apache.directory.server</groupId>
       <artifactId>apacheds-protocol-dns</artifactId>
diff --git a/resolver-dns/src/main/java/io/netty/resolver/dns/DnsNameResolver.java b/resolver-dns/src/main/java/io/netty/resolver/dns/DnsNameResolver.java
index 344be375c6d..eb3b8046c6d 100644
--- a/resolver-dns/src/main/java/io/netty/resolver/dns/DnsNameResolver.java
+++ b/resolver-dns/src/main/java/io/netty/resolver/dns/DnsNameResolver.java
@@ -399,8 +399,8 @@ public DnsNameResolver(
         this.cnameCache = checkNotNull(cnameCache, "cnameCache");
         this.dnsQueryLifecycleObserverFactory = traceEnabled ?
                 dnsQueryLifecycleObserverFactory instanceof NoopDnsQueryLifecycleObserverFactory ?
-                        new TraceDnsQueryLifeCycleObserverFactory() :
-                        new BiDnsQueryLifecycleObserverFactory(new TraceDnsQueryLifeCycleObserverFactory(),
+                        new LoggingDnsQueryLifeCycleObserverFactory() :
+                        new BiDnsQueryLifecycleObserverFactory(new LoggingDnsQueryLifeCycleObserverFactory(),
                                                                dnsQueryLifecycleObserverFactory) :
                 checkNotNull(dnsQueryLifecycleObserverFactory, "dnsQueryLifecycleObserverFactory");
         this.searchDomains = searchDomains != null ? searchDomains.clone() : DEFAULT_SEARCH_DOMAINS;
diff --git a/resolver-dns/src/main/java/io/netty/resolver/dns/DnsNameResolverBuilder.java b/resolver-dns/src/main/java/io/netty/resolver/dns/DnsNameResolverBuilder.java
index 2afaf1c68ff..3ec59ff2907 100644
--- a/resolver-dns/src/main/java/io/netty/resolver/dns/DnsNameResolverBuilder.java
+++ b/resolver-dns/src/main/java/io/netty/resolver/dns/DnsNameResolverBuilder.java
@@ -324,7 +324,10 @@ public DnsNameResolverBuilder maxQueriesPerResolve(int maxQueriesPerResolve) {
      *
      * @param traceEnabled true if trace is enabled
      * @return {@code this}
+     * @deprecated Prefer to {@linkplain #dnsQueryLifecycleObserverFactory(DnsQueryLifecycleObserverFactory) configure}
+     * a {@link LoggingDnsQueryLifeCycleObserverFactory} instead.
      */
+    @Deprecated
     public DnsNameResolverBuilder traceEnabled(boolean traceEnabled) {
         this.traceEnabled = traceEnabled;
         return this;
diff --git a/resolver-dns/src/main/java/io/netty/resolver/dns/LoggingDnsQueryLifeCycleObserverFactory.java b/resolver-dns/src/main/java/io/netty/resolver/dns/LoggingDnsQueryLifeCycleObserverFactory.java
new file mode 100644
index 00000000000..d33a47e9b7b
--- /dev/null
+++ b/resolver-dns/src/main/java/io/netty/resolver/dns/LoggingDnsQueryLifeCycleObserverFactory.java
@@ -0,0 +1,85 @@
+/*
+ * Copyright 2020 The Netty Project
+ *
+ * The Netty Project licenses this file to you under the Apache License,
+ * version 2.0 (the "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at:
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ */
+package io.netty.resolver.dns;
+
+import io.netty.handler.codec.dns.DnsQuestion;
+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;
+
+import static io.netty.util.internal.ObjectUtil.checkNotNull;
+
+/**
+ * A {@link DnsQueryLifecycleObserverFactory} that enables detailed logging in the {@link DnsNameResolver}.
+ * <p>
+ * When {@linkplain DnsNameResolverBuilder#dnsQueryLifecycleObserverFactory(DnsQueryLifecycleObserverFactory)
+ * configured on the resolver}, detailed trace information will be generated so that it is easier to understand the
+ * cause of resolution failure.
+ */
+public final class LoggingDnsQueryLifeCycleObserverFactory implements DnsQueryLifecycleObserverFactory {
+    private static final InternalLogger DEFAULT_LOGGER =
+            InternalLoggerFactory.getInstance(LoggingDnsQueryLifeCycleObserverFactory.class);
+    private final InternalLogger logger;
+    private final InternalLogLevel level;
+
+    /**
+     * Create {@link DnsQueryLifecycleObserver} instances that log events at the default {@link LogLevel#DEBUG} level.
+     */
+    public LoggingDnsQueryLifeCycleObserverFactory() {
+        this(LogLevel.DEBUG);
+    }
+
+    /**
+     * Create {@link DnsQueryLifecycleObserver} instances that log events at the given log level.
+     * @param level The log level to use for logging resolver events.
+     */
+    public LoggingDnsQueryLifeCycleObserverFactory(LogLevel level) {
+        this.level = checkAndConvertLevel(level);
+        logger = DEFAULT_LOGGER;
+    }
+
+    /**
+     * Create {@link DnsQueryLifecycleObserver} instances that log events to a logger with the given class context,
+     * at the given log level.
+     * @param classContext The class context for the logger to use.
+     * @param level The log level to use for logging resolver events.
+     */
+    public LoggingDnsQueryLifeCycleObserverFactory(Class<?> classContext, LogLevel level) {
+        this.level = checkAndConvertLevel(level);
+        logger = InternalLoggerFactory.getInstance(checkNotNull(classContext, "classContext"));
+    }
+
+    /**
+     * Create {@link DnsQueryLifecycleObserver} instances that log events to a logger with the given name context,
+     * at the given log level.
+     * @param name The name for the logger to use.
+     * @param level The log level to use for logging resolver events.
+     */
+    public LoggingDnsQueryLifeCycleObserverFactory(String name, LogLevel level) {
+        this.level = checkAndConvertLevel(level);
+        logger = InternalLoggerFactory.getInstance(checkNotNull(name, "name"));
+    }
+
+    private static InternalLogLevel checkAndConvertLevel(LogLevel level) {
+        return checkNotNull(level, "level").toInternalLevel();
+    }
+
+    @Override
+    public DnsQueryLifecycleObserver newDnsQueryLifecycleObserver(DnsQuestion question) {
+        return new LoggingDnsQueryLifecycleObserver(question, logger, level);
+    }
+}
diff --git a/resolver-dns/src/main/java/io/netty/resolver/dns/TraceDnsQueryLifecycleObserver.java b/resolver-dns/src/main/java/io/netty/resolver/dns/LoggingDnsQueryLifecycleObserver.java
similarity index 93%
rename from resolver-dns/src/main/java/io/netty/resolver/dns/TraceDnsQueryLifecycleObserver.java
rename to resolver-dns/src/main/java/io/netty/resolver/dns/LoggingDnsQueryLifecycleObserver.java
index 8166ff2c301..6f1aa629680 100644
--- a/resolver-dns/src/main/java/io/netty/resolver/dns/TraceDnsQueryLifecycleObserver.java
+++ b/resolver-dns/src/main/java/io/netty/resolver/dns/LoggingDnsQueryLifecycleObserver.java
@@ -26,13 +26,13 @@
 
 import static io.netty.util.internal.ObjectUtil.checkNotNull;
 
-final class TraceDnsQueryLifecycleObserver implements DnsQueryLifecycleObserver {
+final class LoggingDnsQueryLifecycleObserver implements DnsQueryLifecycleObserver {
     private final InternalLogger logger;
     private final InternalLogLevel level;
     private final DnsQuestion question;
     private InetSocketAddress dnsServerAddress;
 
-    TraceDnsQueryLifecycleObserver(DnsQuestion question, InternalLogger logger, InternalLogLevel level) {
+    LoggingDnsQueryLifecycleObserver(DnsQuestion question, InternalLogger logger, InternalLogLevel level) {
         this.question = checkNotNull(question, "question");
         this.logger = checkNotNull(logger, "logger");
         this.level = checkNotNull(level, "level");
diff --git a/resolver-dns/src/main/java/io/netty/resolver/dns/TraceDnsQueryLifeCycleObserverFactory.java b/resolver-dns/src/main/java/io/netty/resolver/dns/TraceDnsQueryLifeCycleObserverFactory.java
deleted file mode 100644
index dc7572bca17..00000000000
--- a/resolver-dns/src/main/java/io/netty/resolver/dns/TraceDnsQueryLifeCycleObserverFactory.java
+++ /dev/null
@@ -1,45 +0,0 @@
-/*
- * Copyright 2017 The Netty Project
- *
- * The Netty Project licenses this file to you under the Apache License,
- * version 2.0 (the "License"); you may not use this file except in compliance
- * with the License. You may obtain a copy of the License at:
- *
- *   http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
- * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
- * License for the specific language governing permissions and limitations
- * under the License.
- */
-package io.netty.resolver.dns;
-
-import io.netty.handler.codec.dns.DnsQuestion;
-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;
-
-final class TraceDnsQueryLifeCycleObserverFactory implements DnsQueryLifecycleObserverFactory {
-    private static final InternalLogger DEFAULT_LOGGER =
-            InternalLoggerFactory.getInstance(TraceDnsQueryLifeCycleObserverFactory.class);
-    private static final InternalLogLevel DEFAULT_LEVEL = InternalLogLevel.DEBUG;
-    private final InternalLogger logger;
-    private final InternalLogLevel level;
-
-    TraceDnsQueryLifeCycleObserverFactory() {
-        this(DEFAULT_LOGGER, DEFAULT_LEVEL);
-    }
-
-    TraceDnsQueryLifeCycleObserverFactory(InternalLogger logger, InternalLogLevel level) {
-        this.logger = checkNotNull(logger, "logger");
-        this.level = checkNotNull(level, "level");
-    }
-
-    @Override
-    public DnsQueryLifecycleObserver newDnsQueryLifecycleObserver(DnsQuestion question) {
-        return new TraceDnsQueryLifecycleObserver(question, logger, level);
-    }
-}


Test patch: