Expose a LoggingDnsQueryLifeCycleObserverFactory (#10488)

Expose a LoggingDnsQueryLifeCycleObserverFactory

Motivation:
 There is a use case for having logging in the DnsNameResolver, similar to the LoggingHandler.
 Previously, one could set `traceEnabled` on the DnsNameResolverBuilder, but this is not very configurable.
 Specifically, the log level and the logger context cannot be changed.

Modification:
 Expose a LoggingDnsQueryLifeCycleObserverFactory, that permit changing the log-level
 and logger context.

Result:
 It is now possible to get logging in the DnsNameResolver at a custom log level and logger,
 without very much effort.

Fixes #10485
This commit is contained in:
Chris Vest 2020-08-19 17:38:29 +02:00 committed by GitHub
parent 514d349e1f
commit 6150a0e3c5
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 100 additions and 79 deletions

View File

@ -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>

View File

@ -272,7 +272,6 @@ public class DnsNameResolver extends InetNameResolver {
* @param resolvedAddressTypes the preferred address types
* @param recursionDesired if recursion desired flag must be set
* @param maxQueriesPerResolve the maximum allowed number of DNS queries for a given name resolution
* @param traceEnabled if trace is enabled
* @param maxPayloadSize the capacity of the datagram packet buffer
* @param optResourceEnabled if automatic inclusion of a optional records is enabled
* @param hostsFileEntriesResolver the {@link HostsFileEntriesResolver} used to check for local aliases
@ -296,7 +295,6 @@ public class DnsNameResolver extends InetNameResolver {
ResolvedAddressTypes resolvedAddressTypes,
boolean recursionDesired,
int maxQueriesPerResolve,
boolean traceEnabled,
int maxPayloadSize,
boolean optResourceEnabled,
HostsFileEntriesResolver hostsFileEntriesResolver,
@ -306,7 +304,7 @@ public class DnsNameResolver extends InetNameResolver {
boolean decodeIdn) {
this(eventLoop, channelFactory, resolveCache,
new AuthoritativeDnsServerCacheAdapter(authoritativeDnsServerCache), dnsQueryLifecycleObserverFactory,
queryTimeoutMillis, resolvedAddressTypes, recursionDesired, maxQueriesPerResolve, traceEnabled,
queryTimeoutMillis, resolvedAddressTypes, recursionDesired, maxQueriesPerResolve,
maxPayloadSize, optResourceEnabled, hostsFileEntriesResolver, dnsServerAddressStreamProvider,
searchDomains, ndots, decodeIdn);
}
@ -324,7 +322,6 @@ public class DnsNameResolver extends InetNameResolver {
* @param resolvedAddressTypes the preferred address types
* @param recursionDesired if recursion desired flag must be set
* @param maxQueriesPerResolve the maximum allowed number of DNS queries for a given name resolution
* @param traceEnabled if trace is enabled
* @param maxPayloadSize the capacity of the datagram packet buffer
* @param optResourceEnabled if automatic inclusion of a optional records is enabled
* @param hostsFileEntriesResolver the {@link HostsFileEntriesResolver} used to check for local aliases
@ -348,7 +345,6 @@ public class DnsNameResolver extends InetNameResolver {
ResolvedAddressTypes resolvedAddressTypes,
boolean recursionDesired,
int maxQueriesPerResolve,
boolean traceEnabled,
int maxPayloadSize,
boolean optResourceEnabled,
HostsFileEntriesResolver hostsFileEntriesResolver,
@ -358,7 +354,7 @@ public class DnsNameResolver extends InetNameResolver {
boolean decodeIdn) {
this(eventLoop, channelFactory, null, resolveCache, NoopDnsCnameCache.INSTANCE, authoritativeDnsServerCache,
dnsQueryLifecycleObserverFactory, queryTimeoutMillis, resolvedAddressTypes, recursionDesired,
maxQueriesPerResolve, traceEnabled, maxPayloadSize, optResourceEnabled, hostsFileEntriesResolver,
maxQueriesPerResolve, maxPayloadSize, optResourceEnabled, hostsFileEntriesResolver,
dnsServerAddressStreamProvider, searchDomains, ndots, decodeIdn, false);
}
@ -374,7 +370,6 @@ public class DnsNameResolver extends InetNameResolver {
ResolvedAddressTypes resolvedAddressTypes,
boolean recursionDesired,
int maxQueriesPerResolve,
boolean traceEnabled,
int maxPayloadSize,
boolean optResourceEnabled,
HostsFileEntriesResolver hostsFileEntriesResolver,
@ -397,11 +392,7 @@ public class DnsNameResolver extends InetNameResolver {
requireNonNull(dnsServerAddressStreamProvider, "dnsServerAddressStreamProvider");
this.resolveCache = requireNonNull(resolveCache, "resolveCache");
this.cnameCache = requireNonNull(cnameCache, "cnameCache");
this.dnsQueryLifecycleObserverFactory = traceEnabled ?
dnsQueryLifecycleObserverFactory instanceof NoopDnsQueryLifecycleObserverFactory ?
new TraceDnsQueryLifeCycleObserverFactory() :
new BiDnsQueryLifecycleObserverFactory(new TraceDnsQueryLifeCycleObserverFactory(),
dnsQueryLifecycleObserverFactory) :
this.dnsQueryLifecycleObserverFactory =
requireNonNull(dnsQueryLifecycleObserverFactory, "dnsQueryLifecycleObserverFactory");
this.searchDomains = searchDomains != null ? searchDomains.clone() : DEFAULT_SEARCH_DOMAINS;
this.ndots = ndots >= 0 ? ndots : DEFAULT_OPTIONS.ndots();

View File

@ -318,18 +318,6 @@ public final class DnsNameResolverBuilder {
return this;
}
/**
* Sets if this resolver should generate the detailed trace information in an exception message so that
* it is easier to understand the cause of resolution failure.
*
* @param traceEnabled true if trace is enabled
* @return {@code this}
*/
public DnsNameResolverBuilder traceEnabled(boolean traceEnabled) {
this.traceEnabled = traceEnabled;
return this;
}
/**
* Sets the capacity of the datagram packet buffer (in bytes). The default value is {@code 4096} bytes.
*
@ -482,7 +470,6 @@ public final class DnsNameResolverBuilder {
resolvedAddressTypes,
recursionDesired,
maxQueriesPerResolve,
traceEnabled,
maxPayloadSize,
optResourceEnabled,
hostsFileEntriesResolver,
@ -540,7 +527,6 @@ public final class DnsNameResolverBuilder {
copiedBuilder.resolvedAddressTypes(resolvedAddressTypes);
copiedBuilder.recursionDesired(recursionDesired);
copiedBuilder.maxQueriesPerResolve(maxQueriesPerResolve);
copiedBuilder.traceEnabled(traceEnabled);
copiedBuilder.maxPayloadSize(maxPayloadSize);
copiedBuilder.optResourceEnabled(optResourceEnabled);
copiedBuilder.hostsFileEntriesResolver(hostsFileEntriesResolver);

View File

@ -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 java.util.Objects.requireNonNull;
/**
* 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(requireNonNull(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(requireNonNull(name, "name"));
}
private static InternalLogLevel checkAndConvertLevel(LogLevel level) {
return requireNonNull(level, "level").toInternalLevel();
}
@Override
public DnsQueryLifecycleObserver newDnsQueryLifecycleObserver(DnsQuestion question) {
return new LoggingDnsQueryLifecycleObserver(question, logger, level);
}
}

View File

@ -26,13 +26,13 @@ import java.util.List;
import static java.util.Objects.requireNonNull;
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 = requireNonNull(question, "question");
this.logger = requireNonNull(logger, "logger");
this.level = requireNonNull(level, "level");

View File

@ -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 java.util.Objects.requireNonNull;
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 = requireNonNull(logger, "logger");
this.level = requireNonNull(level, "level");
}
@Override
public DnsQueryLifecycleObserver newDnsQueryLifecycleObserver(DnsQuestion question) {
return new TraceDnsQueryLifecycleObserver(question, logger, level);
}
}

View File

@ -922,7 +922,7 @@ public class DnsNameResolverTest {
group.next(), new ReflectiveChannelFactory<DatagramChannel>(NioDatagramChannel.class),
NoopDnsCache.INSTANCE, nsCache, NoopDnsQueryLifecycleObserverFactory.INSTANCE, 3000,
ipv4Preferred ? ResolvedAddressTypes.IPV4_ONLY : ResolvedAddressTypes.IPV6_ONLY, true,
10, true, 4096, false, HostsFileEntriesResolver.DEFAULT,
10, 4096, false, HostsFileEntriesResolver.DEFAULT,
new SequentialDnsServerAddressStreamProvider(dnsServer2.localAddress(), dnsServer3.localAddress()),
DnsNameResolver.DEFAULT_SEARCH_DOMAINS, 0, true) {
@Override
@ -1268,7 +1268,7 @@ public class DnsNameResolverTest {
final DnsNameResolver resolver = new DnsNameResolver(
group.next(), new ReflectiveChannelFactory<DatagramChannel>(NioDatagramChannel.class),
NoopDnsCache.INSTANCE, nsCache, lifecycleObserverFactory, 3000, ResolvedAddressTypes.IPV4_ONLY, true,
10, true, 4096, false, HostsFileEntriesResolver.DEFAULT,
10, 4096, false, HostsFileEntriesResolver.DEFAULT,
new SingletonDnsServerAddressStreamProvider(dnsServer.localAddress()),
DnsNameResolver.DEFAULT_SEARCH_DOMAINS, 0, true) {
@Override
@ -1425,7 +1425,7 @@ public class DnsNameResolverTest {
final DnsNameResolver resolver = new DnsNameResolver(
group.next(), new ReflectiveChannelFactory<DatagramChannel>(NioDatagramChannel.class),
cache, authoritativeDnsServerCache, NoopDnsQueryLifecycleObserverFactory.INSTANCE, 2000,
ResolvedAddressTypes.IPV4_ONLY, true, 10, true, 4096,
ResolvedAddressTypes.IPV4_ONLY, true, 10, 4096,
false, HostsFileEntriesResolver.DEFAULT,
new SingletonDnsServerAddressStreamProvider(redirectServer.localAddress()),
DnsNameResolver.DEFAULT_SEARCH_DOMAINS, 0, true) {
@ -1587,7 +1587,7 @@ public class DnsNameResolverTest {
group.next(), new ReflectiveChannelFactory<DatagramChannel>(NioDatagramChannel.class),
NoopDnsCache.INSTANCE, authoritativeDnsServerCache,
NoopDnsQueryLifecycleObserverFactory.INSTANCE, 2000, ResolvedAddressTypes.IPV4_ONLY,
true, 10, true, 4096,
true, 10, 4096,
false, HostsFileEntriesResolver.DEFAULT,
new SingletonDnsServerAddressStreamProvider(redirectServer.localAddress()),
DnsNameResolver.DEFAULT_SEARCH_DOMAINS, 0, true) {
@ -1724,7 +1724,7 @@ public class DnsNameResolverTest {
loop, new ReflectiveChannelFactory<DatagramChannel>(NioDatagramChannel.class),
cache, authoritativeDnsServerCache,
NoopDnsQueryLifecycleObserverFactory.INSTANCE, 2000, ResolvedAddressTypes.IPV4_ONLY,
true, 10, true, 4096,
true, 10, 4096,
false, HostsFileEntriesResolver.DEFAULT,
new SingletonDnsServerAddressStreamProvider(redirectServer.localAddress()),
DnsNameResolver.DEFAULT_SEARCH_DOMAINS, 0, true) {
@ -2532,7 +2532,6 @@ public class DnsNameResolverTest {
null, // resolvedAddressTypes, see https://github.com/netty/netty/pull/8445
true, // recursionDesired
1, // maxQueriesPerResolve
false, // traceEnabled
4096, // maxPayloadSize
true, // optResourceEnabled
HostsFileEntriesResolver.DEFAULT, // hostsFileEntriesResolver