From a1e5debc0b7dfc28cf5e376f809076f0bc80efb5 Mon Sep 17 00:00:00 2001 From: Etan Shaul Date: Tue, 20 Jun 2017 16:40:48 -0400 Subject: [PATCH 1/6] Adds CloudSdk parameters for setting structured logs value --- .../tools/appengine/cloudsdk/CloudSdk.java | 24 ++++++++++++++----- 1 file changed, 18 insertions(+), 6 deletions(-) diff --git a/src/main/java/com/google/cloud/tools/appengine/cloudsdk/CloudSdk.java b/src/main/java/com/google/cloud/tools/appengine/cloudsdk/CloudSdk.java index bad8413ea..0db0aa836 100644 --- a/src/main/java/com/google/cloud/tools/appengine/cloudsdk/CloudSdk.java +++ b/src/main/java/com/google/cloud/tools/appengine/cloudsdk/CloudSdk.java @@ -79,15 +79,18 @@ public class CloudSdk { private final ProcessRunner processRunner; private final String appCommandMetricsEnvironment; private final String appCommandMetricsEnvironmentVersion; - @Nullable private final File appCommandCredentialFile; private final String appCommandOutputFormat; + private final String appCommandShowStructuredLogs; private final WaitingProcessOutputLineListener runDevAppServerWaitListener; - private CloudSdk(Path sdkPath, Path javaHomePath, String appCommandMetricsEnvironment, - String appCommandMetricsEnvironmentVersion, + private CloudSdk(Path sdkPath, Path javaHomePath, + @Nullable String appCommandMetricsEnvironment, + @Nullable String appCommandMetricsEnvironmentVersion, @Nullable File appCommandCredentialFile, - String appCommandOutputFormat, ProcessRunner processRunner, + @Nullable String appCommandOutputFormat, + @Nullable String appCommandShowStructuredLogs, + ProcessRunner processRunner, WaitingProcessOutputLineListener runDevAppServerWaitListener) { this.sdkPath = sdkPath; this.javaHomePath = javaHomePath; @@ -95,6 +98,7 @@ private CloudSdk(Path sdkPath, Path javaHomePath, String appCommandMetricsEnviro this.appCommandMetricsEnvironmentVersion = appCommandMetricsEnvironmentVersion; this.appCommandCredentialFile = appCommandCredentialFile; this.appCommandOutputFormat = appCommandOutputFormat; + this.appCommandShowStructuredLogs = appCommandShowStructuredLogs; this.processRunner = processRunner; this.runDevAppServerWaitListener = runDevAppServerWaitListener; @@ -176,6 +180,9 @@ private void runGcloudCommand(List args, File workingDirectory, String.. if (appCommandMetricsEnvironmentVersion != null) { environment.put("CLOUDSDK_METRICS_ENVIRONMENT_VERSION", appCommandMetricsEnvironmentVersion); } + if (appCommandShowStructuredLogs != null) { + environment.put("CLOUDSDK_SHOW_STRUCTURED_LOGS", appCommandShowStructuredLogs); + } // This is to ensure IDE credentials get correctly passed to the gcloud commands, in Windows. // It's a temporary workaround until a fix is released. // https://github.com/GoogleCloudPlatform/google-cloud-intellij/issues/985 @@ -525,9 +532,9 @@ public static class Builder { private Path sdkPath; private String appCommandMetricsEnvironment; private String appCommandMetricsEnvironmentVersion; - @Nullable private File appCommandCredentialFile; private String appCommandOutputFormat; + private String appCommandShowStructuredLogs; private boolean async = false; private List stdOutLineListeners = new ArrayList<>(); private List stdErrLineListeners = new ArrayList<>(); @@ -584,6 +591,11 @@ public Builder appCommandOutputFormat(String appCommandOutputFormat) { return this; } + public Builder appCommandShowStructuredLogs(String appCommandShowStructuredLogs) { + this.appCommandShowStructuredLogs = appCommandShowStructuredLogs; + return this; + } + /** * Whether to run commands asynchronously. */ @@ -713,7 +725,7 @@ public CloudSdk build() { return new CloudSdk(sdkPath, javaHomePath, appCommandMetricsEnvironment, appCommandMetricsEnvironmentVersion, appCommandCredentialFile, appCommandOutputFormat, - processRunner, runDevAppServerWaitListener); + appCommandShowStructuredLogs, processRunner, runDevAppServerWaitListener); } /** From c66ce8b648ea3a38f24042eeb06eb7f13d455feb Mon Sep 17 00:00:00 2001 From: Etan Shaul Date: Wed, 21 Jun 2017 11:26:02 -0400 Subject: [PATCH 2/6] fix env var name --- .../com/google/cloud/tools/appengine/cloudsdk/CloudSdk.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/com/google/cloud/tools/appengine/cloudsdk/CloudSdk.java b/src/main/java/com/google/cloud/tools/appengine/cloudsdk/CloudSdk.java index 0db0aa836..6f58ef8d0 100644 --- a/src/main/java/com/google/cloud/tools/appengine/cloudsdk/CloudSdk.java +++ b/src/main/java/com/google/cloud/tools/appengine/cloudsdk/CloudSdk.java @@ -181,7 +181,7 @@ private void runGcloudCommand(List args, File workingDirectory, String.. environment.put("CLOUDSDK_METRICS_ENVIRONMENT_VERSION", appCommandMetricsEnvironmentVersion); } if (appCommandShowStructuredLogs != null) { - environment.put("CLOUDSDK_SHOW_STRUCTURED_LOGS", appCommandShowStructuredLogs); + environment.put("CLOUDSDK_CORE_SHOW_STRUCTURED_LOGS", appCommandShowStructuredLogs); } // This is to ensure IDE credentials get correctly passed to the gcloud commands, in Windows. // It's a temporary workaround until a fix is released. From 04ecfbdcc4de0090d3784d74af789072c06ac406 Mon Sep 17 00:00:00 2001 From: Etan Shaul Date: Wed, 21 Jun 2017 11:31:44 -0400 Subject: [PATCH 3/6] javadoc --- .../com/google/cloud/tools/appengine/cloudsdk/CloudSdk.java | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/main/java/com/google/cloud/tools/appengine/cloudsdk/CloudSdk.java b/src/main/java/com/google/cloud/tools/appengine/cloudsdk/CloudSdk.java index 6f58ef8d0..6332d2823 100644 --- a/src/main/java/com/google/cloud/tools/appengine/cloudsdk/CloudSdk.java +++ b/src/main/java/com/google/cloud/tools/appengine/cloudsdk/CloudSdk.java @@ -591,6 +591,10 @@ public Builder appCommandOutputFormat(String appCommandOutputFormat) { return this; } + /** + * Sets structured json logs for the stderr output. Supported values include 'never' (default), + * 'always', 'terminal', etc. + */ public Builder appCommandShowStructuredLogs(String appCommandShowStructuredLogs) { this.appCommandShowStructuredLogs = appCommandShowStructuredLogs; return this; From 747fdb6901586ae20bde6c16051ee03976b661ea Mon Sep 17 00:00:00 2001 From: Etan Shaul Date: Wed, 21 Jun 2017 13:33:50 -0400 Subject: [PATCH 4/6] bump min version requirement --- .../com/google/cloud/tools/appengine/cloudsdk/CloudSdk.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/com/google/cloud/tools/appengine/cloudsdk/CloudSdk.java b/src/main/java/com/google/cloud/tools/appengine/cloudsdk/CloudSdk.java index 6332d2823..a5577ba1e 100644 --- a/src/main/java/com/google/cloud/tools/appengine/cloudsdk/CloudSdk.java +++ b/src/main/java/com/google/cloud/tools/appengine/cloudsdk/CloudSdk.java @@ -59,7 +59,7 @@ */ public class CloudSdk { - public static final CloudSdkVersion MINIMUM_VERSION = new CloudSdkVersion("145.0.0"); + public static final CloudSdkVersion MINIMUM_VERSION = new CloudSdkVersion("159.0.0"); private static final Logger logger = Logger.getLogger(CloudSdk.class.getName()); private static final Joiner WHITESPACE_JOINER = Joiner.on(" "); From 1a46a2984aa6a1f3b39c2fc64ef6856a7011a561 Mon Sep 17 00:00:00 2001 From: Etan Shaul Date: Wed, 21 Jun 2017 14:17:46 -0400 Subject: [PATCH 5/6] unit tests for gcloud command environment variables --- .../tools/appengine/cloudsdk/CloudSdk.java | 18 +++++-- .../appengine/cloudsdk/CloudSdkTest.java | 53 ++++++++++++------- 2 files changed, 48 insertions(+), 23 deletions(-) diff --git a/src/main/java/com/google/cloud/tools/appengine/cloudsdk/CloudSdk.java b/src/main/java/com/google/cloud/tools/appengine/cloudsdk/CloudSdk.java index a5577ba1e..b099527f8 100644 --- a/src/main/java/com/google/cloud/tools/appengine/cloudsdk/CloudSdk.java +++ b/src/main/java/com/google/cloud/tools/appengine/cloudsdk/CloudSdk.java @@ -169,9 +169,20 @@ private void runGcloudCommand(List args, File workingDirectory, String.. command.addAll(args); command.addAll(GcloudArgs.get("format", appCommandOutputFormat)); - Map environment = Maps.newHashMap(); if (appCommandCredentialFile != null) { command.addAll(GcloudArgs.get("credential-file-override", appCommandCredentialFile)); + } + + logCommand(command); + processRunner.setEnvironment(getGcloudCommandEnvironment()); + processRunner.setWorkingDirectory(workingDirectory); + processRunner.run(command.toArray(new String[command.size()])); + } + + @VisibleForTesting + Map getGcloudCommandEnvironment() { + Map environment = Maps.newHashMap(); + if (appCommandCredentialFile != null) { environment.put("CLOUDSDK_APP_USE_GSUTIL", "0"); } if (appCommandMetricsEnvironment != null) { @@ -192,10 +203,7 @@ private void runGcloudCommand(List args, File workingDirectory, String.. environment.put("CLOUDSDK_CORE_DISABLE_PROMPTS", "1"); - logCommand(command); - processRunner.setEnvironment(environment); - processRunner.setWorkingDirectory(workingDirectory); - processRunner.run(command.toArray(new String[command.size()])); + return environment; } // Runs a gcloud command synchronously, with a new ProcessRunner. This method is intended to be diff --git a/src/test/java/com/google/cloud/tools/appengine/cloudsdk/CloudSdkTest.java b/src/test/java/com/google/cloud/tools/appengine/cloudsdk/CloudSdkTest.java index 0e323c276..e7fefdd89 100644 --- a/src/test/java/com/google/cloud/tools/appengine/cloudsdk/CloudSdkTest.java +++ b/src/test/java/com/google/cloud/tools/appengine/cloudsdk/CloudSdkTest.java @@ -16,24 +16,6 @@ package com.google.cloud.tools.appengine.cloudsdk; -import com.google.cloud.tools.appengine.cloudsdk.CloudSdk.Builder; -import com.google.cloud.tools.appengine.cloudsdk.process.ProcessOutputLineListener; -import com.google.common.io.Files; - -import org.junit.Before; -import org.junit.Test; -import org.junit.runner.RunWith; -import org.mockito.Mock; -import org.mockito.Mockito; -import org.mockito.junit.MockitoJUnitRunner; - -import java.io.IOException; -import java.nio.charset.Charset; -import java.nio.file.Path; -import java.nio.file.Paths; -import java.util.Arrays; -import java.util.List; - import static org.hamcrest.core.AnyOf.anyOf; import static org.hamcrest.core.Is.is; import static org.hamcrest.core.StringEndsWith.endsWith; @@ -42,8 +24,27 @@ import static org.junit.Assert.assertNull; import static org.junit.Assert.assertThat; import static org.junit.Assert.fail; +import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; +import com.google.cloud.tools.appengine.cloudsdk.CloudSdk.Builder; +import com.google.cloud.tools.appengine.cloudsdk.process.ProcessOutputLineListener; +import com.google.common.io.Files; +import java.io.File; +import java.io.IOException; +import java.nio.charset.Charset; +import java.nio.file.Path; +import java.nio.file.Paths; +import java.util.Arrays; +import java.util.List; +import java.util.Map; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.Mock; +import org.mockito.Mockito; +import org.mockito.junit.MockitoJUnitRunner; + /** * Unit tests for {@link CloudSdk}. */ @@ -239,4 +240,20 @@ public void testGetJavaBinary() { System.getProperty("os.name").contains("Windows") ? "java.exe" : "java").toAbsolutePath(), sdk.getJavaExecutablePath()); } + + @Test + public void testGcloudCommandEnvironment() { + builder.appCommandShowStructuredLogs("always"); + builder.appCommandCredentialFile(mock(File.class)); + builder.appCommandMetricsEnvironment("intellij"); + builder.appCommandMetricsEnvironmentVersion("99"); + CloudSdk sdk = builder.build(); + + Map env = sdk.getGcloudCommandEnvironment(); + assertEquals("0", env.get("CLOUDSDK_APP_USE_GSUTIL")); + assertEquals("always", env.get("CLOUDSDK_CORE_SHOW_STRUCTURED_LOGS")); + assertEquals("intellij", env.get("CLOUDSDK_METRICS_ENVIRONMENT")); + assertEquals("99", env.get("CLOUDSDK_METRICS_ENVIRONMENT_VERSION")); + assertEquals("1", env.get("CLOUDSDK_CORE_DISABLE_PROMPTS")); + } } From 1c83d0b18fee12f925e0824065f53ec23f82f640 Mon Sep 17 00:00:00 2001 From: Etan Shaul Date: Fri, 23 Jun 2017 10:43:25 -0400 Subject: [PATCH 6/6] add @Nullable --- .../com/google/cloud/tools/appengine/cloudsdk/CloudSdk.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/main/java/com/google/cloud/tools/appengine/cloudsdk/CloudSdk.java b/src/main/java/com/google/cloud/tools/appengine/cloudsdk/CloudSdk.java index b099527f8..452123f07 100644 --- a/src/main/java/com/google/cloud/tools/appengine/cloudsdk/CloudSdk.java +++ b/src/main/java/com/google/cloud/tools/appengine/cloudsdk/CloudSdk.java @@ -84,7 +84,8 @@ public class CloudSdk { private final String appCommandShowStructuredLogs; private final WaitingProcessOutputLineListener runDevAppServerWaitListener; - private CloudSdk(Path sdkPath, Path javaHomePath, + private CloudSdk(Path sdkPath, + @Nullable Path javaHomePath, @Nullable String appCommandMetricsEnvironment, @Nullable String appCommandMetricsEnvironmentVersion, @Nullable File appCommandCredentialFile,