Produce histograms from run_tbmv3_metric

The script can now write out histogram set json. Currently we're using a
json config file to specify details in the histogram that are not
available in trace processor metric result, but this will likely change
soon. No support for diagnostics yet.

Bug: chromium:1012687
Change-Id: I8654bdb8ea6a596f899b919d0884bee25ed7af1a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1860334
Commit-Queue: Deepanjan Roy <dproy@chromium.org>
Reviewed-by: Juan Antonio Navarro Pérez <perezju@chromium.org>
Cr-Commit-Position: refs/heads/master@{#706039}

tools/perf/core/tbmv3/metrics/console_error_metric.proto

@@ -9,9 +9,9 @@ package perfetto.protos;
 import "protos/perfetto/metrics/metrics.proto";
 
 message ConsoleErrorMetric {
-  optional int64 all = 1;
-  optional int64 js = 2;
-  optional int64 network = 3;
+  optional int64 all_errors = 1;
+  optional int64 js_errors = 2;
+  optional int64 network_errors = 3;
 }
 
 extend TraceMetrics {

tools/perf/core/tbmv3/metrics/console_error_metric.sql

@@ -9,7 +9,6 @@ where slices.name = "ConsoleMessage::Error"
   and slices.category = "blink.console"
   and args.flat_key = "debug.source"
 UNION ALL
--- Can't find a trace with v8 console errors so this path is untested.
 SELECT "JS" AS source
 FROM slices
 WHERE slices.category = 'v8.console' AND (
@@ -21,12 +20,12 @@ WHERE slices.category = 'v8.console' AND (
 CREATE VIEW console_error_metric AS
 SELECT
   (SELECT COUNT(*) FROM console_error_events) as all_errors,
-  (SELECT COUNT(*) FROM console_error_events where source = "JS") as js,
-  (SELECT COUNT(*) FROM console_error_events where source = "Network") as network;
+  (SELECT COUNT(*) FROM console_error_events where source = "JS") as js_errors,
+  (SELECT COUNT(*) FROM console_error_events where source = "Network") as network_errors;
 
 CREATE VIEW console_error_metric_output AS
 SELECT ConsoleErrorMetric(
-  'all', all_errors,
-  'js', js,
-  'network', network)
+  'all_errors', all_errors,
+  'js_errors', js_errors,
+  'network_errors', network_errors)
 FROM console_error_metric

tools/perf/core/tbmv3/metrics/console_error_metric_config.json

+{
+  "name": "Console Error Metrics",
+  "description": "Counts the number of console errors seen in the trace.",
+  "histograms": [
+    {
+      "name": "all_errors",
+      "description": "Number of all console error messages",
+      "requiredCategories": ["v8", "blink.console"],
+      "unit": "count_smallerIsBetter"
+    },
+    {
+      "name": "js_errors",
+      "description": "Number of js console error messages",
+      "unit": "count_smallerIsBetter"
+    },
+    {
+      "name": "network_errors",
+      "description": "Number of network console error messages",
+      "unit": "count_smallerIsBetter"
+    }
+  ]
+}

tools/perf/core/tbmv3/run_tbmv3_metric.py

@@ -3,6 +3,8 @@
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
 
+from __future__ import print_function
+
 import argparse
 import json
 import os
@@ -11,14 +13,32 @@ import sys
 
 from collections import namedtuple
 
+# TODO(crbug.com/1012687): Adding tools/perf to path. We can remove this when
+# we have a wrapper script under tools/perf that sets up import paths more
+# nicely.
+sys.path.append(os.path.join(os.path.dirname(__file__), '..', '..'))
+
+from core import path_util
+path_util.AddPyUtilsToPath()
+path_util.AddTracingToPath()
+
+from tracing.value import histogram_set
 
 _CHROMIUM_SRC_PATH  = os.path.join(
     os.path.dirname(__file__), '..', '..', '..', '..')
-_DEFAULT_TP_PATH = os.path.abspath(os.path.join(
+_DEFAULT_TP_PATH = os.path.realpath(os.path.join(
     _CHROMIUM_SRC_PATH, 'out', 'Debug', 'trace_processor_shell'))
-_METRICS_PATH = os.path.join(os.path.dirname(__file__), 'metrics')
+_METRICS_PATH = os.path.realpath(os.path.join(os.path.dirname(__file__),
+                                              'metrics'))
+
+MetricFiles = namedtuple('MetricFiles', ('sql', 'proto', 'config'))
+
 
-MetricFiles = namedtuple('MetricFiles', ('sql', 'proto'))
+def CreateMetricFiles(metric_name):
+  return MetricFiles(
+    sql=os.path.join(_METRICS_PATH, metric_name + '.sql'),
+    proto=os.path.join(_METRICS_PATH, metric_name + '.proto'),
+    config=os.path.join(_METRICS_PATH, metric_name + '_config.json'))
 
 
 class MetricFileNotFound(Exception):
@@ -35,20 +55,76 @@ class TraceProcessorError(Exception):
 
 def _CheckFilesExist(trace_processor_path, metric_files):
   if not os.path.exists(trace_processor_path):
-    raise TraceProcessorNotFound("Could not find trace processor shell at %s"
+    raise TraceProcessorNotFound('Could not find trace processor shell at %s'
                                  % trace_processor_path)
 
   # Currently assuming all metric files live in tbmv3/metrics directory. We will
   # revise this decision later.
   for filetype, path in metric_files._asdict().iteritems():
     if not os.path.exists(path):
-      raise MetricFileNotFound("metric %s file not found at %s"
+      raise MetricFileNotFound('metric %s file not found at %s'
                                % (filetype, path))
 
 
+def _RunTraceProcessorMetric(trace_processor_path, trace, metric_files):
+  trace_processor_args = [trace_processor_path, trace,
+                          '--run-metrics', metric_files.sql,
+                          '--metrics-output=json',
+                          '--extra-metrics', _METRICS_PATH]
+
+  try:
+    output = subprocess.check_output(trace_processor_args)
+  except subprocess.CalledProcessError:
+    raise TraceProcessorError(
+        'Failed to compute metrics. Check trace processor logs.')
+  return json.loads(output)
+
+
+def _ScopedHistogramName(metric_name, histogram_name):
+  """Returns scoped histogram name by preprending metric name.
+
+  This is useful for avoiding histogram name collision. The '_metric' suffix of
+  the metric name is dropped from scoped name. Example:
+  _ScopedHistogramName("console_error_metric", "js_errors")
+  => "console_error::js_errors"
+  """
+  metric_suffix = '_metric'
+  suffix_length = len(metric_suffix)
+  # TODO(crbug.com/1012687): Decide on whether metrics should always have
+  # '_metric' suffix.
+  if metric_name[-suffix_length:] == metric_suffix:
+    scope = metric_name[:-suffix_length]
+  else:
+    scope = metric_name
+  return '::'.join([scope, histogram_name])
+
+
+def _ProduceHistograms(metric_name, metric_files, measurements):
+  histograms = histogram_set.HistogramSet()
+  with open(metric_files.config) as f:
+    config = json.load(f)
+  metric_root_field = 'perfetto.protos.' + metric_name
+  for histogram_config in config['histograms']:
+    histogram_name = histogram_config['name']
+    samples = measurements[metric_root_field][histogram_name]
+    scoped_histogram_name = _ScopedHistogramName(metric_name, histogram_name)
+    description = histogram_config['description']
+    histograms.CreateHistogram(scoped_histogram_name,
+                               histogram_config['unit'], samples,
+                               description=description)
+  return histograms
+
+
+def _WriteHistogramSetToFile(histograms, outfile):
+  with open(outfile, 'w') as f:
+    json.dump(histograms.AsDicts(), f, indent=2, sort_keys=True,
+              separators=(',', ': '))
+
+
 def Main():
   parser = argparse.ArgumentParser(
-    description='[Experimental] Runs TBMv3 metrics on local traces.')
+    description='[Experimental] Runs TBMv3 metrics on local traces and '
+    'produces histogram json.')
   parser.add_argument('--trace', required=True,
                       help='Trace file you want to compute metric on')
   parser.add_argument('--metric', required=True,
@@ -56,29 +132,18 @@ def Main():
   parser.add_argument('--trace_processor_path', default=_DEFAULT_TP_PATH,
                       help='Path to trace processor shell. '
                       'Default: %(default)s')
+  parser.add_argument('--outfile', default='results.json',
+                      help='Path to output file. Default: %(default)s')
   args = parser.parse_args()
 
-  metric_files = MetricFiles(
-    sql=os.path.join(_METRICS_PATH, args.metric + '.sql'),
-    proto=os.path.join(_METRICS_PATH, args.metric + '.proto')
-  )
+  metric_files = CreateMetricFiles(args.metric)
   _CheckFilesExist(args.trace_processor_path, metric_files)
 
-  trace_processor_args = [args.trace_processor_path,
-                          args.trace,
-                          '--run-metrics', metric_files.sql,
-                          '--metrics-output=json',
-                          '--extra-metrics', _METRICS_PATH]
-
-  try:
-    json_output = subprocess.check_output(trace_processor_args)
-  except subprocess.CalledProcessError:
-    raise TraceProcessorError("Failed to compute metrics. " +
-                              "Check trace processor logs.")
-
-  json_result = json.loads(json_output)
-  print "Metric result:"
-  print json_result
+  measurements = _RunTraceProcessorMetric(args.trace_processor_path,
+                                               args.trace, metric_files)
+  histograms = _ProduceHistograms(args.metric, metric_files, measurements)
+  _WriteHistogramSetToFile(histograms, args.outfile)
+  print('JSON result created in file://%s' % (args.outfile))
 
 
 if __name__ == '__main__':