New issue
Advanced search Search tips

Issue 884678 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Sep 19
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

Consider requiring ad bytes on page for ads.resources.* metrics

Project Member Reported by jkarlin@chromium.org, Sep 17

Issue description

The new pageload.clients.ads.resources.* metrics are reporting the number of ad bytes and total bytes for every page load, while the other ads metrics are reporting only on pages in which there are at least some ad bytes. 

The advantage of filtering out results with no ad bytes is that we can more easily determine percentiles of the metrics for pages which don't have ads. Also, it's what the other metrics do so they can be more easily compared.

The advantage to keeping them as is is that we learn what fraction of pages don't have ads on them, but it seems like we could track that in a separate metric.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Sep 17

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

commit 8e825d974bb0271c5587f2a10f710a1d2e4c4465
Author: John Delaney <johnidel@chromium.org>
Date: Mon Sep 17 18:19:46 2018

Record ad bytes only on pages that have loaded ads

Current metrics recording ad totals on pages are only required if
there are > 0 ad bytes on the page. Add this logic to
pageload.clients.ad.resources.* so that they can be easily compared
to existing metrics. Allows easier creation of percentiles for
pages that have ads.

Bug:  884678 
Change-Id: I4c1088c98bf86a06b4c0a01eb3077d27c06f8ee5
Reviewed-on: https://chromium-review.googlesource.com/1228433
Reviewed-by: Josh Karlin <jkarlin@chromium.org>
Commit-Queue: John Delaney <johnidel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#591744}
[modify] https://crrev.com/8e825d974bb0271c5587f2a10f710a1d2e4c4465/chrome/browser/page_load_metrics/observers/ads_page_load_metrics_observer.cc

Project Member

Comment 2 by bugdroid1@chromium.org, Sep 18

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

commit 8b87d09c2e19fa9eabd7bcfb5874472037342613
Author: John Delaney <johnidel@chromium.org>
Date: Tue Sep 18 21:44:14 2018

Fix histogram summaries for PageLoad.Clients.Ads.Resource.*

THere metrics are now gated on there being non-zero ad bytes on
page. Reflect this condition in the histogram descriptions. See:
https://chromium-review.googlesource.com/c/chromium/src/+/1228433

Bug:  884678 
Change-Id: I5320c4b8fff1e3e57ba45cb2c0100db9ea577d62
Reviewed-on: https://chromium-review.googlesource.com/1231154
Reviewed-by: Jesse Doherty <jwd@chromium.org>
Reviewed-by: Josh Karlin <jkarlin@chromium.org>
Commit-Queue: John Delaney <johnidel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#592210}
[modify] https://crrev.com/8b87d09c2e19fa9eabd7bcfb5874472037342613/tools/metrics/histograms/histograms.xml

Status: Fixed (was: Assigned)

Sign in to add a comment