telemetry: ide_editCodeFile is noisy aws#5491

## Problem:
- Pseudo-documents like `foo.git`, `output://…`, and
`~/.vscode/argv.json` are included in `ide_editCodeFile` metrics even
though they were not actually opened by the user.
- `onDidOpenTextDocument` doesn't correctly represent user activity: it
fires for *all* documents opened with `onDidOpenTextDocument()`, which
includes all documents/buffers opened for programmatic purposes by
application or library code.

## Solution:
- Listen to `onDidChangeActiveTextEditor` instead of
`onDidOpenTextDocument`. This represents when a user actually opens a
file in the IDE.
- Ignore various "noise" documents.

Author: justinmk3

packages/core/src/shared/filetypes.ts

@@ -62,8 +62,17 @@ export function isAwsFiletype(doc: vscode.TextDocument): boolean | undefined {
 
 export function activate(): void {
     globals.context.subscriptions.push(
-        // TODO: onDidChangeTextDocument ?
-        vscode.workspace.onDidOpenTextDocument(async (doc: vscode.TextDocument) => {
+        vscode.window.onDidChangeActiveTextEditor(async (editor) => {
+            const doc = editor?.document
+            // Ignore output:// files.
+            // Ignore *.git files (from the builtin git extension).
+            // Ignore ~/.vscode/argv.json (vscode internal file).
+            const isNoise =
+                !doc || doc.uri.scheme === 'git' || doc.uri.scheme === 'output' || doc.fileName.endsWith('argv.json')
+            if (isNoise) {
+                return
+            }
+
             const basename = path.basename(doc.fileName)
             let fileExt: string | undefined = path.extname(doc.fileName).trim()
             fileExt = fileExt !== '' ? fileExt : undefined // Telemetry client will fail on empty string.
@@ -77,11 +86,10 @@ export function activate(): void {
                 const isSameMetricPending = await globals.telemetry.findPendingMetric('ide_editCodeFile', metric, [
                     'filenameExt',
                 ])
-                if (isSameMetricPending) {
-                    return // Avoid redundant/duplicate metrics.
+                if (!isSameMetricPending) {
+                    // Avoid redundant/duplicate metrics.
+                    telemetry.ide_editCodeFile.emit(metric)
                 }
-
-                telemetry.ide_editCodeFile.emit(metric)
             }
 
             const isAwsFileExt = isAwsFiletype(doc)

packages/core/src/shared/fs/watchedFiles.ts

@@ -9,7 +9,6 @@ import * as pathutils from '../utilities/pathUtils'
 import * as path from 'path'
 import { globDirPatterns, isUntitledScheme, normalizeVSCodeUri } from '../utilities/vsCodeUtils'
 import { Settings } from '../settings'
-import { once } from '../utilities/functionUtils'
 import { Timeout } from '../utilities/timeoutUtils'
 
 /**
@@ -60,7 +59,6 @@ export function getExcludePattern() {
     const excludePattern = `**/{${excludePatternsStr}}/`
     return excludePattern
 }
-const getExcludePatternOnce = once(getExcludePattern)
 
 /**
  * WatchedFiles lets us watch files on the filesystem. It is used
@@ -180,6 +178,9 @@ export abstract class WatchedFiles<T> implements vscode.Disposable {
 
     /**
      * Adds a regex pattern to ignore paths containing the pattern
+     *
+     * TODO: this does NOT prevent addWatchPatterns() from creating a massive, expensive, recursive
+     * filewatcher for these patterns 🤦, it merely skips processing at event-time.
      */
     public addExcludedPattern(pattern: RegExp) {
         if (this._isDisposed) {
@@ -309,7 +310,7 @@ export abstract class WatchedFiles<T> implements vscode.Disposable {
         let skips = 0
         const found: vscode.Uri[] = []
 
-        const exclude = getExcludePatternOnce()
+        const exclude = getExcludePattern()
         getLogger().info(`${this.name}: building with: ${this.outputPatterns()}`)
 
         for (let i = 0; i < this.globs.length && !cancel?.completed; i++) {

packages/core/src/test/shared/filetypes.test.ts

@@ -32,27 +32,23 @@ describe('ide_editCodeFile telemetry', function () {
     let jsonUri1: vscode.Uri | undefined
     let jsonUri2: vscode.Uri | undefined
     let tsUri: vscode.Uri | undefined
+    let noiseUri1!: vscode.Uri
+    let noiseUri2!: vscode.Uri
 
     beforeEach(async function () {
         await testUtil.closeAllEditors()
-
-        const jsonFile1 = workspaceUtils.tryGetAbsolutePath(
-            vscode.workspace.workspaceFolders?.[0],
-            'ts-plain-sam-app/tsconfig.json'
-        )
-        jsonUri1 = vscode.Uri.file(jsonFile1)
-
-        const jsonFile2 = workspaceUtils.tryGetAbsolutePath(
-            vscode.workspace.workspaceFolders?.[0],
-            'ts-plain-sam-app/package.json'
-        )
-        jsonUri2 = vscode.Uri.file(jsonFile2)
-
-        const tsFile = workspaceUtils.tryGetAbsolutePath(
-            vscode.workspace.workspaceFolders?.[0],
-            'ts-plain-sam-app/src/app.ts'
-        )
-        tsUri = vscode.Uri.file(tsFile)
+        const ws = vscode.workspace.workspaceFolders?.[0]
+
+        jsonUri1 = vscode.Uri.file(workspaceUtils.tryGetAbsolutePath(ws, 'ts-plain-sam-app/tsconfig.json'))
+        jsonUri2 = vscode.Uri.file(workspaceUtils.tryGetAbsolutePath(ws, 'ts-plain-sam-app/package.json'))
+        tsUri = vscode.Uri.file(workspaceUtils.tryGetAbsolutePath(ws, 'ts-plain-sam-app/src/app.ts'))
+        // Simulate ~/.vscode/argv.json which is emitted by vscode `onDidOpenTextDocument` event.
+        // filetypes.ts no longer listens to `onDidOpenTextDocument`, but this test protects against
+        // regressions.
+        noiseUri1 = vscode.Uri.file(workspaceUtils.tryGetAbsolutePath(ws, '.vscode/argv.json'))
+        await testUtil.toFile('fake argv.json', noiseUri1)
+        noiseUri2 = vscode.Uri.file(workspaceUtils.tryGetAbsolutePath(ws, 'foo/bar/zub.git'))
+        await testUtil.toFile('fake *.git file', noiseUri2)
     })
 
     after(async function () {
@@ -67,6 +63,14 @@ describe('ide_editCodeFile telemetry', function () {
         await vscode.commands.executeCommand('vscode.open', jsonUri1)
         // Different file, same extension (.json), thus `ide_editCodeFile` should be skipped/deduped.
         await vscode.commands.executeCommand('vscode.open', jsonUri2)
+
+        // Open some "noise" documents WITHOUT showing them in a text editor. This triggers
+        // `onDidOpenTextDocument` but not `onDidChangeActiveTextEditor`. This simulates typical
+        // vscode activity. Noise documents should NOT trigger `ide_editCodeFile` or any similar
+        // metric.
+        await vscode.workspace.openTextDocument(noiseUri1)
+        await vscode.workspace.openTextDocument(noiseUri2)
+
         // Wait for metrics...
         const r1 = await getMetrics(2, 'ide_editCodeFile')
         const m1 = r1?.[0]
@@ -92,10 +96,12 @@ describe('ide_editCodeFile telemetry', function () {
 
 describe('file_editAwsFile telemetry', function () {
     let awsConfigUri: vscode.Uri | undefined
+    let ssmJsonUri: vscode.Uri | undefined
     let cfnUri: vscode.Uri | undefined
 
     beforeEach(async function () {
         await testUtil.closeAllEditors()
+        const ws = vscode.workspace.workspaceFolders?.[0]
 
         // Create a dummy file in ~/.aws on the system.
         // Note: We consider _any_ file in ~/.aws to be an "AWS config" file,
@@ -104,11 +110,8 @@ describe('file_editAwsFile telemetry', function () {
         awsConfigUri = vscode.Uri.file(awsConfigFile)
         await testUtil.toFile('Test file from the aws-toolkit-vscode test suite.', awsConfigFile)
 
-        const cfnFile = workspaceUtils.tryGetAbsolutePath(
-            vscode.workspace.workspaceFolders?.[0],
-            'python3.7-plain-sam-app/template.yaml'
-        )
-        cfnUri = vscode.Uri.file(cfnFile)
+        ssmJsonUri = vscode.Uri.file(workspaceUtils.tryGetAbsolutePath(ws, 'test.ssm.json'))
+        cfnUri = vscode.Uri.file(workspaceUtils.tryGetAbsolutePath(ws, 'python3.7-plain-sam-app/template.yaml'))
     })
 
     after(async function () {
@@ -118,10 +121,7 @@ describe('file_editAwsFile telemetry', function () {
     it('emits when opened by user', async function () {
         await vscode.commands.executeCommand('vscode.open', cfnUri)
         await vscode.commands.executeCommand('vscode.open', awsConfigUri)
-        await vscode.workspace.openTextDocument({
-            content: 'test content for SSM JSON',
-            language: 'ssm-json',
-        })
+        await vscode.commands.executeCommand('vscode.open', ssmJsonUri)
 
         const r = await getMetrics(3, 'file_editAwsFile', 5000)
 

packages/core/src/test/testUtil.ts

@@ -20,16 +20,16 @@ import { DeclaredCommand } from '../shared/vscode/commands2'
 const testTempDirs: string[] = []
 
 /**
- * Writes the string form of `o` to `filePathParts` as UTF-8 text.
+ * Writes the string form of `o` to `filepath` as UTF-8 text.
  *
- * Creates parent directories in `filePathParts`, if necessary.
+ * Creates parent directories in `filepath`, if necessary.
  */
-export async function toFile(o: any, ...filePathParts: string[]) {
-    const text = o ? o.toString() : ''
-    const filePath = path.join(...filePathParts)
-    const dir = path.dirname(filePath)
+export async function toFile(o: any, filepath: string | vscode.Uri) {
+    const file = typeof filepath === 'string' ? filepath : filepath.fsPath
+    const text = o === undefined ? '' : o.toString()
+    const dir = path.dirname(file)
     await fs2.mkdir(dir)
-    await fs2.writeFile(filePath, text)
+    await fs2.writeFile(file, text)
 }
 
 /**

packages/core/src/testFixtures/workspaceFolder/test.ssm.json

@@ -0,0 +1,28 @@
+{
+    "description": "Attach IAM Policy to Instance",
+    "assumeRole": "{{ AutomationAssumeRole }}",
+    "schemaVersion": "0.3",
+    "parameters": {
+        "AutomationAssumeRole": {
+            "description": "ARN role",
+            "type": "String"
+        },
+        "InstanceId": {
+            "description": "Instance ID",
+            "type": "String"
+        }
+    },
+    "mainSteps": [
+        {
+            "inputs": {
+                "FunctionName": "SSMOnboardingLambda",
+                "InputPayload": {
+                    "instance_id": "{{ InstanceId }}"
+                }
+            },
+            "name": "AttachPoliciesToInstance",
+            "action": "aws:invokeLambdaFunction",
+            "onFailure": "Abort"
+        }
+    ]
+}

packages/core/src/testInteg/shared/utilities/workspaceUtils.test.ts

@@ -211,7 +211,7 @@ describe('collectFiles', function () {
         const workspaceFolder = await createTestWorkspace(fileAmount, { fileNamePrefix, fileContent })
 
         const writeFile = (pathParts: string[], fileContent: string) => {
-            return toFile(fileContent, workspaceFolder.uri.fsPath, ...pathParts)
+            return toFile(fileContent, path.join(workspaceFolder.uri.fsPath, ...pathParts))
         }
 
         sinon.stub(vscode.workspace, 'workspaceFolders').value([workspaceFolder])
@@ -307,18 +307,18 @@ describe('collectFiles', function () {
         const fileContent = ''
         for (const fmt of ['txt', 'md']) {
             // root license files
-            await toFile(fileContent, workspace.uri.fsPath, `license.${fmt}`)
-            await toFile(fileContent, workspace.uri.fsPath, `License.${fmt}`)
-            await toFile(fileContent, workspace.uri.fsPath, `LICENSE.${fmt}`)
+            await toFile(fileContent, path.join(workspace.uri.fsPath, `license.${fmt}`))
+            await toFile(fileContent, path.join(workspace.uri.fsPath, `License.${fmt}`))
+            await toFile(fileContent, path.join(workspace.uri.fsPath, `LICENSE.${fmt}`))
 
             // nested license files
-            await toFile(fileContent, workspace.uri.fsPath, 'src', `license.${fmt}`)
-            await toFile(fileContent, workspace.uri.fsPath, 'src', `License.${fmt}`)
-            await toFile(fileContent, workspace.uri.fsPath, 'src', `LICENSE.${fmt}`)
+            await toFile(fileContent, path.join(workspace.uri.fsPath, `src/license.${fmt}`))
+            await toFile(fileContent, path.join(workspace.uri.fsPath, `src/License.${fmt}`))
+            await toFile(fileContent, path.join(workspace.uri.fsPath, `src/LICENSE.${fmt}`))
         }
 
         // add a non license file too, to make sure it is returned
-        await toFile(fileContent, workspace.uri.fsPath, 'non-license.md')
+        await toFile(fileContent, path.join(workspace.uri.fsPath, 'non-license.md'))
 
         const result = await collectFiles([workspace.uri.fsPath], [workspace], true)
 
@@ -438,7 +438,7 @@ describe('collectFilesForIndex', function () {
         const workspaceFolder = await createTestWorkspace(fileAmount, { fileNamePrefix, fileContent })
 
         const writeFile = (pathParts: string[], fileContent: string) => {
-            return toFile(fileContent, workspaceFolder.uri.fsPath, ...pathParts)
+            return toFile(fileContent, path.join(workspaceFolder.uri.fsPath, ...pathParts))
         }
 
         sinon.stub(vscode.workspace, 'workspaceFolders').value([workspaceFolder])