Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

api, core: make channel logger accessible through NameResolver.Args #6430

Merged
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
35 changes: 34 additions & 1 deletion api/src/main/java/io/grpc/NameResolver.java
Expand Up @@ -412,18 +412,21 @@ 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;

Args(
Integer defaultPort,
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;
}

Expand Down Expand Up @@ -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.
Expand All @@ -485,6 +501,7 @@ public String toString() {
.add("proxyDetector", proxyDetector)
.add("syncContext", syncContext)
.add("serviceConfigParser", serviceConfigParser)
.add("channelLogger", channelLogger)
.add("executor", executor)
.toString();
}
Expand All @@ -500,6 +517,7 @@ public Builder toBuilder() {
builder.setProxyDetector(proxyDetector);
builder.setSynchronizationContext(syncContext);
builder.setServiceConfigParser(serviceConfigParser);
builder.setChannelLogger(channelLogger);
builder.setOffloadExecutor(executor);
return builder;
}
Expand All @@ -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() {
Expand Down Expand Up @@ -568,6 +587,17 @@ public Builder setServiceConfigParser(ServiceConfigParser parser) {
return this;
}

/**
* See {@link Args#getChannelLogger}. This is a required field.
*
* @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.
*
Expand All @@ -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);
}
}
}
Expand Down
1 change: 1 addition & 0 deletions api/src/test/java/io/grpc/NameResolverRegistryTest.java
Expand Up @@ -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
Expand Down
4 changes: 4 additions & 0 deletions api/src/test/java/io/grpc/NameResolverTest.java
Expand Up @@ -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);
Expand All @@ -61,13 +62,15 @@ 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();
assertThat(args2.getDefaultPort()).isEqualTo(defaultPort);
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);
Expand Down Expand Up @@ -251,6 +254,7 @@ private NameResolver.Args createArgs() {
.setProxyDetector(proxyDetector)
.setSynchronizationContext(syncContext)
.setServiceConfigParser(parser)
.setChannelLogger(channelLogger)
.setOffloadExecutor(executor)
.build();
}
Expand Down
13 changes: 7 additions & 6 deletions core/src/main/java/io/grpc/internal/ManagedChannelImpl.java
Expand Up @@ -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;
Expand All @@ -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() {
Expand All @@ -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);
Expand Down
Expand Up @@ -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;
Expand All @@ -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();
Expand Down
4 changes: 4 additions & 0 deletions core/src/test/java/io/grpc/internal/DnsNameResolverTest.java
Expand Up @@ -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;
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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);
}
Expand Down Expand Up @@ -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
Expand Down
Expand Up @@ -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;
Expand All @@ -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
Expand Down
Expand Up @@ -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;
Expand All @@ -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
Expand Down
Expand Up @@ -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;
Expand All @@ -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();
Expand Down
2 changes: 2 additions & 0 deletions xds/src/test/java/io/grpc/xds/XdsNameResolverTest.java
Expand Up @@ -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;
Expand Down Expand Up @@ -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();
Expand Down