New issue
Advanced search Search tips

Issue 762357 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Nov 27
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Android , Windows , Chrome , Mac
Pri: 1
Type: Task


Sign in to add a comment

NetS13nServiceWorker: add UMA

Project Member Reported by falken@chromium.org, Sep 6 2017

Issue description

Make sure we have correct and complete UMA for when NetS13nSW/NetworkService is enabled.

Two goals:
- Ensure we have UMA to compare for a field trial with S13nSW on.
- Ensure the existing UMA doesn't get imbalanced by broken or missing UMA depending on S13nSW/NetworkService.

For the field trial, we should look at the following:
ServiceWorker.StartWorker.Time
ServiceWorker.StartWorker.Status
ServiceWorker.FetchEvent.MainResource.Status
ServiceWorker.FetchEvent.Subresource.Status
PageLoad.Clients.ServiceWorker.PaintTiming.NavigationToFirstContentfulPaint

For the second, we should list up all SW UMAs and see whether they are being recorded correctly with NS and whether it matters.

Rough doc: https://docs.google.com/document/d/1sG15uQsPe5QddEGuCmsxQKVI5S8Vfx9PdHKHWt5zZMs/edit#heading=h.csgt6ave2p57
(internal only for now)


Original description:
ServiceWorkerURLLoaderJob (S13nServiceWorker) should have similar UMA to ServiceWorkerURLRequestJob (non-S13nServiceWorker).

If we're reusing the UMA names as the non-S13nSW case, we should enable them logging all UMA at the same time so the metrics don't get imbalanced.
 
Description: Show this description
Project Member

Comment 2 by bugdroid1@chromium.org, Sep 8 2017

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

commit 2ed306fbd6cd40a04109815303e9766a31ea4dec
Author: Matt Falkenhagen <falken@chromium.org>
Date: Fri Sep 08 02:53:12 2017

S13nServiceWorker: Set navigation preload flag in SWURLLoaderJob.

This field is currently only used for UseCounter. Using it in
S13nServiceWorker somewhat violates the plan in 762357 to enable
all UMA at the same time, but this is just UseCounter data which
is not as important as keeping the reliability UMA consistent, and
I expect S13nSW to have negligible impact on UseCounter until launched.

Bug:  715640 ,  762357 
Change-Id: I5e32d5251fa61c0e8978296f3e76cb4899af1bc2
Reviewed-on: https://chromium-review.googlesource.com/655417
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Commit-Queue: Matt Falkenhagen <falken@chromium.org>
Cr-Commit-Position: refs/heads/master@{#500491}
[modify] https://crrev.com/2ed306fbd6cd40a04109815303e9766a31ea4dec/content/browser/service_worker/service_worker_url_loader_job.cc
[modify] https://crrev.com/2ed306fbd6cd40a04109815303e9766a31ea4dec/content/browser/service_worker/service_worker_url_loader_job_unittest.cc

Cc: nhiroki@chromium.org
Summary: Add UMA for ServiceWorkerURLLoaderJob and ServiceWorkerScriptURLLoader (was: Add UMA for ServiceWorkerURLLoaderJob)
Let me piggyback...

Comment 4 by laforge@google.com, Nov 7 2017

Components: Internals>Network>Service

Comment 5 by laforge@google.com, Nov 7 2017

Components: -Internals>Network>Service Internals>Services>Network
Apologies, applied the wrong component in bulk.

Comment 6 by dxie@chromium.org, May 17 2018

Labels: -Pri-3 Proj-Servicification-Canary OS-All Pri-1

Comment 7 by dxie@chromium.org, May 18 2018

Labels: -OS-All OS-Windows OS-Linux OS-Mac OS-Chrome OS-Android

Comment 8 by falken@chromium.org, Jun 10 2018

Description: Show this description

Comment 9 by falken@chromium.org, Jun 10 2018

Summary: NetS13nServiceWorker: add UMA (was: Add UMA for ServiceWorkerURLLoaderJob and ServiceWorkerScriptURLLoader)
Owner: falken@chromium.org
Status: Started (was: Available)
Project Member

Comment 11 by bugdroid1@chromium.org, Jun 13 2018

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

commit 6d08d5279809ccdd7b498fc6b811a7e3ce72b8d7
Author: Matt Falkenhagen <falken@chromium.org>
Date: Wed Jun 13 00:46:31 2018

service worker: Cleanup URLRequestJobResult enum.

- Remove some unused values.
- Document the rest.

Bug:  762357 
Change-Id: Id32abd4474b58c9577c12757944fb879319673c8
Reviewed-on: https://chromium-review.googlesource.com/1096514
Commit-Queue: Matt Falkenhagen <falken@chromium.org>
Reviewed-by: Matt Falkenhagen <falken@chromium.org>
Reviewed-by: Helen Li <xunjieli@chromium.org>
Reviewed-by: Makoto Shimazu <shimazu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#566659}
[modify] https://crrev.com/6d08d5279809ccdd7b498fc6b811a7e3ce72b8d7/content/browser/service_worker/service_worker_metrics.h
[modify] https://crrev.com/6d08d5279809ccdd7b498fc6b811a7e3ce72b8d7/content/browser/service_worker/service_worker_url_request_job.cc
[modify] https://crrev.com/6d08d5279809ccdd7b498fc6b811a7e3ce72b8d7/net/log/net_log_event_type_list.h
[modify] https://crrev.com/6d08d5279809ccdd7b498fc6b811a7e3ce72b8d7/tools/metrics/histograms/enums.xml

Description: Show this description
Blockedon: 852658
Project Member

Comment 14 by bugdroid1@chromium.org, Jun 14 2018

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

commit 182e86d6caaea926a164aa89c19bb0b96a528d3e
Author: Matt Falkenhagen <falken@chromium.org>
Date: Thu Jun 14 05:07:54 2018

NetS13nServiceWorker: Refactoring and documentation around loaders and events.

Factoring out pure code health changes from a CL to add UMA to loaders
https://chromium-review.googlesource.com/c/chromium/src/+/1098855.

Bug:  762357 
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_mojo
Change-Id: I6af1a5c9ce410b84404c182d835702891138bdc8
TBR: kinuko
Reviewed-on: https://chromium-review.googlesource.com/1099095
Reviewed-by: Matt Falkenhagen <falken@chromium.org>
Reviewed-by: Makoto Shimazu <shimazu@chromium.org>
Commit-Queue: Matt Falkenhagen <falken@chromium.org>
Cr-Commit-Position: refs/heads/master@{#567147}
[modify] https://crrev.com/182e86d6caaea926a164aa89c19bb0b96a528d3e/content/renderer/service_worker/service_worker_subresource_loader.cc
[modify] https://crrev.com/182e86d6caaea926a164aa89c19bb0b96a528d3e/content/renderer/service_worker/service_worker_subresource_loader_unittest.cc
[modify] https://crrev.com/182e86d6caaea926a164aa89c19bb0b96a528d3e/third_party/blink/public/mojom/service_worker/service_worker_event_status.mojom

Blockedon: 852664
Blockedon: 852666
Blockedon: 852667
Blockedon: 852668
Blockedon: 852670
Blocking: 852672
Blockedon: 852673
Project Member

Comment 22 by bugdroid1@chromium.org, Jun 15 2018

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

commit 6d1da9def8eaab2680241fd4a5466e6c960ff13c
Author: Matt Falkenhagen <falken@chromium.org>
Date: Fri Jun 15 11:01:30 2018

NetS13nServiceWorker: Add UMA for navigation and subresource loaders.

Starts recording the following in NetS13nServiceWorker:
- ServiceWorker.FetchEvent.MainResource.Status
- ServiceWorker.FetchEvent.Subresource.Status

These are important health metrics for service worker interception,
and can be used for comparisons in a field trial for NetS13nSW.

Bug:  762357 
Change-Id: I34d735fdde34b535d0604ce187de3c1acb450ddc
Reviewed-on: https://chromium-review.googlesource.com/1098855
Commit-Queue: Matt Falkenhagen <falken@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Reviewed-by: Ilya Sherman <isherman@chromium.org>
Reviewed-by: Makoto Shimazu <shimazu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#567604}
[modify] https://crrev.com/6d1da9def8eaab2680241fd4a5466e6c960ff13c/content/browser/background_sync/background_sync_manager.cc
[modify] https://crrev.com/6d1da9def8eaab2680241fd4a5466e6c960ff13c/content/browser/service_worker/service_worker_browsertest.cc
[modify] https://crrev.com/6d1da9def8eaab2680241fd4a5466e6c960ff13c/content/browser/service_worker/service_worker_navigation_loader.cc
[modify] https://crrev.com/6d1da9def8eaab2680241fd4a5466e6c960ff13c/content/browser/service_worker/service_worker_navigation_loader_unittest.cc
[modify] https://crrev.com/6d1da9def8eaab2680241fd4a5466e6c960ff13c/content/browser/service_worker/service_worker_register_job.cc
[modify] https://crrev.com/6d1da9def8eaab2680241fd4a5466e6c960ff13c/content/browser/service_worker/service_worker_type_converters.cc
[modify] https://crrev.com/6d1da9def8eaab2680241fd4a5466e6c960ff13c/content/browser/service_worker/service_worker_type_converters.h
[modify] https://crrev.com/6d1da9def8eaab2680241fd4a5466e6c960ff13c/content/browser/service_worker/service_worker_version.cc
[modify] https://crrev.com/6d1da9def8eaab2680241fd4a5466e6c960ff13c/content/common/BUILD.gn
[add] https://crrev.com/6d1da9def8eaab2680241fd4a5466e6c960ff13c/content/common/service_worker/service_worker_type_converters.cc
[add] https://crrev.com/6d1da9def8eaab2680241fd4a5466e6c960ff13c/content/common/service_worker/service_worker_type_converters.h
[modify] https://crrev.com/6d1da9def8eaab2680241fd4a5466e6c960ff13c/content/renderer/service_worker/service_worker_subresource_loader.cc
[modify] https://crrev.com/6d1da9def8eaab2680241fd4a5466e6c960ff13c/content/renderer/service_worker/service_worker_subresource_loader.h
[modify] https://crrev.com/6d1da9def8eaab2680241fd4a5466e6c960ff13c/content/renderer/service_worker/service_worker_subresource_loader_unittest.cc
[modify] https://crrev.com/6d1da9def8eaab2680241fd4a5466e6c960ff13c/tools/metrics/histograms/histograms.xml

Labels: -Proj-Servicification-Canary Proj-Servicification
Labels: knon
Labels: -knon Hotlist-KnownIssue
Status: Fixed (was: Started)
We added the desired UMA.

Sign in to add a comment