From 119b6d193f141c57f75d5faec59b2279f0e7fd1e Mon Sep 17 00:00:00 2001 From: mohanli-ml Date: Thu, 14 Oct 2021 23:52:36 +0000 Subject: [PATCH 1/9] feat: add google-c2p dependence to DirectPath --- build.gradle | 3 +- gax-grpc/build.gradle | 3 +- .../InstantiatingGrpcChannelProvider.java | 29 ++++++++++++++----- 3 files changed, 25 insertions(+), 10 deletions(-) diff --git a/build.gradle b/build.gradle index 22b115b91..e494e5642 100644 --- a/build.gradle +++ b/build.gradle @@ -133,7 +133,8 @@ subprojects { 'maven.io_grpc_grpc_netty_shaded': "io.grpc:grpc-netty-shaded:${libraries['version.io_grpc']}", 'maven.io_grpc_grpc_alts': "io.grpc:grpc-alts:${libraries['version.io_grpc']}", 'maven.com_google_protobuf': "com.google.protobuf:protobuf-java:${libraries['version.com_google_protobuf']}", - 'maven.com_google_protobuf_java_util': "com.google.protobuf:protobuf-java-util:${libraries['version.com_google_protobuf']}" + 'maven.com_google_protobuf_java_util': "com.google.protobuf:protobuf-java-util:${libraries['version.com_google_protobuf']}", + 'maven.io_grpc_grpc_xds': "io.grpc:grpc-xds:${libraries['version.io_grpc']}" ]) } diff --git a/gax-grpc/build.gradle b/gax-grpc/build.gradle index bed13b8cf..9135473e5 100644 --- a/gax-grpc/build.gradle +++ b/gax-grpc/build.gradle @@ -16,7 +16,8 @@ dependencies { libraries['maven.com_google_api_grpc_proto_google_common_protos'], libraries['maven.com_google_api_api_common'], libraries['maven.io_grpc_grpc_netty_shaded'], - libraries['maven.io_grpc_grpc_alts'] + libraries['maven.io_grpc_grpc_alts'], + libraries['maven.io_grpc_grpc_xds'] compileOnly libraries['maven.com_google_auto_value_auto_value'] diff --git a/gax-grpc/src/main/java/com/google/api/gax/grpc/InstantiatingGrpcChannelProvider.java b/gax-grpc/src/main/java/com/google/api/gax/grpc/InstantiatingGrpcChannelProvider.java index 225d76d6d..b42be78ef 100644 --- a/gax-grpc/src/main/java/com/google/api/gax/grpc/InstantiatingGrpcChannelProvider.java +++ b/gax-grpc/src/main/java/com/google/api/gax/grpc/InstantiatingGrpcChannelProvider.java @@ -83,6 +83,7 @@ public final class InstantiatingGrpcChannelProvider implements TransportChannelP static final String DIRECT_PATH_ENV_VAR = "GOOGLE_CLOUD_ENABLE_DIRECT_PATH"; private static final String DIRECT_PATH_ENV_DISABLE_DIRECT_PATH = "GOOGLE_CLOUD_DISABLE_DIRECT_PATH"; + private static final String DIRECT_PATH_ENABLE_XDS = "GOOGLE_CLOUD_ENABLE_DIRECT_PATH_XDS"; static final long DIRECT_PATH_KEEP_ALIVE_TIME_SECONDS = 3600; static final long DIRECT_PATH_KEEP_ALIVE_TIMEOUT_SECONDS = 20; // reduce the thundering herd problem of too many channels trying to (re)connect at the same time @@ -331,15 +332,22 @@ private ManagedChannel createSingleChannel() throws IOException, GeneralSecurity ManagedChannelBuilder builder; // TODO(weiranf): Add API in ComputeEngineCredentials to check default service account. + String directPathXdsEnv = envProvider.getenv(DIRECT_PATH_ENABLE_XDS); + boolean isDirectPathXdsEnabled = Boolean.parseBoolean(directPathXdsEnv); if (isDirectPathEnabled(serviceAddress) && isNonDefaultServiceAccountAllowed() && isOnComputeEngine()) { - builder = ComputeEngineChannelBuilder.forAddress(serviceAddress, port); - // Set default keepAliveTime and keepAliveTimeout when directpath environment is enabled. - // Will be overridden by user defined values if any. - builder.keepAliveTime(DIRECT_PATH_KEEP_ALIVE_TIME_SECONDS, TimeUnit.SECONDS); - builder.keepAliveTimeout(DIRECT_PATH_KEEP_ALIVE_TIMEOUT_SECONDS, TimeUnit.SECONDS); - builder.defaultServiceConfig(directPathServiceConfig); + if (isDirectPathXdsEnabled) { + // google-c2p resolver target must not have a port number + builder = ComputeEngineChannelBuilder.forTarget("google-c2p:///" + serviceAddress); + } else { + builder = ComputeEngineChannelBuilder.forAddress(serviceAddress, port); + // Set default keepAliveTime and keepAliveTimeout when directpath environment is enabled. + // Will be overridden by user defined values if any. + builder.keepAliveTime(DIRECT_PATH_KEEP_ALIVE_TIME_SECONDS, TimeUnit.SECONDS); + builder.keepAliveTimeout(DIRECT_PATH_KEEP_ALIVE_TIMEOUT_SECONDS, TimeUnit.SECONDS); + builder.defaultServiceConfig(directPathServiceConfig); + } } else { ChannelCredentials channelCredentials = createMtlsChannelCredentials(); if (channelCredentials != null) { @@ -348,10 +356,15 @@ && isOnComputeEngine()) { builder = ManagedChannelBuilder.forAddress(serviceAddress, port); } } + // google-c2p resolver requires service config lookup + if (!isDirectPathXdsEnabled) { + builder = + builder + // See https://github.com/googleapis/gapic-generator/issues/2816 + .disableServiceConfigLookUp(); + } builder = builder - // See https://github.com/googleapis/gapic-generator/issues/2816 - .disableServiceConfigLookUp() .intercept(new GrpcChannelUUIDInterceptor()) .intercept(headerInterceptor) .intercept(metadataHandlerInterceptor) From ce1227a83ff81aaf0458ed7924efd46c5f155dc8 Mon Sep 17 00:00:00 2001 From: mohanli-ml Date: Fri, 15 Oct 2021 03:55:23 +0000 Subject: [PATCH 2/9] feat: add google-c2p dependence to DirectPath --- .../api/gax/grpc/InstantiatingGrpcChannelProvider.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/gax-grpc/src/main/java/com/google/api/gax/grpc/InstantiatingGrpcChannelProvider.java b/gax-grpc/src/main/java/com/google/api/gax/grpc/InstantiatingGrpcChannelProvider.java index b42be78ef..6c47bf31e 100644 --- a/gax-grpc/src/main/java/com/google/api/gax/grpc/InstantiatingGrpcChannelProvider.java +++ b/gax-grpc/src/main/java/com/google/api/gax/grpc/InstantiatingGrpcChannelProvider.java @@ -331,12 +331,12 @@ private ManagedChannel createSingleChannel() throws IOException, GeneralSecurity ManagedChannelBuilder builder; - // TODO(weiranf): Add API in ComputeEngineCredentials to check default service account. - String directPathXdsEnv = envProvider.getenv(DIRECT_PATH_ENABLE_XDS); - boolean isDirectPathXdsEnabled = Boolean.parseBoolean(directPathXdsEnv); + // Check DirectPath traffic. + boolean isDirectPathXdsEnabled = false; if (isDirectPathEnabled(serviceAddress) && isNonDefaultServiceAccountAllowed() && isOnComputeEngine()) { + isDirectPathXdsEnabled = Boolean.parseBoolean(envProvider.getenv(DIRECT_PATH_ENABLE_XDS)); if (isDirectPathXdsEnabled) { // google-c2p resolver target must not have a port number builder = ComputeEngineChannelBuilder.forTarget("google-c2p:///" + serviceAddress); From d8188d315c75c067d251a01859b47501aa05e98b Mon Sep 17 00:00:00 2001 From: mohanli-ml Date: Fri, 15 Oct 2021 23:42:30 +0000 Subject: [PATCH 3/9] feat: add google-c2p dependence to DirectPath --- .../api/gax/grpc/InstantiatingGrpcChannelProvider.java | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/gax-grpc/src/main/java/com/google/api/gax/grpc/InstantiatingGrpcChannelProvider.java b/gax-grpc/src/main/java/com/google/api/gax/grpc/InstantiatingGrpcChannelProvider.java index 6c47bf31e..963830494 100644 --- a/gax-grpc/src/main/java/com/google/api/gax/grpc/InstantiatingGrpcChannelProvider.java +++ b/gax-grpc/src/main/java/com/google/api/gax/grpc/InstantiatingGrpcChannelProvider.java @@ -83,7 +83,7 @@ public final class InstantiatingGrpcChannelProvider implements TransportChannelP static final String DIRECT_PATH_ENV_VAR = "GOOGLE_CLOUD_ENABLE_DIRECT_PATH"; private static final String DIRECT_PATH_ENV_DISABLE_DIRECT_PATH = "GOOGLE_CLOUD_DISABLE_DIRECT_PATH"; - private static final String DIRECT_PATH_ENABLE_XDS = "GOOGLE_CLOUD_ENABLE_DIRECT_PATH_XDS"; + private static final String DIRECT_PATH_ENV_ENABLE_XDS = "GOOGLE_CLOUD_ENABLE_DIRECT_PATH_XDS"; static final long DIRECT_PATH_KEEP_ALIVE_TIME_SECONDS = 3600; static final long DIRECT_PATH_KEEP_ALIVE_TIMEOUT_SECONDS = 20; // reduce the thundering herd problem of too many channels trying to (re)connect at the same time @@ -336,7 +336,7 @@ private ManagedChannel createSingleChannel() throws IOException, GeneralSecurity if (isDirectPathEnabled(serviceAddress) && isNonDefaultServiceAccountAllowed() && isOnComputeEngine()) { - isDirectPathXdsEnabled = Boolean.parseBoolean(envProvider.getenv(DIRECT_PATH_ENABLE_XDS)); + isDirectPathXdsEnabled = Boolean.parseBoolean(envProvider.getenv(DIRECT_PATH_ENV_ENABLE_XDS)); if (isDirectPathXdsEnabled) { // google-c2p resolver target must not have a port number builder = ComputeEngineChannelBuilder.forTarget("google-c2p:///" + serviceAddress); @@ -358,10 +358,8 @@ && isOnComputeEngine()) { } // google-c2p resolver requires service config lookup if (!isDirectPathXdsEnabled) { - builder = - builder - // See https://github.com/googleapis/gapic-generator/issues/2816 - .disableServiceConfigLookUp(); + // See https://github.com/googleapis/gapic-generator/issues/2816 + builder.disableServiceConfigLookUp(); } builder = builder From 8bfcd101e6638161e04bf6a7347f88f43fb50278 Mon Sep 17 00:00:00 2001 From: mohanli-ml Date: Tue, 19 Oct 2021 04:13:49 +0000 Subject: [PATCH 4/9] feat: add google-c2p dependence to DirectPath --- .../InstantiatingGrpcChannelProvider.java | 26 +++++++++--- .../InstantiatingGrpcChannelProviderTest.java | 41 +++++++++++++++++++ 2 files changed, 61 insertions(+), 6 deletions(-) diff --git a/gax-grpc/src/main/java/com/google/api/gax/grpc/InstantiatingGrpcChannelProvider.java b/gax-grpc/src/main/java/com/google/api/gax/grpc/InstantiatingGrpcChannelProvider.java index 963830494..3451609a4 100644 --- a/gax-grpc/src/main/java/com/google/api/gax/grpc/InstantiatingGrpcChannelProvider.java +++ b/gax-grpc/src/main/java/com/google/api/gax/grpc/InstantiatingGrpcChannelProvider.java @@ -109,6 +109,7 @@ public final class InstantiatingGrpcChannelProvider implements TransportChannelP @Nullable private final Boolean allowNonDefaultServiceAccount; @VisibleForTesting final ImmutableMap directPathServiceConfig; @Nullable private final MtlsProvider mtlsProvider; + @Nullable @VisibleForTesting private String activeEndpoint; @Nullable private final ApiFunction channelConfigurator; @@ -136,6 +137,7 @@ private InstantiatingGrpcChannelProvider(Builder builder) { builder.directPathServiceConfig == null ? getDefaultDirectPathServiceConfig() : builder.directPathServiceConfig; + this.activeEndpoint = builder.endpoint; } /** @@ -339,15 +341,16 @@ && isOnComputeEngine()) { isDirectPathXdsEnabled = Boolean.parseBoolean(envProvider.getenv(DIRECT_PATH_ENV_ENABLE_XDS)); if (isDirectPathXdsEnabled) { // google-c2p resolver target must not have a port number - builder = ComputeEngineChannelBuilder.forTarget("google-c2p:///" + serviceAddress); + activeEndpoint = "google-c2p:///" + serviceAddress; + builder = ComputeEngineChannelBuilder.forTarget(activeEndpoint); } else { builder = ComputeEngineChannelBuilder.forAddress(serviceAddress, port); - // Set default keepAliveTime and keepAliveTimeout when directpath environment is enabled. - // Will be overridden by user defined values if any. - builder.keepAliveTime(DIRECT_PATH_KEEP_ALIVE_TIME_SECONDS, TimeUnit.SECONDS); - builder.keepAliveTimeout(DIRECT_PATH_KEEP_ALIVE_TIMEOUT_SECONDS, TimeUnit.SECONDS); builder.defaultServiceConfig(directPathServiceConfig); } + // Set default keepAliveTime and keepAliveTimeout when directpath environment is enabled. + // Will be overridden by user defined values if any. + builder.keepAliveTime(DIRECT_PATH_KEEP_ALIVE_TIME_SECONDS, TimeUnit.SECONDS); + builder.keepAliveTimeout(DIRECT_PATH_KEEP_ALIVE_TIMEOUT_SECONDS, TimeUnit.SECONDS); } else { ChannelCredentials channelCredentials = createMtlsChannelCredentials(); if (channelCredentials != null) { @@ -398,11 +401,16 @@ && isOnComputeEngine()) { return managedChannel; } - /** The endpoint to be used for the channel. */ + /** The endpoint passed to the builder. It must have a pot, and must not have a URI scheme. */ public String getEndpoint() { return endpoint; } + /** The actual endpoint to be used for building a channel. */ + public String getActiveEndpoint() { + return activeEndpoint; + } + /** The time without read activity before sending a keepalive ping. */ public Duration getKeepAliveTime() { return keepAliveTime; @@ -535,6 +543,12 @@ Builder setMtlsProvider(MtlsProvider mtlsProvider) { return this; } + @VisibleForTesting + Builder setEnvProvider(EnvironmentProvider envProvider) { + this.envProvider = envProvider; + return this; + } + /** * Sets the GrpcInterceptorProvider for this TransportChannelProvider. * diff --git a/gax-grpc/src/test/java/com/google/api/gax/grpc/InstantiatingGrpcChannelProviderTest.java b/gax-grpc/src/test/java/com/google/api/gax/grpc/InstantiatingGrpcChannelProviderTest.java index 72f3cc464..8476df890 100644 --- a/gax-grpc/src/test/java/com/google/api/gax/grpc/InstantiatingGrpcChannelProviderTest.java +++ b/gax-grpc/src/test/java/com/google/api/gax/grpc/InstantiatingGrpcChannelProviderTest.java @@ -67,6 +67,23 @@ @RunWith(JUnit4.class) public class InstantiatingGrpcChannelProviderTest extends AbstractMtlsTransportChannelTest { + static class TestEnvironmentProvider + implements InstantiatingGrpcChannelProvider.EnvironmentProvider { + private final String isDirectPathXdsEnabled; + + TestEnvironmentProvider(String isDirectPathXdsEnabled) { + this.isDirectPathXdsEnabled = isDirectPathXdsEnabled; + } + + @Override + public String getenv(String env) { + if (env.equals("GOOGLE_CLOUD_ENABLE_DIRECT_PATH_XDS")) { + return isDirectPathXdsEnabled; + } + return System.getenv(env); + } + } + @Test public void testEndpoint() { String endpoint = "localhost:8080"; @@ -386,6 +403,30 @@ public ManagedChannelBuilder apply(ManagedChannelBuilder channelBuilder) { provider.getTransportChannel().shutdownNow(); } + @Test + public void testWithDirectPathXds() throws IOException { + ScheduledExecutorService executor = new ScheduledThreadPoolExecutor(1); + executor.shutdown(); + + InstantiatingGrpcChannelProvider iProvider = + InstantiatingGrpcChannelProvider.newBuilder() + .setAttemptDirectPath(true) + .setEnvProvider(new TestEnvironmentProvider(/*isDirectPathXdsEnabled = */ "true")) + .build(); + + TransportChannelProvider provider = + iProvider + .withHeaders(Collections.emptyMap()) + .withExecutor((Executor) executor) + .withEndpoint("localhost:8080") + .withCredentials(ComputeEngineCredentials.create()); + ; + + provider.getTransportChannel().shutdownNow(); + + assertEquals(iProvider.getActiveEndpoint(), "google-c2p:///localhost"); + } + @Test public void testWithIPv6Address() throws IOException { ScheduledExecutorService executor = new ScheduledThreadPoolExecutor(1); From 77e81d794fcea75597bf44eb1385ecaf8451b1b4 Mon Sep 17 00:00:00 2001 From: mohanli-ml Date: Tue, 19 Oct 2021 04:43:45 +0000 Subject: [PATCH 5/9] feat: add google-c2p dependence to DirectPath --- .../gax/grpc/InstantiatingGrpcChannelProvider.java | 2 +- .../grpc/InstantiatingGrpcChannelProviderTest.java | 11 +++++------ 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/gax-grpc/src/main/java/com/google/api/gax/grpc/InstantiatingGrpcChannelProvider.java b/gax-grpc/src/main/java/com/google/api/gax/grpc/InstantiatingGrpcChannelProvider.java index 3451609a4..84df6529d 100644 --- a/gax-grpc/src/main/java/com/google/api/gax/grpc/InstantiatingGrpcChannelProvider.java +++ b/gax-grpc/src/main/java/com/google/api/gax/grpc/InstantiatingGrpcChannelProvider.java @@ -401,7 +401,7 @@ && isOnComputeEngine()) { return managedChannel; } - /** The endpoint passed to the builder. It must have a pot, and must not have a URI scheme. */ + /** The endpoint passed to the builder, which must have a port. */ public String getEndpoint() { return endpoint; } diff --git a/gax-grpc/src/test/java/com/google/api/gax/grpc/InstantiatingGrpcChannelProviderTest.java b/gax-grpc/src/test/java/com/google/api/gax/grpc/InstantiatingGrpcChannelProviderTest.java index 8476df890..2f5f756ba 100644 --- a/gax-grpc/src/test/java/com/google/api/gax/grpc/InstantiatingGrpcChannelProviderTest.java +++ b/gax-grpc/src/test/java/com/google/api/gax/grpc/InstantiatingGrpcChannelProviderTest.java @@ -408,23 +408,22 @@ public void testWithDirectPathXds() throws IOException { ScheduledExecutorService executor = new ScheduledThreadPoolExecutor(1); executor.shutdown(); - InstantiatingGrpcChannelProvider iProvider = + InstantiatingGrpcChannelProvider grpcChannelProvider = InstantiatingGrpcChannelProvider.newBuilder() .setAttemptDirectPath(true) .setEnvProvider(new TestEnvironmentProvider(/*isDirectPathXdsEnabled = */ "true")) .build(); - TransportChannelProvider provider = - iProvider + TransportChannelProvider transportChannelProvider = + grpcChannelProvider .withHeaders(Collections.emptyMap()) .withExecutor((Executor) executor) .withEndpoint("localhost:8080") .withCredentials(ComputeEngineCredentials.create()); - ; - provider.getTransportChannel().shutdownNow(); + transportChannelProvider.getTransportChannel().shutdownNow(); - assertEquals(iProvider.getActiveEndpoint(), "google-c2p:///localhost"); + assertEquals(grpcChannelProvider.getActiveEndpoint(), "google-c2p:///localhost"); } @Test From baaff1811da812285ddd41342a06582d4540d70b Mon Sep 17 00:00:00 2001 From: Chanseok Oh Date: Wed, 20 Oct 2021 15:47:39 -0400 Subject: [PATCH 6/9] Simplify code --- .../InstantiatingGrpcChannelProviderTest.java | 68 +++++++------------ .../com/google/api/gax/rpc/StubSettings.java | 2 + 2 files changed, 25 insertions(+), 45 deletions(-) diff --git a/gax-grpc/src/test/java/com/google/api/gax/grpc/InstantiatingGrpcChannelProviderTest.java b/gax-grpc/src/test/java/com/google/api/gax/grpc/InstantiatingGrpcChannelProviderTest.java index 28b96ca77..c7dedc87a 100644 --- a/gax-grpc/src/test/java/com/google/api/gax/grpc/InstantiatingGrpcChannelProviderTest.java +++ b/gax-grpc/src/test/java/com/google/api/gax/grpc/InstantiatingGrpcChannelProviderTest.java @@ -67,22 +67,6 @@ @RunWith(JUnit4.class) public class InstantiatingGrpcChannelProviderTest extends AbstractMtlsTransportChannelTest { - static class TestEnvironmentProvider - implements InstantiatingGrpcChannelProvider.EnvironmentProvider { - private final String isDirectPathXdsEnabled; - - TestEnvironmentProvider(String isDirectPathXdsEnabled) { - this.isDirectPathXdsEnabled = isDirectPathXdsEnabled; - } - - @Override - public String getenv(String env) { - if (env.equals("GOOGLE_CLOUD_ENABLE_DIRECT_PATH_XDS")) { - return isDirectPathXdsEnabled; - } - return System.getenv(env); - } - } @Test public void testEndpoint() { @@ -181,11 +165,8 @@ public void testToBuilder() { Duration keepaliveTime = Duration.ofSeconds(1); Duration keepaliveTimeout = Duration.ofSeconds(2); ApiFunction channelConfigurator = - new ApiFunction() { - @Override - public ManagedChannelBuilder apply(ManagedChannelBuilder input) { - throw new UnsupportedOperationException(); - } + builder -> { + throw new UnsupportedOperationException(); }; Map directPathServiceConfig = ImmutableMap.of("loadbalancingConfig", "grpclb"); @@ -283,16 +264,13 @@ public void testWithGCECredentials() throws IOException { executor.shutdown(); ApiFunction channelConfigurator = - new ApiFunction() { - @Override - public ManagedChannelBuilder apply(ManagedChannelBuilder channelBuilder) { - if (InstantiatingGrpcChannelProvider.isOnComputeEngine()) { - assertThat(channelBuilder instanceof ComputeEngineChannelBuilder).isTrue(); - } else { - assertThat(channelBuilder instanceof ComputeEngineChannelBuilder).isFalse(); - } - return channelBuilder; + channelBuilder -> { + if (InstantiatingGrpcChannelProvider.isOnComputeEngine()) { + assertThat(channelBuilder).isInstanceOf(ComputeEngineChannelBuilder.class); + } else { + assertThat(channelBuilder).isNotInstanceOf(ComputeEngineChannelBuilder.class); } + return channelBuilder; }; TransportChannelProvider provider = @@ -321,13 +299,10 @@ public void testWithNonGCECredentials() throws IOException { executor.shutdown(); ApiFunction channelConfigurator = - new ApiFunction() { - @Override - public ManagedChannelBuilder apply(ManagedChannelBuilder channelBuilder) { - // Clients with non-GCE credentials will not attempt DirectPath. - assertThat(channelBuilder instanceof ComputeEngineChannelBuilder).isFalse(); - return channelBuilder; - } + channelBuilder -> { + // Clients with non-GCE credentials will not attempt DirectPath. + assertThat(channelBuilder instanceof ComputeEngineChannelBuilder).isFalse(); + return channelBuilder; }; TransportChannelProvider provider = @@ -383,13 +358,10 @@ public void testWithNoDirectPathFlagSet() throws IOException { executor.shutdown(); ApiFunction channelConfigurator = - new ApiFunction() { - @Override - public ManagedChannelBuilder apply(ManagedChannelBuilder channelBuilder) { - // Clients without setting attemptDirectPath flag will not attempt DirectPath - assertThat(channelBuilder instanceof ComputeEngineChannelBuilder).isFalse(); - return channelBuilder; - } + channelBuilder -> { + // Clients without setting attemptDirectPath flag will not attempt DirectPath + assertThat(channelBuilder instanceof ComputeEngineChannelBuilder).isFalse(); + return channelBuilder; }; TransportChannelProvider provider = @@ -415,7 +387,13 @@ public void testWithDirectPathXds() throws IOException { InstantiatingGrpcChannelProvider grpcChannelProvider = InstantiatingGrpcChannelProvider.newBuilder() .setAttemptDirectPath(true) - .setEnvProvider(new TestEnvironmentProvider(/*isDirectPathXdsEnabled = */ "true")) + .setEnvProvider( + envVar -> { + if (envVar.equals("GOOGLE_CLOUD_ENABLE_DIRECT_PATH_XDS")) { + return "true"; + } + return null; + }) .build(); TransportChannelProvider transportChannelProvider = diff --git a/gax/src/main/java/com/google/api/gax/rpc/StubSettings.java b/gax/src/main/java/com/google/api/gax/rpc/StubSettings.java index 825aa9d7a..04f1d59c9 100644 --- a/gax/src/main/java/com/google/api/gax/rpc/StubSettings.java +++ b/gax/src/main/java/com/google/api/gax/rpc/StubSettings.java @@ -176,6 +176,7 @@ public ApiTracerFactory getTracerFactory() { return tracerFactory; } + @Override public String toString() { return MoreObjects.toStringHelper(this) .add("backgroundExecutorProvider", backgroundExecutorProvider) @@ -535,6 +536,7 @@ protected static void applyToAllUnaryMethods( public abstract > StubSettings build() throws IOException; + @Override public String toString() { return MoreObjects.toStringHelper(this) .add("backgroundExecutorProvider", backgroundExecutorProvider) From db57e6767f43535093f349fca7029b9e46bb25df Mon Sep 17 00:00:00 2001 From: Chanseok Oh Date: Wed, 20 Oct 2021 16:35:18 -0400 Subject: [PATCH 7/9] Simplify --- .../api/gax/grpc/InstantiatingGrpcChannelProviderTest.java | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/gax-grpc/src/test/java/com/google/api/gax/grpc/InstantiatingGrpcChannelProviderTest.java b/gax-grpc/src/test/java/com/google/api/gax/grpc/InstantiatingGrpcChannelProviderTest.java index c7dedc87a..50d15b957 100644 --- a/gax-grpc/src/test/java/com/google/api/gax/grpc/InstantiatingGrpcChannelProviderTest.java +++ b/gax-grpc/src/test/java/com/google/api/gax/grpc/InstantiatingGrpcChannelProviderTest.java @@ -388,12 +388,7 @@ public void testWithDirectPathXds() throws IOException { InstantiatingGrpcChannelProvider.newBuilder() .setAttemptDirectPath(true) .setEnvProvider( - envVar -> { - if (envVar.equals("GOOGLE_CLOUD_ENABLE_DIRECT_PATH_XDS")) { - return "true"; - } - return null; - }) + envVar -> envVar.equals("GOOGLE_CLOUD_ENABLE_DIRECT_PATH_XDS") ? "true" : null) .build(); TransportChannelProvider transportChannelProvider = From 1c288e09ebcd489801a3c09ac56b058169e0e3a2 Mon Sep 17 00:00:00 2001 From: mohanli-ml Date: Wed, 20 Oct 2021 22:53:58 +0000 Subject: [PATCH 8/9] feat: add google-c2p dependence to DirectPath --- .../InstantiatingGrpcChannelProvider.java | 18 ++------------ .../InstantiatingGrpcChannelProviderTest.java | 24 ------------------- 2 files changed, 2 insertions(+), 40 deletions(-) diff --git a/gax-grpc/src/main/java/com/google/api/gax/grpc/InstantiatingGrpcChannelProvider.java b/gax-grpc/src/main/java/com/google/api/gax/grpc/InstantiatingGrpcChannelProvider.java index 84df6529d..d441a0c9a 100644 --- a/gax-grpc/src/main/java/com/google/api/gax/grpc/InstantiatingGrpcChannelProvider.java +++ b/gax-grpc/src/main/java/com/google/api/gax/grpc/InstantiatingGrpcChannelProvider.java @@ -109,7 +109,6 @@ public final class InstantiatingGrpcChannelProvider implements TransportChannelP @Nullable private final Boolean allowNonDefaultServiceAccount; @VisibleForTesting final ImmutableMap directPathServiceConfig; @Nullable private final MtlsProvider mtlsProvider; - @Nullable @VisibleForTesting private String activeEndpoint; @Nullable private final ApiFunction channelConfigurator; @@ -137,7 +136,6 @@ private InstantiatingGrpcChannelProvider(Builder builder) { builder.directPathServiceConfig == null ? getDefaultDirectPathServiceConfig() : builder.directPathServiceConfig; - this.activeEndpoint = builder.endpoint; } /** @@ -341,8 +339,7 @@ && isOnComputeEngine()) { isDirectPathXdsEnabled = Boolean.parseBoolean(envProvider.getenv(DIRECT_PATH_ENV_ENABLE_XDS)); if (isDirectPathXdsEnabled) { // google-c2p resolver target must not have a port number - activeEndpoint = "google-c2p:///" + serviceAddress; - builder = ComputeEngineChannelBuilder.forTarget(activeEndpoint); + builder = ComputeEngineChannelBuilder.forTarget("google-c2p:///" + serviceAddress); } else { builder = ComputeEngineChannelBuilder.forAddress(serviceAddress, port); builder.defaultServiceConfig(directPathServiceConfig); @@ -401,16 +398,11 @@ && isOnComputeEngine()) { return managedChannel; } - /** The endpoint passed to the builder, which must have a port. */ + /** The endpoint to be used for the channel. */ public String getEndpoint() { return endpoint; } - /** The actual endpoint to be used for building a channel. */ - public String getActiveEndpoint() { - return activeEndpoint; - } - /** The time without read activity before sending a keepalive ping. */ public Duration getKeepAliveTime() { return keepAliveTime; @@ -543,12 +535,6 @@ Builder setMtlsProvider(MtlsProvider mtlsProvider) { return this; } - @VisibleForTesting - Builder setEnvProvider(EnvironmentProvider envProvider) { - this.envProvider = envProvider; - return this; - } - /** * Sets the GrpcInterceptorProvider for this TransportChannelProvider. * diff --git a/gax-grpc/src/test/java/com/google/api/gax/grpc/InstantiatingGrpcChannelProviderTest.java b/gax-grpc/src/test/java/com/google/api/gax/grpc/InstantiatingGrpcChannelProviderTest.java index 50d15b957..ed01d241e 100644 --- a/gax-grpc/src/test/java/com/google/api/gax/grpc/InstantiatingGrpcChannelProviderTest.java +++ b/gax-grpc/src/test/java/com/google/api/gax/grpc/InstantiatingGrpcChannelProviderTest.java @@ -379,30 +379,6 @@ public void testWithNoDirectPathFlagSet() throws IOException { provider.getTransportChannel().shutdownNow(); } - @Test - public void testWithDirectPathXds() throws IOException { - ScheduledExecutorService executor = new ScheduledThreadPoolExecutor(1); - executor.shutdown(); - - InstantiatingGrpcChannelProvider grpcChannelProvider = - InstantiatingGrpcChannelProvider.newBuilder() - .setAttemptDirectPath(true) - .setEnvProvider( - envVar -> envVar.equals("GOOGLE_CLOUD_ENABLE_DIRECT_PATH_XDS") ? "true" : null) - .build(); - - TransportChannelProvider transportChannelProvider = - grpcChannelProvider - .withHeaders(Collections.emptyMap()) - .withExecutor((Executor) executor) - .withEndpoint("localhost:8080") - .withCredentials(ComputeEngineCredentials.create()); - - transportChannelProvider.getTransportChannel().shutdownNow(); - - assertEquals(grpcChannelProvider.getActiveEndpoint(), "google-c2p:///localhost"); - } - @Test public void testWithIPv6Address() throws IOException { ScheduledExecutorService executor = new ScheduledThreadPoolExecutor(1); From c8636082ec5ede3e1d3b824b7d6fac24df579021 Mon Sep 17 00:00:00 2001 From: mohanli-ml Date: Thu, 21 Oct 2021 00:47:18 +0000 Subject: [PATCH 9/9] feat: add google-c2p dependence to DirectPath --- gax-grpc/build.gradle | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/gax-grpc/build.gradle b/gax-grpc/build.gradle index cae2fe436..f08a01f99 100644 --- a/gax-grpc/build.gradle +++ b/gax-grpc/build.gradle @@ -18,8 +18,9 @@ dependencies { libraries['maven.io_grpc_grpc_auth'], libraries['maven.io_grpc_grpc_netty_shaded'], libraries['maven.io_grpc_grpc_protobuf'], - libraries['maven.io_grpc_grpc_stub'], - libraries['maven.io_grpc_grpc_xds']) + libraries['maven.io_grpc_grpc_stub']) + + runtimeOnly libraries['maven.io_grpc_grpc_xds'] compileOnly libraries['maven.com_google_auto_value_auto_value']