Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add telemetry #278

Merged
merged 1 commit into from
Aug 26, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 31 additions & 21 deletions lib/clients/metric-client/index.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
const uuid = require('uuid/v4');
const axios = require('axios');
const AppConfig = require('@src/model/app-config');
const { METRICS } = require('@src/utils/constants');
const pck = require('../../../package.json');

const MetricActionResult = {
SUCCESS: 'Success',
Expand Down Expand Up @@ -57,37 +60,25 @@ class MetricAction {
}

class MetricClient {
/**
* A metric client options
* @typedef {Object} MetricClientOptions
* @property {string} version - The application version. Typically, version form package.json
* @property {string} machineId - The machine id
* @property {boolean} newUser - is new user
* @property {string} clientId - The client id. Typically, application name. For example, ask cli.
* @property {string} serverUrl - The server url where to send metrics data.
* @property {number} sendTimeout - The send timeout to send data to the metrics server.
*/

/**
* @constructor
* @param {MetricClientOptions} options - The options for constructor
*/
constructor(options) {
const { version, machineId, newUser, clientId, serverUrl, sendTimeout, enabled } = options;
constructor() {
this.httpClient = axios.create({
timeout: sendTimeout || 3000,
timeout: 3000,
headers: { 'Content-Type': 'text/plain' }
});
this.serverUrl = serverUrl;
this.serverUrl = METRICS.ENDPOINT;
RonWang marked this conversation as resolved.
Show resolved Hide resolved
this.postRetries = 3;
this.enabled = enabled !== false;

this.enabled = this._isEnabled();
this.data = {
version,
machineId,
version: pck.version,
machineId: this._getMachineId(),
timeStarted: new Date(),
newUser,
newUser: false, // default to false since unused.
timeUploaded: null,
clientId,
clientId: pck.name,
actions: []
};
}
Expand Down Expand Up @@ -157,6 +148,25 @@ class MetricClient {
_retry(retries, fn) {
return fn().catch(err => (retries > 1 ? this._retry(retries - 1, fn) : Promise.reject(err)));
}

_isEnabled() {
if (process.env.ASK_SHARE_USAGE === 'false') return false;
RonWang marked this conversation as resolved.
Show resolved Hide resolved
if (!AppConfig.configFileExists()) return false;
RonWang marked this conversation as resolved.
Show resolved Hide resolved

new AppConfig();
return AppConfig.getInstance().getShareUsage();
}

_getMachineId() {
if (!this.enabled) return;
const appConfig = AppConfig.getInstance();
if (!appConfig.getMachineId()) {
appConfig.setMachineId(uuid());
appConfig.write();
}

return appConfig.getMachineId();
}
}

module.exports = { MetricClient, MetricActionResult };
2 changes: 1 addition & 1 deletion lib/commands/abstract-command.js
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ class AbstractCommand {
Messenger.getInstance().doDebug = commandInstance.debug;

// Start metric client
metricClient.startAction(commandInstance._name, 'command');
metricClient.startAction(commandInstance._name, '');
const isCredentialHelperCmd = commandInstance._name === 'git-credentials-helper';

// Check if a new CLI version is released
Expand Down
21 changes: 13 additions & 8 deletions lib/commands/smapi/smapi-commander.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,10 @@ const { ModelIntrospector } = require('ask-smapi-sdk');
const { kebabCase } = require('@src/utils/string-utils');
const { CliCustomizationProcessor } = require('@src/commands/smapi/cli-customization-processor');
const optionModel = require('@src/commands/option-model.json');
const { smapiCommandHandler } = require('@src/commands/smapi/smapi-command-handler');
const handler = require('@src/commands/smapi/smapi-command-handler');
const aliases = require('@src/commands/smapi/customizations/aliases.json');
const { apiToCommanderMap } = require('@src/commands/smapi/customizations/parameters-map');
const metricClient = require('@src/utils/metrics');

const uploadCatalog = require('./appended-commands/upload-catalog');
const exportPackage = require('./appended-commands/export-package');
Expand Down Expand Up @@ -99,13 +100,17 @@ const makeSmapiCommander = () => {
defaultOptions.forEach(option => {
commanderInstance.option(option.flags, option.description);
});
commanderInstance.action((inputCmdObj) => smapiCommandHandler(
apiOperationName,
flatParamsMap,
commanderToApiCustomizationMap,
inputCmdObj,
modelIntrospector
));
commanderInstance.action((inputCmdObj) => {
metricClient.startAction('smapi', inputCmdObj.name());
return handler.smapiCommandHandler(
apiOperationName,
flatParamsMap,
commanderToApiCustomizationMap,
inputCmdObj,
modelIntrospector
).then(res => metricClient.sendData().then(() => res))
.catch(err => metricClient.sendData(err).then(() => Promise.reject(err)));
});
});

// register hand-written appended commands
Expand Down
20 changes: 20 additions & 0 deletions lib/model/app-config.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
const fs = require('fs-extra');
const os = require('os');
const path = require('path');

Expand Down Expand Up @@ -36,6 +37,10 @@ module.exports = class AppConfig extends ConfigFile {
instance = null;
}

static configFileExists() {
return fs.existsSync(defaultFilePath);
}

// getter and setter

getAwsProfile(profile) {
Expand Down Expand Up @@ -68,6 +73,21 @@ module.exports = class AppConfig extends ConfigFile {
this.setProperty(['profiles', profile, 'vendor_id'], vendorId);
}

setMachineId(machineId) {
this.setProperty(['machine_id'], machineId);
}

getMachineId() {
return this.getProperty(['machine_id']);
}

getShareUsage() {
const shareUsage = this.getProperty(['share_usage']);
if (shareUsage !== undefined) return shareUsage;

return true;
}

/**
* Returns all profile names and their associated aws profile names (if any) as list of objects.
* return profilesList. Eg: [{ askProfile: 'askProfile1', awsProfile: 'awsProfile1'}, { askProfile: 'askProfile2', awsProfile: 'awsProfile2'}].
Expand Down
44 changes: 0 additions & 44 deletions lib/model/metric-config.js

This file was deleted.

3 changes: 1 addition & 2 deletions lib/utils/constants.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,7 @@ module.exports.APPLICATION_NAME = 'ask-cli';
module.exports.NPM_REGISTRY_URL_BASE = 'http://registry.npmjs.org';

module.exports.METRICS = {
ENDPOINT: '', // TODO add the official endpoint when we have it
NEW_USER_LENGTH_DAYS: 3
ENDPOINT: 'https://client-telemetry.amazonalexa.com'
};

module.exports.DEPLOYER_TYPE = {
Expand Down
16 changes: 2 additions & 14 deletions lib/utils/metrics.js
Original file line number Diff line number Diff line change
@@ -1,18 +1,6 @@
const { MetricClient } = require('@src/clients/metric-client');
// const MetricConfig = require('@src/model/metric-config');
const { METRICS } = require('@src/utils/constants');
const { name, version } = require('./../../package.json');

// TODO enable when we have configure command prompting for telemetry
// const metricConfig = new MetricConfig();

const metricClient = new MetricClient({
version,
machineId: '', // metricConfig.machineId,
newUser: false, // metricConfig.isNewUser(),
clientId: name,
serverUrl: METRICS.ENDPOINT,
enabled: false // TODO make it dependent on configure command
});
// metric client singleton
const metricClient = new MetricClient();

module.exports = metricClient;
3 changes: 2 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,8 @@
"functional-test": "mocha -t 180000 test/functional/run-test.js",
"lint": "eslint lib/builtins lib/clients lib/commands lib/controllers lib/model lib/view",
"pre-release": "standard-version",
"prism": "prism"
"prism": "prism",
"postinstall": "node postinstall.js"
},
"dependencies": {
"adm-zip": "^0.4.13",
Expand Down
9 changes: 9 additions & 0 deletions postinstall.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
console.log(`
================================================================================
ASK CLI collects telemetry to better understand customer needs. You can
OPT OUT and disable telemetry by setting the 'share_usage' key to 'false'
in '~/.ask/cli_config'.

Learn more: https://developer.amazon.com/docs/alexa/smapi/ask-cli-telemetry.html
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do you have this link ready?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I dont.

Copy link
Contributor

@RonWang RonWang Aug 25, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Then we have to make sure this link and the feat go public together, could you talk to the docs team?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have created a ticket. not sure how long will it take.

================================================================================
`);
2 changes: 2 additions & 0 deletions test/functional/run-test.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
require('module-alias/register');

process.env.ASK_SHARE_USAGE = false;
RonWang marked this conversation as resolved.
Show resolved Hide resolved

[
'@test/functional/commands/high-level-commands-test.js'
].forEach((testFile) => {
Expand Down
2 changes: 2 additions & 0 deletions test/integration/run-test.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
require('module-alias/register');

process.env.ASK_SHARE_USAGE = false;
RonWang marked this conversation as resolved.
Show resolved Hide resolved

[
'@test/integration/commands/smapi-commands-test.js'
].forEach((testFile) => {
Expand Down
Loading