Commit 2eab7dfd authored by Yue Ru Sun's avatar Yue Ru Sun Committed by Commit Bot

Obsolete tag for UKM enums

UKM enums are used by `UKM.Entries.Recorded.ByEntryHash` and
`UKM.Entries.Dropped.ByEntryHash` when displaying a breakdown of event
counts by name.

Some events were incorrectly displayed as "obsolete" in the histogram
table here https://screenshot.googleplex.com/CRmtGDOcAwQ.png
compared to the the event definition https://source.chromium.org/chromium/chromium/src/+/master:tools/metrics/ukm/ukm.xml;l=4254;drc=ec5f934779d00e3bf817be19297f72ef5bd98e9c

This patch fixes the logic to consider an event as obsolete iff
- the event itself is marked as obsolete, or
- all the metrics under this event are marked as obsolete.

Tested locally with
chromium/src/tools/metrics/histograms$ ./merge_xml.py --output output.xml ../ukm/ukm.xml enums.xml

Bug: b/163387426
Change-Id: I06ae60edb4e025cf693ad37063a45dc3bf6edd31
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2347386
Commit-Queue: Yue Ru Sun <yrsun@chromium.org>
Reviewed-by: default avatarAlexei Svitkine <asvitkine@chromium.org>
Cr-Commit-Position: refs/heads/master@{#796858}
parent 047efa5d
......@@ -84,8 +84,8 @@ class MergeXmlTest(unittest.TestCase):
"""Checks that the UkmEventNameHash enum is populated correctly.
If ukm.xml is provided, populate a list of ints to the UkmEventNameHash enum
where where each value is a xml event name hash truncated to 31 bits and
each label is the corresponding event name.
where each value is a truncated hash of the event name and each label is the
corresponding event name, with obsolete label when applicable.
"""
merged = merge_xml.PrettyPrintMergedFiles(histogram_paths.ALL_TEST_XMLS)
expected_merged_xml = """
......@@ -109,9 +109,11 @@ class MergeXmlTest(unittest.TestCase):
This gets populated by the GetEnumsNodes function in merge_xml.py when
producing the merged XML file.
</summary>
<int value="324605288" label="AbusiveExperienceHeuristic.WindowOpen"/>
<int value="1621538456" label="AbusiveExperienceHeuristic.TabUnder"/>
<int value="1913876024" label="Autofill.SelectedMaskedServerCard (Obsolete)"/>
<int value="151676257" label="AbusiveExperienceHeuristic.TestEvent1"/>
<int value="898353372"
label="AbusiveExperienceHeuristic.TestEvent2 (Obsolete)"/>
<int value="1052089961" label="Autofill.TestEvent3"/>
<int value="1758741469" label="FullyObsolete.TestEvent4 (Obsolete)"/>
</enum>
</enums>
......
......@@ -22,18 +22,32 @@ def _GetEventDetails(event):
"""Returns a simple struct containing the event details.
Args:
event: An event.
event: An event description as defined in ukm.xml.
Returns:
A struct containing the event name, name hash, and whether the event is
obsolete.
A struct containing the event name, truncated hash, and whether the event is
considered obsolete.
"""
name = event.getAttribute('name')
# The value is UKM event name hash truncated to 31 bits. This is recorded in
# https://cs.chromium.org/chromium/src/components/ukm/ukm_recorder_impl.cc?rcl=728ad079d8e52ada4e321fb4f53713e4f0588072&l=114
# https://cs.chromium.org/chromium/src/components/ukm/ukm_recorder_impl.cc?q=LogEventHashasUmaHistogram
hash = codegen.HashName(name) & 0x7fffffff
is_obsolete = event.getElementsByTagName('obsolete')
return EventDetails(name=name, hash=hash, is_obsolete=is_obsolete)
def _HasDirectObsoleteTag(node):
return any(
isinstance(child, xml.dom.minidom.Element)
and child.tagName == 'obsolete' for child in node.childNodes)
# The UKM event is considered obsolete if the event itself is marked as
# obsolete with a tag or all of its metrics are marked as obsolete.
is_event_obsolete = _HasDirectObsoleteTag(event)
are_all_metrics_obsolete = all(
_HasDirectObsoleteTag(metric)
for metric in event.getElementsByTagName('metric'))
return EventDetails(name=name,
hash=hash,
is_obsolete=is_event_obsolete or are_all_metrics_obsolete)
def PopulateEnumWithUkmEvents(doc, enum, ukm_events):
......
<ukm-configuration>
<event name="AbusiveExperienceHeuristic.TabUnder">
<owner>csharrison@chromium.org</owner>
<metric name="DidTabUnder">
<event name="AbusiveExperienceHeuristic.TestEvent1">
<owner>owner1@chromium.org</owner>
<metric name="Metric">
<summary>
True if the page attempted a tab-under navigation.
A summary of AbusiveExperienceHeuristic.TestEvent1
</summary>
</metric>
</event>
<event name="AbusiveExperienceHeuristic.WindowOpen">
<owner>yaoxia@chromium.org</owner>
<event name="AbusiveExperienceHeuristic.TestEvent2">
<owner>owner3@chromium.org</owner>
<obsolete>
Deprecated 2/2019
</obsolete>
<metric name="Metric">
<summary>
Recorded whenever window.open() is called when AdTagging is enabled.
A summary of the metric.
</summary>
<metric name="FromAdScript">
</metric>
</event>
<event name="Autofill.TestEvent3">
<owner>owner3@chromium.org</owner>
<summary>
A summary of Autofill.TestEvent3.
</summary>
<metric name="MetricVersion1">
<summary>
True if the page called window.open() with an ad script in the stack.
MetricVersion1 summary.
</summary>
<obsolete>
Deprecated and replaced by MetricVersion2.
</obsolete>
</metric>
<metric name="FromAdSubframe">
<metric name="MetricVersion2">
<summary>
True if the page called window.open() from an ad subframe.
MetricVersion2 summary.
</summary>
</metric>
</event>
<event name="Autofill.SelectedMaskedServerCard">
<event name="FullyObsolete.TestEvent4">
<owner>owner4@chromium.org</owner>
<summary>
All metrics of this event are marked as obsolete.
</summary>
<metric name="MetricVersion1">
<summary>
MetricVersion1 summary.
</summary>
<obsolete>
Deprecated 2/2019
Deprecated and replaced by MetricVersion2.
</obsolete>
<owner>jiahuiguo@google.com</owner>
<metric name="MillisecondsSinceFormParsed">
</metric>
<metric name="MetricVersion2">
<summary>
Obsolete.
Time since form parse.
MetricVersion2 summary.
</summary>
<obsolete/>
</metric>
</event>
......
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