From 04ce649df36c92661e88a3dd8b1693fd30814d12 Mon Sep 17 00:00:00 2001 From: Lachlan Roberts Date: Thu, 23 Nov 2023 14:19:42 +1100 Subject: [PATCH 1/2] Improve debugging for JavaRuntimeViaHttpBase and SpringBootTest Signed-off-by: Lachlan Roberts --- .../runtime/jetty/proxy/JettyHttpProxy.java | 4 --- runtime/test/pom.xml | 23 ++++++++++++ .../jetty9/JavaRuntimeViaHttpBase.java | 35 ++++++++++++------- .../runtime/jetty9/SpringBootTest.java | 2 +- 4 files changed, 47 insertions(+), 17 deletions(-) diff --git a/runtime/runtime_impl_jetty12/src/main/java/com/google/apphosting/runtime/jetty/proxy/JettyHttpProxy.java b/runtime/runtime_impl_jetty12/src/main/java/com/google/apphosting/runtime/jetty/proxy/JettyHttpProxy.java index 152fd8af..3e8d463f 100644 --- a/runtime/runtime_impl_jetty12/src/main/java/com/google/apphosting/runtime/jetty/proxy/JettyHttpProxy.java +++ b/runtime/runtime_impl_jetty12/src/main/java/com/google/apphosting/runtime/jetty/proxy/JettyHttpProxy.java @@ -73,8 +73,6 @@ */ public class JettyHttpProxy { private static final GoogleLogger logger = GoogleLogger.forEnclosingClass(); - private static final String JETTY_LOG_CLASS = "org.eclipse.jetty.util.log.class"; - private static final String JETTY_STDERRLOG = "org.eclipse.jetty.util.log.StdErrLog"; private static final long MAX_REQUEST_SIZE = 32 * 1024 * 1024; /** @@ -83,8 +81,6 @@ public class JettyHttpProxy { */ public static void startServer(ServletEngineAdapter.Config runtimeOptions) { try { - System.setProperty(JETTY_LOG_CLASS, JETTY_STDERRLOG); - ForwardingHandler handler = new ForwardingHandler(runtimeOptions, System.getenv()); handler.init(); Server server = newServer(runtimeOptions, handler); diff --git a/runtime/test/pom.xml b/runtime/test/pom.xml index cfdcd879..d5ff7cef 100644 --- a/runtime/test/pom.xml +++ b/runtime/test/pom.xml @@ -30,6 +30,7 @@ true + false @@ -108,5 +109,27 @@ appengine-utils test + + org.awaitility + awaitility + 4.2.0 + test + + + + + + + org.apache.maven.plugins + maven-surefire-plugin + + + ${debug.forked.process} + + + + + + diff --git a/runtime/test/src/test/java/com/google/apphosting/runtime/jetty9/JavaRuntimeViaHttpBase.java b/runtime/test/src/test/java/com/google/apphosting/runtime/jetty9/JavaRuntimeViaHttpBase.java index 463386d2..2e964821 100644 --- a/runtime/test/src/test/java/com/google/apphosting/runtime/jetty9/JavaRuntimeViaHttpBase.java +++ b/runtime/test/src/test/java/com/google/apphosting/runtime/jetty9/JavaRuntimeViaHttpBase.java @@ -29,6 +29,7 @@ import static java.nio.file.StandardCopyOption.REPLACE_EXISTING; import static java.util.concurrent.TimeUnit.MILLISECONDS; import static java.util.concurrent.TimeUnit.SECONDS; +import static org.awaitility.Awaitility.await; import com.google.apphosting.base.protos.api.RemoteApiPb; import com.google.apphosting.testing.PortPicker; @@ -63,6 +64,7 @@ import java.io.InputStreamReader; import java.io.OutputStream; import java.net.InetSocketAddress; +import java.net.Socket; import java.net.URL; import java.nio.file.Files; import java.nio.file.Path; @@ -243,10 +245,12 @@ static RuntimeContext create( .isTrue(); InetSocketAddress apiSocketAddress = new InetSocketAddress(apiPort); - ImmutableList runtimeArgs = - ImmutableList.builder() - .add( - JAVA_HOME.value() + "/bin/java", + ImmutableList.Builder builder = ImmutableList.builder(); + builder.add(JAVA_HOME.value() + "/bin/java"); + if (Boolean.getBoolean("debug.forked.process")) { + builder.add("-agentlib:jdwp=transport=dt_socket,server=y,suspend=y,address=*:8000"); + } + builder.add( "-Dcom.google.apphosting.runtime.jetty94.LEGACY_MODE=" + useJetty94LegacyMode(), "-Dappengine.use.EE8=" + Boolean.getBoolean("appengine.use.EE8"), "-Dappengine.use.EE10=" + Boolean.getBoolean("appengine.use.EE10"), @@ -254,8 +258,8 @@ static RuntimeContext create( "-cp", useMavenJars() ? new File(runtimeDir, "jars/runtime-main.jar").getAbsolutePath() - : new File(runtimeDir, "runtime-main.jar").getAbsolutePath()) - .addAll(optionalFlags()) + : new File(runtimeDir, "runtime-main.jar").getAbsolutePath()); + builder.addAll(optionalFlags()) .addAll(jvmFlagsFromEnvironment(config.environmentEntries())) .add( "com.google.apphosting.runtime.JavaRuntimeMainWithDefaults", @@ -264,18 +268,15 @@ static RuntimeContext create( "--trusted_host=" + HostAndPort.fromParts(apiSocketAddress.getHostString(), apiPort), runtimeDir.getAbsolutePath()) - .addAll(config.launcherFlags()) - .build(); + .addAll(config.launcherFlags()); + ImmutableList runtimeArgs = builder.build(); Process runtimeProcess = launchRuntime(runtimeArgs, config.environmentEntries()); OutputPump outPump = new OutputPump(runtimeProcess.getInputStream(), "[stdout] "); OutputPump errPump = new OutputPump(runtimeProcess.getErrorStream(), "[stderr] "); new Thread(outPump).start(); new Thread(errPump).start(); - // TODO(b/192665275): - // For some reason, a Maven build does not emit anymore this log, need to investigate. - // For now, just wait a bit so the server is started in tests. - Thread.sleep(SERVER_START_TIMEOUT_SECONDS * 100); + await().atMost(30, SECONDS).until(() -> isPortAvailable("localhost", jettyPort)); int timeoutMillis = 30_000; RequestConfig requestConfig = @@ -292,6 +293,16 @@ static RuntimeContext create( runtimeProcess, httpApiServer, httpClient, jettyPort, outPump, errPump); } + public static boolean isPortAvailable(String host, int port) { + try { + Socket socket = new Socket(host, port); + socket.close(); + return true; + } catch (Exception e) { + return false; + } + } + private static List jvmFlagsFromEnvironment(ImmutableMap env) { return Splitter.on(' ').omitEmptyStrings().splitToList(env.getOrDefault("GAE_JAVA_OPTS", "")); } diff --git a/runtime/test/src/test/java/com/google/apphosting/runtime/jetty9/SpringBootTest.java b/runtime/test/src/test/java/com/google/apphosting/runtime/jetty9/SpringBootTest.java index ced7aa95..f97ab707 100644 --- a/runtime/test/src/test/java/com/google/apphosting/runtime/jetty9/SpringBootTest.java +++ b/runtime/test/src/test/java/com/google/apphosting/runtime/jetty9/SpringBootTest.java @@ -62,7 +62,7 @@ private RuntimeContext runtimeContext() throws IOException, Inte private static List readOutput(InputStream inputStream) throws IOException { try (BufferedReader output = new BufferedReader(new InputStreamReader(inputStream))) { - return output.lines().collect(Collectors.toList()); + return output.lines().map(l -> l + "\n").collect(Collectors.toList()); } } From f461fd779f7b68f3876a6050a1be4b0f478ce5e8 Mon Sep 17 00:00:00 2001 From: Lachlan Roberts Date: Tue, 28 Nov 2023 14:26:53 +1100 Subject: [PATCH 2/2] use port number instead of boolean for debugging JavaRuntimeViaHttpBase Signed-off-by: Lachlan Roberts --- runtime/test/pom.xml | 3 +-- .../apphosting/runtime/jetty9/JavaRuntimeViaHttpBase.java | 5 +++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/runtime/test/pom.xml b/runtime/test/pom.xml index d5ff7cef..6964defb 100644 --- a/runtime/test/pom.xml +++ b/runtime/test/pom.xml @@ -30,7 +30,6 @@ true - false @@ -125,7 +124,7 @@ maven-surefire-plugin - ${debug.forked.process} + ${appengine.debug.port} diff --git a/runtime/test/src/test/java/com/google/apphosting/runtime/jetty9/JavaRuntimeViaHttpBase.java b/runtime/test/src/test/java/com/google/apphosting/runtime/jetty9/JavaRuntimeViaHttpBase.java index 2e964821..1ebc526d 100644 --- a/runtime/test/src/test/java/com/google/apphosting/runtime/jetty9/JavaRuntimeViaHttpBase.java +++ b/runtime/test/src/test/java/com/google/apphosting/runtime/jetty9/JavaRuntimeViaHttpBase.java @@ -247,8 +247,9 @@ static RuntimeContext create( ImmutableList.Builder builder = ImmutableList.builder(); builder.add(JAVA_HOME.value() + "/bin/java"); - if (Boolean.getBoolean("debug.forked.process")) { - builder.add("-agentlib:jdwp=transport=dt_socket,server=y,suspend=y,address=*:8000"); + Integer debugPort = Integer.getInteger("appengine.debug.port"); + if (debugPort != null) { + builder.add("-agentlib:jdwp=transport=dt_socket,server=y,suspend=y,address=*:" + debugPort); } builder.add( "-Dcom.google.apphosting.runtime.jetty94.LEGACY_MODE=" + useJetty94LegacyMode(),