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(cli): [STENCIL-12] Adding Telemetry and CLI features. #2964

Merged
merged 24 commits into from
Aug 4, 2021
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
f1deae7
feat(cli): [STENCIL-12] Adding Telemetry and CLI for users to toggle …
splitinfinities Jul 26, 2021
628c0c7
Fixes from Ryan
splitinfinities Jul 26, 2021
d52a7d1
Fixes from Ryan + better interop by using fetch instead of request.
splitinfinities Jul 27, 2021
f524372
Update src/cli/task-telemetry.ts
splitinfinities Jul 27, 2021
d889830
Add missed new line in telemetry
splitinfinities Jul 27, 2021
c635329
Update src/cli/telemetry/shouldTrack.ts
splitinfinities Jul 27, 2021
739e62c
Update src/cli/telemetry/telemetry.ts
splitinfinities Jul 27, 2021
cdf1879
Update src/cli/telemetry/telemetry.ts
splitinfinities Jul 27, 2021
dbbb0cb
Feedback from Ryan
splitinfinities Jul 27, 2021
aa5bb85
Feedback from Lars + Ryan
splitinfinities Jul 28, 2021
5ca6854
Adding guard clause on fetch in node-sys
splitinfinities Jul 28, 2021
3d466ed
Update src/cli/telemetry/telemetry.ts
splitinfinities Jul 28, 2021
871ac40
Update src/cli/telemetry/telemetry.ts
splitinfinities Jul 28, 2021
60865ea
Fix ts issues and revert the global pointer for fetch because it may …
splitinfinities Jul 28, 2021
bce8319
Update src/cli/telemetry/telemetry.ts
splitinfinities Jul 28, 2021
6448a29
Feedback from Lars
splitinfinities Jul 28, 2021
250f28c
Add session to the Metric as opposed to the data sent
splitinfinities Jul 28, 2021
66c8019
Update src/cli/ionic-config.ts to match Ionic's CLI
splitinfinities Jul 30, 2021
c251962
Feedback from Lars, adding the pwa prop instead of overloading the ta…
splitinfinities Jul 30, 2021
45f5a11
Add validation on reading the telemetry token
splitinfinities Jul 30, 2021
2e01743
Write tests to a temporary file.
splitinfinities Aug 2, 2021
b8986c7
Improve testability of Telemetry code
splitinfinities Aug 2, 2021
f9644b8
Fix from Ryan
splitinfinities Aug 3, 2021
063cc1c
Fixes from Ryan
splitinfinities Aug 3, 2021
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
4 changes: 4 additions & 0 deletions src/cli/state/stencil-cli-config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,10 @@ export default class StencilCLIConfig {
return StencilCLIConfig.instance;
}

public resetInstance() {
delete StencilCLIConfig.instance;
}

public get logger() {
return this._logger;
}
Expand Down
92 changes: 42 additions & 50 deletions src/cli/telemetry/telemetry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,37 +47,16 @@ export interface TrackableData {
* @param result The results of a compiler build.
*/
export async function telemetryBuildFinishedAction(result: CompilerBuildResults) {
const { sys, flags, logger } = getStencilCLIConfig();
const { flags, logger } = getStencilCLIConfig();
const tracking = await shouldTrack(flags.ci);

if (!tracking) {
return;
}

const details = sys.details;
const packages = await getInstalledPackages();
const targets = await getActiveTargets();
const versions = getCoreCompiler()?.versions || { typescript: 'unknown', rollup: 'unknown' };
const component_count = Object.keys(result.componentGraph).length;

const data: TrackableData = {
yarn: isUsingYarn(),
duration_ms: result.duration,
component_count,
targets,
packages: packages || [],
arguments: flags.args,
task: flags.task,
stencil: getCoreCompiler()?.version || 'unknown',
system: `${sys.name} ${sys.version}`,
os_name: details.platform,
os_version: details.release,
cpu_model: `${details.cpuModel}`,
build: getCoreCompiler()?.buildId || 'unknown',
typescript: versions.typescript,
rollup: versions.rollup,
has_app_pwa_config: hasAppTarget(),
};
const data = await prepareData(result.duration, component_count);

await sendMetric('stencil_cli_command', data);
logger.debug(`${logger.blue('Telemetry')}: ${logger.gray(JSON.stringify(data))}`);
Expand All @@ -89,11 +68,9 @@ export async function telemetryBuildFinishedAction(result: CompilerBuildResults)
* @returns void
*/
export async function telemetryAction(action?: TelemetryCallback) {
const { sys, flags, logger } = getStencilCLIConfig();
const { flags, logger } = getStencilCLIConfig();
const tracking = await shouldTrack(!!flags.ci);

const details = sys.details;
const versions = getCoreCompiler()?.versions || { typescript: 'unknown', rollup: 'unknown' };
let duration = undefined;
let error: any;

Expand All @@ -115,26 +92,7 @@ export async function telemetryAction(action?: TelemetryCallback) {
return;
}

const packages = await getInstalledPackages();
const targets = await getActiveTargets();

const data: TrackableData = {
yarn: isUsingYarn(),
duration_ms: duration,
targets,
packages: packages || [],
arguments: flags.args,
task: flags.task,
stencil: getCoreCompiler()?.version || 'unknown',
system: `${sys.name} ${sys.version}`,
os_name: details.platform,
os_version: details.release,
cpu_model: `${details.cpuModel}`,
build: getCoreCompiler()?.buildId || 'unknown',
typescript: versions.typescript,
rollup: versions.rollup,
has_app_pwa_config: hasAppTarget(),
};
const data = await prepareData(duration);

await sendMetric('stencil_cli_command', data);
logger.debug(`${logger.blue('Telemetry')}: ${logger.gray(JSON.stringify(data))}`);
Expand All @@ -144,21 +102,55 @@ export async function telemetryAction(action?: TelemetryCallback) {
}
}

function hasAppTarget(): boolean {
export function hasAppTarget(): boolean {
return getStencilCLIConfig().validatedConfig.config.outputTargets.some(
target => target.type === 'www' && (!!target.serviceWorker || target.baseUrl !== '/'),
target => target.type === 'www' && (!!target.serviceWorker || (!!target.baseUrl && target.baseUrl !== '/')),
);
}

function isUsingYarn() {
export function isUsingYarn() {
return getCompilerSystem().getEnvironmentVar('npm_execpath')?.includes('yarn') || false;
}

async function getActiveTargets(): Promise<string[]> {
export async function getActiveTargets(): Promise<string[]> {
const result = getStencilCLIConfig().validatedConfig.config.outputTargets.map(t => t.type);
return Array.from(new Set(result));
}

export const prepareData = async (duration_ms: number, component_count: number = undefined): Promise<TrackableData> => {
const { flags, sys } = getStencilCLIConfig();
const { typescript, rollup } = getCoreCompiler()?.versions || { typescript: 'unknown', rollup: 'unknown' };
const packages = await getInstalledPackages() || [];
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this always returns an array, and don't think we need the || [] here

const targets = await getActiveTargets();
const yarn = isUsingYarn()
const stencil = getCoreCompiler()?.version || 'unknown';
const system = `${sys.name} ${sys.version}`;
const os_name = sys.details.platform;
const os_version = sys.details.release;
const cpu_model = sys.details.cpuModel;
const build = getCoreCompiler()?.buildId || 'unknown';
const has_app_pwa_config = hasAppTarget();

return {
yarn,
duration_ms,
component_count,
targets,
packages,
arguments: flags.args,
task: flags.task,
stencil,
system,
os_name,
os_version,
cpu_model,
build,
typescript,
rollup,
has_app_pwa_config,
};
}

/**
* Reads package-lock.json and package.json files in order to cross references the dependencies and devDependencies properties. Pull the current installed version of each package under the @stencil, @ionic, and @capacitor scopes.
* @returns string[]
Expand Down
131 changes: 130 additions & 1 deletion src/cli/telemetry/test/telemetry.spec.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { initializeStencilCLIConfig } from '../../state/stencil-cli-config';
import { getStencilCLIConfig, initializeStencilCLIConfig } from '../../state/stencil-cli-config';
import * as telemetry from '../telemetry';
import * as shouldTrack from '../shouldTrack';
import { createSystem } from '../../../compiler/sys/stencil-sys';
Expand Down Expand Up @@ -76,3 +76,132 @@ describe('checkTelemetry', () => {
expect(await telemetry.checkTelemetry()).toBe(true);
});
});

describe('hasAppTarget', () => {

beforeEach(() => {
getStencilCLIConfig()?.resetInstance();

initializeStencilCLIConfig({
sys: createSystem(),
logger: mockLogger(),
flags: { ci: false },
validatedConfig: { config: { outputTargets: [] } } as LoadConfigResults,
args: [],
});
});


it('Result is correct when outputTargets are empty', () => {
getStencilCLIConfig().validatedConfig = { config: { outputTargets: [] } } as LoadConfigResults
expect(telemetry.hasAppTarget()).toBe(false)
});

it('Result is correct when outputTargets contains www with no baseUrl or serviceWorker', () => {
getStencilCLIConfig().validatedConfig = { config: { outputTargets: [{ type: "www" }] } } as LoadConfigResults;
expect(telemetry.hasAppTarget()).toBe(false)
});

it('Result is correct when outputTargets contains www with default baseUrl value', () => {
getStencilCLIConfig().validatedConfig = { config: { outputTargets: [{ type: "www", baseUrl: "/" }] } } as LoadConfigResults;
expect(telemetry.hasAppTarget()).toBe(false)
});

it('Result is correct when outputTargets contains www with serviceWorker', () => {
getStencilCLIConfig().validatedConfig = { config: { outputTargets: [{ type: "www", serviceWorker: { swDest: "./tmp" }, }] } } as LoadConfigResults
expect(telemetry.hasAppTarget()).toBe(true)
});

it('Result is correct when outputTargets contains www with serviceWorker', () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this test name needs to be updated, there's no service worker field set in this test

getStencilCLIConfig().validatedConfig = { config: { outputTargets: [{ type: "www", baseUrl: "https://example.com" }] } } as LoadConfigResults
expect(telemetry.hasAppTarget()).toBe(true)
});

it('Result is correct when outputTargets contains www with serviceWorker', () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this test name needs to be updated, it mimics the two previous test names

getStencilCLIConfig().validatedConfig = { config: { outputTargets: [{ type: "www", baseUrl: "https://example.com", serviceWorker: { swDest: "./tmp" } }] } } as LoadConfigResults
expect(telemetry.hasAppTarget()).toBe(true)
});
});

describe('hasAppTarget', () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

This outer level describe matches the one on line 80. Did you mean 'prepareData'?

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 did!


beforeEach(() => {
getStencilCLIConfig()?.resetInstance();

initializeStencilCLIConfig({
sys: createSystem(),
logger: mockLogger(),
flags: { ci: false },
validatedConfig: { config: { outputTargets: [] } } as LoadConfigResults,
args: [],
});
});

it('provides an object', async () => {
const data = await telemetry.prepareData(1000);
expect(data).toEqual({
"arguments": undefined,
"build": "unknown",
"component_count": undefined,
"cpu_model": "",
"duration_ms": 1000,
"has_app_pwa_config": false,
"os_name": "",
"os_version": "",
"packages": [],
"rollup": "unknown",
"stencil": "unknown",
"system": "in-memory __VERSION:STENCIL__",
"targets": [],
"task": undefined,
"typescript": "unknown",
"yarn": false
})
})

it('updates when there is a PWA config', async () => {
getStencilCLIConfig().validatedConfig = { config: { outputTargets: [{ type: "www", baseUrl: "https://example.com", serviceWorker: {swDest: "./tmp" } }] } } as LoadConfigResults
const data = await telemetry.prepareData(1000);
expect(data).toEqual({
"arguments": undefined,
"build": "unknown",
"component_count": undefined,
"cpu_model": "",
"duration_ms": 1000,
"has_app_pwa_config": true,
"os_name": "",
"os_version": "",
"packages": [],
"rollup": "unknown",
"stencil": "unknown",
"system": "in-memory __VERSION:STENCIL__",
"targets": ["www"],
"task": undefined,
"typescript": "unknown",
"yarn": false
})
})

it('updates when there is a component count passed in', async () => {
getStencilCLIConfig().validatedConfig = { config: { outputTargets: [{ type: "www", baseUrl: "https://example.com", serviceWorker: {swDest: "./tmp" } }] } } as LoadConfigResults
const data = await telemetry.prepareData(1000, 12);
expect(data).toEqual({
"arguments": undefined,
"build": "unknown",
"component_count": 12,
"cpu_model": "",
"duration_ms": 1000,
"has_app_pwa_config": true,
"os_name": "",
"os_version": "",
"packages": [],
"rollup": "unknown",
"stencil": "unknown",
"system": "in-memory __VERSION:STENCIL__",
"targets": ["www"],
"task": undefined,
"typescript": "unknown",
"yarn": false
})
})
})