diff --git a/src/common/index.ts b/src/common/index.ts index 0c3533ce..c509c8b8 100644 --- a/src/common/index.ts +++ b/src/common/index.ts @@ -20,5 +20,5 @@ export { setupNode, } from './github' export {WithCustomLintPaths, addLinters} from './lint' -export {IWithTelemetryReportUrl, TelemetryOptions, WithTelemetry, collectTelemetry, setupTelemetry} from './telemetry' +export {IWithTelemetryReportUrl, WithTelemetry, collectTelemetry, setupTelemetry} from './telemetry' export {addVsCode} from './vscode-settings' diff --git a/src/common/telemetry/__tests__/index.ts b/src/common/telemetry/__tests__/index.ts index ab8fbf35..41e04e54 100644 --- a/src/common/telemetry/__tests__/index.ts +++ b/src/common/telemetry/__tests__/index.ts @@ -15,7 +15,7 @@ describe('setupTelemetry function', () => { test('sets up telemetry if requested in options', () => { const project = new TestProject({}) - setupTelemetry(project, {telemetry: {reportUrl: 'http://localhost:3000/telemetry'}}) + setupTelemetry(project, {isTelemetryEnabled: true, telemetryUrl: 'http://localhost:3000/telemetry'}) expect(project.telemetryReportUrl).toBeDefined() const snapshot = synthSnapshot(project) diff --git a/src/common/telemetry/collect-telemetry/__tests__/index.ts b/src/common/telemetry/collect-telemetry/__tests__/index.ts index 5412456b..a7be0206 100644 --- a/src/common/telemetry/collect-telemetry/__tests__/index.ts +++ b/src/common/telemetry/collect-telemetry/__tests__/index.ts @@ -3,7 +3,7 @@ import {readFileSync} from 'fs' import fetch, {RequestInfo, RequestInit, Response} from 'node-fetch' import {NodeProject, NodeProjectOptions} from 'projen/lib/javascript' import {collectTelemetry} from '..' -import {IWithTelemetryReportUrl, TelemetryOptions, WithTelemetry, setupTelemetry} from '../..' +import {IWithTelemetryReportUrl, WithTelemetry, setupTelemetry} from '../..' import {telemetryEnableEnvVar} from '../collect-telemetry' jest.mock('child_process') @@ -17,7 +17,7 @@ jest.mock('fs', () => ({ describe('collectTelemetry function', () => { // eslint-disable-next-line @typescript-eslint/consistent-type-assertions -- TS is not aware of the Jest mock, thus casting is needed const mockedNodeFetch = fetch as unknown as jest.Mock, [url: RequestInfo, init: RequestInit]> - const telemetry: TelemetryOptions = {reportUrl: 'http://localhost:3000/telemetry'} + const telemetryOptions: WithTelemetry = {isTelemetryEnabled: true, telemetryUrl: 'http://localhost:3000/telemetry'} const mockStringify = jest.fn< string, @@ -48,7 +48,6 @@ describe('collectTelemetry function', () => { test('does nothing if telemetryReportUrl not set', () => { const project = new TestProject() - setupTelemetry(project, {}) collectTelemetry(project) expect(mockedNodeFetch).not.toBeCalled() @@ -56,26 +55,23 @@ describe('collectTelemetry function', () => { test('does nothing if IS_OTTOFELLER_TEMPLATES_TELEMETRY_COLLECTED env var not set', () => { delete process.env[telemetryEnableEnvVar] - const project = new TestProject({telemetry}) - setupTelemetry(project, {telemetry}) + const project = new TestProject(telemetryOptions) collectTelemetry(project) expect(mockedNodeFetch).not.toBeCalled() }) test('collects templates version', () => { - const version = '1.1.0' - const project = new TestProject({telemetry, devDeps: [`@ottofeller/templates@${version}`]}) - setupTelemetry(project, {telemetry}) + const templateVersion = '1.1.0' + const project = new TestProject({...telemetryOptions, devDeps: [`@ottofeller/templates@${templateVersion}`]}) collectTelemetry(project) - expect(mockStringify).lastCalledWith(expect.objectContaining({version})) - expect(mockedNodeFetch).toBeCalledWith(telemetry.reportUrl, fetchOptions) + expect(mockStringify).lastCalledWith(expect.objectContaining({templateVersion})) + expect(mockedNodeFetch).toBeCalledWith(telemetryOptions.telemetryUrl, fetchOptions) }) test('collects git URLs', () => { - const project = new TestProject({telemetry}) - setupTelemetry(project, {telemetry}) + const project = new TestProject(telemetryOptions) const gitUrlsRaw = [ 'origin https://github.com/ottofeller/sampleProject.git (fetch)', @@ -87,12 +83,11 @@ describe('collectTelemetry function', () => { const gitUrls = ['https://github.com/ottofeller/sampleProject.git'] expect(mockStringify).lastCalledWith(expect.objectContaining({gitUrls})) - expect(mockedNodeFetch).toBeCalledWith(telemetry.reportUrl, fetchOptions) + expect(mockedNodeFetch).toBeCalledWith(telemetryOptions.telemetryUrl, fetchOptions) }) test('collects escape hatches utilized in projenrc file', () => { - const project = new TestProject({telemetry}) - setupTelemetry(project, {telemetry}) + const project = new TestProject(telemetryOptions) const escapeHatches = { files: { @@ -131,15 +126,14 @@ describe('collectTelemetry function', () => { collectTelemetry(project) expect(mockStringify).lastCalledWith(expect.objectContaining({escapeHatches})) - expect(mockedNodeFetch).toBeCalledWith(telemetry.reportUrl, fetchOptions) + expect(mockedNodeFetch).toBeCalledWith(telemetryOptions.telemetryUrl, fetchOptions) }) test('collects GitHub workflow data', () => { - const project = new TestProject({telemetry}) + const project = new TestProject(telemetryOptions) const updatedWorkflowName = 'build' const updatedTrigger = {fork: {}} project.github!.tryFindWorkflow(updatedWorkflowName)!.on(updatedTrigger) - setupTelemetry(project, {telemetry}) collectTelemetry(project) const telemetryWorkflow = { @@ -199,21 +193,37 @@ describe('collectTelemetry function', () => { } expect(mockStringify).lastCalledWith(expect.objectContaining({workflows})) - expect(mockedNodeFetch).toBeCalledWith(telemetry.reportUrl, fetchOptions) + expect(mockedNodeFetch).toBeCalledWith(telemetryOptions.telemetryUrl, fetchOptions) }) - test('collects errors', () => { - const project = new TestProject({telemetry}) - setupTelemetry(project, {telemetry}) - collectTelemetry(project) + describe('collects errors', () => { + test('if failed to collect git URLs', () => { + const project = new TestProject(telemetryOptions) + mockedReadFileSync.mockReturnValue('') + collectTelemetry(project) - const errors: unknown[] = [ - new TypeError("Cannot read properties of undefined (reading 'split')"), // undefined returned from execSync mock in gitUrls - new TypeError("Cannot read properties of undefined (reading 'matchAll')"), // undefined returned from readFileSync mock in escape hatches - ] + const errors: unknown[] = [ + // undefined returned from execSync mock in gitUrls + new TypeError("Cannot read properties of undefined (reading 'split')"), + ] + + expect(mockStringify).lastCalledWith(expect.objectContaining({errors})) + expect(mockedNodeFetch).toBeCalledWith(telemetryOptions.telemetryUrl, fetchOptions) + }) - expect(mockStringify).lastCalledWith(expect.objectContaining({errors})) - expect(mockedNodeFetch).toBeCalledWith(telemetry.reportUrl, fetchOptions) + test('if failed to read projenrc file', () => { + const project = new TestProject(telemetryOptions) + mockedExecSync.mockReturnValue('') + collectTelemetry(project) + + const errors: unknown[] = [ + // undefined returned from readFileSync mock in escape hatches + new TypeError("Cannot read properties of undefined (reading 'matchAll')"), + ] + + expect(mockStringify).lastCalledWith(expect.objectContaining({errors})) + expect(mockedNodeFetch).toBeCalledWith(telemetryOptions.telemetryUrl, fetchOptions) + }) }) }) @@ -227,5 +237,7 @@ class TestProject extends NodeProject implements IWithTelemetryReportUrl { defaultReleaseBranch: 'main', licensed: false, // NOTE License component interferes with readFileSync mock }) + + setupTelemetry(this, options) } } diff --git a/src/common/telemetry/collect-telemetry/clone-workflow.ts b/src/common/telemetry/collect-telemetry/clone-workflow.ts index a612ce1f..345e936e 100644 --- a/src/common/telemetry/collect-telemetry/clone-workflow.ts +++ b/src/common/telemetry/collect-telemetry/clone-workflow.ts @@ -1,6 +1,10 @@ import type {GithubWorkflow} from 'projen/lib/github' -export const cloneWorkflow = (workflow: GithubWorkflow) => { +/** + * Get a GithubWorkflow component and create a copy with only essential properties: + * all internal projen staff is deleted in order to avoid circular references. + */ +export const cloneWorkflow = (workflow: GithubWorkflow): Record => { const propsToDelete = ['project', 'file', 'github'] const entries = Object.entries(workflow).filter(([key]) => !propsToDelete.includes(key)) return Object.fromEntries(entries) diff --git a/src/common/telemetry/collect-telemetry/get-escape-hatches.ts b/src/common/telemetry/collect-telemetry/collect-escape-hatches.ts similarity index 59% rename from src/common/telemetry/collect-telemetry/get-escape-hatches.ts rename to src/common/telemetry/collect-telemetry/collect-escape-hatches.ts index 48305851..fd5af204 100644 --- a/src/common/telemetry/collect-telemetry/get-escape-hatches.ts +++ b/src/common/telemetry/collect-telemetry/collect-escape-hatches.ts @@ -2,7 +2,16 @@ import type {MaybePlural} from '../../MaybePlural' export type EscapeHatches = Record> -export const getEscapeHatches = (source: string, methods: Array): EscapeHatches => { +/** + * Search for given symbols (provided as a list) in source code. + * For each found method call reports arguments. + * + * @param source Source code content + * @param methods Methods to report + * + * @returns A mapping object between found method calls and the call arguments. + */ +export const collectEscapeHatches = (source: string, methods: Array): EscapeHatches => { const regex = new RegExp(`\\.(${methods.join('|')})\\((.*)\\)`, 'g') const handles = source.matchAll(regex) diff --git a/src/common/telemetry/collect-telemetry/collect-telemetry.ts b/src/common/telemetry/collect-telemetry/collect-telemetry.ts index 2eb03ff2..28f5b9a8 100644 --- a/src/common/telemetry/collect-telemetry/collect-telemetry.ts +++ b/src/common/telemetry/collect-telemetry/collect-telemetry.ts @@ -4,25 +4,53 @@ import fetch from 'node-fetch' import * as path from 'path' import {ProjenrcFile} from 'projen' import type {NodeProject, NodeProjectOptions} from 'projen/lib/javascript' -import type {IWithTelemetryReportUrl} from '../with-telemetry' +import type {IWithTelemetryReportUrl} from '../i-with-telemetry-report-url' import {cloneWorkflow} from './clone-workflow' +import {EscapeHatches, collectEscapeHatches} from './collect-escape-hatches' import {diff} from './diff' -import {EscapeHatches, getEscapeHatches} from './get-escape-hatches' interface TelemetryPayload { - version?: string + /** + * Template package version used in the project + */ + templateVersion?: string + + /** + * Remote URLs set in git for the project repo + */ gitUrls?: Array + + /** + * A collection of escape hatches used to tune the project setup. + * Projen has some methods to alter the content of generated files. + * Even though these changes might be required by a project, + * projen mechanisms might lack versatility and disallow some kind of edits. + * We try to find such measures in order to improve the template functionality. + * + * @see http://projen.io/escape-hatches.html + */ escapeHatches?: { files?: EscapeHatches overrides?: EscapeHatches tasks?: EscapeHatches } + + /** + * Github workflows set up in the project. + * Reports the new workflows added as well as deletion + * or modification of the default one. + */ workflows?: { deleted?: Array new?: Array updated?: Array } - errors?: unknown + + /** + * Any errors caught collecting other metrics - mostly related to external process calls, + * such as `execSync`, `readFileSync`, etc. + */ + errors?: Array } export const telemetryEnableEnvVar = 'IS_OTTOFELLER_TEMPLATES_TELEMETRY_COLLECTED' as const @@ -38,10 +66,10 @@ export const collectTelemetry = async (project: NodeProject & IWithTelemetryRepo // ANCHOR template version try { - const version = project.package.tryResolveDependencyVersion('@ottofeller/templates') + const templateVersion = project.package.tryResolveDependencyVersion('@ottofeller/templates') - if (version) { - payload.version = version + if (templateVersion) { + payload.templateVersion = templateVersion } } catch (e) { errors.push(e) @@ -73,11 +101,11 @@ export const collectTelemetry = async (project: NodeProject & IWithTelemetryRepo const rcFilePath = path.resolve(project.outdir, rcFile) const projenrcContents = fs.readFileSync(rcFilePath, 'utf-8') const fileHandles = ['tryFindFile', 'tryFindObjectFile', 'tryRemoveFile', 'tryFindWorkflow'] - const files = getEscapeHatches(projenrcContents, fileHandles) + const files = collectEscapeHatches(projenrcContents, fileHandles) const overrideHandles = ['addOverride', 'addDeletionOverride', 'addToArray', 'patch'] - const overrides = getEscapeHatches(projenrcContents, overrideHandles) + const overrides = collectEscapeHatches(projenrcContents, overrideHandles) const taskHandles = ['tryFind', 'addTask', 'removeTask', 'addScripts', 'removeScript'] - const tasks = getEscapeHatches(projenrcContents, taskHandles) + const tasks = collectEscapeHatches(projenrcContents, taskHandles) if (files || overrides || tasks) { payload.escapeHatches = {files, overrides, tasks} @@ -117,6 +145,8 @@ export const collectTelemetry = async (project: NodeProject & IWithTelemetryRepo .github!.workflows.map((defaultWorkflow) => [defaultWorkflow, github.tryFindWorkflow(defaultWorkflow.name)]) .filter(([, updatedWorkflow]) => updatedWorkflow) .map(([defaultWorkflow, updatedWorkflow]) => ({ + // NOTE Here we create object clones in order to eliminate all circular refs + // and leave only the properties essential to GitHub workflows. ...diff(cloneWorkflow(defaultWorkflow!), cloneWorkflow(updatedWorkflow!)), name: defaultWorkflow!.name, })) diff --git a/src/common/telemetry/i-with-telemetry-report-url.ts b/src/common/telemetry/i-with-telemetry-report-url.ts new file mode 100644 index 00000000..b79efd28 --- /dev/null +++ b/src/common/telemetry/i-with-telemetry-report-url.ts @@ -0,0 +1,6 @@ +export interface IWithTelemetryReportUrl { + /** + * URL used for telemetry. + */ + readonly telemetryReportUrl?: string +} diff --git a/src/common/telemetry/index.ts b/src/common/telemetry/index.ts index eb4d93a7..586d9af7 100644 --- a/src/common/telemetry/index.ts +++ b/src/common/telemetry/index.ts @@ -1,3 +1,4 @@ export {collectTelemetry} from './collect-telemetry' +export {IWithTelemetryReportUrl} from './i-with-telemetry-report-url' export {setupTelemetry} from './setup-telemetry' -export {IWithTelemetryReportUrl, TelemetryOptions, WithTelemetry} from './with-telemetry' +export {WithTelemetry} from './with-telemetry' diff --git a/src/common/telemetry/setup-telemetry.ts b/src/common/telemetry/setup-telemetry.ts index 59aed812..9929eb11 100644 --- a/src/common/telemetry/setup-telemetry.ts +++ b/src/common/telemetry/setup-telemetry.ts @@ -1,19 +1,34 @@ import type {NodeProject} from 'projen/lib/javascript' +import type {IWithTelemetryReportUrl} from './i-with-telemetry-report-url' import {TelemetryWorkflow, TelemetryWorkflowOptions} from './telemetry-workflow' -import type {IWithTelemetryReportUrl, WithTelemetry} from './with-telemetry' +import type {WithTelemetry} from './with-telemetry' type Writeable = {-readonly [P in keyof T]: T[P]} +/** + * Check options for telemetry being requested + * and configure the project to have: + * - a URL available for telemetry code; + * - a special GitHub workflow that runs telemetry collection and sends the data to the predefined URL. + */ export const setupTelemetry = ( project: NodeProject & Writeable, options: WithTelemetry & TelemetryWorkflowOptions, ) => { - const reportUrl = options.telemetry?.reportUrl + const {isTelemetryEnabled = false, telemetryUrl} = options - if (!reportUrl) { + if (!isTelemetryEnabled && telemetryUrl) { + throw Error('Telemetry is disabled, thus "telemetryUrl" won\'t have any effect.') + } + + if (!isTelemetryEnabled) { return } - project.telemetryReportUrl = reportUrl + if (!telemetryUrl) { + throw Error('A valid URL is required to be set for telemetry.') + } + + project.telemetryReportUrl = telemetryUrl TelemetryWorkflow.addToProject(project, options) } diff --git a/src/common/telemetry/with-telemetry.ts b/src/common/telemetry/with-telemetry.ts index e4e524dd..028fed2b 100644 --- a/src/common/telemetry/with-telemetry.ts +++ b/src/common/telemetry/with-telemetry.ts @@ -1,11 +1,15 @@ -export interface TelemetryOptions { - readonly reportUrl: string -} - export interface WithTelemetry { - readonly telemetry?: TelemetryOptions -} + /** + * Enable template usage telemetry. + * Collects the data on the template package version, connected git remotes, + * applied escape hatches, configured GitHub workflows. + * + * @default false + */ + readonly isTelemetryEnabled?: boolean -export interface IWithTelemetryReportUrl { - readonly telemetryReportUrl?: string + /** + * Endpoint URL to send telemetry data to. + */ + readonly telemetryUrl?: string }