Integrate metrics

lib/containers/remote-container.js

@@ -2,6 +2,7 @@ import React from 'react';
 import PropTypes from 'prop-types';
 import {QueryRenderer, graphql} from 'react-relay';
 
+import {incrementCounter} from '../reporter-proxy';
 import {autobind} from '../helpers';
 import {RemotePropType, BranchSetPropType, OperationStateObserverPropType} from '../prop-types';
 import RelayNetworkLayerManager from '../relay-network-layer-manager';
@@ -132,10 +133,12 @@ export default class RemoteContainer extends React.Component {
   }
 
   handleLogin(token) {
+    incrementCounter('github-login');
     this.props.loginModel.setToken(this.props.host, token);
   }
 
   handleLogout() {
+    incrementCounter('github-logout');
     this.props.loginModel.removeToken(this.props.host);
   }
 }

lib/controllers/remote-controller.js

@@ -3,6 +3,7 @@ import PropTypes from 'prop-types';
 import {shell} from 'electron';
 
 import {autobind} from '../helpers';
+import {incrementCounter} from '../reporter-proxy';
 import {RemotePropType, BranchSetPropType, OperationStateObserverPropType} from '../prop-types';
 import IssueishSearchesController from './issueish-searches-controller';
 
@@ -69,10 +70,13 @@ export default class RemoteController extends React.Component {
     createPrUrl += '/compare/' + encodeURIComponent(currentBranch.getName());
     createPrUrl += '?expand=1';
 
-    return new Promise((resolve, reject) => {
-      shell.openExternal(createPrUrl, {}, err => {
-        if (err) { reject(err); } else { resolve(); }
-      });
-    });
+    let returnPromise;
+    try {
+      returnPromise = await shell.openExternal(createPrUrl);
+      incrementCounter('create-pull-request');
+    } catch (err) {
+      returnPromise = Promise.reject(err);
+    }
+    return returnPromise;
   }
 }

lib/controllers/root-controller.js

@@ -26,6 +26,7 @@ import Conflict from '../models/conflicts/conflict';
 import Switchboard from '../switchboard';
 import {destroyFilePatchPaneItems, destroyEmptyFilePatchPaneItems, autobind} from '../helpers';
 import {GitError} from '../git-shell-out-strategy';
+import {incrementCounter} from '../reporter-proxy';
 
 export default class RootController extends React.Component {
   static propTypes = {
@@ -788,10 +789,12 @@ class TabTracker {
   }
 
   reveal() {
+    incrementCounter(`${this.name}-tab-open`);
     return this.getWorkspace().open(this.uri, {searchAllPanes: true, activateItem: true, activatePane: true});
   }
 
   hide() {
+    incrementCounter(`${this.name}-tab-close`);
     return this.getWorkspace().hide(this.uri);
   }
 

lib/git-shell-out-strategy.js

@@ -13,6 +13,7 @@ import {parse as parseStatus} from 'what-the-status';
 import GitPromptServer from './git-prompt-server';
 import GitTempDir from './git-temp-dir';
 import AsyncQueue from './async-queue';
+import {incrementCounter} from './reporter-proxy';
 import {
   getDugitePath, getSharedModulePath, getAtomHelperPath,
   extractCoAuthorsAndRawCommitMessage, fileExists, isFileExecutable, isFileSymlink, isBinary,
@@ -42,6 +43,9 @@ export class LargeRepoError extends Error {
   }
 }
 
+// ignored for the purposes of usage metrics tracking because they're noisy
+const IGNORED_GIT_COMMANDS = ['cat-file', 'config', 'diff', 'for-each-ref', 'log', 'rev-parse', 'status'];
+
 const DISABLE_COLOR_FLAGS = [
   'branch', 'diff', 'showBranch', 'status', 'ui',
 ].reduce((acc, type) => {
@@ -89,6 +93,7 @@ export default class GitShellOutStrategy {
   async exec(args, options = GitShellOutStrategy.defaultExecArgs) {
     /* eslint-disable no-console,no-control-regex */
     const {stdin, useGitPromptServer, useGpgWrapper, useGpgAtomPrompt, writeOperation} = options;
+    const commandName = args[0];
     const subscriptions = new CompositeDisposable();
     const diagnosticsEnabled = process.env.ATOM_GITHUB_GIT_DIAGNOSTICS || atom.config.get('github.gitDiagnostics');
 
@@ -311,6 +316,10 @@ export default class GitShellOutStrategy {
           err.command = formattedArgs;
           reject(err);
         }
+
+        if (!IGNORED_GIT_COMMANDS.includes(commandName)) {
+          incrementCounter(commandName);
+        }
         resolve(stdout);
       });
     }, {parallel: !writeOperation});
@@ -473,7 +482,7 @@ export default class GitShellOutStrategy {
 
     if (amend) { args.push('--amend'); }
     if (allowEmpty) { args.push('--allow-empty'); }
-    return this.gpgExec(args, {writeOperation: true});
+    return await this.gpgExec(args, {writeOperation: true});
   }
 
   addCoAuthorsToMessage(message, coAuthors = []) {

lib/github-package.js

@@ -21,6 +21,7 @@ import ContextMenuInterceptor from './context-menu-interceptor';
 import AsyncQueue from './async-queue';
 import WorkerManager from './worker-manager';
 import getRepoPipelineManager from './get-repo-pipeline-manager';
+import {reporterProxy} from './reporter-proxy';
 
 const defaultState = {
   newProject: true,
@@ -310,6 +311,10 @@ export default class GithubPackage {
     this.rerender();
   }
 
+  consumeReporter(reporter) {
+    reporterProxy.setReporter(reporter);
+  }
+
   createGitTimingsView() {
     return StubItem.create('git-timings-view', {
       title: 'GitHub Package Timings View',

lib/reporter-proxy.js

@@ -0,0 +1,63 @@
+const pjson = require('../package.json');
+
+// this class allows us to call reporter methods
+// before the reporter is actually loaded, since we don't want to
+// assume that the metrics package will load before the GitHub package.
+
+// maybe this should be an object instead of a class.
+// IDK.
+class ReporterProxy {
+  constructor() {
+    this.reporter = null;
+    this.events = [];
+    this.timings = [];
+    this.counters = [];
+    this.gitHubPackageVersion = pjson.version;
+  }
+
+  // function that is called after the reporter is actually loaded, to
+  // set the reporter and send any data that have accumulated while it was loading.
+  setReporter(reporter) {
+    this.reporter = reporter;
+
+    this.events.forEach(customEvent => {
+      this.reporter.addCustomEvent(customEvent.eventType, customEvent.event);
+    });
+
+    this.timings.forEach(timing => {
+      this.reporter.addTiming(timing.eventType, timing.durationInMilliseconds, timing.metadata);
+    });
+
+    this.counters.forEach(counterName => {
+      this.reporter.incrementCounter(counterName);
+    });
+  }
+}
+
+export const reporterProxy = new ReporterProxy();
+
+export const incrementCounter = function(counterName) {
+  if (reporterProxy.reporter) {
+    reporterProxy.reporter.incrementCounter(counterName);
+  } else {
+    reporterProxy.counters.push(counterName);
+  }
+};
+
+export const addTiming = function(eventType, durationInMilliseconds, metadata = {}) {
+  metadata.gitHubPackageVersion = reporterProxy.gitHubPackageVersion;
+  if (reporterProxy.reporter) {
+    reporterProxy.reporter.addTiming(eventType, durationInMilliseconds, metadata);
+  } else {
+    reporterProxy.timings.push({eventType, durationInMilliseconds, metadata});
+  }
+};
+
+export const addEvent = function(eventType, event) {
+  event.gitHubPackageVersion = reporterProxy.gitHubPackageVersion;
+  if (reporterProxy.reporter) {
+    reporterProxy.reporter.addCustomEvent(eventType, event);
+  } else {
+    reporterProxy.events.push({eventType, event});
+  }
+};

package.json

@@ -96,6 +96,11 @@
       "versions": {
         "^1.0.0": "consumeStatusBar"
       }
+    },
+    "metrics-reporter": {
+      "versions": {
+        "^1.1.0": "consumeReporter"
+      }
     }
   },
   "configSchema": {

test/containers/remote-container.test.js

@@ -1,6 +1,7 @@
 import React from 'react';
 import {mount} from 'enzyme';
 
+import * as reporterProxy from '../../lib/reporter-proxy';
 import {createRepositoryResult} from '../fixtures/factories/repository-result';
 import Remote from '../../lib/models/remote';
 import Branch, {nullBranch} from '../../lib/models/branch';
@@ -131,6 +132,28 @@ describe('RemoteContainer', function() {
     assert.strictEqual(qev.prop('error'), e);
   });
 
+  it('increments a counter on login', function() {
+    const token = '1234';
+    sinon.stub(model, 'getScopes').resolves(GithubLoginModel.REQUIRED_SCOPES);
+    const incrementCounterStub = sinon.stub(reporterProxy, 'incrementCounter');
+
+    const wrapper = mount(buildApp());
+    wrapper.instance().handleLogin(token);
+    assert.isTrue(incrementCounterStub.calledOnceWith('github-login'));
+  });
+
+  it('increments a counter on logout', function() {
+    const token = '1234';
+    sinon.stub(model, 'getScopes').resolves(GithubLoginModel.REQUIRED_SCOPES);
+
+    const wrapper = mount(buildApp());
+    wrapper.instance().handleLogin(token);
+
+    const incrementCounterStub = sinon.stub(reporterProxy, 'incrementCounter');
+    wrapper.instance().handleLogout();
+    assert.isTrue(incrementCounterStub.calledOnceWith('github-logout'));
+  });
+
   it('renders the controller once results have arrived', async function() {
     const {resolve} = expectRepositoryQuery();
     expectEmptyIssueishQuery();

test/controllers/remote-controller.test.js

@@ -1,11 +1,13 @@
 import React from 'react';
 import {shallow} from 'enzyme';
+import {shell} from 'electron';
 
 import BranchSet from '../../lib/models/branch-set';
 import Branch, {nullBranch} from '../../lib/models/branch';
 import Remote from '../../lib/models/remote';
 import {nullOperationStateObserver} from '../../lib/models/operation-state-observer';
 import RemoteController from '../../lib/controllers/remote-controller';
+import * as reporterProxy from '../../lib/reporter-proxy';
 
 describe('RemoteController', function() {
   let atomEnv, remote, branchSet, currentBranch;
@@ -49,6 +51,29 @@ describe('RemoteController', function() {
     );
   }
 
+  it('increments a counter when onCreatePr is called', async function() {
+    const wrapper = shallow(createApp());
+    sinon.stub(shell, 'openExternal').resolves();
+    sinon.stub(reporterProxy, 'incrementCounter');
+
+    await wrapper.instance().onCreatePr();
+    assert.equal(reporterProxy.incrementCounter.callCount, 1);
+    assert.deepEqual(reporterProxy.incrementCounter.lastCall.args, ['create-pull-request']);
+  });
+
+  it('handles error when onCreatePr fails', async function() {
+    const wrapper = shallow(createApp());
+    sinon.stub(shell, 'openExternal').rejects(new Error('oh noes'));
+    sinon.stub(reporterProxy, 'incrementCounter');
+
+    try {
+      await wrapper.instance().onCreatePr();
+    } catch (err) {
+      assert.equal(err.message, 'oh noes');
+    }
+    assert.equal(reporterProxy.incrementCounter.callCount, 0);
+  });
+
   it('renders issueish searches', function() {
     const wrapper = shallow(createApp());
 

test/controllers/root-controller.test.js

@@ -11,6 +11,7 @@ import Repository from '../../lib/models/repository';
 import GitTabItem from '../../lib/items/git-tab-item';
 import GithubTabController from '../../lib/controllers/github-tab-controller';
 import ResolutionProgress from '../../lib/models/conflicts/resolution-progress';
+import * as reporterProxy from '../../lib/reporter-proxy';
 
 import RootController from '../../lib/controllers/root-controller';
 
@@ -120,6 +121,14 @@ describe('RootController', function() {
             {searchAllPanes: true, activateItem: true, activatePane: true},
           ]);
         });
+        it('increments counter with correct name', function() {
+          sinon.stub(workspace, 'open');
+          const incrementCounterStub = sinon.stub(reporterProxy, 'incrementCounter');
+
+          tabTracker.reveal();
+          assert.equal(incrementCounterStub.callCount, 1);
+          assert.deepEqual(incrementCounterStub.lastCall.args, [`${tabName}-tab-open`]);
+        });
       });
 
       describe('hide', function() {
@@ -132,6 +141,14 @@ describe('RootController', function() {
             `atom-github://dock-item/${tabName}`,
           ]);
         });
+        it('increments counter with correct name', function() {
+          sinon.stub(workspace, 'hide');
+          const incrementCounterStub = sinon.stub(reporterProxy, 'incrementCounter');
+
+          tabTracker.hide();
+          assert.equal(incrementCounterStub.callCount, 1);
+          assert.deepEqual(incrementCounterStub.lastCall.args, [`${tabName}-tab-close`]);
+        });
       });
 
       describe('toggle()', function() {

test/git-strategies.test.js

@@ -13,6 +13,7 @@ import WorkerManager from '../lib/worker-manager';
 
 import {cloneRepository, initRepository, assertDeepPropertyVals} from './helpers';
 import {normalizeGitHelperPath, getTempDir} from '../lib/helpers';
+import * as reporterProxy from '../lib/reporter-proxy';
 
 /**
  * KU Thoughts: The GitShellOutStrategy methods are tested in Repository tests for the most part
@@ -1298,7 +1299,24 @@ import {normalizeGitHelperPath, getTempDir} from '../lib/helpers';
         });
       });
     });
+    describe('exec', function() {
+      let workingDirPath, git, incrementCounterStub;
+      beforeEach(async function() {
+        workingDirPath = await cloneRepository('three-files');
+        git = createTestStrategy(workingDirPath);
+        incrementCounterStub = sinon.stub(reporterProxy, 'incrementCounter');
+      });
+      it('does not call incrementCounter when git command is on the ignore list', async function() {
+        await git.exec(['status']);
+        assert.equal(incrementCounterStub.callCount, 0);
+      });
+      it('does call incrementCounter when git command is NOT on the ignore list', async function() {
+        await git.exec(['commit', '--allow-empty', '-m', 'make an empty commit']);
 
+        assert.equal(incrementCounterStub.callCount, 1);
+        assert.deepEqual(incrementCounterStub.lastCall.args, ['commit']);
+      });
+    });
     describe('executeGitCommand', function() {
       it('shells out in process until WorkerManager instance is ready', async function() {
         if (process.env.ATOM_GITHUB_INLINE_GIT_EXEC) {