From 15ac29c8b96b70ad98077efbe32b44a5c62df026 Mon Sep 17 00:00:00 2001 From: Chengyuan Zhang Date: Mon, 18 Nov 2019 16:33:39 -0800 Subject: [PATCH] api, core: make channel logger accessible through NameResolver.Args (#6430) --- api/src/main/java/io/grpc/NameResolver.java | 35 ++++++++++++++++++- .../io/grpc/NameResolverRegistryTest.java | 1 + .../test/java/io/grpc/NameResolverTest.java | 4 +++ .../io/grpc/internal/ManagedChannelImpl.java | 13 +++---- .../internal/DnsNameResolverProviderTest.java | 2 ++ .../io/grpc/internal/DnsNameResolverTest.java | 4 +++ ...ManagedChannelImplGetNameResolverTest.java | 2 ++ .../OverrideAuthorityNameResolverTest.java | 2 ++ .../grpc/xds/XdsNameResolverProviderTest.java | 2 ++ .../java/io/grpc/xds/XdsNameResolverTest.java | 2 ++ 10 files changed, 60 insertions(+), 7 deletions(-) diff --git a/api/src/main/java/io/grpc/NameResolver.java b/api/src/main/java/io/grpc/NameResolver.java index 6e36db64a2de..fd7302658c5f 100644 --- a/api/src/main/java/io/grpc/NameResolver.java +++ b/api/src/main/java/io/grpc/NameResolver.java @@ -412,6 +412,7 @@ public static final class Args { private final ProxyDetector proxyDetector; private final SynchronizationContext syncContext; private final ServiceConfigParser serviceConfigParser; + @Nullable private final ChannelLogger channelLogger; @Nullable private final Executor executor; private Args( @@ -419,11 +420,13 @@ private Args( ProxyDetector proxyDetector, SynchronizationContext syncContext, ServiceConfigParser serviceConfigParser, + @Nullable ChannelLogger channelLogger, @Nullable Executor executor) { this.defaultPort = checkNotNull(defaultPort, "defaultPort not set"); this.proxyDetector = checkNotNull(proxyDetector, "proxyDetector not set"); this.syncContext = checkNotNull(syncContext, "syncContext not set"); this.serviceConfigParser = checkNotNull(serviceConfigParser, "serviceConfigParser not set"); + this.channelLogger = channelLogger; this.executor = executor; } @@ -466,6 +469,19 @@ public ServiceConfigParser getServiceConfigParser() { return serviceConfigParser; } + /** + * Returns the {@link ChannelLogger} for the Channel served by this NameResolver. + * + * @since 1.26.0 + */ + @ExperimentalApi("https://github.com/grpc/grpc-java/issues/6438") + public ChannelLogger getChannelLogger() { + if (channelLogger == null) { + throw new IllegalStateException("ChannelLogger is not set in Builder"); + } + return channelLogger; + } + /** * Returns the Executor on which this resolver should execute long-running or I/O bound work. * Null if no Executor was set. @@ -485,6 +501,7 @@ public String toString() { .add("proxyDetector", proxyDetector) .add("syncContext", syncContext) .add("serviceConfigParser", serviceConfigParser) + .add("channelLogger", channelLogger) .add("executor", executor) .toString(); } @@ -500,6 +517,7 @@ public Builder toBuilder() { builder.setProxyDetector(proxyDetector); builder.setSynchronizationContext(syncContext); builder.setServiceConfigParser(serviceConfigParser); + builder.setChannelLogger(channelLogger); builder.setOffloadExecutor(executor); return builder; } @@ -523,6 +541,7 @@ public static final class Builder { private ProxyDetector proxyDetector; private SynchronizationContext syncContext; private ServiceConfigParser serviceConfigParser; + private ChannelLogger channelLogger; private Executor executor; Builder() { @@ -568,6 +587,17 @@ public Builder setServiceConfigParser(ServiceConfigParser parser) { return this; } + /** + * See {@link Args#getChannelLogger}. + * + * @since 1.26.0 + */ + @ExperimentalApi("https://github.com/grpc/grpc-java/issues/6438") + public Builder setChannelLogger(ChannelLogger channelLogger) { + this.channelLogger = checkNotNull(channelLogger); + return this; + } + /** * See {@link Args#getOffloadExecutor}. This is an optional field. * @@ -585,7 +615,10 @@ public Builder setOffloadExecutor(Executor executor) { * @since 1.21.0 */ public Args build() { - return new Args(defaultPort, proxyDetector, syncContext, serviceConfigParser, executor); + return + new Args( + defaultPort, proxyDetector, syncContext, serviceConfigParser, + channelLogger, executor); } } } diff --git a/api/src/test/java/io/grpc/NameResolverRegistryTest.java b/api/src/test/java/io/grpc/NameResolverRegistryTest.java index abbe10af2aca..f35f2f00b93b 100644 --- a/api/src/test/java/io/grpc/NameResolverRegistryTest.java +++ b/api/src/test/java/io/grpc/NameResolverRegistryTest.java @@ -39,6 +39,7 @@ public class NameResolverRegistryTest { .setProxyDetector(mock(ProxyDetector.class)) .setSynchronizationContext(new SynchronizationContext(mock(UncaughtExceptionHandler.class))) .setServiceConfigParser(mock(ServiceConfigParser.class)) + .setChannelLogger(mock(ChannelLogger.class)) .build(); @Test diff --git a/api/src/test/java/io/grpc/NameResolverTest.java b/api/src/test/java/io/grpc/NameResolverTest.java index 3206f4be4ae5..828047443a4e 100644 --- a/api/src/test/java/io/grpc/NameResolverTest.java +++ b/api/src/test/java/io/grpc/NameResolverTest.java @@ -45,6 +45,7 @@ public class NameResolverTest { private final SynchronizationContext syncContext = new SynchronizationContext(mock(UncaughtExceptionHandler.class)); private final ServiceConfigParser parser = mock(ServiceConfigParser.class); + private final ChannelLogger channelLogger = mock(ChannelLogger.class); private final Executor executor = Executors.newSingleThreadExecutor(); private URI uri; private final NameResolver nameResolver = mock(NameResolver.class); @@ -61,6 +62,7 @@ public void args() { assertThat(args.getProxyDetector()).isSameInstanceAs(proxyDetector); assertThat(args.getSynchronizationContext()).isSameInstanceAs(syncContext); assertThat(args.getServiceConfigParser()).isSameInstanceAs(parser); + assertThat(args.getChannelLogger()).isSameInstanceAs(channelLogger); assertThat(args.getOffloadExecutor()).isSameInstanceAs(executor); NameResolver.Args args2 = args.toBuilder().build(); @@ -68,6 +70,7 @@ public void args() { assertThat(args2.getProxyDetector()).isSameInstanceAs(proxyDetector); assertThat(args2.getSynchronizationContext()).isSameInstanceAs(syncContext); assertThat(args2.getServiceConfigParser()).isSameInstanceAs(parser); + assertThat(args2.getChannelLogger()).isSameInstanceAs(channelLogger); assertThat(args2.getOffloadExecutor()).isSameInstanceAs(executor); assertThat(args2).isNotSameInstanceAs(args); @@ -251,6 +254,7 @@ private NameResolver.Args createArgs() { .setProxyDetector(proxyDetector) .setSynchronizationContext(syncContext) .setServiceConfigParser(parser) + .setChannelLogger(channelLogger) .setOffloadExecutor(executor) .build(); } diff --git a/core/src/main/java/io/grpc/internal/ManagedChannelImpl.java b/core/src/main/java/io/grpc/internal/ManagedChannelImpl.java index 560f8cba03b9..de3a472501c6 100644 --- a/core/src/main/java/io/grpc/internal/ManagedChannelImpl.java +++ b/core/src/main/java/io/grpc/internal/ManagedChannelImpl.java @@ -561,6 +561,12 @@ ClientStream newSubstream(ClientStreamTracer.Factory tracerFactory, Metadata new final TimeProvider timeProvider) { this.target = checkNotNull(builder.target, "target"); this.logId = InternalLogId.allocate("Channel", target); + this.timeProvider = checkNotNull(timeProvider, "timeProvider"); + maxTraceEvents = builder.maxTraceEvents; + channelTracer = new ChannelTracer( + logId, builder.maxTraceEvents, timeProvider.currentTimeNanos(), + "Channel for '" + target + "'"); + channelLogger = new ChannelLoggerImpl(channelTracer, timeProvider); this.nameResolverFactory = builder.getNameResolverFactory(); ProxyDetector proxyDetector = builder.proxyDetector != null ? builder.proxyDetector : GrpcUtil.DEFAULT_PROXY_DETECTOR; @@ -581,6 +587,7 @@ ClientStream newSubstream(ClientStreamTracer.Factory tracerFactory, Metadata new builder.maxRetryAttempts, builder.maxHedgedAttempts, loadBalancerFactory)) + .setChannelLogger(channelLogger) .setOffloadExecutor( // Avoid creating the offloadExecutor until it is first used new Executor() { @@ -591,12 +598,6 @@ public void execute(Runnable command) { }) .build(); this.nameResolver = getNameResolver(target, nameResolverFactory, nameResolverArgs); - this.timeProvider = checkNotNull(timeProvider, "timeProvider"); - maxTraceEvents = builder.maxTraceEvents; - channelTracer = new ChannelTracer( - logId, builder.maxTraceEvents, timeProvider.currentTimeNanos(), - "Channel for '" + target + "'"); - channelLogger = new ChannelLoggerImpl(channelTracer, timeProvider); this.executorPool = checkNotNull(builder.executorPool, "executorPool"); this.balancerRpcExecutorPool = checkNotNull(balancerRpcExecutorPool, "balancerRpcExecutorPool"); this.balancerRpcExecutorHolder = new ExecutorHolder(balancerRpcExecutorPool); diff --git a/core/src/test/java/io/grpc/internal/DnsNameResolverProviderTest.java b/core/src/test/java/io/grpc/internal/DnsNameResolverProviderTest.java index 3f5df9bdbbbb..fa52a7d8511d 100644 --- a/core/src/test/java/io/grpc/internal/DnsNameResolverProviderTest.java +++ b/core/src/test/java/io/grpc/internal/DnsNameResolverProviderTest.java @@ -24,6 +24,7 @@ import static org.junit.Assume.assumeTrue; import static org.mockito.Mockito.mock; +import io.grpc.ChannelLogger; import io.grpc.NameResolver; import io.grpc.NameResolver.ServiceConfigParser; import io.grpc.SynchronizationContext; @@ -47,6 +48,7 @@ public void uncaughtException(Thread t, Throwable e) { .setProxyDetector(GrpcUtil.DEFAULT_PROXY_DETECTOR) .setSynchronizationContext(syncContext) .setServiceConfigParser(mock(ServiceConfigParser.class)) + .setChannelLogger(mock(ChannelLogger.class)) .build(); private DnsNameResolverProvider provider = new DnsNameResolverProvider(); diff --git a/core/src/test/java/io/grpc/internal/DnsNameResolverTest.java b/core/src/test/java/io/grpc/internal/DnsNameResolverTest.java index 4edc2473daca..192d03d33412 100644 --- a/core/src/test/java/io/grpc/internal/DnsNameResolverTest.java +++ b/core/src/test/java/io/grpc/internal/DnsNameResolverTest.java @@ -36,6 +36,7 @@ import com.google.common.collect.Iterables; import com.google.common.net.InetAddresses; import com.google.common.testing.FakeTicker; +import io.grpc.ChannelLogger; import io.grpc.EquivalentAddressGroup; import io.grpc.HttpConnectProxiedSocketAddress; import io.grpc.NameResolver; @@ -115,6 +116,7 @@ public void uncaughtException(Thread t, Throwable e) { .setProxyDetector(GrpcUtil.DEFAULT_PROXY_DETECTOR) .setSynchronizationContext(syncContext) .setServiceConfigParser(mock(ServiceConfigParser.class)) + .setChannelLogger(mock(ChannelLogger.class)) .build(); private final DnsNameResolverProvider provider = new DnsNameResolverProvider(); @@ -175,6 +177,7 @@ private DnsNameResolver newResolver( .setProxyDetector(proxyDetector) .setSynchronizationContext(syncContext) .setServiceConfigParser(mock(ServiceConfigParser.class)) + .setChannelLogger(mock(ChannelLogger.class)) .build(); return newResolver(name, stopwatch, isAndroid, args); } @@ -331,6 +334,7 @@ public void testExecutor_custom() throws Exception { .setProxyDetector(GrpcUtil.NOOP_PROXY_DETECTOR) .setSynchronizationContext(syncContext) .setServiceConfigParser(mock(ServiceConfigParser.class)) + .setChannelLogger(mock(ChannelLogger.class)) .setOffloadExecutor( new Executor() { @Override diff --git a/core/src/test/java/io/grpc/internal/ManagedChannelImplGetNameResolverTest.java b/core/src/test/java/io/grpc/internal/ManagedChannelImplGetNameResolverTest.java index c620ce20dcbe..c1d84eb76bf9 100644 --- a/core/src/test/java/io/grpc/internal/ManagedChannelImplGetNameResolverTest.java +++ b/core/src/test/java/io/grpc/internal/ManagedChannelImplGetNameResolverTest.java @@ -21,6 +21,7 @@ import static org.junit.Assert.fail; import static org.mockito.Mockito.mock; +import io.grpc.ChannelLogger; import io.grpc.NameResolver; import io.grpc.NameResolver.Factory; import io.grpc.NameResolver.ServiceConfigParser; @@ -40,6 +41,7 @@ public class ManagedChannelImplGetNameResolverTest { .setProxyDetector(mock(ProxyDetector.class)) .setSynchronizationContext(new SynchronizationContext(mock(UncaughtExceptionHandler.class))) .setServiceConfigParser(mock(ServiceConfigParser.class)) + .setChannelLogger(mock(ChannelLogger.class)) .build(); @Test diff --git a/core/src/test/java/io/grpc/internal/OverrideAuthorityNameResolverTest.java b/core/src/test/java/io/grpc/internal/OverrideAuthorityNameResolverTest.java index 58b17d52fae5..8d23ce3f884d 100644 --- a/core/src/test/java/io/grpc/internal/OverrideAuthorityNameResolverTest.java +++ b/core/src/test/java/io/grpc/internal/OverrideAuthorityNameResolverTest.java @@ -23,6 +23,7 @@ import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; +import io.grpc.ChannelLogger; import io.grpc.NameResolver; import io.grpc.NameResolver.ServiceConfigParser; import io.grpc.ProxyDetector; @@ -41,6 +42,7 @@ public class OverrideAuthorityNameResolverTest { .setProxyDetector(mock(ProxyDetector.class)) .setSynchronizationContext(new SynchronizationContext(mock(UncaughtExceptionHandler.class))) .setServiceConfigParser(mock(ServiceConfigParser.class)) + .setChannelLogger(mock(ChannelLogger.class)) .build(); @Test diff --git a/xds/src/test/java/io/grpc/xds/XdsNameResolverProviderTest.java b/xds/src/test/java/io/grpc/xds/XdsNameResolverProviderTest.java index 549c3c66a8ff..275cf7207393 100644 --- a/xds/src/test/java/io/grpc/xds/XdsNameResolverProviderTest.java +++ b/xds/src/test/java/io/grpc/xds/XdsNameResolverProviderTest.java @@ -20,6 +20,7 @@ import static org.junit.Assert.fail; import static org.mockito.Mockito.mock; +import io.grpc.ChannelLogger; import io.grpc.InternalServiceProviders; import io.grpc.NameResolver; import io.grpc.NameResolver.ServiceConfigParser; @@ -46,6 +47,7 @@ public void uncaughtException(Thread t, Throwable e) { .setProxyDetector(GrpcUtil.NOOP_PROXY_DETECTOR) .setSynchronizationContext(syncContext) .setServiceConfigParser(mock(ServiceConfigParser.class)) + .setChannelLogger(mock(ChannelLogger.class)) .build(); private XdsNameResolverProvider provider = new XdsNameResolverProvider(); diff --git a/xds/src/test/java/io/grpc/xds/XdsNameResolverTest.java b/xds/src/test/java/io/grpc/xds/XdsNameResolverTest.java index d5811067fd03..d3efc826a5f6 100644 --- a/xds/src/test/java/io/grpc/xds/XdsNameResolverTest.java +++ b/xds/src/test/java/io/grpc/xds/XdsNameResolverTest.java @@ -24,6 +24,7 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.Iterables; import io.envoyproxy.envoy.api.v2.core.Node; +import io.grpc.ChannelLogger; import io.grpc.NameResolver; import io.grpc.NameResolver.ResolutionResult; import io.grpc.NameResolver.ServiceConfigParser; @@ -69,6 +70,7 @@ public void uncaughtException(Thread t, Throwable e) { .setProxyDetector(GrpcUtil.NOOP_PROXY_DETECTOR) .setSynchronizationContext(syncContext) .setServiceConfigParser(mock(ServiceConfigParser.class)) + .setChannelLogger(mock(ChannelLogger.class)) .build(); private final XdsNameResolverProvider provider = new XdsNameResolverProvider();