test(logging): Add verbose logging and reroute to test files via mave…

…n failsafe plugin (#2295) test(logging): Add verbose logging and reroute to test files via maven failsafe plugin
googleapis · Jul 29, 2024 · 46d95ca · 46d95ca
1 parent 4888836
commit 46d95ca
Show file tree

Hide file tree

Showing 8 changed files with 30 additions and 56 deletions.
diff --git a/.kokoro/nightly/integration.cfg b/.kokoro/nightly/integration.cfg
@@ -8,7 +8,7 @@ env_vars: {
 
 env_vars: {
     key: "INTEGRATION_TEST_ARGS"
-    value: "-P bigtable-emulator-it,bigtable-prod-it,bigtable-prod-batch-it -Dbigtable.project=gcloud-devel -Dbigtable.instance=google-cloud-bigtable -Dbigtable.table=integration-tests -Dbigtable.kms_key_name=projects/gcloud-devel/locations/us-east1/keyRings/cmek-test-key-ring/cryptoKeys/cmek-test-key -Dbigtable.wait-for-cmek-key-status=true"
+    value: "-P bigtable-emulator-it,bigtable-prod-it,bigtable-prod-batch-it,enable-verbose-grpc-logs -Dbigtable.project=gcloud-devel -Dbigtable.instance=google-cloud-bigtable -Dbigtable.table=integration-tests -Dbigtable.kms_key_name=projects/gcloud-devel/locations/us-east1/keyRings/cmek-test-key-ring/cryptoKeys/cmek-test-key -Dbigtable.wait-for-cmek-key-status=true"
 }
 
 env_vars: {

diff --git a/.kokoro/presubmit/graalvm-native-17.cfg b/.kokoro/presubmit/graalvm-native-17.cfg
@@ -13,7 +13,7 @@ env_vars: {
 
 env_vars: {
     key: "INTEGRATION_TEST_ARGS"
-    value: "-P bigtable-emulator-it,bigtable-prod-it,bigtable-prod-batch-it -Dbigtable.project=gcloud-devel -Dbigtable.instance=google-cloud-bigtable -Dbigtable.table=integration-tests -Dbigtable.kms_key_name=projects/gcloud-devel/locations/us-east1/keyRings/cmek-test-key-ring/cryptoKeys/cmek-test-key -Dbigtable.wait-for-cmek-key-status=true"
+    value: "-P bigtable-emulator-it,bigtable-prod-it,bigtable-prod-batch-it,enable-verbose-grpc-logs -Dbigtable.project=gcloud-devel -Dbigtable.instance=google-cloud-bigtable -Dbigtable.table=integration-tests -Dbigtable.kms_key_name=projects/gcloud-devel/locations/us-east1/keyRings/cmek-test-key-ring/cryptoKeys/cmek-test-key -Dbigtable.wait-for-cmek-key-status=true"
 }
 
 # TODO: remove this after we've migrated all tests and scripts

diff --git a/.kokoro/presubmit/graalvm-native.cfg b/.kokoro/presubmit/graalvm-native.cfg
@@ -19,7 +19,7 @@ env_vars: {
 
 env_vars: {
     key: "INTEGRATION_TEST_ARGS"
-    value: "-P bigtable-emulator-it,bigtable-prod-it,bigtable-prod-batch-it -Dbigtable.project=gcloud-devel -Dbigtable.instance=google-cloud-bigtable -Dbigtable.table=integration-tests -Dbigtable.kms_key_name=projects/gcloud-devel/locations/us-east1/keyRings/cmek-test-key-ring/cryptoKeys/cmek-test-key -Dbigtable.wait-for-cmek-key-status=true"
+    value: "-P bigtable-emulator-it,bigtable-prod-it,bigtable-prod-batch-it,enable-verbose-grpc-logs -Dbigtable.project=gcloud-devel -Dbigtable.instance=google-cloud-bigtable -Dbigtable.table=integration-tests -Dbigtable.kms_key_name=projects/gcloud-devel/locations/us-east1/keyRings/cmek-test-key-ring/cryptoKeys/cmek-test-key -Dbigtable.wait-for-cmek-key-status=true"
 }
 
 env_vars: {

diff --git a/.kokoro/presubmit/integration.cfg b/.kokoro/presubmit/integration.cfg
@@ -8,7 +8,7 @@ env_vars: {
 
 env_vars: {
     key: "INTEGRATION_TEST_ARGS"
-    value: "-P bigtable-emulator-it,bigtable-prod-it,bigtable-prod-batch-it -Dbigtable.project=gcloud-devel -Dbigtable.instance=google-cloud-bigtable -Dbigtable.table=integration-tests -Dbigtable.kms_key_name=projects/gcloud-devel/locations/us-east1/keyRings/cmek-test-key-ring/cryptoKeys/cmek-test-key -Dbigtable.wait-for-cmek-key-status=true"
+    value: "-P bigtable-emulator-it,bigtable-prod-it,bigtable-prod-batch-it,enable-verbose-grpc-logs -Dbigtable.project=gcloud-devel -Dbigtable.instance=google-cloud-bigtable -Dbigtable.table=integration-tests -Dbigtable.kms_key_name=projects/gcloud-devel/locations/us-east1/keyRings/cmek-test-key-ring/cryptoKeys/cmek-test-key -Dbigtable.wait-for-cmek-key-status=true"
 }
 
 env_vars: {

diff --git a/google-cloud-bigtable/pom.xml b/google-cloud-bigtable/pom.xml
@@ -25,7 +25,12 @@
     <skipTests>false</skipTests>
     <skipUnitTests>${skipTests}</skipUnitTests>
     <skipITs>${skipTests}</skipITs>
-
+    <!-- Configure test logging output. By default tests print info logs to stdout/err.
+     However for kokoro runs to enable better debugging of connection failures,
+     the `enable-verbose-grpc-logs` profile is enabled.
+     It will enable verbose logs and redirect output to a test specific file in the integration tests report directory.
+     These propeties should only be set by the `enable-verbose-grpc-logs` profile -->
+    <java-log-config>src/test/resources/logging.properties</java-log-config>
     <!-- Use client defined default endpoints -->
     <!-- Can be overriden on the commandline via `-Dbigtable.cfe-data-endpoint=bigtableadmin.googleapis.com:443` -->
     <bigtable.cfe-data-endpoint/>
@@ -335,13 +340,10 @@
 
   <profiles>
     <profile>
-      <!-- Enable per-test verbose grpc logs
-      However, parallel test execution must be disabled to allow TestEnvRule to add/remove the
-      appender -->
       <id>enable-verbose-grpc-logs</id>
       <properties>
         <bigtable.enable-grpc-logs>true</bigtable.enable-grpc-logs>
-        <!-- NOTE: bigtable.grpc-log-dir is configured separately for each execution -->
+        <java-log-config>src/test/resources/logging-verbose.properties</java-log-config>
       </properties>
       <build>
         <plugins>
@@ -351,6 +353,7 @@
             <configuration>
               <forkCount>1</forkCount>
               <parallel>none</parallel>
+              <redirectTestOutputToFile>true</redirectTestOutputToFile>
             </configuration>
           </plugin>
         </plugins>
@@ -495,7 +498,6 @@
                 </goals>
                 <configuration>
                   <skip>false</skip>
-
                   <systemPropertyVariables>
                     <bigtable.env>cloud</bigtable.env>
                     <bigtable.data-endpoint>${bigtable.directpath-data-endpoint}</bigtable.data-endpoint>
@@ -730,6 +732,9 @@
 
           <!-- print full stacktraces by default -->
           <trimStackTrace>false</trimStackTrace>
+          <systemPropertyVariables>
+            <java.util.logging.config.file>${java-log-config}</java.util.logging.config.file>
+          </systemPropertyVariables>
         </configuration>
       </plugin>
 

diff --git a/...-cloud-bigtable/src/test/java/com/google/cloud/bigtable/test_helpers/env/TestEnvRule.java b/...-cloud-bigtable/src/test/java/com/google/cloud/bigtable/test_helpers/env/TestEnvRule.java
@@ -25,23 +25,15 @@
 import com.google.cloud.bigtable.admin.v2.models.Cluster;
 import com.google.cloud.bigtable.admin.v2.models.Instance;
 import com.google.cloud.bigtable.admin.v2.models.UpdateAuthorizedViewRequest;
-import com.google.common.base.Joiner;
-import com.google.common.base.Preconditions;
-import com.google.common.base.Strings;
 import com.google.common.collect.ImmutableSet;
 import java.io.IOException;
-import java.nio.file.Files;
-import java.nio.file.Path;
-import java.nio.file.Paths;
 import java.util.ArrayList;
 import java.util.List;
 import java.util.Set;
 import java.util.concurrent.ExecutionException;
-import java.util.logging.FileHandler;
 import java.util.logging.Handler;
 import java.util.logging.Level;
 import java.util.logging.Logger;
-import java.util.logging.SimpleFormatter;
 import org.junit.rules.TestRule;
 import org.junit.runner.Description;
 import org.junit.runners.model.Statement;
@@ -107,8 +99,6 @@ protected void before(Description description) throws Throwable {
         .that(System.getenv())
         .doesNotContainKey(BIGTABLE_EMULATOR_HOST_ENV_VAR);
 
-    configureLogging(description);
-
     switch (env) {
       case "emulator":
         testEnv = EmulatorEnv.createBundled();
@@ -125,33 +115,6 @@ protected void before(Description description) throws Throwable {
     testEnv.start();
   }
 
-  private void configureLogging(Description description) throws IOException {
-    if (!BIGTABLE_ENABLE_VERBOSE_GRPC_LOGS) {
-      return;
-    }
-    Preconditions.checkState(
-        !Strings.isNullOrEmpty(BIGTABLE_GRPC_LOG_DIR),
-        "The property "
-            + BIGTABLE_GRPC_LOG_DIR
-            + " must be set when verbose grpc logs are enabled");
-
-    Files.createDirectories(Paths.get(BIGTABLE_GRPC_LOG_DIR));
-
-    String basename =
-        Joiner.on("-").useForNull("").join(description.getClassName(), description.getMethodName());
-    Path logPath = Paths.get(BIGTABLE_GRPC_LOG_DIR, basename + ".log");
-
-    grpcLogHandler = new FileHandler(logPath.toString());
-    grpcLogHandler.setFormatter(new SimpleFormatter());
-    grpcLogHandler.setLevel(Level.ALL);
-
-    for (String grpcLoggerName : GRPC_LOGGER_NAMES) {
-      Logger logger = Logger.getLogger(grpcLoggerName);
-      logger.setLevel(Level.ALL);
-      logger.addHandler(grpcLogHandler);
-    }
-  }
-
   private void after() {
     try {
       cleanUpStale();

diff --git a/google-cloud-bigtable/src/test/resources/logging-verbose.properties b/google-cloud-bigtable/src/test/resources/logging-verbose.properties
@@ -0,0 +1,11 @@
+# Verbose logging configuration used by enable-verbose-grpc-logs profile.
+handlers= java.util.logging.ConsoleHandler
+.level= FINEST
+
+# hide "Connecting to the Bigtable emulator at localhost:XXXX" lines
+com.google.cloud.bigtable.data.v2.BigtableDataSettings.level=WARNING
+java.util.logging.ConsoleHandler.level = FINEST
+
+java.util.logging.ConsoleHandler.formatter = java.util.logging.SimpleFormatter
+# time [level] loggerName: message
+java.util.logging.SimpleFormatter.format=%1$tT [%4$-7s] %2$s: %5$s%n
diff --git a/google-cloud-bigtable/src/test/resources/logging.properties b/google-cloud-bigtable/src/test/resources/logging.properties
@@ -1,17 +1,12 @@
 handlers= java.util.logging.ConsoleHandler
 .level= INFO
 
+# hide "Connecting to the Bigtable emulator at localhost:XXXX" lines
+com.google.cloud.bigtable.data.v2.BigtableDataSettings.level=WARNING
 java.util.logging.ConsoleHandler.level = INFO
-java.util.logging.ConsoleHandler.formatter = java.util.logging.SimpleFormatter
-
-# Example to customize the SimpleFormatter output format
-# to print one-line log message like this:
-#     <level>: <log message> [<date/time>]
-#
 
-#java.util.logging.SimpleFormatter.format=%4$s: %5$s [%1$tc]%n
+java.util.logging.ConsoleHandler.formatter = java.util.logging.SimpleFormatter
 # time [level] loggerName: message
 java.util.logging.SimpleFormatter.format=%1$tT [%4$-7s] %2$s: %5$s%n
 
-# hide "Connecting to the Bigtable emulator at localhost:XXXX" lines
-com.google.cloud.bigtable.data.v2.BigtableDataSettings.level=WARNING
+