[Telemetry] Allow users to select TBMv1 metrics to run alongside TBMv2

telemetry/telemetry/web_perf/timeline_based_measurement.py

Index: telemetry/telemetry/web_perf/timeline_based_measurement.py
diff --git a/telemetry/telemetry/web_perf/timeline_based_measurement.py b/telemetry/telemetry/web_perf/timeline_based_measurement.py
index e476424ae1d9191a5bd4dcda47ab9297ec9d611b..3ec08528ce437e5d9f4db3fd10834ffab71b4abf 100644
--- a/telemetry/telemetry/web_perf/timeline_based_measurement.py
+++ b/telemetry/telemetry/web_perf/timeline_based_measurement.py
@@ -191,7 +191,7 @@ class Options(object):
                       " Given overhead level: %s" % overhead_level)
 
     self._timeline_based_metric = None
-    self._legacy_timeline_based_metrics = _GetAllLegacyTimelineBasedMetrics()
+    self._legacy_timeline_based_metrics = []
 
 
   def ExtendTraceCategoryFilter(self, filters):
@@ -218,14 +218,12 @@ class Options(object):
       metric: A string metric path under //tracing/tracing/metrics.
     """
     assert isinstance(metric, basestring)
-    self._legacy_timeline_based_metrics = None
     self._timeline_based_metric = metric
 
   def GetTimelineBasedMetric(self):
     return self._timeline_based_metric
 
   def SetLegacyTimelineBasedMetrics(self, metrics):
-    assert self._timeline_based_metric == None
     assert isinstance(metrics, collections.Iterable)
     for m in metrics:
       assert isinstance(m, timeline_based_metric.TimelineBasedMetric)
@@ -282,11 +280,19 @@ class TimelineBasedMeasurement(story_test.StoryTest):
 
     if self._tbm_options.GetTimelineBasedMetric():
       self._ComputeTimelineBasedMetric(results, trace_value)
+      # Legacy metrics can be computed, but only if explicitly specified.
+      if self._tbm_options.GetLegacyTimelineBasedMetrics():
+        self._ComputeLegacyTimelineBasedMetrics(results, trace_result)
     else:
-      assert self._tbm_options.GetLegacyTimelineBasedMetrics()
+      # Run all TBMv1 metrics if no other metric is specified (legacy behavior)
+      if not self._tbm_options.GetLegacyTimelineBasedMetrics():
+        logging.warn('Please specify the TBMv1 metrics you are interested in '
+                     'explicitly. This implicit functionality will be removed '
+                     'on July 17, 2016.')

[nednguyen, 2017/02/02 13:59:57]

Ethan, is this code path ready to be killed now?

[eakuefner, 2017/02/07 20:13:08]

Just about. Looking at this is on my radar for this week. There are two
behaviors to consider here:

1. Allowing users to select both TBMv1 and TBMv2 metrics to be computed: pretty
sure this functionality can be killed right now. I'm inclined to just do that
first and send an email to telemetry-announce.
2. Selecting all TBMv1 metrics implicitly if TimelineBasedMeasurement is
specified but SetTimelineBasedMetrics or SetLegacyTimelineBasedMetrics are not
called. This functionality is still used by smoothness but probably nothing
else, and could probably be fixed by just inserting
SetLegacyTimelineBasedMetrics into smoothness. I just need to finish checking
the rest of the TBMv1 tests to make sure that this call is present.

+        self._tbm_options.SetLegacyTimelineBasedMetrics(
+            _GetAllLegacyTimelineBasedMetrics())
       self._ComputeLegacyTimelineBasedMetrics(results, trace_result)

[nednguyen, 2016/06/20 23:17:05]

Probably should modify existing test in timeline_based_page_test_unittest to
cover this code path rather than the code path of AllLegacyTimelineBasedMetric

[eakuefner, 2016/06/21 21:36:27]

Done in https://codereview.chromium.org/2084943003

 
-
   def DidRunStory(self, platform):
     """Clean up after running the story."""
     if platform.tracing_controller.is_tracing_running: