PLA-263 Update telemetry options, add JSDoc comments

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'

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)

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<Promise<Response>, [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,34 +48,30 @@ describe('collectTelemetry function', () => {
 
   test('does nothing if telemetryReportUrl not set', () => {
     const project = new TestProject()
-    setupTelemetry(project, {})
     collectTelemetry(project)
 
     expect(mockedNodeFetch).not.toBeCalled()
   })
 
   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)
   }
 }

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<string, unknown> => {
   const propsToDelete = ['project', 'file', 'github']
   const entries = Object.entries(workflow).filter(([key]) => !propsToDelete.includes(key))
   return Object.fromEntries(entries)

src/common/telemetry/collect-telemetry/get-escape-hatches.ts → src/common/telemetry/collect-telemetry/collect-escape-hatches.ts

@@ -2,7 +2,16 @@ import type {MaybePlural} from '../../MaybePlural'
 
 export type EscapeHatches = Record<string, MaybePlural<string>>
 
-export const getEscapeHatches = (source: string, methods: Array<string>): 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<string>): EscapeHatches => {
   const regex = new RegExp(`\\.(${methods.join('|')})\\((.*)\\)`, 'g')
   const handles = source.matchAll(regex)
 

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<string>
+
+  /**
+   * 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<string>
     new?: Array<object>
     updated?: Array<object>
   }
-  errors?: unknown
+
+  /**
+   * Any errors caught collecting other metrics - mostly related to external process calls,
+   * such as `execSync`, `readFileSync`, etc.
+   */
+  errors?: Array<unknown>
 }
 
 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,
         }))

src/common/telemetry/i-with-telemetry-report-url.ts

@@ -0,0 +1,6 @@
+export interface IWithTelemetryReportUrl {
+  /**
+   * URL used for telemetry.
+   */
+  readonly telemetryReportUrl?: string
+}

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'

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<T extends object> = {-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<IWithTelemetryReportUrl>,
   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)
 }

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
 }