Refactor JavaRuntimeParams and JavaRuntimeFactory, removing many unused command-line flags, remove jCommander dependency for simpler parser with simple types.

This change significantly refactors `JavaRuntimeParams` and `JavaRuntimeFactory`. Many command-line parameters have been removed from `JavaRuntimeParams` because they were unused.

The values for several parameters are now hardcoded as constants within `AppEngineConstants` and `JavaRuntimeFactory`, reflecting their static nature in the current runtime configuration.

Specifically, `MAX_RUNTIME_LOG_PER_REQUEST`, `BYTE_COUNT_BEFORE_FLUSHING`, `MAX_LOG_LINE_SIZE`, `MAX_LOG_FLUSH_TIME`, `THREAD_STOP_TERMINATES_CLONE`, `FORCE_URLFETCH_URL_STREAM_HANDLER`, `MAX_REQUEST_SIZE`, and `MAX_RESPONSE_SIZE` have been moved to `AppEngineConstants`.

Additionally, `LEGACY_MODE` and `ASYNC_MODE` in `AppEngineConstants` have been converted from static final fields to static methods (`isLegacyMode()` and `isAsyncMode()`). This allows their values to be dynamically read from system properties, which is necessary because `AppEngineConstants` is now used both before and after application loading, where system properties can change. This change fixes test failures in `AsyncServletTest`.

The command-line parsing library JCommander has been removed. A simpler manual parsing mechanism has been implemented in `JavaRuntimeParams` for the remaining four flags: `--trusted_host`, `--fixed_application_path`, `--jetty_http_port`, and `--clone_max_outstanding_api_rpcs`. Note that while `DEFAULT_MAX_OUTSTANDING_API_RPCS` is now defined in `AppEngineConstants`, the `--clone_max_outstanding_api_rpcs` flag still allows overriding this default.

The unused Cloud Profiler integration, previously configurable via `--enable_cloud_cpu_profiler` and `--enable_cloud_heap_profiler`, has been removed.

The selection of the `ServletEngineAdapter` is now based on system properties (`appengine.use.EE8`, `appengine.use.EE10`, or `appengine.use.EE11`) rather than a command-line parameter.

Other changes include:
*   The system properties `appengine.jetty.also_log_to_apiproxy` and `appengine.urlfetch.deriveResponseMessage` are now unconditionally set to `true` in `JavaRuntimeFactory`.
*   The `disable_api_call_logging_in_apiproxy` setting is now *only* controlled by the system property, removing its command-line parameter control.
*   The constants `SKIP_ADMIN_CHECK_ATTR` and `X_GOOGLE_INTERNAL_SKIPADMINCHECK_UC` have been centralized in `AppEngineConstants`.

PiperOrigin-RevId: 868781401
Change-Id: Ib4e50d2fdd4946133e47713c320338e2617b65ec

Author: ludoch

THIRD-PARTY.txt

@@ -193,7 +193,6 @@ The repository contains 3rd-party code under the following licenses:
         * Java Concurrency Tools Core Library (org.jctools:jctools-core:4.0.5 - https://github.com/JCTools)
         * JBoss Logging 3 (org.jboss.logging:jboss-logging:3.6.0.Final - http://www.jboss.org)
         * JBoss Threads (org.jboss.threads:jboss-threads:3.6.1.Final - http://www.jboss.org)
-        * jcommander (com.beust:jcommander:1.82 - https://jcommander.org)
         * jffi (com.github.jnr:jffi:1.3.9 - http://github.com/jnr/jffi)
         * JGroups (org.jgroups:jgroups:5.4.5.Final - http://www.jgroups.org)
         * jnr-a64asm (com.github.jnr:jnr-a64asm:1.0.0 - http://nexus.sonatype.org/oss-repository-hosting.html/jnr-a64asm)

runtime/impl/pom.xml

@@ -32,10 +32,7 @@
     <description>App Engine runtime implementation.</description>
 
     <dependencies>
-        <dependency>
-            <groupId>com.beust</groupId>
-            <artifactId>jcommander</artifactId>
-        </dependency>
+
         <dependency>
             <groupId>com.contrastsecurity</groupId>
             <artifactId>yamlbeans</artifactId>

runtime/impl/src/main/java/com/google/apphosting/runtime/AppEngineConstants.java

@@ -17,19 +17,55 @@
 package com.google.apphosting.runtime;
 
 import com.google.common.collect.ImmutableSet;
+import java.time.Duration;
 
 /** {@code AppEngineConstants} centralizes some constants that are specific to our use of Jetty. */
 public final class AppEngineConstants {
 
+  // We need these system properties based value to be static methods as now used
+  // in the pre boostrap of the runtime, before these system properties are changed based on 
+  // app settings in appengine-web.xml, so they might change after the system properties are
+  // initialized.
+
   /**
    * If Legacy Mode is turned on, then Jetty is configured to be more forgiving of bad requests and
    * to act more in the style of Jetty-9.3
    */
-  public static final boolean LEGACY_MODE =
-          Boolean.getBoolean("com.google.apphosting.runtime.jetty94.LEGACY_MODE");
+  public static boolean isLegacyMode() {
+    return Boolean.getBoolean("com.google.apphosting.runtime.jetty94.LEGACY_MODE");
+  }
 
   /** Set the Jetty request with Async mode. */
-  public static final boolean ASYNC_MODE = Boolean.getBoolean("com.google.appengine.enable_async");
+  public static boolean isAsyncMode() {
+    return Boolean.getBoolean("com.google.appengine.enable_async");
+  }
+
+  /** The maximum allowed size in bytes of the Runtime Log per request, returned in the UPResponse. */
+  public static final long MAX_RUNTIME_LOG_PER_REQUEST = 3000L * 1024L;
+
+  /** The maximum number of simultaneous APIHost RPCs. */
+  public static final int DEFAULT_MAX_OUTSTANDING_API_RPCS = 100;
+
+  /** Flush application logs when they grow to this size. */
+  public static final long BYTE_COUNT_BEFORE_FLUSHING = 100 * 1024L;
+
+  /** Maximum application log line size. */
+  public static final int MAX_LOG_LINE_SIZE = 16 * 1024;
+
+  /**
+   * Maximum time a log record should be allowed to to be cached in the runtime before being flushed
+   * to the appserver (only applies to non-frontend requests).
+   */
+  public static final Duration MAX_LOG_FLUSH_TIME = Duration.ofSeconds(60);
+
+  /** Always terminate the clone when Thread.stop() is used. */
+  public static final boolean THREAD_STOP_TERMINATES_CLONE = true;
+
+  /**
+   * Force url-stream-handler to 'urlfetch' irrespective of the contents of the appengine-web.xml
+   * descriptor.
+   */
+  public static final boolean FORCE_URLFETCH_URL_STREAM_HANDLER = false;
 
   public static final String GAE_RUNTIME = System.getenv("GAE_RUNTIME");
 
@@ -80,6 +116,8 @@ public final class AppEngineConstants {
   public static final String X_APPENGINE_QUEUENAME = "x-appengine-queuename";
   public static final String X_APPENGINE_TIMEOUT_MS = "x-appengine-timeout-ms";
   public static final String X_GOOGLE_INTERNAL_SKIPADMINCHECK = "x-google-internal-skipadmincheck";
+  // This is the same as X_GOOGLE_INTERNAL_SKIPADMINCHECK, but in upper case. This is used for
+  // case-insensitive comparisons.
   public static final String X_GOOGLE_INTERNAL_SKIPADMINCHECK_UC =
       "X-Google-Internal-SkipAdminCheck";
   public static final String X_GOOGLE_INTERNAL_PROFILER = "x-google-internal-profiler";
@@ -152,5 +190,11 @@ public final class AppEngineConstants {
    */
   public static final int JETTY_RESPONSE_HEADER_SIZE = 262144;
 
+  /** The maximum request size in bytes (32M). */
+  public static final int MAX_REQUEST_SIZE = 32 * 1024 * 1024;
+
+  /** The maximum response size in bytes (32M). */
+  public static final int MAX_RESPONSE_SIZE = 32 * 1024 * 1024;
+
   private AppEngineConstants() {}
 }

runtime/impl/src/main/java/com/google/apphosting/runtime/JavaRuntimeFactory.java

@@ -16,13 +16,23 @@
 
 package com.google.apphosting.runtime;
 
+import static com.google.apphosting.runtime.AppEngineConstants.BYTE_COUNT_BEFORE_FLUSHING;
+import static com.google.apphosting.runtime.AppEngineConstants.CYCLES_PER_SECOND;
+import static com.google.apphosting.runtime.AppEngineConstants.FORCE_URLFETCH_URL_STREAM_HANDLER;
+import static com.google.apphosting.runtime.AppEngineConstants.JETTY_REQUEST_HEADER_SIZE;
+import static com.google.apphosting.runtime.AppEngineConstants.JETTY_RESPONSE_HEADER_SIZE;
+import static com.google.apphosting.runtime.AppEngineConstants.MAX_LOG_FLUSH_TIME;
+import static com.google.apphosting.runtime.AppEngineConstants.MAX_LOG_LINE_SIZE;
+import static com.google.apphosting.runtime.AppEngineConstants.MAX_RUNTIME_LOG_PER_REQUEST;
+import static com.google.apphosting.runtime.AppEngineConstants.SOFT_DEADLINE_DELAY_MS;
+import static com.google.apphosting.runtime.AppEngineConstants.THREAD_STOP_TERMINATES_CLONE;
+
 import com.google.apphosting.api.ApiProxy;
 import com.google.common.annotations.VisibleForTesting;
+import com.google.common.base.VerifyException;
 import com.google.common.flogger.GoogleLogger;
 import com.google.common.net.HostAndPort;
 import java.io.File;
-import java.lang.reflect.Method;
-import java.time.Duration;
 import java.util.List;
 import java.util.Optional;
 import java.util.OptionalInt;
@@ -54,27 +64,24 @@ public JavaRuntime getStartedRuntime(NullSandboxPlugin sandboxPlugin, String[] a
       logger.atWarning().log("Unknown command line arguments: %s", unknownParams);
     }
 
-    RuntimeLogSink logSink = new RuntimeLogSink(params.getMaxRuntimeLogPerRequest());
+    RuntimeLogSink logSink = new RuntimeLogSink(MAX_RUNTIME_LOG_PER_REQUEST);
     logSink.addHandlerToRootLogger();
 
-    maybeConfigureProfiler(params);
+    // Cloud Profiler is now set up externally, typically at the application entry point.
+    // See:
+    // https://github.com/GoogleCloudPlatform/appengine-java-standard?tab=readme-ov-file#entry-point-features
 
-    if (params.getLogJettyExceptionsToAppLogs()) {
-      // This system property is checked by JettyLogger, which is
-      // instantiated via reflection by Jetty and therefore we cannot
-      // pass options into it directly.
-      //
-      // TODO: This logic should really move down to
-      // JettyServletAdapter.
-      System.setProperty("appengine.jetty.also_log_to_apiproxy", "true");
-    }
+    // This system property is checked by JettyLogger, which is
+    // instantiated via reflection by Jetty and therefore we cannot
+    // pass options into it directly.
+    //
+    // TODO: This logic should really move down to
+    // JettyServletAdapter.
+    System.setProperty("appengine.jetty.also_log_to_apiproxy", "true");
 
-    if (params.getUrlfetchDeriveResponseMessage()) {
-      // This system property is checked by URLFetch's Connection class, which
-      // is directly instantiated by the Java API and cannot take constructor
-      // arguments.
-      System.setProperty("appengine.urlfetch.deriveResponseMessage", "true");
-    }
+    // This system property is always set to true and is checked by URLFetch's Connection class,
+    // which is directly instantiated by the Java API and cannot take constructor arguments.
+    System.setProperty("appengine.urlfetch.deriveResponseMessage", "true");
 
     // This system property is checked by GMTransport, which is directly
     // registered with JavaMail and cannot take additional constructor arguments.
@@ -86,37 +93,41 @@ public JavaRuntime getStartedRuntime(NullSandboxPlugin sandboxPlugin, String[] a
     // registered with JavaMail and cannot take additional constructor arguments.
     System.setProperty("appengine.mail.filenamePreventsInlining", "true");
 
-    ServletEngineAdapter servletEngine = createServletEngine(params);
-    ApiDeadlineOracle deadlineOracle =
-        new ApiDeadlineOracle.Builder().initDeadlineMap().build();
+    ServletEngineAdapter servletEngine = createServletEngine();
+    ApiDeadlineOracle deadlineOracle = new ApiDeadlineOracle.Builder().initDeadlineMap().build();
 
     ApiHostClientFactory apiHostFactory = new ApiHostClientFactory();
 
     BackgroundRequestCoordinator coordinator = new BackgroundRequestCoordinator();
 
+    // The disableApiCallLogging setting is controlled by a system property,
+    // which is consistent with other runtime settings configured via system properties.
+    // If command-line parameter control is also needed, this logic should be updated
+    // to potentially check a value parsed from 'args' via JavaRuntimeParams.
+    boolean disableApiCallLogging = Boolean.getBoolean("disable_api_call_logging_in_apiproxy");
+
     ApiProxyImpl apiProxyImpl =
         ApiProxyImpl.builder()
             .setApiHost(
                 apiHostFactory.newAPIHost(
-                    params.getTrustedHost(),
-                    OptionalInt.of(params.getCloneMaxOutstandingApiRpcs())))
+                    params.getTrustedHost(), OptionalInt.of(params.getMaxOutstandingApiRpcs())))
             .setDeadlineOracle(deadlineOracle)
             .setExternalDatacenterName("MARS")
-            .setByteCountBeforeFlushing(params.getByteCountBeforeFlushing())
-            .setMaxLogLineSize(params.getMaxLogLineSize())
-            .setMaxLogFlushTime(Duration.ofSeconds(params.getMaxLogFlushSeconds()))
+            .setByteCountBeforeFlushing(BYTE_COUNT_BEFORE_FLUSHING)
+            .setMaxLogLineSize(MAX_LOG_LINE_SIZE)
+            .setMaxLogFlushTime(MAX_LOG_FLUSH_TIME)
             .setCoordinator(coordinator)
-            .setDisableApiCallLogging(params.getDisableApiCallLogging())
+            .setDisableApiCallLogging(disableApiCallLogging)
             .build();
 
     RequestManager.Builder requestManagerBuilder =
         RequestManager.builder()
-            .setSoftDeadlineDelay(AppEngineConstants.SOFT_DEADLINE_DELAY_MS)
+            .setSoftDeadlineDelay(SOFT_DEADLINE_DELAY_MS)
             .setRuntimeLogSink(Optional.of(logSink))
             .setApiProxyImpl(apiProxyImpl)
-            .setMaxOutstandingApiRpcs(params.getCloneMaxOutstandingApiRpcs())
-            .setThreadStopTerminatesClone(params.getThreadStopTerminatesClone())
-            .setCyclesPerSecond(AppEngineConstants.CYCLES_PER_SECOND);
+            .setMaxOutstandingApiRpcs(params.getMaxOutstandingApiRpcs())
+            .setThreadStopTerminatesClone(THREAD_STOP_TERMINATES_CLONE)
+            .setCyclesPerSecond(CYCLES_PER_SECOND);
 
     RequestManager requestManager = makeRequestManager(requestManagerBuilder);
     apiProxyImpl.setRequestManager(requestManager);
@@ -134,7 +145,7 @@ public JavaRuntime getStartedRuntime(NullSandboxPlugin sandboxPlugin, String[] a
             .setConfiguration(configuration)
             .setDeadlineOracle(deadlineOracle)
             .setCoordinator(coordinator)
-            .setForceUrlfetchUrlStreamHandler(params.getForceUrlfetchUrlStreamHandler())
+            .setForceUrlfetchUrlStreamHandler(FORCE_URLFETCH_URL_STREAM_HANDLER)
             .setFixedApplicationPath(params.getFixedApplicationPath());
 
     JavaRuntime runtime = makeRuntime(runtimeBuilder);
@@ -145,8 +156,8 @@ public JavaRuntime getStartedRuntime(NullSandboxPlugin sandboxPlugin, String[] a
             .setApplicationRoot("notused")
             .setFixedApplicationPath(params.getFixedApplicationPath())
             .setJettyHttpAddress(HostAndPort.fromParts("0.0.0.0", params.getJettyHttpPort()))
-            .setJettyRequestHeaderSize(AppEngineConstants.JETTY_REQUEST_HEADER_SIZE)
-            .setJettyResponseHeaderSize(AppEngineConstants.JETTY_RESPONSE_HEADER_SIZE)
+            .setJettyRequestHeaderSize(JETTY_REQUEST_HEADER_SIZE)
+            .setJettyResponseHeaderSize(JETTY_RESPONSE_HEADER_SIZE)
             .setEvaluationRuntimeServerInterface(runtime)
             .build();
     try {
@@ -170,37 +181,23 @@ public RequestManager makeRequestManager(RequestManager.Builder builder) {
     return builder.build();
   }
 
-  /** Creates the ServletEngineAdapter specifies by the --servlet_engine flag. */
-  private static ServletEngineAdapter createServletEngine(JavaRuntimeParams params) {
-    Class<? extends ServletEngineAdapter> engineClazz = params.getServletEngine();
-    if (engineClazz == null) {
-      throw new RuntimeException("No servlet engine (--servlet_engine) defined in the parameters.");
+  /** Creates the ServletEngineAdapter. */
+  private static ServletEngineAdapter createServletEngine() {
+    String servletEngine;
+    if (Boolean.getBoolean("appengine.use.EE8")
+        || Boolean.getBoolean("appengine.use.EE10")
+        || Boolean.getBoolean("appengine.use.EE11")) {
+      servletEngine = "com.google.apphosting.runtime.jetty.JettyServletEngineAdapter";
+    } else {
+      servletEngine = "com.google.apphosting.runtime.jetty9.JettyServletEngineAdapter";
     }
+
     try {
+      Class<? extends ServletEngineAdapter> engineClazz =
+          Class.forName(servletEngine).asSubclass(ServletEngineAdapter.class);
       return engineClazz.getConstructor().newInstance();
     } catch (ReflectiveOperationException ex) {
-      throw new RuntimeException("Failed to instantiate " + engineClazz, ex);
-    }
-  }
-
-  private static void maybeConfigureProfiler(JavaRuntimeParams params) {
-    if (params.getEnableCloudCpuProfiler() || params.getEnableCloudHeapProfiler()) {
-      try {
-        Class<?> profilerClass = Class.forName("com.google.cloud.profiler.agent.Profiler");
-        Class<?> profilerConfigClass =
-            Class.forName("com.google.cloud.profiler.agent.Profiler$Config");
-        Object profilerConfig = profilerConfigClass.getConstructor().newInstance();
-        Method setCpuProfilerEnabled =
-            profilerConfigClass.getMethod("setCpuProfilerEnabled", boolean.class);
-        Method setHeapProfilerEnabled =
-            profilerConfigClass.getMethod("setHeapProfilerEnabled", boolean.class);
-        setCpuProfilerEnabled.invoke(profilerConfig, params.getEnableCloudCpuProfiler());
-        setHeapProfilerEnabled.invoke(profilerConfig, params.getEnableCloudHeapProfiler());
-        Method start = profilerClass.getMethod("start", profilerConfigClass);
-        start.invoke(null, profilerConfig);
-      } catch (Exception e) {
-        logger.atWarning().withCause(e).log("Failed to start the profiler");
-      }
+      throw new VerifyException("Failed to instantiate " + servletEngine, ex);
     }
   }
 }