From 4a08b97dd4a339ead8c92306f34ae9c6c80b41f3 Mon Sep 17 00:00:00 2001 From: Junji Watanabe Date: Thu, 29 Aug 2024 11:17:57 +0000 Subject: [PATCH] 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 Commit-Queue: Junji Watanabe Reviewed-by: Takuto Ikuta --- build_telemetry.py | 36 ++++++++++++++++++++-------------- ninjalog_uploader.py | 5 +++-- tests/build_telemetry_test.py | 37 ++++++++++++++++------------------- 3 files changed, 41 insertions(+), 37 deletions(-) diff --git a/build_telemetry.py b/build_telemetry.py index f3e77c539..afe6d8211 100755 --- a/build_telemetry.py +++ b/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 @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.""" diff --git a/ninjalog_uploader.py b/ninjalog_uploader.py index ddcdc2d75..e9aaaa0c9 100755 --- a/ninjalog_uploader.py +++ b/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() diff --git a/tests/build_telemetry_test.py b/tests/build_telemetry_test.py index f71c6bd26..d59fe3e3b 100755 --- a/tests/build_telemetry_test.py +++ b/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())