New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 611740 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Jul 2016
Cc:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug


Sign in to add a comment

Address issues with page load metrics data loss due to logging in OnComplete

Project Member Reported by bmcquade@chromium.org, May 13 2016

Issue description

Address issues with page load metrics data loss due to logging in OnComplete
 
Cc: csharrison@chromium.org
We landed a metric that logs immediately and merged to M51, so we're starting to get data from Beta channel on Android (other platforms coming soon). Here's what we've learned so far:

The 2 histograms being compared are PageLoad.Timing2.NavigationToFirstContentfulPaint, which is logged at the end of the page life cycle, and PageLoad.Timing2.NavigationToFirstContentfulPaint.Immediate, which is the newly added variant that logs this value immediately after it is observed.

By comparing counts and aggregate statistics between these we can get a sense for data loss and bias introduced by logging at the end of a page load.

Across all M51 beta users on Android, we're seeing 13% loss in metrics on Android as a result of logging at the end of the page's lifetime, even though our logging happens in the browser process. This may be because, after Chrome gets backgrounded, even the bg process can get killed without notice, and we do not currently do anything to log at the time Chrome gets a notification that it's being backgrounded.

Further, we see a skew in the mean as a result of this loss. The mean is 7% lower as a result of not logging metrics immediately.

Given the above, I'm concerned that our current metrics approach is not as reliable on Android as we'd like, and we need to move our key metrics to use the immediate logging approach. I think it's a relatively small change to do this (see https://codereview.chromium.org/1971913002 for a prototype that introduces a helper class to ease this transition). For M51 I'd like to try to introduce additional immediate log events (unclear if merges will be accepted at this point, but I'd like to try), keeping the existing events in place. As we collect more data we can make a decision about whether and when to migrate to immediate logging by default in M52+.
Blockedon: 611811
Blockedon: 611813
Project Member

Comment 4 by bugdroid1@chromium.org, May 13 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/9a521d48e39c57ee5cb42326e9e50bd76dbb4bb2

commit 9a521d48e39c57ee5cb42326e9e50bd76dbb4bb2
Author: bmcquade <bmcquade@chromium.org>
Date: Fri May 13 19:15:54 2016

Improved and generalized page load metric immediate event logging.

Our current page load metric logging approach has been to log at
the very end of the page lifecycle, in the PageLoadMetricsObserver
OnComplete callback. This keeps metric logging logic simple, but
has some costs. In particular, it means that metrics may be logged
long after events were actually observed on a page load, which leads
to metrics landing in the 'wrong' UMA record, and on Android we see
>10% loss in logging and 7% skew in the mean, likely due to Android
killing the browser process after backgrounding, leading to our code
failing to log metrics for some pages.

This patch introduces a class that makes it easy for us to log metrics
immediately. In a follow up change, this class will be used to log
additional metrics.

BUG= 611740 

Review-Url: https://codereview.chromium.org/1975963003
Cr-Commit-Position: refs/heads/master@{#393599}

[modify] https://crrev.com/9a521d48e39c57ee5cb42326e9e50bd76dbb4bb2/chrome/browser/page_load_metrics/observers/core_page_load_metrics_observer.cc
[modify] https://crrev.com/9a521d48e39c57ee5cb42326e9e50bd76dbb4bb2/chrome/browser/page_load_metrics/observers/core_page_load_metrics_observer.h
[modify] https://crrev.com/9a521d48e39c57ee5cb42326e9e50bd76dbb4bb2/components/page_load_metrics/browser/metrics_web_contents_observer.cc
[modify] https://crrev.com/9a521d48e39c57ee5cb42326e9e50bd76dbb4bb2/components/page_load_metrics/browser/page_load_metrics_observer.h

Blockedon: 612547
Blockedon: 612561
Blockedon: 612576
The plan here is to change metrics to use immediate logging, and since the logging behavior has changed, rename these metrics. We will split metrics in the PageLoad.Timing2 category into 3 new sub-categories: PageLoad.PaintTiming, PageLoad.DocumentTiming, and PageLoad.ParseTiming.
Project Member

Comment 10 by bugdroid1@chromium.org, Jul 20 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ea728cb128c672e9cb9c99c16c00206575584d0a

commit ea728cb128c672e9cb9c99c16c00206575584d0a
Author: bmcquade <bmcquade@chromium.org>
Date: Wed Jul 20 12:55:46 2016

Remove non-immediate core page load metrics

This change removes the PageLoad.Timing2.* metrics, as they have
been replaced with
PageLoad.(Paint|Document|Parse)Timing.* equivalents.

Additionally, histograms that use dom loading are removed, as it
has been pointed out that dom loading is not well spec'd and has
been marked as deprecated
(https://github.com/w3c/navigation-timing/issues/13).

This change does not obsolete these histograms in histograms.xml,
as some experiments continue to depend on them and removing
them from histograms.xml makes them inaccessible in the Finch
dashboard. We'll remove these histograms from histograms.xml in
a follow up change in a few releases, when everyone has migrated
to the new metrics.

A few PageLoad.Timing2.* metrics are not removed in this change.
Those metrics will eventually need to be migrated to new names,
but for the time being they do not have new equivalents, so we
have not migrated them yet.

BUG= 611740 

Review-Url: https://codereview.chromium.org/2156093002
Cr-Commit-Position: refs/heads/master@{#406530}

[modify] https://crrev.com/ea728cb128c672e9cb9c99c16c00206575584d0a/chrome/browser/page_load_metrics/observers/core_page_load_metrics_observer.cc
[modify] https://crrev.com/ea728cb128c672e9cb9c99c16c00206575584d0a/chrome/browser/page_load_metrics/observers/core_page_load_metrics_observer.h
[modify] https://crrev.com/ea728cb128c672e9cb9c99c16c00206575584d0a/chrome/browser/page_load_metrics/observers/core_page_load_metrics_observer_unittest.cc
[modify] https://crrev.com/ea728cb128c672e9cb9c99c16c00206575584d0a/tools/metrics/histograms/histograms.xml

Status: Fixed (was: Untriaged)

Sign in to add a comment