Merge branch 'main' into metrics-units

satyakigh · satyakigh · commit 78a73e852ee4 · 2025-11-03T12:17:45.000-05:00
diff --git a/src/handlers/ConfigurationHandler.ts b/src/handlers/ConfigurationHandler.ts
@@ -7,9 +7,9 @@ const log = LoggerFactory.getLogger('ConfigurationHandler');
 
 export function configurationHandler(components: ServerComponents): (params: DidChangeConfigurationParams) => void {
     return (_params: DidChangeConfigurationParams): void => {
+        TelemetryService.instance.get('ConfigurationHandler').count('count', 1);
         // Pull configuration from LSP workspace and notify all components via subscriptions (fire-and-forget)
         components.settingsManager.syncConfiguration().catch((error) => {
-            TelemetryService.instance.get('ConfigurationHandler').count('count', 1);
             log.error(error, `Failed to sync configuration`);
         });
     };
diff --git a/src/handlers/ExecutionHandler.ts b/src/handlers/ExecutionHandler.ts
@@ -37,6 +37,7 @@ export function executionHandler(
                 });
             }
             case ANALYZE_DIAGNOSTIC: {
+                TelemetryService.instance.get('CodeAction').count(`accepted.diagnoseWithAI`, 1);
                 return executeWithError(components, async () => {
                     const message = await components.cfnAI.analyzeDiagnostic(
                         params.arguments?.[0] as string,
@@ -66,6 +67,15 @@ export function executionHandler(
                     components.diagnosticCoordinator
                         .handleClearCfnDiagnostic(uri, diagnosticId)
                         .catch((err) => log.error(err, `Error clearing diagnostic`));
+                    TelemetryService.instance.get('CodeAction').count(`accepted.clearDiagnostic`, 1);
+                }
+                break;
+            }
+            case TRACK_CODE_ACTION_ACCEPTED: {
+                const args = params.arguments ?? [];
+                if (args.length > 0) {
+                    const actionType = args[0] as string;
+                    TelemetryService.instance.get('CodeAction').count(`accepted.${actionType}`, 1);
                 }
                 break;
             }
@@ -91,3 +101,4 @@ export const OPTIMIZE_TEMPLATE = '/command/llm/template/optimize';
 export const ANALYZE_DIAGNOSTIC = '/command/llm/diagnostic/analyze';
 export const RECOMMEND_RELATED_RESOURCES = '/command/llm/template/recommend-related';
 export const CLEAR_DIAGNOSTIC = '/command/template/clear-diagnostic';
+export const TRACK_CODE_ACTION_ACCEPTED = '/command/codeAction/track';
diff --git a/src/server/CfnInfraCore.ts b/src/server/CfnInfraCore.ts
@@ -9,6 +9,7 @@ import { LspComponents } from '../protocol/LspComponents';
 import { DiagnosticCoordinator } from '../services/DiagnosticCoordinator';
 import { SettingsManager } from '../settings/SettingsManager';
 import { ClientMessage } from '../telemetry/ClientMessage';
+import { LoggerFactory } from '../telemetry/LoggerFactory';
 import { TelemetryService } from '../telemetry/TelemetryService';
 import { Closeable, closeSafely } from '../utils/Closeable';
 import { Configurable, Configurables } from '../utils/Configurable';
@@ -71,6 +72,8 @@ export class CfnInfraCore implements Configurables, Closeable {
     }
 
     async close() {
-        return await closeSafely(this.dataStoreFactory, TelemetryService.instance);
+        // eslint-disable-next-line @typescript-eslint/ban-ts-comment
+        // @ts-ignore
+        return await closeSafely(this.dataStoreFactory, TelemetryService.instance, LoggerFactory._instance);
     }
 }
diff --git a/src/services/CodeActionService.ts b/src/services/CodeActionService.ts
@@ -15,23 +15,26 @@ import { SyntaxTreeManager } from '../context/syntaxtree/SyntaxTreeManager';
 import { NodeSearch } from '../context/syntaxtree/utils/NodeSearch';
 import { NodeType } from '../context/syntaxtree/utils/NodeType';
 import { DocumentManager } from '../document/DocumentManager';
-import { ANALYZE_DIAGNOSTIC } from '../handlers/ExecutionHandler';
+import { ANALYZE_DIAGNOSTIC, TRACK_CODE_ACTION_ACCEPTED } from '../handlers/ExecutionHandler';
 import { CfnInfraCore } from '../server/CfnInfraCore';
 import { CFN_VALIDATION_SOURCE } from '../stacks/actions/ValidationWorkflow';
 import { LoggerFactory } from '../telemetry/LoggerFactory';
-import { Track } from '../telemetry/TelemetryDecorator';
+import { ScopedTelemetry } from '../telemetry/ScopedTelemetry';
+import { Telemetry, Track } from '../telemetry/TelemetryDecorator';
 import { pointToPosition } from '../utils/TypeConverters';
 import { ExtractToParameterProvider } from './extractToParameter/ExtractToParameterProvider';
 
 export interface CodeActionFix {
     title: string;
     kind: string;
+    actionType: string;
     diagnostic: Diagnostic;
     textEdits: TextEdit[];
     command?: Command;
 }
 
 export class CodeActionService {
+    @Telemetry() private readonly telemetry!: ScopedTelemetry;
     private static readonly REMOVE_ERROR_TITLE = 'Hide validation error';
     private readonly log = LoggerFactory.getLogger(CodeActionService);
 
@@ -43,8 +46,10 @@ export class CodeActionService {
         private readonly syntaxTreeManager: SyntaxTreeManager,
         private readonly documentManager: DocumentManager,
         private readonly contextManager: ContextManager,
-        private readonly extractToParameterProvider?: ExtractToParameterProvider,
-    ) {}
+        private readonly extractToParameterProvider: ExtractToParameterProvider,
+    ) {
+        this.initializeCounters();
+    }
 
     /**
      * Process diagnostics and generate code actions with fixes
@@ -76,6 +81,13 @@ export class CodeActionService {
         return codeActions;
     }
 
+    private initializeCounters(): void {
+        this.telemetry.count('quickfix.cfnLintFixOffered', 0);
+        this.telemetry.count('quickfix.clearDiagnosticOffered', 0);
+        this.telemetry.count('refactor.extractToParameterOffered', 0);
+        this.telemetry.count('refactor.extractAllToParameterOffered', 0);
+    }
+
     /**
      * Generate fixes for a specific diagnostic
      */
@@ -112,10 +124,12 @@ export class CodeActionService {
      * Generate fixes for CFN Validation diagnostics
      */
     private generateCfnValidationFixes(diagnostic: Diagnostic, uri: string): CodeActionFix[] {
+        this.telemetry.count('quickfix.clearDiagnosticOffered', 1);
         return [
             {
                 title: CodeActionService.REMOVE_ERROR_TITLE,
                 kind: 'quickfix',
+                actionType: 'clearDiagnostic',
                 diagnostic,
                 textEdits: [],
                 command: {
@@ -165,6 +179,8 @@ export class CodeActionService {
             }
         }
 
+        this.telemetry.count('quickfix.cfnLintFixOffered', fixes.length);
+
         return fixes;
     }
 
@@ -182,6 +198,7 @@ export class CodeActionService {
             fixes.push({
                 title: `Remove invalid property '${propertyName}'`,
                 kind: 'quickfix',
+                actionType: 'removeProperty',
                 diagnostic,
                 textEdits: [
                     {
@@ -284,6 +301,7 @@ export class CodeActionService {
                 fixes.push({
                     title: `Add required property '${propertyName}'`,
                     kind: 'quickfix',
+                    actionType: 'addRequiredProperty',
                     diagnostic,
                     textEdits: [
                         {
@@ -324,6 +342,12 @@ export class CodeActionService {
 
             if (fix.command) {
                 codeAction.command = fix.command;
+            } else {
+                codeAction.command = {
+                    title: 'Track code action',
+                    command: TRACK_CODE_ACTION_ACCEPTED,
+                    arguments: [fix.actionType],
+                };
             }
 
             return codeAction;
@@ -520,20 +544,27 @@ export class CodeActionService {
             const canExtract = this.extractToParameterProvider.canExtract(context);
 
             if (canExtract) {
-                const extractAction = this.generateExtractToParameterAction(params, context);
+                const extractAction = this.telemetry.measure('refactor.extractToParameter', () =>
+                    this.generateExtractToParameterAction(params, context),
+                );
+
                 if (extractAction) {
                     refactorActions.push(extractAction);
+                    this.telemetry.count('refactor.extractToParameterOffered', 1);
                 }
 
-                const hasMultiple = this.extractToParameterProvider.hasMultipleOccurrences(
-                    context,
-                    params.textDocument.uri,
+                const hasMultiple = this.telemetry.measure('refactor.hasMultipleOccurrences', () =>
+                    this.extractToParameterProvider.hasMultipleOccurrences(context, params.textDocument.uri),
                 );
 
                 if (hasMultiple) {
-                    const extractAllAction = this.generateExtractAllOccurrencesToParameterAction(params, context);
+                    const extractAllAction = this.telemetry.measure('refactor.extractAllToParameter', () =>
+                        this.generateExtractAllOccurrencesToParameterAction(params, context),
+                    );
+
                     if (extractAllAction) {
                         refactorActions.push(extractAllAction);
+                        this.telemetry.count('refactor.extractAllToParameterOffered', 1);
                     }
                 }
             }
@@ -579,7 +610,13 @@ export class CodeActionService {
                 command: {
                     title: 'Position cursor in parameter description',
                     command: 'aws.cloudformation.extractToParameter.positionCursor',
-                    arguments: [params.textDocument.uri, extractionResult.parameterName, context.documentType],
+                    arguments: [
+                        params.textDocument.uri,
+                        extractionResult.parameterName,
+                        context.documentType,
+                        TRACK_CODE_ACTION_ACCEPTED,
+                        'extractToParameter',
+                    ],
                 },
             };
         } catch (error) {
@@ -625,7 +662,13 @@ export class CodeActionService {
                 command: {
                     title: 'Position cursor in parameter description',
                     command: 'aws.cloudformation.extractToParameter.positionCursor',
-                    arguments: [params.textDocument.uri, extractionResult.parameterName, context.documentType],
+                    arguments: [
+                        params.textDocument.uri,
+                        extractionResult.parameterName,
+                        context.documentType,
+                        TRACK_CODE_ACTION_ACCEPTED,
+                        'extractAllToParameter',
+                    ],
                 },
             };
         } catch (error) {
diff --git a/src/telemetry/LoggerFactory.ts b/src/telemetry/LoggerFactory.ts
@@ -1,7 +1,12 @@
 /* eslint-disable @typescript-eslint/no-unsafe-function-type */
+import { readdir, stat, unlink, readFile, writeFile } from 'fs/promises';
+import { join } from 'path';
+import { DateTime } from 'luxon';
 import pino, { LevelWithSilent, Logger } from 'pino';
 import { AwsMetadata } from '../server/InitParams';
-import { ExtensionName } from '../utils/ExtensionConfig';
+import { pathToArtifact } from '../utils/ArtifactsDir';
+import { Closeable } from '../utils/Closeable';
+import { ExtensionId, ExtensionName } from '../utils/ExtensionConfig';
 import { TelemetrySettings } from './TelemetryConfig';
 
 export const LogLevel: Record<LevelWithSilent, number> = {
@@ -14,12 +19,16 @@ export const LogLevel: Record<LevelWithSilent, number> = {
     trace: 6,
 } as const;
 
-export class LoggerFactory {
+export class LoggerFactory implements Closeable {
+    private static readonly LogsDirectory = pathToArtifact('logs');
+    private static readonly MaxFileSize = 50 * 1024 * 1024; // 50MB
+
     private static readonly _instance: LoggerFactory = new LoggerFactory();
 
     private readonly baseLogger: Logger;
     private readonly logLevel: LevelWithSilent;
     private readonly loggers = new Map<string, Logger>();
+    private readonly interval: NodeJS.Timeout;
 
     private constructor(level?: LevelWithSilent) {
         this.logLevel = level ?? TelemetrySettings.logLevel;
@@ -28,15 +37,81 @@ export class LoggerFactory {
             name: ExtensionName,
             level: this.logLevel,
             transport: {
-                target: 'pino-pretty',
-                options: {
-                    colorize: false,
-                    translateTime: 'SYS:hh:MM:ss TT',
-                    ignore: 'pid,hostname,name,clazz',
-                    messageFormat: '[{clazz}] {msg}',
-                },
+                targets: [
+                    {
+                        target: 'pino-pretty',
+                        options: {
+                            colorize: false,
+                            translateTime: 'SYS:hh:MM:ss TT',
+                            ignore: 'pid,hostname,name,clazz',
+                            messageFormat: '[{clazz}] {msg}',
+                        },
+                    },
+                    {
+                        target: 'pino/file',
+                        options: {
+                            destination: join(
+                                LoggerFactory.LogsDirectory,
+                                `${ExtensionId}-${DateTime.utc().toFormat('yyyy-MM-dd')}.log`,
+                            ),
+                            mkdir: true,
+                        },
+                    },
+                ],
             },
         });
+
+        void this.cleanOldLogs();
+        this.interval = setInterval(
+            () => {
+                void this.trimLogs();
+            },
+            10 * 60 * 1000,
+        );
+    }
+
+    private async cleanOldLogs() {
+        try {
+            const files = await readdir(LoggerFactory.LogsDirectory);
+            const oneWeekAgo = DateTime.utc().minus({ weeks: 1 });
+
+            for (const file of files) {
+                if (!file.endsWith('.log')) continue;
+
+                const filePath = join(LoggerFactory.LogsDirectory, file);
+                const stats = await stat(filePath);
+
+                if (DateTime.fromJSDate(stats.mtime) < oneWeekAgo) {
+                    await unlink(filePath);
+                }
+            }
+
+            await this.trimLogs();
+        } catch (err) {
+            this.baseLogger.error(err, 'Error cleaning up old logs');
+        }
+    }
+
+    private async trimLogs() {
+        try {
+            const files = await readdir(LoggerFactory.LogsDirectory);
+
+            for (const file of files) {
+                if (!file.endsWith('.log')) continue;
+
+                const filePath = join(LoggerFactory.LogsDirectory, file);
+                const stats = await stat(filePath);
+
+                if (stats.size > LoggerFactory.MaxFileSize) {
+                    const content = await readFile(filePath, 'utf8');
+                    const lines = content.split('\n');
+                    const trimmed = lines.slice(-Math.floor(lines.length / 2)).join('\n');
+                    await writeFile(filePath, trimmed);
+                }
+            }
+        } catch (err) {
+            this.baseLogger.error(err, 'Error trimming old logs');
+        }
     }
 
     private reconfigure(newLevel: LevelWithSilent) {
@@ -57,6 +132,10 @@ export class LoggerFactory {
         return logger;
     }
 
+    close() {
+        clearInterval(this.interval);
+    }
+
     static getLogger(clazz: string | Function): Logger {
         return LoggerFactory._instance.getLogger(clazz);
     }
diff --git a/tst/integration/ExtractToParameter.e2e.test.ts b/tst/integration/ExtractToParameter.e2e.test.ts
@@ -110,10 +110,12 @@ describe('Extract to Parameter - End-to-End CodeAction Workflow Tests', () => {
             expect(extractAction?.command).toBeDefined();
             expect(extractAction?.command?.command).toBe('aws.cloudformation.extractToParameter.positionCursor');
             expect(extractAction?.command?.arguments).toBeDefined();
-            expect(extractAction?.command?.arguments?.length).toBe(3);
+            expect(extractAction?.command?.arguments?.length).toBe(5);
             expect(extractAction?.command?.arguments?.[0]).toBe(uri);
             expect(typeof extractAction?.command?.arguments?.[1]).toBe('string'); // parameter name
             expect(extractAction?.command?.arguments?.[2]).toBe('JSON'); // document type
+            expect(extractAction?.command?.arguments?.[3]).toBe('/command/codeAction/track'); // tracking command
+            expect(extractAction?.command?.arguments?.[4]).toBe('extractToParameter'); // action type
 
             // Step 10: Sequential extraction test - Apply first extraction and perform second
             const firstUpdatedContent = applyWorkspaceEdit(template, changes);
@@ -292,10 +294,12 @@ Resources:
                 expect(extractAction.command).toBeDefined();
                 expect(extractAction.command?.command).toBe('aws.cloudformation.extractToParameter.positionCursor');
                 expect(extractAction.command?.arguments).toBeDefined();
-                expect(extractAction.command?.arguments?.length).toBe(3);
+                expect(extractAction.command?.arguments?.length).toBe(5);
                 expect(extractAction.command?.arguments?.[0]).toBe(uri);
                 expect(typeof extractAction.command?.arguments?.[1]).toBe('string'); // parameter name
                 expect(extractAction.command?.arguments?.[2]).toBe('YAML'); // document type
+                expect(extractAction.command?.arguments?.[3]).toBe('/command/codeAction/track'); // tracking command
+                expect(extractAction.command?.arguments?.[4]).toBe('extractToParameter'); // action type
 
                 // Sequential extraction test: Apply first extraction and perform second
                 const firstUpdatedContent = applyWorkspaceEdit(template, changes);
diff --git a/tst/unit/services/CodeActionService.extractToParameter.test.ts b/tst/unit/services/CodeActionService.extractToParameter.test.ts
diff --git a/tst/unit/services/CodeActionService.test.ts b/tst/unit/services/CodeActionService.test.ts

Original file line number	Diff line number	Diff line change
`@@ -9,6 +9,7 @@ import { LspComponents } from '../protocol/LspComponents';`
`9`	`9`	`import { DiagnosticCoordinator } from '../services/DiagnosticCoordinator';`
`10`	`10`	`import { SettingsManager } from '../settings/SettingsManager';`
`11`	`11`	`import { ClientMessage } from '../telemetry/ClientMessage';`
	`12`	`+import { LoggerFactory } from '../telemetry/LoggerFactory';`
`12`	`13`	`import { TelemetryService } from '../telemetry/TelemetryService';`
`13`	`14`	`import { Closeable, closeSafely } from '../utils/Closeable';`
`14`	`15`	`import { Configurable, Configurables } from '../utils/Configurable';`
`@@ -71,6 +72,8 @@ export class CfnInfraCore implements Configurables, Closeable {`
`71`	`72`	`}`
`72`	`73`
`73`	`74`	`async close() {`
`74`		`- return await closeSafely(this.dataStoreFactory, TelemetryService.instance);`
	`75`	`+ // eslint-disable-next-line @typescript-eslint/ban-ts-comment`
	`76`	`+ // @ts-ignore`
	`77`	`+ return await closeSafely(this.dataStoreFactory, TelemetryService.instance, LoggerFactory._instance);`
`75`	`78`	`}`
`76`	`79`	`}`