[CacheStorage] Add metrics to the scheduler

[CacheStorage] Add metrics to the scheduler

This CL adds UMA to keep track of operation stats such as how long the queue is
when adding an operation, how long each operation waits on the queue, how long
each operation takes, and whether operations freeze.

BUG=630285
Committed: https://crrev.com/8fdce7026ee44b2d26fa31bdd3f713d1fd9f92ff
Cr-Commit-Position: refs/heads/master@{#407878}

[jkarlin, 2016-07-21 17:00:32]

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

[commit-bot: I haz the power, 2016-07-21 17:00:52]

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

[jkarlin, 2016-07-21 17:33:36]

jkarlin@chromium.org changed reviewers:
+ jsbell@chromium.org

[jkarlin, 2016-07-21 17:33:36]

jsbell@ PTAL at everything, thanks!

[jkarlin, 2016-07-21 17:42:44]

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

[commit-bot: I haz the power, 2016-07-21 17:43:06]

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

[jsbell, 2016-07-21 18:13:31]

lgtm with a few nits

https://codereview.chromium.org/2168123002/diff/100001/content/browser/cache_...
File content/browser/cache_storage/cache_storage_operation_unittest.cc (right):

https://codereview.chromium.org/2168123002/diff/100001/content/browser/cache_...
content/browser/cache_storage/cache_storage_operation_unittest.cc:14: #include
"content/public/test/test_browser_thread_bundle.h"
Needed?

https://codereview.chromium.org/2168123002/diff/100001/content/browser/cache_...
content/browser/cache_storage/cache_storage_operation_unittest.cc:28: virtual
void Run() { callback_count_++; }
It doesn't look like this needs to be virtual. copy/paste?

https://codereview.chromium.org/2168123002/diff/100001/content/browser/cache_...
content/browser/cache_storage/cache_storage_operation_unittest.cc:47: // 
TestBrowserThreadBundle browser_thread_bundle_;
Remove

https://codereview.chromium.org/2168123002/diff/100001/content/browser/cache_...
File content/browser/cache_storage/cache_storage_scheduler.h (right):

https://codereview.chromium.org/2168123002/diff/100001/content/browser/cache_...
content/browser/cache_storage/cache_storage_scheduler.h:13: #include
"base/memory/ref_counted.h"
Needed?

https://codereview.chromium.org/2168123002/diff/100001/content/browser/cache_...
content/browser/cache_storage/cache_storage_scheduler.h:20: class
SingleThreadTaskRunner;
Needed?

https://codereview.chromium.org/2168123002/diff/100001/tools/metrics/histogra...
File tools/metrics/histograms/histograms.xml (right):

https://codereview.chromium.org/2168123002/diff/100001/tools/metrics/histogra...
tools/metrics/histograms/histograms.xml:51999: +    The time in ms from when an
operation's is started until it completes.
nit: remove 's

[jkarlin, 2016-07-21 18:24:54]

jkarlin@chromium.org changed reviewers:
+ rkaplow@chromium.org

[jkarlin, 2016-07-21 18:24:54]

rkaplow@ PTAL at histograms.xml, thanks!

[jkarlin, 2016-07-21 18:29:16]

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

[jkarlin, 2016-07-21 18:29:24]

Thanks jsbell!

https://codereview.chromium.org/2168123002/diff/100001/content/browser/cache_...
File content/browser/cache_storage/cache_storage_operation_unittest.cc (right):

https://codereview.chromium.org/2168123002/diff/100001/content/browser/cache_...
content/browser/cache_storage/cache_storage_operation_unittest.cc:14: #include
"content/public/test/test_browser_thread_bundle.h"
On 2016/07/21 18:13:31, jsbell wrote:
> Needed?

Done.

https://codereview.chromium.org/2168123002/diff/100001/content/browser/cache_...
content/browser/cache_storage/cache_storage_operation_unittest.cc:28: virtual
void Run() { callback_count_++; }
On 2016/07/21 18:13:31, jsbell wrote:
> It doesn't look like this needs to be virtual. copy/paste?

Done.

https://codereview.chromium.org/2168123002/diff/100001/content/browser/cache_...
content/browser/cache_storage/cache_storage_operation_unittest.cc:47: // 
TestBrowserThreadBundle browser_thread_bundle_;
On 2016/07/21 18:13:31, jsbell wrote:
> Remove

Done.

https://codereview.chromium.org/2168123002/diff/100001/tools/metrics/histogra...
File tools/metrics/histograms/histograms.xml (right):

https://codereview.chromium.org/2168123002/diff/100001/tools/metrics/histogra...
tools/metrics/histograms/histograms.xml:51999: +    The time in ms from when an
operation's is started until it completes.
On 2016/07/21 18:13:31, jsbell wrote:
> nit: remove 's

Done.

[jkarlin, 2016-07-21 18:29:49]

https://codereview.chromium.org/2168123002/diff/100001/content/browser/cache_...
File content/browser/cache_storage/cache_storage_scheduler.h (right):

https://codereview.chromium.org/2168123002/diff/100001/content/browser/cache_...
content/browser/cache_storage/cache_storage_scheduler.h:13: #include
"base/memory/ref_counted.h"
On 2016/07/21 18:13:31, jsbell wrote:
> Needed?

Done.

https://codereview.chromium.org/2168123002/diff/100001/content/browser/cache_...
content/browser/cache_storage/cache_storage_scheduler.h:20: class
SingleThreadTaskRunner;
On 2016/07/21 18:13:31, jsbell wrote:
> Needed?

Done.

[commit-bot: I haz the power, 2016-07-21 18:30:03]

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

[rkaplow, 2016-07-21 18:40:01]

https://codereview.chromium.org/2168123002/diff/120001/tools/metrics/histogra...
File tools/metrics/histograms/histograms.xml (right):

https://codereview.chromium.org/2168123002/diff/120001/tools/metrics/histogra...
tools/metrics/histograms/histograms.xml:52003: +<histogram
name="ServiceWorkerCache.Scheduler.OperationSlow" enum="BooleanSlow">
instead of making a custom enum, why not just use Boolean and call the metric
IsOperationSlow

https://codereview.chromium.org/2168123002/diff/120001/tools/metrics/histogra...
tools/metrics/histograms/histograms.xml:52018: +<histogram
name="ServiceWorkerCache.Scheduler.QueueLength">
can you add a unit

[commit-bot: I haz the power, 2016-07-21 19:47:55]

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

[commit-bot: I haz the power, 2016-07-21 19:47:56]

Dry run: Try jobs failed on following builders:
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[jkarlin, 2016-07-21 20:19:20]

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

[jkarlin, 2016-07-21 20:19:30]

Thanks rkaplow@, PTAL

https://codereview.chromium.org/2168123002/diff/120001/tools/metrics/histogra...
File tools/metrics/histograms/histograms.xml (right):

https://codereview.chromium.org/2168123002/diff/120001/tools/metrics/histogra...
tools/metrics/histograms/histograms.xml:52003: +<histogram
name="ServiceWorkerCache.Scheduler.OperationSlow" enum="BooleanSlow">
On 2016/07/21 18:40:01, rkaplow wrote:
> instead of making a custom enum, why not just use Boolean and call the metric
> IsOperationSlow

Done.

https://codereview.chromium.org/2168123002/diff/120001/tools/metrics/histogra...
tools/metrics/histograms/histograms.xml:52018: +<histogram
name="ServiceWorkerCache.Scheduler.QueueLength">
On 2016/07/21 18:40:01, rkaplow wrote:
> can you add a unit

It's a count, no unit needed.

[commit-bot: I haz the power, 2016-07-21 20:19:42]

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-07-21 20:27:54]

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

[commit-bot: I haz the power, 2016-07-21 20:27:55]

Dry run: Try jobs failed on following builders:
  android_arm64_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_arm6...)

[jkarlin, 2016-07-21 21:17:02]

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

[commit-bot: I haz the power, 2016-07-21 21:17:40]

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

[jkarlin, 2016-07-21 21:17:48]

Patchset #8 (id:140001) has been deleted

[jkarlin, 2016-07-21 23:07:04]

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

[commit-bot: I haz the power, 2016-07-21 23:07:53]

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-07-22 02:03:19]

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

[commit-bot: I haz the power, 2016-07-22 02:03:20]

Dry run: This issue passed the CQ dry run.

[jkarlin, 2016-07-22 19:05:52]

rkaplow@ PTAL, thanks!

[rkaplow, 2016-07-25 18:32:35]

On 2016/07/21 20:19:30, jkarlin wrote:
> Thanks rkaplow@, PTAL
> 
>
https://codereview.chromium.org/2168123002/diff/120001/tools/metrics/histogra...
> File tools/metrics/histograms/histograms.xml (right):
> 
>
https://codereview.chromium.org/2168123002/diff/120001/tools/metrics/histogra...
> tools/metrics/histograms/histograms.xml:52003: +<histogram
> name="ServiceWorkerCache.Scheduler.OperationSlow" enum="BooleanSlow">
> On 2016/07/21 18:40:01, rkaplow wrote:
> > instead of making a custom enum, why not just use Boolean and call the
metric
> > IsOperationSlow
> 
> Done.
> 
>
https://codereview.chromium.org/2168123002/diff/120001/tools/metrics/histogra...
> tools/metrics/histograms/histograms.xml:52018: +<histogram
> name="ServiceWorkerCache.Scheduler.QueueLength">
> On 2016/07/21 18:40:01, rkaplow wrote:
> > can you add a unit
> 
> It's a count, no unit needed.

You can still add a unit to what it refers to - in this case "elements" seems
fine. We prefer all histograms to have some unit - this way the graphs are
correctly labelled.

[rkaplow, 2016-07-25 18:33:16]

lgtm

[jkarlin, 2016-07-26 18:11:16]

On 2016/07/25 18:32:35, rkaplow wrote:
> On 2016/07/21 20:19:30, jkarlin wrote:
> > Thanks rkaplow@, PTAL
> > 
> >
>
https://codereview.chromium.org/2168123002/diff/120001/tools/metrics/histogra...
> > File tools/metrics/histograms/histograms.xml (right):
> > 
> >
>
https://codereview.chromium.org/2168123002/diff/120001/tools/metrics/histogra...
> > tools/metrics/histograms/histograms.xml:52003: +<histogram
> > name="ServiceWorkerCache.Scheduler.OperationSlow" enum="BooleanSlow">
> > On 2016/07/21 18:40:01, rkaplow wrote:
> > > instead of making a custom enum, why not just use Boolean and call the
> metric
> > > IsOperationSlow
> > 
> > Done.
> > 
> >
>
https://codereview.chromium.org/2168123002/diff/120001/tools/metrics/histogra...
> > tools/metrics/histograms/histograms.xml:52018: +<histogram
> > name="ServiceWorkerCache.Scheduler.QueueLength">
> > On 2016/07/21 18:40:01, rkaplow wrote:
> > > can you add a unit
> > 
> > It's a count, no unit needed.
> 
> You can still add a unit to what it refers to - in this case "elements" seems
> fine. We prefer all histograms to have some unit - this way the graphs are
> correctly labelled.

Got it, done!

[jkarlin, 2016-07-26 18:11:36]

The CQ bit was checked by jkarlin@chromium.org

[jkarlin, 2016-07-26 18:11:36]

The patchset sent to the CQ was uploaded after l-g-t-m from jsbell@chromium.org,
rkaplow@chromium.org
Link to the patchset: https://codereview.chromium.org/2168123002/#ps220001
(title: "Add unit")

[commit-bot: I haz the power, 2016-07-26 18:12:09]

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-07-26 19:30:31]

Committed patchset #11 (id:220001)

[commit-bot: I haz the power, 2016-07-26 19:33:52]

Description was changed from

==========
[CacheStorage] Add metrics to the scheduler

This CL adds UMA to keep track of operation stats such as how long the queue is
when adding an operation, how long each operation waits on the queue, how long
each operation takes, and whether operations freeze.

BUG=630285
==========

to

==========
[CacheStorage] Add metrics to the scheduler

This CL adds UMA to keep track of operation stats such as how long the queue is
when adding an operation, how long each operation waits on the queue, how long
each operation takes, and whether operations freeze.

BUG=630285

Committed: https://crrev.com/8fdce7026ee44b2d26fa31bdd3f713d1fd9f92ff
Cr-Commit-Position: refs/heads/master@{#407878}
==========

[commit-bot: I haz the power, 2016-07-26 19:33:53]

Patchset 11 (id:??) landed as
https://crrev.com/8fdce7026ee44b2d26fa31bdd3f713d1fd9f92ff
Cr-Commit-Position: refs/heads/master@{#407878}