Files.app: Add analytics code to instrument cloud import flows.

ui/file_manager/file_manager/common/js/metrics_events.js

+// Copyright 2015 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+// namespace
+var metrics = metrics || metricsBase;
+
+/** @enum {string} */
+metrics.Categories = {

[Steve McKay, 2015/02/17 23:47:51]

No need for string constants. Just inline the value where used, then build off
of that.

[Ben Kwa, 2015/02/17 23:54:24]

I'd agree if this value was only used in the metrics namespace, but it's used in
client code (e.g.
this.tracker_.sendTiming(metrics.Categories.IMPORT, ...) as well, so I think the
enum works better.  WDYT?

+  IMPORT: 'Import'
+};
+
+/**
+ * The values of these enums come from the analytics console.
+ * @private @enum {number}
+ */
+metrics.Dimension_ = {
+  USER_TYPE: 1,
+  SESSION_TYPE: 2,
+  DEDUPE_TYPE: 3,

[Steve McKay, 2015/02/17 23:47:50]

Remove DEDUPE_TYPE and IMPORT_RESULT

[Ben Kwa, 2015/02/18 22:23:02]

Done.

+  IMPORT_RESULT: 4
+};
+
+/**
+ * @enum {!analytics.EventBuilder.Dimension}
+ */
+metrics.Dimensions = {
+  USER_TYPE_NON_IMPORT: {
+    index: metrics.Dimension_.USER_TYPE,
+    value: 'Non-import'
+  },
+  USER_TYPE_IMPORT: {
+    index: metrics.Dimension_.USER_TYPE,
+    value: 'Import'
+  },
+  SESSION_TYPE_NON_IMPORT: {
+    index: metrics.Dimension_.SESSION_TYPE,
+    value: 'Non-import'
+  },
+  SESSION_TYPE_IMPORT: {
+    index: metrics.Dimension_.SESSION_TYPE,
+    value: 'Import'
+  }
+};
+
+// namespace
+metrics.event = metrics.event || {};
+
+/**
+ * Base event builder for all import-related events.
+ * @private {analytics.EventBuilder}
+ */
+metrics.event.Import_ = analytics.EventBuilder.builder()

[Steve McKay, 2015/02/17 23:47:50]

Make this an enum:


@private @enum {analytics.EventBuilder}
metrics.event.Shared_ = {
  IMPORT: metrics.event.Import_
};

The, as needed you can hoist shared events up here.

[Ben Kwa, 2015/02/18 22:23:02]

Done.

+    .category(metrics.Categories.IMPORT);

[Steve McKay, 2015/02/17 23:47:50]

IMPORT > Acquisition, because we may want to track manually imported stuff
differently.

[Ben Kwa, 2015/02/18 22:23:02]

Done.

+
+/**
+ * @enum {!analytics.EventBuilder}
+ */
+metrics.Events = {

[Steve McKay, 2015/02/17 23:47:50]

metrics.ImportEvents, then you can remove the IMPORT_ qualification on each
entry.

[Ben Kwa, 2015/02/18 22:23:02]

Done.

+  IMPORT_STARTED: metrics.event.Import_
+      .action('ImportStart')

[Steve McKay, 2015/02/17 23:47:50]

Prefer regular phrases in your strings...

"Import Started"
"Import Ended"

[Ben Kwa, 2015/02/18 22:23:02]

Done.

+      .dimension(metrics.Dimensions.SESSION_TYPE_IMPORT)
+      .dimension(metrics.Dimensions.USER_TYPE_IMPORT),
+
+  IMPORT_ENDED: metrics.event.Import_
+      .action('ImportSuccess'),

[Steve McKay, 2015/02/17 23:47:50]

Import Completed

you get the idea

[Ben Kwa, 2015/02/18 22:23:01]

Done.

+
+  IMPORT_CANCELLED: metrics.event.Import_
+      .action('importCancelled'),
+
+  IMPORT_ERROR: metrics.event.Import_
+      .action('importError'),
+
+  IMPORT_FILE_COUNT: metrics.event.Import_
+      .action('importFileCount'),

[Steve McKay, 2015/02/17 23:47:50]

Files Imported

[Ben Kwa, 2015/02/18 22:23:01]

Done.

+
+  IMPORT_BYTE_COUNT: metrics.event.Import_
+      .action('importByteCount'),

[Steve McKay, 2015/02/17 23:47:50]

Total Bytes Imported

[Ben Kwa, 2015/02/18 22:23:01]

Done.

+
+  DEVICE_YANKED: metrics.event.Import_
+      .action('deviceYanked'),
+
+  HISTORY_DEDUPE_COUNT: metrics.event.Import_
+      .action('historyDedupeCount'),

[Steve McKay, 2015/02/17 23:47:50]

Num. Files History Deduped
Num. Files Content Deduped

[Ben Kwa, 2015/02/18 22:23:01]

Done.

+
+  CONTENT_DEDUPE_COUNT: metrics.event.Import_
+      .action('contentDedupeCount')
+};
+
+// namespace
+metrics.timing = metrics.timing || {};
+
+/** @enum {string} */
+metrics.timing.Variables = {
+  COMPUTE_HASH: 'ComputeHash',

[Steve McKay, 2015/02/17 23:47:50]

Any reason we need to qualify this? We're computing it in-place, maybe we should
include that. "Compute Content Hash (in-place)". Or maybe you can apply a label
for "in-place". I don't recall if timings support labels.

[Ben Kwa, 2015/02/18 22:23:01]

I'll use a label.

+  SEARCH_BY_HASH: 'SearchByHash'
+};