Commit feafb439 authored by Caitlin Fischer's avatar Caitlin Fischer Committed by Commit Bot

Re-add support for the components tag and OWNERS expansion.

This is done by updating the histograms config model and by uncommenting
the call to ExpandHistogramsOWNERS() in merge_xml.py. This ensures
<owner>src/dir/OWNERS</owner> is expanded into actual owners, e.g.
<owner>name1@google.com</owner> and <owner>name2@chromium.org</owner>.
Uncommenting this allows components to be extracted from OWNERS files as
well. That way, component info, if available in an OWNERS file, is added
to histogram expiry bugs, e.g. http://crbug/1088985.

Also, add component documentation and add a component to two
Variations.Headers.* histograms.

Support was previously added for component tags, see http://crbug/989112
for details, but it wasn't explicitly used.

Bug: 1133699
Change-Id: I572c877be8929923fd14df352e762a7a293ca5ea
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2442136Reviewed-by: default avatarSteven Holte <holte@chromium.org>
Reviewed-by: default avatarWeilun Shi <sweilun@chromium.org>
Commit-Queue: Caitlin Fischer <caitlinfischer@google.com>
Cr-Commit-Position: refs/heads/master@{#813914}
parent a60f84e9
...@@ -516,6 +516,19 @@ usefulness. When a histogram is nearing expiry, a robot will file a reminder ...@@ -516,6 +516,19 @@ usefulness. When a histogram is nearing expiry, a robot will file a reminder
bug in Monorail. It's important that somebody familiar with the histogram bug in Monorail. It's important that somebody familiar with the histogram
notices and triages such bugs! notices and triages such bugs!
### Components
Histograms may be associated with components, which can help make sure that
histogram expiry bugs don't fall through the cracks.
There are two ways in which components may be associated with a histogram. The
first and recommended way is to add a tag to a histogram or histogram suffix,
e.g. <component>UI&gt;Shell</component>. The second way is to specify an OWNERS
file as a secondary owner for a histogram. If the OWNERS file contains a
component, then the component is associated with the histogram. If the specified
OWNERS file doesn't have a component, but an OWNERS file in a parent directory
does, then the parent directory's component is used.
### Cleaning Up Histogram Entries ### Cleaning Up Histogram Entries
Do not delete histograms from histograms.xml. Instead, mark unused Do not delete histograms from histograms.xml. Instead, mark unused
......
...@@ -370,6 +370,29 @@ def _ExtractOwners(node): ...@@ -370,6 +370,29 @@ def _ExtractOwners(node):
return owners, has_owner return owners, has_owner
def _ExtractComponents(histogram):
"""Extracts component information from the given histogram element.
Components are present when a histogram has a component tag, e.g.
<component>UI&gt;Browser</component>. Components may also be present when an
OWNERS file is given as a histogram owner, e.g. <owner>src/dir/OWNERS</owner>.
See _ExtractComponentFromOWNERS() in the following file for details:
chromium/src/tools/metrics/histograms/expand_owners.py.
Args:
histogram: A DOM Element corresponding to a histogram.
Returns:
A list of the components associated with the histogram, e.g.
['UI>Browser>Spellcheck'].
"""
component_nodes = histogram.getElementsByTagName('component')
return [
_GetTextFromChildNodes(component_node)
for component_node in component_nodes
]
def _ValidateDateString(date_str): def _ValidateDateString(date_str):
"""Checks if |date_str| matches 'YYYY-MM-DD'. """Checks if |date_str| matches 'YYYY-MM-DD'.
...@@ -553,6 +576,11 @@ def _ExtractHistogramsFromXmlTree(tree, enums): ...@@ -553,6 +576,11 @@ def _ExtractHistogramsFromXmlTree(tree, enums):
if owners: if owners:
histogram_entry['owners'] = owners histogram_entry['owners'] = owners
# Find <component> tag.
components = _ExtractComponents(histogram)
if components:
histogram_entry['components'] = components
# Find <summary> tag. # Find <summary> tag.
summary_nodes = list(IterElementsWithTag(histogram, 'summary')) summary_nodes = list(IterElementsWithTag(histogram, 'summary'))
......
...@@ -445,6 +445,33 @@ class ExtractHistogramsTest(unittest.TestCase): ...@@ -445,6 +445,33 @@ class ExtractHistogramsTest(unittest.TestCase):
'Multi-paragraph sample description UI>Browser. Words.\n\n' 'Multi-paragraph sample description UI>Browser. Words.\n\n'
'Still multi-paragraph sample description.\n\nHere.') 'Still multi-paragraph sample description.\n\nHere.')
def testComponentExtraction(self):
"""Checks that components are successfully extracted from histograms."""
histogram = xml.dom.minidom.parseString("""
<histogram-configuration>
<histograms>
<histogram name="Coffee" expires_after="2022-01-01">
<owner>histogram_owner@google.com</owner>
<summary>An ode to coffee.</summary>
<component>Liquid&gt;Hot</component>
<component>Caffeine</component>
</histogram>
</histograms>
<histogram_suffixes_list>
<histogram_suffixes name="Brand" separator=".">
<suffix name="Dunkies" label="The coffee is from Dunkin."/>
<affected-histogram name="Coffee"/>
</histogram_suffixes>
</histogram_suffixes_list>
</histogram-configuration>
""")
histograms, _ = extract_histograms.ExtractHistogramsFromDom(histogram)
self.assertEqual(histograms['Coffee']['components'],
['Liquid>Hot', 'Caffeine'])
self.assertEqual(histograms['Coffee.Dunkies']['components'],
['Liquid>Hot', 'Caffeine'])
def testNewHistogramWithoutSummary(self): def testNewHistogramWithoutSummary(self):
histogram_without_summary = xml.dom.minidom.parseString(""" histogram_without_summary = xml.dom.minidom.parseString("""
<histogram-configuration> <histogram-configuration>
......
...@@ -12,6 +12,7 @@ import models ...@@ -12,6 +12,7 @@ import models
_OBSOLETE_TYPE = models.TextNodeType('obsolete') _OBSOLETE_TYPE = models.TextNodeType('obsolete')
_OWNER_TYPE = models.TextNodeType('owner', single_line=True) _OWNER_TYPE = models.TextNodeType('owner', single_line=True)
_COMPONENT_TYPE = models.TextNodeType('component', single_line=True)
_SUMMARY_TYPE = models.TextNodeType('summary', single_line=True) _SUMMARY_TYPE = models.TextNodeType('summary', single_line=True)
_DETAILS_TYPE = models.TextNodeType('details') _DETAILS_TYPE = models.TextNodeType('details')
...@@ -142,6 +143,7 @@ _HISTOGRAM_TYPE = models.ObjectNodeType( ...@@ -142,6 +143,7 @@ _HISTOGRAM_TYPE = models.ObjectNodeType(
alphabetization=[ alphabetization=[
(_OBSOLETE_TYPE.tag, _KEEP_ORDER), (_OBSOLETE_TYPE.tag, _KEEP_ORDER),
(_OWNER_TYPE.tag, _KEEP_ORDER), (_OWNER_TYPE.tag, _KEEP_ORDER),
(_COMPONENT_TYPE.tag, _KEEP_ORDER),
(_SUMMARY_TYPE.tag, _KEEP_ORDER), (_SUMMARY_TYPE.tag, _KEEP_ORDER),
(_DETAILS_TYPE.tag, _KEEP_ORDER), (_DETAILS_TYPE.tag, _KEEP_ORDER),
(_TOKEN_TYPE.tag, _KEEP_ORDER), (_TOKEN_TYPE.tag, _KEEP_ORDER),
...@@ -150,6 +152,7 @@ _HISTOGRAM_TYPE = models.ObjectNodeType( ...@@ -150,6 +152,7 @@ _HISTOGRAM_TYPE = models.ObjectNodeType(
children=[ children=[
models.ChildType(_OBSOLETE_TYPE.tag, _OBSOLETE_TYPE, multiple=False), models.ChildType(_OBSOLETE_TYPE.tag, _OBSOLETE_TYPE, multiple=False),
models.ChildType(_OWNER_TYPE.tag, _OWNER_TYPE, multiple=True), models.ChildType(_OWNER_TYPE.tag, _OWNER_TYPE, multiple=True),
models.ChildType(_COMPONENT_TYPE.tag, _COMPONENT_TYPE, multiple=True),
models.ChildType(_SUMMARY_TYPE.tag, _SUMMARY_TYPE, multiple=False), models.ChildType(_SUMMARY_TYPE.tag, _SUMMARY_TYPE, multiple=False),
models.ChildType(_DETAILS_TYPE.tag, _DETAILS_TYPE, multiple=False), models.ChildType(_DETAILS_TYPE.tag, _DETAILS_TYPE, multiple=False),
models.ChildType(_TOKEN_TYPE.tag, _TOKEN_TYPE, multiple=True), models.ChildType(_TOKEN_TYPE.tag, _TOKEN_TYPE, multiple=True),
......
...@@ -84,6 +84,7 @@ PRETTY_XML = """ ...@@ -84,6 +84,7 @@ PRETTY_XML = """
</obsolete> </obsolete>
<owner>owner1@chromium.org</owner> <owner>owner1@chromium.org</owner>
<owner>owner2@chromium.org</owner> <owner>owner2@chromium.org</owner>
<component>Component&gt;Subcomponent</component>
<summary>Summary text</summary> <summary>Summary text</summary>
</histogram> </histogram>
...@@ -119,6 +120,7 @@ XML_WRONG_ATTRIBUTE_ORDER = """ ...@@ -119,6 +120,7 @@ XML_WRONG_ATTRIBUTE_ORDER = """
</obsolete> </obsolete>
<owner>owner1@chromium.org</owner> <owner>owner1@chromium.org</owner>
<owner>owner2@chromium.org</owner> <owner>owner2@chromium.org</owner>
<component>Component&gt;Subcomponent</component>
<summary>Summary text</summary> <summary>Summary text</summary>
</histogram> </histogram>
...@@ -189,6 +191,7 @@ XML_WRONG_INDENT = """ ...@@ -189,6 +191,7 @@ XML_WRONG_INDENT = """
</obsolete> </obsolete>
<owner>owner1@chromium.org</owner> <owner>owner1@chromium.org</owner>
<owner>owner2@chromium.org</owner> <owner>owner2@chromium.org</owner>
<component>Component&gt;Subcomponent</component>
<summary>Summary text</summary> <summary>Summary text</summary>
</histogram> </histogram>
...@@ -224,6 +227,9 @@ XML_WRONG_SINGLELINE = """ ...@@ -224,6 +227,9 @@ XML_WRONG_SINGLELINE = """
owner1@chromium.org owner1@chromium.org
</owner> </owner>
<owner>owner2@chromium.org</owner> <owner>owner2@chromium.org</owner>
<component>
Component&gt;Subcomponent
</component>
<summary> <summary>
Summary text Summary text
</summary> </summary>
...@@ -259,6 +265,7 @@ XML_WRONG_LINEBREAK = """ ...@@ -259,6 +265,7 @@ XML_WRONG_LINEBREAK = """
</obsolete> </obsolete>
<owner>owner1@chromium.org</owner> <owner>owner1@chromium.org</owner>
<owner>owner2@chromium.org</owner> <owner>owner2@chromium.org</owner>
<component>Component&gt;Subcomponent</component>
<summary>Summary text</summary> <summary>Summary text</summary>
</histogram> </histogram>
...@@ -292,6 +299,7 @@ XML_WRONG_CHILDREN_ORDER = """ ...@@ -292,6 +299,7 @@ XML_WRONG_CHILDREN_ORDER = """
</obsolete> </obsolete>
<summary>Summary text</summary> <summary>Summary text</summary>
<owner>owner1@chromium.org</owner> <owner>owner1@chromium.org</owner>
<component>Component&gt;Subcomponent</component>
<owner>owner2@chromium.org</owner> <owner>owner2@chromium.org</owner>
</histogram> </histogram>
......
...@@ -121,6 +121,7 @@ reviews. Googlers can read more about this at go/gwsq-gerrit. ...@@ -121,6 +121,7 @@ reviews. Googlers can read more about this at go/gwsq-gerrit.
expires_after="2020-12-15"> expires_after="2020-12-15">
<owner>caitlinfischer@google.com</owner> <owner>caitlinfischer@google.com</owner>
<owner>src/base/metrics/OWNERS</owner> <owner>src/base/metrics/OWNERS</owner>
<component>Internals&gt;Metrics</component>
<summary> <summary>
The owner of the top-level domain from which certain subframe-initiated HTTP The owner of the top-level domain from which certain subframe-initiated HTTP
requests are made. It's logged after determining that (i) the request should requests are made. It's logged after determining that (i) the request should
...@@ -148,6 +149,7 @@ reviews. Googlers can read more about this at go/gwsq-gerrit. ...@@ -148,6 +149,7 @@ reviews. Googlers can read more about this at go/gwsq-gerrit.
<owner>jwd@chromium.org</owner> <owner>jwd@chromium.org</owner>
<owner>caitlinfischer@google.com</owner> <owner>caitlinfischer@google.com</owner>
<owner>src/base/metrics/OWNERS</owner> <owner>src/base/metrics/OWNERS</owner>
<component>Internals&gt;Metrics</component>
<summary> <summary>
Details about the request context in which an HTTP request is made. Logged Details about the request context in which an HTTP request is made. Logged
after determining that the request should include variations headers but after determining that the request should include variations headers but
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment