Adding AMP cache PageLoad.* metrics

Adding AMP cache PageLoad.* metrics

Accelerated Mobile Pages (AMP) are pages that have limited sets of
functionality to improve page load performance. AMP pages are
discoverable by google and are served via GSA. These pages are cached by
google to further improve page load time and GSA will use the cached
pages when serving AMP to the user.

The AMP cache is currently located at https://www.google.com/amp/*
but will soon move to https://cdn.ampproject.org. See
https://developers.google.com/amp/cache/overview#amp-cache-url-format
for more details.

A follow-up CL will track page size for AMP cache pages.

BUG=658817
Committed: https://crrev.com/675a97de3cb0bb9b2871de224aeebb27291373b5
Cr-Commit-Position: refs/heads/master@{#440916}

[RyanSturm, 2016-12-22 22:44:50]

The CQ bit was checked by ryansturm@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-12-22 22:45:18]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-12-23 01:00:23]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-12-23 01:00:24]

Dry run: This issue passed the CQ dry run.

[RyanSturm, 2016-12-28 19:42:12]

ryansturm@chromium.org changed reviewers:
+ tbansal@chromium.org

[RyanSturm, 2016-12-28 19:42:13]

tbansal: please take a quick look before I send it out to other reviewers

[Bryan McQuade, 2016-12-28 20:04:25]

On 2016/12/28 at 19:42:13, ryansturm wrote:
> tbansal: please take a quick look before I send it out to other reviewers

wow, very nice - just happened to see this so wanted to send an initial comment
to note that many AMP page loads, in particular those from the Google SRP, are
loaded via a same-page navigation on the google hostname that iframes the AMP
document. the current page load metrics infra doesn't track page loads for
iframed content (and I don't think that it should, as load start times in
iframes are rarely associated with user perceived start load time), and also
doesn't track page load times for same page navigations yet, as doing so is
nontrivial.

i think this is probably fine for now, but we should keep in mind that we're
going to miss all AMP loads from the Google search results page until this is
addressed. I have some ideas to add support for implementing same page nav
tracking which we can discuss if you're interested in taking it on in the
future.

[RyanSturm, 2016-12-28 20:26:19]

On 2016/12/28 20:04:25, Bryan McQuade wrote:
> On 2016/12/28 at 19:42:13, ryansturm wrote:
> > tbansal: please take a quick look before I send it out to other reviewers
> 
> wow, very nice - just happened to see this so wanted to send an initial
comment
> to note that many AMP page loads, in particular those from the Google SRP, are
> loaded via a same-page navigation on the google hostname that iframes the AMP
> document. the current page load metrics infra doesn't track page loads for
> iframed content (and I don't think that it should, as load start times in
> iframes are rarely associated with user perceived start load time), and also
> doesn't track page load times for same page navigations yet, as doing so is
> nontrivial.
> 
> i think this is probably fine for now, but we should keep in mind that we're
> going to miss all AMP loads from the Google search results page until this is
> addressed. I have some ideas to add support for implementing same page nav
> tracking which we can discuss if you're interested in taking it on in the
> future.

Good point. I am going to play around with different ways of accessing AMP pages
(google SRP, GSA, directly, etc.) to judge if there is much value in moving
forward. I will add an appropriate comment to the class about the nature of AMP
pages and what this really tracks.

[Bryan McQuade, 2016-12-28 20:31:46]

On 2016/12/28 at 20:26:19, ryansturm wrote:
> On 2016/12/28 20:04:25, Bryan McQuade wrote:
> > On 2016/12/28 at 19:42:13, ryansturm wrote:
> > > tbansal: please take a quick look before I send it out to other reviewers
> > 
> > wow, very nice - just happened to see this so wanted to send an initial
comment
> > to note that many AMP page loads, in particular those from the Google SRP,
are
> > loaded via a same-page navigation on the google hostname that iframes the
AMP
> > document. the current page load metrics infra doesn't track page loads for
> > iframed content (and I don't think that it should, as load start times in
> > iframes are rarely associated with user perceived start load time), and also
> > doesn't track page load times for same page navigations yet, as doing so is
> > nontrivial.
> > 
> > i think this is probably fine for now, but we should keep in mind that we're
> > going to miss all AMP loads from the Google search results page until this
is
> > addressed. I have some ideas to add support for implementing same page nav
> > tracking which we can discuss if you're interested in taking it on in the
> > future.
> 
> Good point. I am going to play around with different ways of accessing AMP
pages (google SRP, GSA, directly, etc.) to judge if there is much value in
moving forward. I will add an appropriate comment to the class about the nature
of AMP pages and what this really tracks.

Sounds good. I'm fine landing this as is to get some initial data, especially
for tracking data use. Can always refine it in the future.

[RyanSturm, 2016-12-28 21:31:15]

The CQ bit was checked by ryansturm@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-12-28 21:31:27]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[RyanSturm, 2016-12-28 21:33:50]

ryansturm@chromium.org changed reviewers:
+ bmcquade@chromium.org
- tbansal@chromium.org

[RyanSturm, 2016-12-28 21:33:51]

-tbansal

bmcquade: PTAL, I added comments about same page navigations as those are a good
portion of the AMP navigations. In the future, it might be worth looking into
iframe navigations (especially cross domain iframe navigations).

[Bryan McQuade, 2016-12-28 21:42:08]

On 2016/12/28 at 21:33:51, ryansturm wrote:
> -tbansal
> 
> bmcquade: PTAL, I added comments about same page navigations as those are a
good portion of the AMP navigations. In the future, it might be worth looking
into iframe navigations (especially cross domain iframe navigations).

looks good, thanks! just need to fix the name of a few histograms in the
constants.

[RyanSturm, 2016-12-28 21:56:51]

The CQ bit was checked by ryansturm@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-12-28 21:56:57]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[RyanSturm, 2016-12-28 21:58:04]

ryansturm@chromium.org changed reviewers:
+ holte@chromium.org

[RyanSturm, 2016-12-28 21:58:04]

holte: PTAL @ histograms

Bmcquade: PTAL, good catch on the string constants.

[Steven Holte, 2016-12-28 22:51:30]

histograms lgtm

[commit-bot: I haz the power, 2016-12-28 22:52:54]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-12-28 22:52:55]

Dry run: This issue passed the CQ dry run.

[Bryan McQuade, 2016-12-28 23:41:47]

On 2016/12/28 at 22:51:30, holte wrote:
> histograms lgtm

LGTM

[RyanSturm, 2016-12-29 00:54:23]

The CQ bit was checked by ryansturm@chromium.org

[commit-bot: I haz the power, 2016-12-29 00:54:31]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-12-29 00:58:20]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1482972863546900,
"parent_rev": "6af82441b8ec93123d5efa54c2ee53e63d34499e", "commit_rev":
"170236a519483f60ff4a846baeef618f39fd66be"}

[commit-bot: I haz the power, 2016-12-29 00:58:47]

Description was changed from

==========
Adding AMP cache PageLoad.* metrics

Accelerated Mobile Pages (AMP) are pages that have limited sets of
functionality to improve page load performance. AMP pages are
discoverable by google and are served via GSA. These pages are cached by
google to further improve page load time and GSA will use the cached
pages when serving AMP to the user.

The AMP cache is currently located at https://www.google.com/amp/*
but will soon move to https://cdn.ampproject.org. See
https://developers.google.com/amp/cache/overview#amp-cache-url-format
for more details.

A follow-up CL will track page size for AMP cache pages.

BUG=658817
==========

to

==========
Adding AMP cache PageLoad.* metrics

Accelerated Mobile Pages (AMP) are pages that have limited sets of
functionality to improve page load performance. AMP pages are
discoverable by google and are served via GSA. These pages are cached by
google to further improve page load time and GSA will use the cached
pages when serving AMP to the user.

The AMP cache is currently located at https://www.google.com/amp/*
but will soon move to https://cdn.ampproject.org. See
https://developers.google.com/amp/cache/overview#amp-cache-url-format
for more details.

A follow-up CL will track page size for AMP cache pages.

BUG=658817

Review-Url: https://codereview.chromium.org/2595263004
==========

[commit-bot: I haz the power, 2016-12-29 00:58:48]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2017-01-02 15:51:04]

Description was changed from

==========
Adding AMP cache PageLoad.* metrics

Accelerated Mobile Pages (AMP) are pages that have limited sets of
functionality to improve page load performance. AMP pages are
discoverable by google and are served via GSA. These pages are cached by
google to further improve page load time and GSA will use the cached
pages when serving AMP to the user.

The AMP cache is currently located at https://www.google.com/amp/*
but will soon move to https://cdn.ampproject.org. See
https://developers.google.com/amp/cache/overview#amp-cache-url-format
for more details.

A follow-up CL will track page size for AMP cache pages.

BUG=658817

Review-Url: https://codereview.chromium.org/2595263004
==========

to

==========
Adding AMP cache PageLoad.* metrics

Accelerated Mobile Pages (AMP) are pages that have limited sets of
functionality to improve page load performance. AMP pages are
discoverable by google and are served via GSA. These pages are cached by
google to further improve page load time and GSA will use the cached
pages when serving AMP to the user.

The AMP cache is currently located at https://www.google.com/amp/*
but will soon move to https://cdn.ampproject.org. See
https://developers.google.com/amp/cache/overview#amp-cache-url-format
for more details.

A follow-up CL will track page size for AMP cache pages.

BUG=658817

Committed: https://crrev.com/675a97de3cb0bb9b2871de224aeebb27291373b5
Cr-Commit-Position: refs/heads/master@{#440916}
==========

[commit-bot: I haz the power, 2017-01-02 15:51:05]

Patchset 3 (id:??) landed as
https://crrev.com/675a97de3cb0bb9b2871de224aeebb27291373b5
Cr-Commit-Position: refs/heads/master@{#440916}