commit c0b93beb5205fe59b4da77a05bc8acabfb6ba459
author Michael Mortensen <mmortensen@google.com> Thu Jan 07 12:48:32 2021 -0700
committer Commit Bot <commit-bot@chromium.org> Sat Jan 09 01:06:43 2021 +0000
tree ce31131ec12b301b0994aa8968f29b474d313ec8
parent bdd216390a3e26f9ecd56b030d91ff26af5c44ef

cros_logging: Improvements for CloudLogging initialization.

These changes are based on comments to https://crrev.com/c/2472537

BUG=chromium:1128411
TEST=manual test, pytest

Change-Id: I8496a7d5ea7f9c605aea1ad1625820b579f3c3d4
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/chromite/+/2615920
Reviewed-by: Chris McDonald <cjmcdonald@chromium.org>
Commit-Queue: Michael Mortensen <mmortensen@google.com>
Tested-by: Michael Mortensen <mmortensen@google.com>

lib/cros_logging.py

diff --git a/lib/cros_logging.py b/lib/cros_logging.py
index a713aac..68d15b0 100644
--- a/lib/cros_logging.py
+++ b/lib/cros_logging.py
@@ -79,7 +79,7 @@
     # code is only used by python3. Beware though with branches and bisection
     # this could need to be ImportError for a long time. ImportError is the
     # parent class of ModuleNotFoundError and works on both python2 and python3.
-    log(NOTICE, 'Could not import google.cloud.logging %s', e)
+    log(ERROR, 'Could not import google.cloud.logging %s', e)
     return
 
   client = cloud_logging.Client()
@@ -97,10 +97,11 @@
   google_app_creds_env_value = os.environ.get('GOOGLE_APPLICATION_CREDENTIALS')
   # If both variables are set, log their values and return True.
   if cloud_logging_env_value == '1' and google_app_creds_env_value:
-    log(NOTICE, 'CHROMITE_CLOUD_LOGGING is ', cloud_logging_env_value)
-    log(NOTICE,
-        'GOOGLE_APPLICATION_CREDENTIALS is ', google_app_creds_env_value)
+    log(INFO, 'CHROMITE_CLOUD_LOGGING is %s', cloud_logging_env_value)
     return True
+  if cloud_logging_env_value == '1' and not google_app_creds_env_value:
+    log(WARNING,
+        'CHROMITE_CLOUD_LOGGING is set, GOOGLE_APPLICATION_CREDENTIALS is not.')
   return False
 
 

lib/cros_logging_unittest.py

diff --git a/lib/cros_logging_unittest.py b/lib/cros_logging_unittest.py
index 228675a..28cd49b 100644
--- a/lib/cros_logging_unittest.py
+++ b/lib/cros_logging_unittest.py
@@ -36,7 +36,6 @@
   def testSetupCloudLogging(self):
     if self.cloud_logging_import_error:
       return
-    ### client_mock = self.PatchObject(google.cloud.logging, 'Client')
     # Invoke the code that calls logging when env vars are set.
     logging._SetupCloudLogging()
 
@@ -56,6 +55,8 @@
   def testCloudLoggingEnvVariablesAreDefined_envSet(self):
     if self.cloud_logging_import_error:
       return
+    # Set logger to INFO level to check for CHROMITE_CLOUD_LOGGING info message.
+    self.logger.setLevel(logging.INFO)
     # Set the env vars
     os.environ['CHROMITE_CLOUD_LOGGING'] = '1'
     os.environ['GOOGLE_APPLICATION_CREDENTIALS'] = '/some/path/to/creds.json'
@@ -63,18 +64,17 @@
       cloud_env_defined = logging._CloudLoggingEnvVariablesAreDefined()
     # Verify that both variables are logged.
     self.assertTrue(cloud_env_defined)
-    self.assertIn('CHROMITE_CLOUD_LOGGING', output.GetStderr())
-    self.assertIn('GOOGLE_APPLICATION_CREDENTIALS', output.GetStderr())
+    self.assertIn('CHROMITE_CLOUD_LOGGING', output.GetStdout())
 
   def testCloudLoggingEnvVariablesAreDefined_noAllEnvSet(self):
     if self.cloud_logging_import_error:
       return
-    # Set the env vars
+    # Set only one env var.
     os.environ['CHROMITE_CLOUD_LOGGING'] = '1'
     with self.OutputCapturer() as output:
       cloud_env_defined = logging._CloudLoggingEnvVariablesAreDefined()
     # If both are not set, there should be no output.
-    self.assertEqual(output.GetStdout(), '')
+    self.assertIn('GOOGLE_APPLICATION_CREDENTIALS is not', output.GetStdout())
     self.assertEqual(output.GetStderr(), '')
     self.assertFalse(cloud_env_defined)