refactor: use container-provided channel registry (#106)

* refactor: use container-provided channel registry

* fix: update snyk ignore

* ci: update test publish plugin

Author: aaron-steinfeld

.github/workflows/pr-test.yml

@@ -40,8 +40,8 @@ jobs:
           flags: unit
 
       - name: Publish Unit Test Results
-        uses: docker://ghcr.io/enricomi/publish-unit-test-result-action:v1.6
+        uses: docker://ghcr.io/enricomi/publish-unit-test-result-action:v2
         if: github.event_name == 'push' || github.event.pull_request.head.repo.full_name == github.repository
         with:
           github_token: ${{ secrets.GITHUB_TOKEN }}
-          files: ./**/build/test-results/**/*.xml
+          junit_files: ./**/build/test-results/**/*.xml

.snyk

@@ -5,6 +5,6 @@ ignore:
   SNYK-JAVA-IONETTY-1042268:
     - '*':
         reason: No replacement available
-        expires: 2022-07-31T00:00:00.000Z
+        expires: 2022-10-31T00:00:00.000Z
 patch: {}
 

hypertrace-core-graphql-grpc-utils/src/main/java/org/hypertrace/core/graphql/utils/grpc/DefaultGrpcChannelRegistry.java

@@ -1,20 +1,14 @@
 package org.hypertrace.core.graphql.utils.grpc;
 
 import io.grpc.ManagedChannel;
-import javax.inject.Inject;
-import javax.inject.Singleton;
-import org.hypertrace.core.graphql.spi.lifecycle.GraphQlServiceLifecycle;
 import org.hypertrace.core.grpcutils.client.GrpcChannelConfig;
 
-@Singleton
 class DefaultGrpcChannelRegistry implements GrpcChannelRegistry {
 
-  private final org.hypertrace.core.grpcutils.client.GrpcChannelRegistry delegate =
-      new org.hypertrace.core.grpcutils.client.GrpcChannelRegistry();
+  private final org.hypertrace.core.grpcutils.client.GrpcChannelRegistry delegate;
 
-  @Inject
-  DefaultGrpcChannelRegistry(GraphQlServiceLifecycle serviceLifecycle) {
-    serviceLifecycle.shutdownCompletion().thenRun(this.delegate::shutdown);
+  DefaultGrpcChannelRegistry(org.hypertrace.core.grpcutils.client.GrpcChannelRegistry delegate) {
+    this.delegate = delegate;
   }
 
   @Override

hypertrace-core-graphql-grpc-utils/src/main/java/org/hypertrace/core/graphql/utils/grpc/GraphQlGrpcModule.java

@@ -7,11 +7,19 @@
 
 public class GraphQlGrpcModule extends AbstractModule {
 
+  private final org.hypertrace.core.grpcutils.client.GrpcChannelRegistry channelRegistry;
+
+  public GraphQlGrpcModule(
+      org.hypertrace.core.grpcutils.client.GrpcChannelRegistry channelRegistry) {
+    this.channelRegistry = channelRegistry;
+  }
+
   @Override
   protected void configure() {
     bind(CallCredentials.class).toInstance(getClientCallCredsProvider().get());
     bind(GraphQlGrpcContextBuilder.class).to(DefaultGraphQlGrpcContextBuilder.class);
-    bind(GrpcChannelRegistry.class).to(DefaultGrpcChannelRegistry.class);
+    bind(GrpcChannelRegistry.class)
+        .toInstance(new DefaultGrpcChannelRegistry(this.channelRegistry));
     bind(GrpcContextBuilder.class).to(PlatformGrpcContextBuilder.class);
   }
 }

hypertrace-core-graphql-grpc-utils/src/test/java/org/hypertrace/core/graphql/utils/grpc/DefaultGrpcChannelRegistryTest.java

@@ -1,36 +1,23 @@
 package org.hypertrace.core.graphql.utils.grpc;
 
-import static org.junit.jupiter.api.Assertions.assertFalse;
 import static org.junit.jupiter.api.Assertions.assertNotNull;
 import static org.junit.jupiter.api.Assertions.assertNotSame;
 import static org.junit.jupiter.api.Assertions.assertSame;
-import static org.junit.jupiter.api.Assertions.assertThrows;
-import static org.junit.jupiter.api.Assertions.assertTrue;
-import static org.mockito.Mockito.when;
 
 import io.grpc.Channel;
-import io.grpc.ManagedChannel;
-import java.util.concurrent.CompletableFuture;
-import org.hypertrace.core.graphql.spi.lifecycle.GraphQlServiceLifecycle;
+import org.hypertrace.core.grpcutils.client.GrpcChannelRegistry;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.extension.ExtendWith;
-import org.mockito.Mock;
 import org.mockito.junit.jupiter.MockitoExtension;
 
 @ExtendWith(MockitoExtension.class)
 class DefaultGrpcChannelRegistryTest {
-
-  @Mock GraphQlServiceLifecycle mockLifecycle;
-  CompletableFuture<Void> shutdown;
-
   DefaultGrpcChannelRegistry channelRegistry;
 
   @BeforeEach
   void beforeEach() {
-    this.shutdown = new CompletableFuture<>();
-    when(this.mockLifecycle.shutdownCompletion()).thenReturn(this.shutdown);
-    this.channelRegistry = new DefaultGrpcChannelRegistry(this.mockLifecycle);
+    this.channelRegistry = new DefaultGrpcChannelRegistry(new GrpcChannelRegistry());
   }
 
   @Test
@@ -45,21 +32,4 @@ void reusesChannelsForDuplicateRequests() {
     assertNotSame(firstChannel, this.channelRegistry.forAddress("foo", 1001));
     assertNotSame(firstChannel, this.channelRegistry.forAddress("bar", 1000));
   }
-
-  @Test
-  void shutdownAllChannelsOnLifecycleShutdown() {
-    ManagedChannel firstChannel = this.channelRegistry.forAddress("foo", 1000);
-    ManagedChannel secondChannel = this.channelRegistry.forAddress("foo", 1002);
-    assertFalse(firstChannel.isShutdown());
-    assertFalse(secondChannel.isShutdown());
-    this.shutdown.complete(null);
-    assertTrue(firstChannel.isShutdown());
-    assertTrue(secondChannel.isShutdown());
-  }
-
-  @Test
-  void throwsIfNewChannelRequestedAfterLifecycleShutdown() {
-    this.shutdown.complete(null);
-    assertThrows(AssertionError.class, () -> this.channelRegistry.forAddress("foo", 1000));
-  }
 }

hypertrace-core-graphql-impl/build.gradle.kts

@@ -7,6 +7,7 @@ plugins {
 dependencies {
   api(project(":hypertrace-core-graphql-spi"))
   api("com.graphql-java-kickstart:graphql-java-servlet")
+  api("org.hypertrace.core.grpcutils:grpc-client-utils")
 
   implementation(project(":hypertrace-core-graphql-schema-registry"))
   implementation(project(":hypertrace-core-graphql-context"))

hypertrace-core-graphql-impl/src/main/java/org/hypertrace/core/graphql/impl/GraphQlFactory.java

@@ -7,11 +7,15 @@
 import org.hypertrace.core.graphql.context.GraphQlRequestContextBuilder;
 import org.hypertrace.core.graphql.spi.config.GraphQlServiceConfig;
 import org.hypertrace.core.graphql.spi.lifecycle.GraphQlServiceLifecycle;
+import org.hypertrace.core.grpcutils.client.GrpcChannelRegistry;
 
 public class GraphQlFactory {
   public static GraphQLConfiguration build(
-      GraphQlServiceConfig config, GraphQlServiceLifecycle lifecycle) {
-    final Injector injector = Guice.createInjector(new GraphQlModule(config, lifecycle));
+      GraphQlServiceConfig config,
+      GraphQlServiceLifecycle lifecycle,
+      GrpcChannelRegistry grpcChannelRegistry) {
+    final Injector injector =
+        Guice.createInjector(new GraphQlModule(config, lifecycle, grpcChannelRegistry));
 
     return GraphQLConfiguration.with(injector.getInstance(GraphQLSchema.class))
         .with(injector.getInstance(GraphQlRequestContextBuilder.class))

hypertrace-core-graphql-impl/src/main/java/org/hypertrace/core/graphql/impl/GraphQlModule.java

@@ -19,23 +19,29 @@
 import org.hypertrace.core.graphql.utils.gateway.GatewayUtilsModule;
 import org.hypertrace.core.graphql.utils.grpc.GraphQlGrpcModule;
 import org.hypertrace.core.graphql.utils.schema.SchemaUtilsModule;
+import org.hypertrace.core.grpcutils.client.GrpcChannelRegistry;
 
 class GraphQlModule extends AbstractModule {
 
   private final GraphQlServiceConfig config;
   private final GraphQlServiceLifecycle lifecycle;
+  private final org.hypertrace.core.grpcutils.client.GrpcChannelRegistry grpcChannelRegistry;
 
-  public GraphQlModule(final GraphQlServiceConfig config, final GraphQlServiceLifecycle lifecycle) {
+  public GraphQlModule(
+      final GraphQlServiceConfig config,
+      final GraphQlServiceLifecycle lifecycle,
+      final GrpcChannelRegistry grpcChannelRegistry) {
     this.config = config;
     this.lifecycle = lifecycle;
+    this.grpcChannelRegistry = grpcChannelRegistry;
   }
 
   @Override
   protected void configure() {
     bind(GraphQlServiceConfig.class).toInstance(this.config);
     bind(GraphQlServiceLifecycle.class).toInstance(this.lifecycle);
     install(new GraphQlRequestContextModule());
-    install(new GraphQlGrpcModule());
+    install(new GraphQlGrpcModule(this.grpcChannelRegistry));
     install(new GraphQlSchemaRegistryModule());
     install(new GraphQlDeserializationRegistryModule());
     install(new CommonSchemaModule());

hypertrace-core-graphql-impl/src/test/java/org/hypertrace/core/graphql/impl/GraphQlModuleTest.java

@@ -6,6 +6,7 @@
 import com.google.inject.Guice;
 import org.hypertrace.core.graphql.spi.config.GraphQlServiceConfig;
 import org.hypertrace.core.graphql.spi.lifecycle.GraphQlServiceLifecycle;
+import org.hypertrace.core.grpcutils.client.GrpcChannelRegistry;
 import org.junit.jupiter.api.Test;
 
 public class GraphQlModuleTest {
@@ -16,7 +17,9 @@ public void testResolveBindings() {
         () ->
             Guice.createInjector(
                     new GraphQlModule(
-                        mock(GraphQlServiceConfig.class), mock(GraphQlServiceLifecycle.class)))
+                        mock(GraphQlServiceConfig.class),
+                        mock(GraphQlServiceLifecycle.class),
+                        mock(GrpcChannelRegistry.class)))
                 .getAllBindings());
   }
 }

hypertrace-core-graphql-service/src/main/java/org/hypertrace/core/graphql/service/GraphQlService.java

@@ -34,7 +34,10 @@ List<HttpHandlerDefinition> buildHandlerDefinition(HttpContainerEnvironment envi
             .port(config.getServicePort())
             .contextPath(config.getGraphQlUrlPath())
             .corsConfig(buildCorsConfig(config))
-            .servlet(new GraphQlServiceHttpServlet(GraphQlFactory.build(config, serviceLifecycle)))
+            .servlet(
+                new GraphQlServiceHttpServlet(
+                    GraphQlFactory.build(
+                        config, serviceLifecycle, environment.getChannelRegistry())))
             .build());
   }