Add enum descriptior to use counter ukm metrics

Use counter ukm metrics measures usage of the same enum
FeatureObserver (defined in enums.xml) as the use counter uma
histograms. This CL adds enum attribute in the use counter ukm xml
that the ukm dashboard can use the label the enum value (int) with
the string label defined in enums.xml

This CL also adds a presubmit checker to verify the enum declared
in the ukm.xml file is defined in enums.xml

Change-Id: I15388109a9a6647275efd02c9999aa77b4db65b2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1629187
Commit-Queue: Luna Lu <loonybear@chromium.org>
Reviewed-by: Luna Lu <loonybear@chromium.org>
Reviewed-by: Robert Kaplow (slow) <rkaplow@chromium.org>
Cr-Commit-Position: refs/heads/master@{#664449}

tools/metrics/BUILD.gn

@@ -99,9 +99,11 @@ group("metrics_python_tests") {
     "//tools/metrics/common/pretty_print_xml.py",
     "//tools/metrics/common/etree_util.py",
 
+    "//tools/metrics/histograms/enums.xml",
     "//tools/metrics/histograms/extract_histograms.py",
     "//tools/metrics/histograms/generate_expired_histograms_array.py",
     "//tools/metrics/histograms/generate_expired_histograms_array_unittest.py",
+    "//tools/metrics/histograms/histogram_paths.py",
     "//tools/metrics/histograms/histograms_print_style.py",
     "//tools/metrics/histograms/merge_xml.py",
     "//tools/metrics/histograms/pretty_print.py",

tools/metrics/histograms/extract_histograms.py

@@ -187,7 +187,7 @@ def _ExpandHistogramNameWithSuffixes(suffix_name, histogram_name,
   return cluster + suffix_name + separator + remainder
 
 
-def _ExtractEnumsFromXmlTree(tree):
+def ExtractEnumsFromXmlTree(tree):
   """Extract all <enum> nodes in the tree into a dictionary."""
 
   enums = {}
@@ -563,7 +563,7 @@ def ExtractHistogramsFromDom(tree):
   """
   _NormalizeAllAttributeValues(tree)
 
-  enums, enum_errors = _ExtractEnumsFromXmlTree(tree)
+  enums, enum_errors = ExtractEnumsFromXmlTree(tree)
   histograms, histogram_errors = _ExtractHistogramsFromXmlTree(tree, enums)
   update_errors = _UpdateHistogramsWithSuffixes(tree, histograms)
 

tools/metrics/ukm/PRESUBMIT.py

@@ -36,7 +36,7 @@ def CheckChange(input_api, output_api):
         return [
             output_api.PresubmitError(
                 '%s does not pass format validation; run %s/validate_format.py '
-                'and fix the reported error(s).' %
+                'and fix the reported error(s) or warning(s).' %
                 (UKM_XML, input_api.PresubmitLocalPath())),
         ]
 

tools/metrics/ukm/ukm.xml

@@ -1995,7 +1995,7 @@ be describing additional metrics about the same event.
     should be used to reason about whether a breaking change is acceptable or
     not.
   </summary>
-  <metric name="Feature">
+  <metric name="Feature" enum="FeatureObserver">
     <summary>
       Opt-in UseCounter feature.
     </summary>
@@ -2007,7 +2007,7 @@ be describing additional metrics about the same event.
       </history>
     </aggregation>
   </metric>
-  <metric name="IsMainFrameFeature">
+  <metric name="IsMainFrameFeature" enum="Boolean">
     <summary>
       Emits True(1) or False(0) to indicate whether the Feature recorded is in
       the main frame or not.

tools/metrics/ukm/ukm_model.py

@@ -74,6 +74,7 @@ _METRIC_TYPE =  models.ObjectNodeType(
     attributes=[
       ('name', unicode, r'^[A-Za-z0-9_.]+$'),
       ('semantic_type', unicode, None),
+      ('enum', unicode, None),
     ],
     alphabetization=[
         (_OBSOLETE_TYPE.tag, _KEEP_ORDER),

tools/metrics/ukm/validate_format.py

@@ -4,11 +4,18 @@
 # found in the LICENSE file.
 """Verifies that the UKM XML file is well-formatted."""
 
+import os
 import sys
+
+sys.path.append(os.path.join(os.path.dirname(__file__), '..', 'common'))
+import path_util
+
 from xml_validations import UkmXmlValidation
 from xml.dom import minidom
 
-UKM_XML = 'ukm.xml'
+UKM_XML = path_util.GetInputFile('tools/metrics/ukm/ukm.xml')
+
+IGNORE_METRIC_CHECK_WARNINGS = True
 
 
 def main():
@@ -18,9 +25,18 @@ def main():
     validator = UkmXmlValidation(config)
 
     ownerCheckSuccess, ownerCheckErrors = validator.checkEventsHaveOwners()
+    metricCheckSuccess, metricCheckErrors, metricCheckWarnings = \
+      validator.checkMetricTypeIsSpecified()
+
+    results = dict();
+
+    if not metricCheckSuccess or not metricCheckSuccess:
+      results['Errors'] = ownerCheckErrors + metricCheckErrors
+    if metricCheckWarnings and not IGNORE_METRIC_CHECK_WARNINGS:
+      results['Warnings'] = metricCheckWarnings
 
-    if not ownerCheckSuccess:
-      return ownerCheckErrors
+    if 'Warnings' in results or 'Errors' in results:
+      return results
 
 
 if __name__ == '__main__':

tools/metrics/ukm/xml_validations.py

@@ -2,6 +2,13 @@
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
 
+import os
+import sys
+
+sys.path.append(os.path.join(os.path.dirname(__file__), '..', 'histograms'))
+import extract_histograms
+import histogram_paths
+import merge_xml
 
 class UkmXmlValidation(object):
   """Validations for the content of ukm.xml."""
@@ -43,3 +50,29 @@ class UkmXmlValidation(object):
     isSuccess = not errors
 
     return (isSuccess, errors)
+
+  def checkMetricTypeIsSpecified(self):
+    """Check each metric is either specified with an enum or a unit."""
+    errors = []
+    warnings = []
+
+    enum_tree = merge_xml.MergeFiles([histogram_paths.ENUMS_XML])
+    enums, _ = extract_histograms.ExtractEnumsFromXmlTree(enum_tree)
+
+    for event_node in self.config.getElementsByTagName('event'):
+      for metric_node in self.config.getElementsByTagName('metric'):
+        if metric_node.hasAttribute('enum'):
+          enum_name = metric_node.getAttribute('enum');
+          # Check if the enum is defined in enums.xml.
+          if enum_name not in enums:
+            errors.append("Unknown enum %s in ukm metric %s:%s." %
+                          (enum_name, event_node.getAttribute('name'),
+                          metric_node.getAttribute('name')))
+        elif not metric_node.hasAttribute('unit'):
+          warnings.append("Warning: Neither \'enum\' or \'unit\' is specified "
+                          "for ukm metric %s:%s."
+                          % (event_node.getAttribute('name'),
+                             metric_node.getAttribute('name')))
+
+    isSuccess = not errors
+    return (isSuccess, errors, warnings)

tools/metrics/ukm/xml_validations_test.py

@@ -51,6 +51,31 @@ class UkmXmlValidationTest(unittest.TestCase):
     self.assertFalse(success)
     self.assertListEqual(expected_errors, errors)
 
+  def testMetricHasUndefinedEnum(self):
+    ukm_config = self.toUkmConfig("""
+        <ukm-configuration>
+          <event name="Event">
+            <metric name="Metric1" enum="BadEnum"/>
+            <metric name="Metric2" enum="FeatureObserver"/>
+            <metric name="Metric3" unit="ms"/>
+            <metric name="Metric4"/>
+          </event>
+        </ukm-configuration>
+        """.strip())
+    expected_errors = [
+        "Unknown enum BadEnum in ukm metric Event:Metric1.",
+    ]
+
+    expected_warnings = [
+        "Warning: Neither 'enum' or 'unit' is specified for ukm metric "
+        "Event:Metric4.",
+    ]
+
+    validator = UkmXmlValidation(ukm_config)
+    success, errors, warnings = validator.checkMetricTypeIsSpecified()
+    self.assertFalse(success)
+    self.assertListEqual(expected_errors, errors)
+    self.assertListEqual(expected_warnings, warnings)
 
 if __name__ == '__main__':
   unittest.main()