git-cl: Report git push information to metrics collection.

Bug: 897394
Change-Id: I52a31bb4840b5de89b96545a3e7544c6708f148f
Reviewed-on: https://chromium-review.googlesource.com/c/1312240Reviewed-by: Andrii Shyshkalov <tandrii@chromium.org>
Commit-Queue: Edward Lesmes <ehmaldonado@chromium.org>

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.*')

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.
 

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()