Files.app: Add metrics of RAW file loading time.

Files.app: Add metrics of RAW file loading time.

BUG=475034
TEST=None

Committed: https://crrev.com/b3db409725e6a1f7f612ecf08115029aa14a6964
Cr-Commit-Position: refs/heads/master@{#325176}

[hirono, 2015-04-10 11:02:16]

hirono@chromium.org changed reviewers:
+ kenobi@chromium.org

[hirono, 2015-04-10 11:02:17]

PTAL, thanks!

[Ben Kwa, 2015-04-10 15:57:31]

lgtm with a couple of minor notes.  The one thing I'd want to double-check
before submitting this, is that our analytics users have agreed on the
granularity of the new timing category in analytics.

https://codereview.chromium.org/1076173002/diff/1/ui/file_manager/file_manage...
File ui/file_manager/file_manager/common/js/metrics_events.js (right):

https://codereview.chromium.org/1076173002/diff/1/ui/file_manager/file_manage...
ui/file_manager/file_manager/common/js/metrics_events.js:23: RAW_IMAGE: 'Raw
Image'
This category sounds maybe a little too specific?  (Question mark because this
is really a judgement call and I don't have full context)

We're looking for a bucketing that corresponds to "I want to be able to see how
much time in my app was spent doing Foo", or "I want to see all the events that
occurred in my app that relate to Foo".  Examples of "Foo" might be:
- Importing media ("Acquisition")
- Browsing media (we don't have this yet, but we might in the future)
- Loading raw files (this case)

So if "Loading raw files" feels like it belongs on that list, then we're fine. 
If it feels too fine-grained, I might suggest something like IMAGE_LOAD for
bucketing all image-loading metrics.

https://codereview.chromium.org/1076173002/diff/1/ui/file_manager/file_manage...
ui/file_manager/file_manager/common/js/metrics_events.js:150: RAW_LOAD_ARW: 'Row
Load ARW',
"Raw"?  Ditto lines below.

https://codereview.chromium.org/1076173002/diff/1/ui/file_manager/image_loade...
File ui/file_manager/image_loader/request.js (right):

https://codereview.chromium.org/1076173002/diff/1/ui/file_manager/image_loade...
ui/file_manager/image_loader/request.js:240: var startTime = new Date();
If you don't want to mess with raw Dates there's also
analytics.Tracker#startTiming which returns a timer object that you call #send
on when you want the timing to stop.  But what you have here also works fine.

[hirono, 2015-04-14 04:01:26]

hirono@chromium.org changed reviewers:
+ smckay@chromium.org

[hirono, 2015-04-14 04:01:27]

I updated the CL as discussion. Please take another look, thanks!

https://codereview.chromium.org/1076173002/diff/1/ui/file_manager/file_manage...
File ui/file_manager/file_manager/common/js/metrics_events.js (right):

https://codereview.chromium.org/1076173002/diff/1/ui/file_manager/file_manage...
ui/file_manager/file_manager/common/js/metrics_events.js:23: RAW_IMAGE: 'Raw
Image'
On 2015/04/10 15:57:31, Ben Kwa wrote:
> This category sounds maybe a little too specific?  (Question mark because this
> is really a judgement call and I don't have full context)
> 
> We're looking for a bucketing that corresponds to "I want to be able to see
how
> much time in my app was spent doing Foo", or "I want to see all the events
that
> occurred in my app that relate to Foo".  Examples of "Foo" might be:
> - Importing media ("Acquisition")
> - Browsing media (we don't have this yet, but we might in the future)
> - Loading raw files (this case)
> 
> So if "Loading raw files" feels like it belongs on that list, then we're fine.

> If it feels too fine-grained, I might suggest something like IMAGE_LOAD for
> bucketing all image-loading metrics.

Turned to use INTERNALS as @smckay suggested.

https://codereview.chromium.org/1076173002/diff/1/ui/file_manager/file_manage...
ui/file_manager/file_manager/common/js/metrics_events.js:150: RAW_LOAD_ARW: 'Row
Load ARW',
On 2015/04/10 15:57:31, Ben Kwa wrote:
> "Raw"?  Ditto lines below.

Done.

[Ben Kwa, 2015-04-14 14:47:28]

One nit, but otherwise, still LGTM.  Thanks!

https://codereview.chromium.org/1076173002/diff/1/ui/file_manager/file_manage...
File ui/file_manager/file_manager/common/js/metrics_events.js (right):

https://codereview.chromium.org/1076173002/diff/1/ui/file_manager/file_manage...
ui/file_manager/file_manager/common/js/metrics_events.js:23: RAW_IMAGE: 'Raw
Image'
On 2015/04/14 04:01:27, hirono wrote:
> On 2015/04/10 15:57:31, Ben Kwa wrote:
> > This category sounds maybe a little too specific?  (Question mark because
this
> > is really a judgement call and I don't have full context)
> > 
> > We're looking for a bucketing that corresponds to "I want to be able to see
> how
> > much time in my app was spent doing Foo", or "I want to see all the events
> that
> > occurred in my app that relate to Foo".  Examples of "Foo" might be:
> > - Importing media ("Acquisition")
> > - Browsing media (we don't have this yet, but we might in the future)
> > - Loading raw files (this case)
> > 
> > So if "Loading raw files" feels like it belongs on that list, then we're
fine.
> 
> > If it feels too fine-grained, I might suggest something like IMAGE_LOAD for
> > bucketing all image-loading metrics.
> 
> Turned to use INTERNALS as @smckay suggested.

Is RAW_IMAGE still needed?  If not, let's remove it.

[Steve McKay, 2015-04-14 18:30:11]

https://codereview.chromium.org/1076173002/diff/20001/ui/file_manager/file_ma...
File ui/file_manager/file_manager/common/js/metrics_events.js (right):

https://codereview.chromium.org/1076173002/diff/20001/ui/file_manager/file_ma...
ui/file_manager/file_manager/common/js/metrics_events.js:149:
EXTRACT_JPEG_FROM_RAW: 'Extract JPEG From RAW'
I thought we were going to include the source extension name in this...that
would allow us to know if there were some extensions that were slow relative to
others.

If you don't want to do that, then replace "JPEG" with "Thumbnail".

https://codereview.chromium.org/1076173002/diff/20001/ui/file_manager/image_l...
File ui/file_manager/image_loader/request.js (right):

https://codereview.chromium.org/1076173002/diff/20001/ui/file_manager/image_l...
ui/file_manager/image_loader/request.js:241: var tracker =
metrics.getTracker().startTiming(
tracker > timer, or timeTracker

[hirono, 2015-04-15 01:57:01]

The CQ bit was checked by hirono@chromium.org

[hirono, 2015-04-15 01:57:02]

The patchset sent to the CQ was uploaded after l-g-t-m from kenobi@chromium.org
Link to the patchset: https://codereview.chromium.org/1076173002/#ps40001
(title: "Fixed.")

[hirono, 2015-04-15 01:57:10]

Thanks!

https://codereview.chromium.org/1076173002/diff/1/ui/file_manager/file_manage...
File ui/file_manager/file_manager/common/js/metrics_events.js (right):

https://codereview.chromium.org/1076173002/diff/1/ui/file_manager/file_manage...
ui/file_manager/file_manager/common/js/metrics_events.js:23: RAW_IMAGE: 'Raw
Image'
On 2015/04/14 14:47:28, Ben Kwa wrote:
> On 2015/04/14 04:01:27, hirono wrote:
> > On 2015/04/10 15:57:31, Ben Kwa wrote:
> > > This category sounds maybe a little too specific?  (Question mark because
> this
> > > is really a judgement call and I don't have full context)
> > > 
> > > We're looking for a bucketing that corresponds to "I want to be able to
see
> > how
> > > much time in my app was spent doing Foo", or "I want to see all the events
> > that
> > > occurred in my app that relate to Foo".  Examples of "Foo" might be:
> > > - Importing media ("Acquisition")
> > > - Browsing media (we don't have this yet, but we might in the future)
> > > - Loading raw files (this case)
> > > 
> > > So if "Loading raw files" feels like it belongs on that list, then we're
> fine.
> > 
> > > If it feels too fine-grained, I might suggest something like IMAGE_LOAD
for
> > > bucketing all image-loading metrics.
> > 
> > Turned to use INTERNALS as @smckay suggested.
> 
> Is RAW_IMAGE still needed?  If not, let's remove it.

Done.

https://codereview.chromium.org/1076173002/diff/20001/ui/file_manager/file_ma...
File ui/file_manager/file_manager/common/js/metrics_events.js (right):

https://codereview.chromium.org/1076173002/diff/20001/ui/file_manager/file_ma...
ui/file_manager/file_manager/common/js/metrics_events.js:149:
EXTRACT_JPEG_FROM_RAW: 'Extract JPEG From RAW'
On 2015/04/14 18:30:11, Steve McKay wrote:
> I thought we were going to include the source extension name in this...that
> would allow us to know if there were some extensions that were slow relative
to
> others.
> 
> If you don't want to do that, then replace "JPEG" with "Thumbnail".

The extension is reported as label (the 4th arguments of startTiming).
JPEG -> Thumbnail is done.

https://codereview.chromium.org/1076173002/diff/20001/ui/file_manager/image_l...
File ui/file_manager/image_loader/request.js (right):

https://codereview.chromium.org/1076173002/diff/20001/ui/file_manager/image_l...
ui/file_manager/image_loader/request.js:241: var tracker =
metrics.getTracker().startTiming(
On 2015/04/14 18:30:11, Steve McKay wrote:
> tracker > timer, or timeTracker

Done.

[commit-bot: I haz the power, 2015-04-15 01:57:37]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1076173002/40001

[commit-bot: I haz the power, 2015-04-15 02:33:39]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2015-04-15 02:34:26]

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