From c702b4486c9e2d7796f00dcd83b6e4cb230a103a Mon Sep 17 00:00:00 2001 From: jianglai Date: Fri, 20 Oct 2017 09:58:14 -0700 Subject: [PATCH] Use standard java thread factory instead of the AppEngine flavor With Java 8 in GAE standard environment, we can now use standard java thread factory to run the metric reporter in the background in daemon mode, which would not interfere with basic scaling idle timeout as App Engine thread would. Because the thread is not created by ThreadManager, no App Engine APIs can be called from it. We therefore use GoogleCredential instead of AppIdentityCredential as HttpRequestInitializer, and NetHttpTransport instead of UlrFetchTransport as HttpTransport. MetricReporter is lazy injected because it depends on jsonCredential retrieved from CloudKms, which is not available in a test environment, causing FrontendServletTest and BackendServletTest to fail. Some minor re-formatting with google-java-format on edited files. Lastly removed moe comments in import statement, which makes the linter unhappy. ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=172896227 --- .../module/backend/BackendComponent.java | 66 ++++++++++--------- .../module/backend/BackendServlet.java | 21 +++--- .../module/frontend/FrontendComponent.java | 53 ++++++++------- .../module/frontend/FrontendServlet.java | 21 +++--- .../registry/module/tools/ToolsComponent.java | 53 ++++++++------- .../google/registry/monitoring/whitebox/BUILD | 1 + .../whitebox/StackdriverModule.java | 14 ++-- java/google/registry/request/Modules.java | 40 ++++++++--- 8 files changed, 151 insertions(+), 118 deletions(-) diff --git a/java/google/registry/module/backend/BackendComponent.java b/java/google/registry/module/backend/BackendComponent.java index 179e3f9bf..edd96b67d 100644 --- a/java/google/registry/module/backend/BackendComponent.java +++ b/java/google/registry/module/backend/BackendComponent.java @@ -15,9 +15,9 @@ package google.registry.module.backend; import dagger.Component; +import dagger.Lazy; import google.registry.bigquery.BigqueryModule; import google.registry.config.RegistryConfig.ConfigModule; -import google.registry.dns.writer.VoidDnsWriterModule; import google.registry.export.DriveModule; import google.registry.export.sheet.SheetsServiceModule; import google.registry.gcs.GcsServiceModule; @@ -36,6 +36,7 @@ import google.registry.request.Modules.DatastoreServiceModule; import google.registry.request.Modules.GoogleCredentialModule; import google.registry.request.Modules.Jackson2Module; import google.registry.request.Modules.ModulesServiceModule; +import google.registry.request.Modules.NetHttpTransportModule; import google.registry.request.Modules.URLFetchServiceModule; import google.registry.request.Modules.UrlFetchTransportModule; import google.registry.request.Modules.UseAppIdentityCredentialForGoogleApisModule; @@ -48,36 +49,39 @@ import javax.inject.Singleton; /** Dagger component with instance lifetime for "backend" App Engine module. */ @Singleton @Component( - modules = { - AppIdentityCredentialModule.class, - AuthModule.class, - BackendRequestComponentModule.class, - BigqueryModule.class, - ConfigModule.class, - DatastoreServiceModule.class, - DirectoryModule.class, - DriveModule.class, - GcsServiceModule.class, - GoogleCredentialModule.class, - GroupsModule.class, - GroupssettingsModule.class, - JSchModule.class, - Jackson2Module.class, - KeyModule.class, - KeyringModule.class, - KmsModule.class, - ModulesServiceModule.class, - SheetsServiceModule.class, - StackdriverModule.class, - SystemClockModule.class, - SystemSleeperModule.class, - URLFetchServiceModule.class, - UrlFetchTransportModule.class, - UseAppIdentityCredentialForGoogleApisModule.class, - UserServiceModule.class, - VoidDnsWriterModule.class, - }) + modules = { + AppIdentityCredentialModule.class, + AuthModule.class, + BackendRequestComponentModule.class, + BigqueryModule.class, + ConfigModule.class, + DatastoreServiceModule.class, + DirectoryModule.class, + DriveModule.class, + GcsServiceModule.class, + GoogleCredentialModule.class, + GroupsModule.class, + GroupssettingsModule.class, + JSchModule.class, + Jackson2Module.class, + KeyModule.class, + KeyringModule.class, + KmsModule.class, + ModulesServiceModule.class, + NetHttpTransportModule.class, + SheetsServiceModule.class, + StackdriverModule.class, + SystemClockModule.class, + SystemSleeperModule.class, + URLFetchServiceModule.class, + UrlFetchTransportModule.class, + UseAppIdentityCredentialForGoogleApisModule.class, + UserServiceModule.class, + google.registry.dns.writer.VoidDnsWriterModule.class, + } +) interface BackendComponent { BackendRequestHandler requestHandler(); - MetricReporter metricReporter(); + + Lazy metricReporter(); } diff --git a/java/google/registry/module/backend/BackendServlet.java b/java/google/registry/module/backend/BackendServlet.java index 76977d0b0..43d093766 100644 --- a/java/google/registry/module/backend/BackendServlet.java +++ b/java/google/registry/module/backend/BackendServlet.java @@ -15,7 +15,7 @@ package google.registry.module.backend; import com.google.appengine.api.LifecycleManager; -import com.google.appengine.api.LifecycleManager.ShutdownHook; +import dagger.Lazy; import google.registry.monitoring.metrics.MetricReporter; import google.registry.util.FormattingLogger; import java.io.IOException; @@ -32,7 +32,7 @@ public final class BackendServlet extends HttpServlet { private static final BackendComponent component = DaggerBackendComponent.create(); private static final BackendRequestHandler requestHandler = component.requestHandler(); - private static final MetricReporter metricReporter = component.metricReporter(); + private static final Lazy metricReporter = component.metricReporter(); private static final FormattingLogger logger = FormattingLogger.getLoggerForCallerClass(); @Override @@ -40,7 +40,7 @@ public final class BackendServlet extends HttpServlet { Security.addProvider(new BouncyCastleProvider()); try { - metricReporter.startAsync().awaitRunning(10, TimeUnit.SECONDS); + metricReporter.get().startAsync().awaitRunning(10, TimeUnit.SECONDS); logger.info("Started up MetricReporter"); } catch (TimeoutException timeoutException) { logger.severefmt("Failed to initialize MetricReporter: %s", timeoutException); @@ -48,15 +48,12 @@ public final class BackendServlet extends HttpServlet { LifecycleManager.getInstance() .setShutdownHook( - new ShutdownHook() { - @Override - public void shutdown() { - try { - metricReporter.stopAsync().awaitTerminated(10, TimeUnit.SECONDS); - logger.info("Shut down MetricReporter"); - } catch (TimeoutException timeoutException) { - logger.severefmt("Failed to stop MetricReporter: %s", timeoutException); - } + () -> { + try { + metricReporter.get().stopAsync().awaitTerminated(10, TimeUnit.SECONDS); + logger.info("Shut down MetricReporter"); + } catch (TimeoutException timeoutException) { + logger.severefmt("Failed to stop MetricReporter: %s", timeoutException); } }); } diff --git a/java/google/registry/module/frontend/FrontendComponent.java b/java/google/registry/module/frontend/FrontendComponent.java index 7aed016e6..4bb4f2499 100644 --- a/java/google/registry/module/frontend/FrontendComponent.java +++ b/java/google/registry/module/frontend/FrontendComponent.java @@ -15,6 +15,7 @@ package google.registry.module.frontend; import dagger.Component; +import dagger.Lazy; import google.registry.braintree.BraintreeModule; import google.registry.config.RegistryConfig.ConfigModule; import google.registry.flows.ServerTridProviderModule; @@ -26,8 +27,10 @@ import google.registry.module.frontend.FrontendRequestComponent.FrontendRequestC import google.registry.monitoring.metrics.MetricReporter; import google.registry.monitoring.whitebox.StackdriverModule; import google.registry.request.Modules.AppIdentityCredentialModule; +import google.registry.request.Modules.GoogleCredentialModule; import google.registry.request.Modules.Jackson2Module; import google.registry.request.Modules.ModulesServiceModule; +import google.registry.request.Modules.NetHttpTransportModule; import google.registry.request.Modules.UrlFetchTransportModule; import google.registry.request.Modules.UseAppIdentityCredentialForGoogleApisModule; import google.registry.request.Modules.UserServiceModule; @@ -40,29 +43,33 @@ import javax.inject.Singleton; /** Dagger component with instance lifetime for "default" App Engine module. */ @Singleton @Component( - modules = { - AppIdentityCredentialModule.class, - AuthModule.class, - BraintreeModule.class, - ConfigModule.class, - ConsoleConfigModule.class, - CustomLogicFactoryModule.class, - FrontendMetricsModule.class, - FrontendRequestComponentModule.class, - Jackson2Module.class, - KeyModule.class, - KeyringModule.class, - KmsModule.class, - ModulesServiceModule.class, - ServerTridProviderModule.class, - StackdriverModule.class, - SystemClockModule.class, - SystemSleeperModule.class, - UrlFetchTransportModule.class, - UseAppIdentityCredentialForGoogleApisModule.class, - UserServiceModule.class, - }) + modules = { + AppIdentityCredentialModule.class, + AuthModule.class, + BraintreeModule.class, + ConfigModule.class, + ConsoleConfigModule.class, + CustomLogicFactoryModule.class, + FrontendMetricsModule.class, + FrontendRequestComponentModule.class, + GoogleCredentialModule.class, + Jackson2Module.class, + KeyModule.class, + KeyringModule.class, + KmsModule.class, + ModulesServiceModule.class, + NetHttpTransportModule.class, + ServerTridProviderModule.class, + StackdriverModule.class, + SystemClockModule.class, + SystemSleeperModule.class, + UrlFetchTransportModule.class, + UseAppIdentityCredentialForGoogleApisModule.class, + UserServiceModule.class, + } +) interface FrontendComponent { FrontendRequestHandler requestHandler(); - MetricReporter metricReporter(); + + Lazy metricReporter(); } diff --git a/java/google/registry/module/frontend/FrontendServlet.java b/java/google/registry/module/frontend/FrontendServlet.java index 05b3b64d8..dd06e3775 100644 --- a/java/google/registry/module/frontend/FrontendServlet.java +++ b/java/google/registry/module/frontend/FrontendServlet.java @@ -15,7 +15,7 @@ package google.registry.module.frontend; import com.google.appengine.api.LifecycleManager; -import com.google.appengine.api.LifecycleManager.ShutdownHook; +import dagger.Lazy; import google.registry.monitoring.metrics.MetricReporter; import google.registry.util.FormattingLogger; import java.io.IOException; @@ -32,7 +32,7 @@ public final class FrontendServlet extends HttpServlet { private static final FrontendComponent component = DaggerFrontendComponent.create(); private static final FrontendRequestHandler requestHandler = component.requestHandler(); - private static final MetricReporter metricReporter = component.metricReporter(); + private static final Lazy metricReporter = component.metricReporter(); private static final FormattingLogger logger = FormattingLogger.getLoggerForCallerClass(); @Override @@ -40,7 +40,7 @@ public final class FrontendServlet extends HttpServlet { Security.addProvider(new BouncyCastleProvider()); try { - metricReporter.startAsync().awaitRunning(10, TimeUnit.SECONDS); + metricReporter.get().startAsync().awaitRunning(10, TimeUnit.SECONDS); logger.info("Started up MetricReporter"); } catch (TimeoutException timeoutException) { logger.severefmt("Failed to initialize MetricReporter: %s", timeoutException); @@ -48,15 +48,12 @@ public final class FrontendServlet extends HttpServlet { LifecycleManager.getInstance() .setShutdownHook( - new ShutdownHook() { - @Override - public void shutdown() { - try { - metricReporter.stopAsync().awaitTerminated(10, TimeUnit.SECONDS); - logger.info("Shut down MetricReporter"); - } catch (TimeoutException timeoutException) { - logger.severefmt("Failed to stop MetricReporter: %s", timeoutException); - } + () -> { + try { + metricReporter.get().stopAsync().awaitTerminated(10, TimeUnit.SECONDS); + logger.info("Shut down MetricReporter"); + } catch (TimeoutException timeoutException) { + logger.severefmt("Failed to stop MetricReporter: %s", timeoutException); } }); } diff --git a/java/google/registry/module/tools/ToolsComponent.java b/java/google/registry/module/tools/ToolsComponent.java index 1518e7801..7f00a22f2 100644 --- a/java/google/registry/module/tools/ToolsComponent.java +++ b/java/google/registry/module/tools/ToolsComponent.java @@ -32,6 +32,7 @@ import google.registry.request.Modules.DatastoreServiceModule; import google.registry.request.Modules.GoogleCredentialModule; import google.registry.request.Modules.Jackson2Module; import google.registry.request.Modules.ModulesServiceModule; +import google.registry.request.Modules.NetHttpTransportModule; import google.registry.request.Modules.UrlFetchTransportModule; import google.registry.request.Modules.UseAppIdentityCredentialForGoogleApisModule; import google.registry.request.Modules.UserServiceModule; @@ -43,31 +44,33 @@ import javax.inject.Singleton; /** Dagger component with instance lifetime for "tools" App Engine module. */ @Singleton @Component( - modules = { - AppIdentityCredentialModule.class, - AuthModule.class, - ConfigModule.class, - CustomLogicFactoryModule.class, - DatastoreServiceModule.class, - DirectoryModule.class, - DriveModule.class, - GcsServiceModule.class, - GoogleCredentialModule.class, - GroupsModule.class, - GroupssettingsModule.class, - Jackson2Module.class, - KeyModule.class, - KeyringModule.class, - KmsModule.class, - ModulesServiceModule.class, - ServerTridProviderModule.class, - SystemClockModule.class, - SystemSleeperModule.class, - ToolsRequestComponentModule.class, - UrlFetchTransportModule.class, - UseAppIdentityCredentialForGoogleApisModule.class, - UserServiceModule.class, - }) + modules = { + AppIdentityCredentialModule.class, + AuthModule.class, + ConfigModule.class, + CustomLogicFactoryModule.class, + DatastoreServiceModule.class, + DirectoryModule.class, + DriveModule.class, + GcsServiceModule.class, + GoogleCredentialModule.class, + GroupsModule.class, + GroupssettingsModule.class, + Jackson2Module.class, + KeyModule.class, + KeyringModule.class, + KmsModule.class, + ModulesServiceModule.class, + NetHttpTransportModule.class, + ServerTridProviderModule.class, + SystemClockModule.class, + SystemSleeperModule.class, + ToolsRequestComponentModule.class, + UrlFetchTransportModule.class, + UseAppIdentityCredentialForGoogleApisModule.class, + UserServiceModule.class, + } +) interface ToolsComponent { ToolsRequestHandler requestHandler(); } diff --git a/java/google/registry/monitoring/whitebox/BUILD b/java/google/registry/monitoring/whitebox/BUILD index dbfc1e4fd..2254efbad 100644 --- a/java/google/registry/monitoring/whitebox/BUILD +++ b/java/google/registry/monitoring/whitebox/BUILD @@ -17,6 +17,7 @@ java_library( "//java/google/registry/request/auth", "//java/google/registry/util", "//third_party/java/objectify:objectify-v4_1", + "@com_google_api_client", "@com_google_apis_google_api_services_bigquery", "@com_google_apis_google_api_services_monitoring", "@com_google_appengine_api_1_0_sdk", diff --git a/java/google/registry/monitoring/whitebox/StackdriverModule.java b/java/google/registry/monitoring/whitebox/StackdriverModule.java index 78e84e8f4..b533ad29a 100644 --- a/java/google/registry/monitoring/whitebox/StackdriverModule.java +++ b/java/google/registry/monitoring/whitebox/StackdriverModule.java @@ -14,16 +14,16 @@ package google.registry.monitoring.whitebox; -import com.google.api.client.http.HttpRequestInitializer; -import com.google.api.client.http.HttpTransport; +import com.google.api.client.googleapis.auth.oauth2.GoogleCredential; +import com.google.api.client.http.javanet.NetHttpTransport; import com.google.api.client.json.JsonFactory; import com.google.api.services.monitoring.v3.Monitoring; import com.google.api.services.monitoring.v3.MonitoringScopes; import com.google.api.services.monitoring.v3.model.MonitoredResource; -import com.google.appengine.api.ThreadManager; import com.google.appengine.api.modules.ModulesService; import com.google.common.base.Function; import com.google.common.collect.ImmutableMap; +import com.google.common.util.concurrent.ThreadFactoryBuilder; import dagger.Module; import dagger.Provides; import google.registry.config.RegistryConfig.Config; @@ -43,9 +43,9 @@ public final class StackdriverModule { @Provides static Monitoring provideMonitoring( - HttpTransport transport, + NetHttpTransport transport, JsonFactory jsonFactory, - Function, ? extends HttpRequestInitializer> credential, + Function, GoogleCredential> credential, @Config("projectId") String projectId) { return new Monitoring.Builder(transport, jsonFactory, credential.apply(MonitoringScopes.all())) .setApplicationName(projectId) @@ -87,6 +87,8 @@ public final class StackdriverModule { static MetricReporter provideMetricReporter( MetricWriter metricWriter, @Config("metricsWriteInterval") Duration writeInterval) { return new MetricReporter( - metricWriter, writeInterval.getStandardSeconds(), ThreadManager.backgroundThreadFactory()); + metricWriter, + writeInterval.getStandardSeconds(), + new ThreadFactoryBuilder().setDaemon(true).build()); } } diff --git a/java/google/registry/request/Modules.java b/java/google/registry/request/Modules.java index 01fe2514e..5d3b7d5c1 100644 --- a/java/google/registry/request/Modules.java +++ b/java/google/registry/request/Modules.java @@ -20,8 +20,10 @@ import static java.nio.charset.StandardCharsets.UTF_8; import com.google.api.client.extensions.appengine.http.UrlFetchTransport; import com.google.api.client.googleapis.auth.oauth2.GoogleCredential; import com.google.api.client.googleapis.extensions.appengine.auth.oauth2.AppIdentityCredential; +import com.google.api.client.googleapis.javanet.GoogleNetHttpTransport; import com.google.api.client.http.HttpRequestInitializer; import com.google.api.client.http.HttpTransport; +import com.google.api.client.http.javanet.NetHttpTransport; import com.google.api.client.json.JsonFactory; import com.google.api.client.json.jackson2.JacksonFactory; import com.google.appengine.api.datastore.DatastoreService; @@ -117,6 +119,24 @@ public final class Modules { } } + /** + * Dagger module that provides standard {@link NetHttpTransport}. Used in non App Engine + * environment. + */ + @Module + public static final class NetHttpTransportModule { + + @Provides + @Singleton + static NetHttpTransport provideNetHttpTransport() { + try { + return GoogleNetHttpTransport.newTrustedTransport(); + } catch (Exception e) { + throw new RuntimeException(e); + } + } + } + /** * Dagger module providing {@link AppIdentityCredential}. * @@ -140,8 +160,8 @@ public final class Modules { @Module public abstract static class UseAppIdentityCredentialForGoogleApisModule { @Binds - abstract Function, ? extends HttpRequestInitializer> - provideHttpRequestInitializer(Function, AppIdentityCredential> credential); + abstract Function, ? extends HttpRequestInitializer> provideHttpRequestInitializer( + Function, AppIdentityCredential> credential); } /** @@ -155,8 +175,8 @@ public final class Modules { @Module public abstract static class UseGoogleCredentialForGoogleApisModule { @Binds - abstract Function, ? extends HttpRequestInitializer> - provideHttpRequestInitializer(Function, GoogleCredential> credential); + abstract Function, ? extends HttpRequestInitializer> provideHttpRequestInitializer( + Function, GoogleCredential> credential); } /** @@ -169,8 +189,8 @@ public final class Modules { * this authentication method should only be used for the following situations: * *
    - *
  1. Locally-running programs (which aren't executing on the App Engine platform) - *
  2. Spreadsheet service (which can't use {@link AppIdentityCredential} due to an old library) + *
  3. Locally-running programs (which aren't executing on the App Engine platform) + *
  4. Spreadsheet service (which can't use {@link AppIdentityCredential} due to an old library) *
* * @see google.registry.keyring.api.Keyring#getJsonCredential() @@ -181,12 +201,14 @@ public final class Modules { @Provides @Singleton static GoogleCredential provideGoogleCredential( - HttpTransport httpTransport, + NetHttpTransport netHttpTransport, JsonFactory jsonFactory, @Key("jsonCredential") String jsonCredential) { try { return GoogleCredential.fromStream( - new ByteArrayInputStream(jsonCredential.getBytes(UTF_8)), httpTransport, jsonFactory); + new ByteArrayInputStream(jsonCredential.getBytes(UTF_8)), + netHttpTransport, + jsonFactory); } catch (IOException e) { throw new RuntimeException(e); } @@ -199,7 +221,7 @@ public final class Modules { } /** - * Provides a GoogleCredential that will connect to GAE using delegated admin access. This is + * Provides a GoogleCredential that will connect to GAE using delegated admin access. This is * needed for API calls requiring domain admin access to the relevant GAFYD using delegated * scopes, e.g. the Directory API and the Groupssettings API. *