From fec80c41355c59f356229dc223808d5c22c87a47 Mon Sep 17 00:00:00 2001
From: Edward Lemur <ehmaldonado@chromium.org>
Date: Thu, 1 Nov 2018 23:14:14 +0000
Subject: [PATCH] git-cl: Report git push information to metrics collection.

Bug: 897394
Change-Id: I52a31bb4840b5de89b96545a3e7544c6708f148f
Reviewed-on: https://chromium-review.googlesource.com/c/1312240
Reviewed-by: Andrii Shyshkalov <tandrii@chromium.org>
Commit-Queue: Edward Lesmes <ehmaldonado@chromium.org>
---
 git_cl.py             | 14 ++++++++++++--
 metrics_utils.py      | 30 ++++++++++++++++++++++++++++++
 tests/metrics_test.py | 14 ++++++++++++++
 3 files changed, 56 insertions(+), 2 deletions(-)

diff --git a/git_cl.py b/git_cl.py
index 8f7f6b77d..176e0b715 100755
--- a/git_cl.py
+++ b/git_cl.py
@@ -29,6 +29,7 @@ import stat
 import sys
 import tempfile
 import textwrap
+import time
 import urllib
 import urllib2
 import urlparse
@@ -184,7 +185,6 @@ def BranchExists(branch):
 def time_sleep(seconds):
   # Use this so that it can be mocked in tests without interfering with python
   # system machinery.
-  import time  # Local import to discourage others from importing time globally.
   return time.sleep(seconds)
 
 
@@ -3008,18 +3008,28 @@ class _GerritChangelistImpl(_ChangelistCodereviewBase):
             GERRIT_ERR_LOGGER.info(line)
 
       filter_fn = FilterHeaders()
+      before_push = time.time()
+      push_returncode = 0
       push_stdout = gclient_utils.CheckCallAndFilter(
           ['git', 'push', self.GetRemoteUrl(), refspec],
           print_stdout=False,
           filter_fn=filter_fn,
           env=env)
-    except subprocess2.CalledProcessError:
+    except subprocess2.CalledProcessError as e:
+      push_returncode = e.returncode
       DieWithError('Failed to create a change. Please examine output above '
                    'for the reason of the failure.\n'
                    'Hint: run command below to diagnose common Git/Gerrit '
                    'credential problems:\n'
                    '  git cl creds-check\n',
                    change_desc)
+    finally:
+      metrics.collector.add_repeated('sub_commands', {
+        'command': 'git push',
+        'execution_time': time.time() - before_push,
+        'exit_code': push_returncode,
+        'arguments': metrics_utils.extract_known_subcommand_args(refspec_opts),
+      })
 
     if options.squash:
       regex = re.compile(r'remote:\s+https?://[\w\-\.\+\/#]*/(\d+)\s.*')
diff --git a/metrics_utils.py b/metrics_utils.py
index 2be13bef1..883c6946e 100644
--- a/metrics_utils.py
+++ b/metrics_utils.py
@@ -142,6 +142,23 @@ GIT_VERSION_RE = re.compile(
   r'git version (\d)\.(\d{0,2})\.(\d{0,2})'
 )
 
+KNOWN_SUBCOMMAND_ARGS = {
+  'cc',
+  'hashtag',
+  'l=Auto-Submit+1',
+  'l=Commit-Queue+1',
+  'l=Commit-Queue+2',
+  'label',
+  'm',
+  'notify=ALL',
+  'notify=NONE',
+  'private',
+  'r',
+  'ready',
+  'topic',
+  'wip'
+}
+
 
 def get_python_version():
   """Return the python version in the major.minor.micro format."""
@@ -178,6 +195,19 @@ def seconds_to_weeks(duration):
   return int(duration) >> 19
 
 
+def extract_known_subcommand_args(args):
+  """Extract the known arguments from the passed list of args."""
+  known_args = []
+  for arg in args:
+    if arg in KNOWN_SUBCOMMAND_ARGS:
+      known_args.append(arg)
+    else:
+      arg = arg.split('=')[0]
+      if arg in KNOWN_SUBCOMMAND_ARGS:
+        known_args.append(arg)
+  return known_args
+
+
 def extract_http_metrics(request_uri, method, status, response_time):
   """Extract metrics from the request URI.
 
diff --git a/tests/metrics_test.py b/tests/metrics_test.py
index ef88b152d..763658882 100644
--- a/tests/metrics_test.py
+++ b/tests/metrics_test.py
@@ -718,6 +718,20 @@ class MetricsUtilsTest(unittest.TestCase):
 
     self.assertIsNone(metrics_utils.get_git_version())
 
+  def test_extract_known_subcommand_args(self):
+    """Tests that we can extract known subcommand args."""
+    result = metrics_utils.extract_known_subcommand_args([
+      'm=Fix issue with ccs', 'cc=foo@example.com', 'cc=bar@example.com'])
+    self.assertEqual(['cc', 'cc', 'm'], sorted(result))
+
+    result = metrics_utils.extract_known_subcommand_args([
+      'm=Some title mentioning cc and hashtag', 'notify=NONE', 'private'])
+    self.assertEqual(['m', 'notify=NONE', 'private'], sorted(result))
+
+    result = metrics_utils.extract_known_subcommand_args([
+      'foo=bar', 'another_unkwnon_arg'])
+    self.assertEqual([], result)
+
 
 if __name__ == '__main__':
   unittest.main()