ninjalog_uploader: Add user email to ninjalog metadata

Bug: 348527311
Change-Id: Idb623a57a2374f4ed5461a9cf159b2eda62c94f4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/5823448
Auto-Submit: Junji Watanabe <jwata@google.com>
Commit-Queue: Junji Watanabe <jwata@google.com>
Reviewed-by: Takuto Ikuta <tikuta@chromium.org>

build_telemetry.py

@@ -5,6 +5,7 @@
 
 import argparse
 import json
+import logging
 import os
 import subprocess
 import sys
@@ -44,11 +45,14 @@ class Config:
 
         if not config:
             config = {
-                "is_googler": is_googler(),
+                "user": check_auth().get("email", ""),
                 "status": None,
                 "countdown": self._countdown,
                 "version": VERSION,
             }
+        if not config.get("user"):
+            config["user"] = check_auth().get("email", "")
+
 
         self._config = config
 
@@ -62,9 +66,14 @@ class Config:
 
     @property
     def is_googler(self):
+        return self.user.endswith("@google.com")
+
+    @property
+    def user(self):
         if not self._config:
             return
-        return self._config.get("is_googler") == True
+        return self._config.get("user", "")
+
 
     @property
     def countdown(self):
@@ -84,7 +93,7 @@ class Config:
                   self._config_path,
                   file=sys.stderr)
             return False
-        if not self._config.get("is_googler"):
+        if not self.is_googler:
             return False
         if self._config.get("status") == "opt-out":
             return False
@@ -138,25 +147,22 @@ def load_config(cfg_path=_DEFAULT_CONFIG_PATH, countdown=_DEFAULT_COUNTDOWN):
     return cfg
 
 
-def is_googler():
-    """Checks whether this user is Googler or not."""
+def check_auth():
+    """Checks auth information."""
     p = subprocess.run(
-        "cipd auth-info",
+        "cipd auth-info --json-output -",
         stdout=subprocess.PIPE,
         stderr=subprocess.PIPE,
         text=True,
         shell=True,
     )
     if p.returncode != 0:
-        return False
-    lines = p.stdout.splitlines()
-    if len(lines) == 0:
-        return False
-    l = lines[0]
-    # |l| will be like 'Logged in as <user>@google.com.' for googler using
-    # reclient.
-    return l.startswith("Logged in as ") and l.endswith("@google.com.")
-
+        return {}
+    try:
+        return json.loads(p.stdout)
+    except json.JSONDecodeError as e:
+        logging.error(e)
+        return {}
 
 def enabled():
     """Checks whether the build can upload build telemetry."""

ninjalog_uploader.py

@@ -123,7 +123,7 @@ def GetJflag(cmdline):
             return int(cmdline[i][len("-j"):])
 
 
-def GetMetadata(cmdline, ninjalog, exit_code, build_duration):
+def GetMetadata(cmdline, ninjalog, exit_code, build_duration, user):
     """Get metadata for uploaded ninjalog.
 
     Returned metadata has schema defined in
@@ -151,6 +151,7 @@ def GetMetadata(cmdline, ninjalog, exit_code, build_duration):
         build_configs[k] = str(build_configs[k])
 
     metadata = {
+        "user": user,
         "exit_code": exit_code,
         "build_duration_sec": build_duration,
         "platform": platform.system(),
@@ -278,7 +279,7 @@ def main():
         return 0
 
     metadata = GetMetadata(args.cmdline, ninjalog, args.exit_code,
-                           args.build_duration)
+                           args.build_duration, cfg.user)
     exit_code = UploadNinjaLog(args.server, ninjalog, metadata)
     if exit_code == 0:
         last_upload_file.touch()

tests/build_telemetry_test.py

@@ -3,6 +3,7 @@
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
 
+import json
 import os
 import sys
 import tempfile
@@ -17,33 +18,29 @@ import build_telemetry
 
 class BuildTelemetryTest(unittest.TestCase):
 
-    def test_is_googler(self):
+    def test_check_auth(self):
         with unittest.mock.patch('subprocess.run') as run_mock:
             run_mock.return_value.returncode = 0
-            run_mock.return_value.stdout = 'Logged in as foo@google.com.\n'
-            self.assertTrue(build_telemetry.is_googler())
+            auth = {'email': 'bob@google.com'}
+            run_mock.return_value.stdout = json.dumps(auth)
+            self.assertEqual(build_telemetry.check_auth(), auth)
 
         with unittest.mock.patch('subprocess.run') as run_mock:
             run_mock.return_value.returncode = 1
-            self.assertFalse(build_telemetry.is_googler())
+            self.assertEqual(build_telemetry.check_auth(), {})
 
         with unittest.mock.patch('subprocess.run') as run_mock:
             run_mock.return_value.returncode = 0
             run_mock.return_value.stdout = ''
-            self.assertFalse(build_telemetry.is_googler())
-
-        with unittest.mock.patch('subprocess.run') as run_mock:
-            run_mock.return_value.returncode = 0
-            run_mock.return_value.stdout = 'Logged in as foo@example.com.\n'
-            self.assertFalse(build_telemetry.is_googler())
+            self.assertEqual(build_telemetry.check_auth(), {})
 
     def test_load_and_save_config(self):
         test_countdown = 2
         with tempfile.TemporaryDirectory() as tmpdir:
             cfg_path = os.path.join(tmpdir, "build_telemetry.cfg")
             with unittest.mock.patch(
-                    'build_telemetry.is_googler') as is_googler:
-                is_googler.return_value = True
+                    'build_telemetry.check_auth') as check_auth:
+                check_auth.return_value = {'email': 'bob@google.com'}
 
                 # Initial config load
                 cfg = build_telemetry.load_config(cfg_path, test_countdown)
@@ -61,9 +58,9 @@ class BuildTelemetryTest(unittest.TestCase):
                 self.assertEqual(cfg.countdown, test_countdown)
                 self.assertEqual(cfg.version, build_telemetry.VERSION)
 
-                # build_telemetry.is_googler() is an expensive call.
+                # build_telemetry.check_auth() is an expensive call.
                 # The cached result should be reused.
-                is_googler.assert_called_once()
+                check_auth.assert_called_once()
 
     def test_enabled(self):
         test_countdown = 2
@@ -72,8 +69,8 @@ class BuildTelemetryTest(unittest.TestCase):
         with tempfile.TemporaryDirectory() as tmpdir:
             cfg_path = os.path.join(tmpdir, "build_telemetry.cfg")
             with unittest.mock.patch(
-                    'build_telemetry.is_googler') as is_googler:
-                is_googler.return_value = True
+                    'build_telemetry.check_auth') as check_auth:
+                check_auth.return_value = {'email': 'bob@google.com'}
 
                 # Initial config load
                 cfg = build_telemetry.load_config(cfg_path, test_countdown)
@@ -115,8 +112,8 @@ class BuildTelemetryTest(unittest.TestCase):
         with tempfile.TemporaryDirectory() as tmpdir:
             cfg_path = os.path.join(tmpdir, "build_telemetry.cfg")
             with unittest.mock.patch(
-                    'build_telemetry.is_googler') as is_googler:
-                is_googler.return_value = True
+                    'build_telemetry.check_auth') as check_auth:
+                check_auth.return_value = {'email': 'bob@google.com'}
                 # After opt-out, it should not display the notice and
                 # change the countdown.
                 cfg = build_telemetry.load_config(cfg_path, test_countdown)
@@ -143,8 +140,8 @@ class BuildTelemetryTest(unittest.TestCase):
         with tempfile.TemporaryDirectory() as tmpdir:
             cfg_path = os.path.join(tmpdir, "build_telemetry.cfg")
             with unittest.mock.patch(
-                    'build_telemetry.is_googler') as is_googler:
-                is_googler.return_value = False
+                    'build_telemetry.check_auth') as check_auth:
+                check_auth.return_value = {'email': 'bob@example.com'}
                 cfg = build_telemetry.load_config(cfg_path)
                 self.assertFalse(cfg.enabled())