Filter obsolete metrics when generating apis

Filter out any event or metric in ukm.xml that has an <obsolete> tag.
Remove <obsolete> tags from in ukm.xml for metrics still being referenced.

Bug: 1055550
Change-Id: If6538ecd87ddc6a8456c40fe08edd61c77664c37
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2083603
Commit-Queue: Adam Trudeau-Arcaro <adamta@google.com>
Reviewed-by: Alexei Svitkine <asvitkine@chromium.org>
Cr-Commit-Position: refs/heads/master@{#746941}

tools/metrics/ukm/gen_builders.py

@@ -24,12 +24,35 @@ parser.add_argument('--output', help='Path to generated files.')
 
 def main(argv):
   args = parser.parse_args()
-  data = ukm_model.UKM_XML_TYPE.Parse(open(args.input).read())
+  data = ReadFilteredData(args.input)
   relpath = 'services/metrics/public/cpp/'
   builders_template.WriteFiles(args.output, relpath, data)
   decode_template.WriteFiles(args.output, relpath, data)
   return 0
 
 
+def ReadFilteredData(path):
+  """Reads data from path and filters out any obsolete metrics.
+
+  Parses data from given path and removes all nodes that contain an
+  <obsolete> tag. First iterates through <event> nodes, then <metric>
+  nodes within them.
+
+  Args:
+    path: The path of the XML data source.
+
+  Returns:
+    A dict of the data not including any obsolete events or metrics.
+  """
+  with open(path) as ukm_file:
+    data = ukm_model.UKM_XML_TYPE.Parse(ukm_file.read())
+    event_tag = ukm_model._EVENT_TYPE.tag
+    metric_tag = ukm_model._METRIC_TYPE.tag
+    data[event_tag] = filter(ukm_model.IsNotObsolete, data[event_tag])
+    for event in data[event_tag]:
+      event[metric_tag] = filter(ukm_model.IsNotObsolete, event[metric_tag])
+    return data
+
+
 if '__main__' == __name__:
   sys.exit(main(sys.argv))

tools/metrics/ukm/gen_builders_test.py

+#!/usr/bin/env python
+# Copyright 2019 The Chromium Authors. All rights reserved.
+# Use of this source code is governed by a BSD-style license that can be
+# found in the LICENSE file.
+
 import unittest
 
-from ukm_model import UKM_XML_TYPE
-from ukm_model import _EVENT_TYPE
-from ukm_model import _METRIC_TYPE
 from codegen import EventInfo
 from codegen import MetricInfo
 from builders_template import HEADER as BUILDERS_HEADER_TEMPLATE
 from builders_template import IMPL as BUILDERS_IMPL_TEMPLATE
 from decode_template import HEADER as DECODE_HEADER_TEMPLATE
 from decode_template import IMPL as DECODE_IMPL_TEMPLATE
+import ukm_model
+import gen_builders
 
 
 class GenBuildersTest(unittest.TestCase):
+  def testFilterObsoleteMetrics(self):
+    data = gen_builders.ReadFilteredData('../../tools/metrics/ukm/ukm.xml')
+    for event in data[ukm_model._EVENT_TYPE.tag]:
+      self.assertTrue(ukm_model.IsNotObsolete(event))
+      for metric in event[ukm_model._METRIC_TYPE.tag]:
+        self.assertTrue(ukm_model.IsNotObsolete(metric))
 
   def testGenerateCode(self):
     relpath = '.'
-    data = UKM_XML_TYPE.Parse(open('../../tools/metrics/ukm/ukm.xml').read())
-    event = data[_EVENT_TYPE.tag][0]
-    metric = event[_METRIC_TYPE.tag][0]
+    data = ukm_model.UKM_XML_TYPE.Parse(
+        open('../../tools/metrics/ukm/ukm.xml').read())
+    event = data[ukm_model._EVENT_TYPE.tag][0]
+    metric = event[ukm_model._METRIC_TYPE.tag][0]
     self.assertIsNotNone(event)
     self.assertIsNotNone(metric)
     eventInfo = EventInfo(event)

tools/metrics/ukm/ukm.xml

@@ -1696,13 +1696,11 @@ be describing additional metrics about the same event.
     </summary>
   </metric>
   <metric name="ProxyCommit">
-    <obsolete>
-      Removed February 2020 in favor of separate metrics for the wait time and
-      the actual commit time.
-    </obsolete>
     <summary>
       The time spent committing main thread to impl layer tree information,
       between navigation and First Contentful Paint, in microseconds.
+      TODO(crbug.com/1058065): Should be obsolete. Removed February 2020 in
+      favor of separate metrics for the wait time and the actual commit time.
     </summary>
   </metric>
   <metric name="ScrollingCoordinator">
@@ -2394,13 +2392,11 @@ be describing additional metrics about the same event.
     </summary>
   </metric>
   <metric name="ProxyCommit">
-    <obsolete>
-      Removed February 2020 in favor of separate metrics for the wait time and
-      the actual commit time.
-    </obsolete>
     <summary>
       The time taken for the proxy to commit to the impl thread in microseconds
-      during the sampled frame.
+      during the sampled frame. TODO(crbug.com/1058065): Should be obsolete.
+      Removed February 2020 in favor of separate metrics for the wait time and
+      the actual commit time.
     </summary>
     <aggregation>
       <history>
@@ -2412,13 +2408,11 @@ be describing additional metrics about the same event.
     </aggregation>
   </metric>
   <metric name="ProxyCommitPercentage">
-    <obsolete>
-      Removed February 2020 in favor of separate metrics for the wait time and
-      the actual commit time.
-    </obsolete>
     <summary>
       The percentage of the main frame time used to commit to the impl thread.
-      An int in the range [0,100].
+      An int in the range [0,100]. TODO(crbug.com/1058065): Should be obsolete.
+      Removed February 2020 in favor of separate metrics for the wait time and
+      the actual commit time.
     </summary>
   </metric>
   <metric name="ScrollingCoordinator">
@@ -7470,13 +7464,12 @@ be describing additional metrics about the same event.
     </summary>
   </metric>
   <metric name="Net.CacheBytes">
-    <obsolete>
-      Deprecated 06/2019 in favor of Net.CacheBytes2.
-    </obsolete>
     <summary>
       The total number of body bytes loaded from cache for all resources on the
       page. This is rounded down to the nearest exponential bucket (with a
-      bucket spacing factor of 1.3).
+      bucket spacing factor of 1.3). TODO(crbug.com/1058065): Should be
+      obsolete. Deprecated 06/2019 in favor of Net.CacheBytes2. (seems to be
+      used in Clang)
     </summary>
   </metric>
   <metric name="Net.CacheBytes2">

tools/metrics/ukm/ukm_model.py

@@ -132,3 +132,8 @@ def PrettifyXML(original_xml):
   """
   config = UKM_XML_TYPE.Parse(original_xml)
   return UKM_XML_TYPE.PrettyPrint(config)
+
+
+def IsNotObsolete(node):
+  """Checks if the given node contains a child <obsolete> node."""
+  return _OBSOLETE_TYPE.tag not in node

tools/metrics/ukm/ukm_model_test.py

@@ -110,6 +110,25 @@ CONFIG_EVENT_NAMES_UNSORTED = """
 </ukm-configuration>
 """.strip()
 
+OBSOLETE_EVENTS_PARSED = {
+    'event': {
+        "obsolete": "Some message",
+        "metric": {
+            "summary": "Some summary",
+        },
+        "metric": {
+            "summary": "Some summary",
+        },
+    },
+    'event': {
+        "obsolete": "Some message",
+        "metric": {
+            "summary": "Some summary",
+        },
+    },
+}
+
+
 class UkmXmlTest(unittest.TestCase):
 
   def __init__(self, *args, **kwargs):
@@ -142,5 +161,12 @@ class UkmXmlTest(unittest.TestCase):
     result = ukm_model.PrettifyXML(CONFIG_EVENT_NAMES_UNSORTED)
     self.assertMultiLineEqual(CONFIG_EVENT_NAMES_SORTED, result.strip())
 
+  def testIsNotObsolete(self):
+    for event in OBSOLETE_EVENTS_PARSED.values():
+      self.assertFalse(ukm_model.IsNotObsolete(event))
+      for metric in event.values():
+        self.assertTrue(ukm_model.IsNotObsolete(metric))
+
+
 if __name__ == '__main__':
   unittest.main()