Fix doubled log prefixes and isolate the telemetry opt-in setting

README.md

@@ -6,7 +6,6 @@
 
 [![Node.js](https://img.shields.io/badge/dynamic/json?url=https%3A%2F%2Fraw.githubusercontent.com%2Faws-cloudformation%2Fcloudformation-vscode%2Frefs%2Fheads%2Fmain%2Fpackage.json&query=%24.engines.node&label=Node.js&color=339933&logo=node.js&logoColor=white)](https://github.com/aws-cloudformation/cloudformation-vscode/blob/main/package.json)
 [![Latest Release](https://img.shields.io/github/v/release/aws-cloudformation/cloudformation-vscode?include_prereleases&sort=semver)](https://github.com/aws-cloudformation/cloudformation-vscode/releases)
-[![Downloads](https://img.shields.io/github/downloads/aws-cloudformation/cloudformation-vscode/total.svg)](https://github.com/aws-cloudformation/cloudformation-vscode/releases)
 
 ## Overview
 
@@ -90,13 +89,14 @@ template validation, and importing resource state, are not available in this ext
 | `aws.cloudformation.diagnostics.cfnGuard.enabledRulePacks` | `["cis-aws-benchmark-level-1"]` | Managed rule packs to validate against                                   |
 | `aws.cloudformation.diagnostics.cfnGuard.rulesFile`        | `""`                            | Path to a custom cfn-guard rules file; empty uses the enabled rule packs |
 | `aws.cloudformation.trace.server`                          | `"off"`                         | Trace communication with the language server (`messages`, `verbose`)     |
-| `aws.cloudformation.telemetry.enabled`                     | `false`                         | Share anonymous usage telemetry with AWS                                 |
+| `aws.iac.telemetry.enabled`                                | `false`                         | Share anonymous usage telemetry with AWS                                 |
 | `aws.cloudformation.telemetry.logLevel`                    | `"info"`                        | Log level of the extension output channel; applies after a window reload |
 
 ## Telemetry
 
 Telemetry is off by default. On first activation the extension asks whether to share anonymous usage data with AWS;
-you can change the choice at any time with `aws.cloudformation.telemetry.enabled`. See the language server's
+you can change the choice at any time with `aws.iac.telemetry.enabled`. The setting is specific to this extension;
+it does not read or change the telemetry preference of any other extension. See the language server's
 [telemetry documentation](https://github.com/aws-cloudformation/cloudformation-languageserver/tree/main/src/telemetry)
 for what is collected.
 

package.json

@@ -128,10 +128,10 @@
                         "default": "off",
                         "description": "Traces the communication between VS Code and the language server"
                     },
-                    "aws.cloudformation.telemetry.enabled": {
+                    "aws.iac.telemetry.enabled": {
                         "type": "boolean",
                         "default": false,
-                        "description": "Configure anonymous telemetry collection (cannot disable unless it is a production release)"
+                        "description": "Share anonymous usage telemetry from the AWS CloudFormation extension with AWS"
                     },
                     "aws.cloudformation.telemetry.logLevel": {
                         "type": "string",

src/telemetry/TelemetryOptIn.ts

@@ -15,7 +15,7 @@ const stateKeys = {
     unpersistedResponse: `${ExtensionConfigKey}.telemetry.unpersistedResponse`,
 } as const;
 
-const telemetrySettingKey = 'aws.cloudformation.telemetry';
+const telemetrySettingKey = 'aws.iac.telemetry';
 const thirtyDaysMs = 30 * 24 * 60 * 60 * 1000;
 const promptTimeoutMs = 2500;
 const telemetryDocsUrl = 'https://github.com/aws-cloudformation/cloudformation-languageserver/tree/main/src/telemetry';
@@ -105,7 +105,7 @@ async function promptTelemetryOptIn(context: ExtensionContext): Promise<boolean>
     const { globalState } = context;
 
     const response = await window.showInformationMessage(
-        'Help us improve the AWS CloudFormation Language Server by sharing anonymous telemetry data with AWS. You can change this preference at any time in aws.cloudformation Settings.',
+        'Help us improve the AWS CloudFormation Language Server by sharing anonymous telemetry data with AWS. You can change this preference at any time with the aws.iac.telemetry.enabled setting.',
         TelemetryChoice.Allow,
         TelemetryChoice.Later,
         TelemetryChoice.Never,

src/utils/Logger.ts

@@ -1,5 +1,5 @@
-import { OutputChannel, workspace } from 'vscode';
-import { toString } from './Utils';
+import { LogOutputChannel, workspace } from 'vscode';
+import { extractErrorMessage, toString } from './Utils';
 
 type LogLevel = 'debug' | 'info' | 'warn' | 'error';
 
@@ -10,23 +10,14 @@ const levelPriority: Record<LogLevel, number> = {
     error: 3,
 };
 
-function formatTime(): string {
-    return new Date().toLocaleTimeString('en-US', {
-        hour: '2-digit',
-        minute: '2-digit',
-        second: '2-digit',
-        hour12: true,
-    });
-}
-
 export class Logger {
     private readonly configuredLevel = workspace
         .getConfiguration('aws.cloudformation.telemetry')
         .get<LogLevel>('logLevel', 'info');
 
     constructor(
         private readonly name: string,
-        private readonly channel: OutputChannel,
+        private readonly channel: LogOutputChannel,
     ) {}
 
     private shouldLog(level: LogLevel): boolean {
@@ -38,15 +29,16 @@ export class Logger {
             return;
         }
 
-        let line = `[${formatTime()}] ${capitalize(level)}: [${this.name}]`;
+        // The channel stamps time and level itself; only the component name is added here.
+        let line = `[${this.name}]`;
         if (message) {
-            line += `${line} ${message}`;
+            line += ` ${message}`;
         }
 
         if (data) {
-            line += ` ${toString(data)}`;
+            line += ` ${data instanceof Error ? (data.stack ?? extractErrorMessage(data)) : toString(data)}`;
         }
-        this.channel.appendLine(line);
+        this.channel[level](line);
     }
 
     debug(data: unknown, message?: string): void {
@@ -67,10 +59,10 @@ export class Logger {
 }
 
 export class LoggerFactory {
-    private static channel: OutputChannel;
+    private static channel: LogOutputChannel;
     private static readonly loggers = new Map<string, Logger>();
 
-    static initialize(channel: OutputChannel) {
+    static initialize(channel: LogOutputChannel) {
         this.channel = channel;
     }
 
@@ -89,14 +81,7 @@ export class LoggerFactory {
 
     /** @internal For testing only */
     static reset(): void {
-        this.channel = undefined as unknown as OutputChannel;
+        this.channel = undefined as unknown as LogOutputChannel;
         this.loggers.clear();
     }
 }
-
-function capitalize(str: string) {
-    return str
-        .split(' ')
-        .map((word) => word.charAt(0).toUpperCase() + word.slice(1))
-        .join(' ');
-}

tst/__mocks__/vscode.ts

@@ -20,6 +20,11 @@ const vscode = {
         }),
         createOutputChannel: vi.fn().mockReturnValue({
             appendLine: vi.fn(),
+            trace: vi.fn(),
+            debug: vi.fn(),
+            info: vi.fn(),
+            warn: vi.fn(),
+            error: vi.fn(),
             dispose: vi.fn(),
         }),
         showQuickPick: vi.fn(),

tst/auth/awsCredentials.test.ts

@@ -21,7 +21,7 @@ describe('AwsCredentialsService', () => {
         vi.clearAllMocks();
 
         LoggerFactory.reset();
-        LoggerFactory.initialize(window.createOutputChannel('test'));
+        LoggerFactory.initialize(window.createOutputChannel('test', { log: true }));
 
         mockStatusBarItem = {
             show: vi.fn(),

tst/lsp-server/CfnDevLspServerProvider.test.ts

@@ -15,7 +15,7 @@ describe('CfnDevLspServerProvider', () => {
 
     beforeEach(() => {
         LoggerFactory.reset();
-        LoggerFactory.initialize(window.createOutputChannel('test'));
+        LoggerFactory.initialize(window.createOutputChannel('test', { log: true }));
         testDir = nodeFs.mkdtempSync(join(tmpdir(), 'cfn-dev-test-'));
     });
 

tst/lsp-server/CfnRemoteLspServerProvider.test.ts

@@ -24,7 +24,7 @@ describe('CfnRemoteLspServerProvider', () => {
 
     beforeEach(() => {
         LoggerFactory.reset();
-        LoggerFactory.initialize(window.createOutputChannel('test'));
+        LoggerFactory.initialize(window.createOutputChannel('test', { log: true }));
         testDir = nodeFs.mkdtempSync(join(tmpdir(), 'cfn-remote-provider-test-'));
     });
 
@@ -125,7 +125,7 @@ describe('Installer → Provider → Launcher integration: failed managed path i
 
     beforeEach(() => {
         LoggerFactory.reset();
-        LoggerFactory.initialize(window.createOutputChannel('test'));
+        LoggerFactory.initialize(window.createOutputChannel('test', { log: true }));
         testDir = nodeFs.mkdtempSync(join(tmpdir(), 'cfn-integration-test-'));
     });
 

tst/lsp-server/LspLauncher.test.ts

@@ -47,7 +47,7 @@ describe('LspLauncher', () => {
 
     beforeEach(() => {
         LoggerFactory.reset();
-        LoggerFactory.initialize(window.createOutputChannel('test'));
+        LoggerFactory.initialize(window.createOutputChannel('test', { log: true }));
 
         let resolveCount = 0;
         resolver = {

tst/lsp-server/LspServerLifecycle.test.ts

@@ -30,7 +30,7 @@ describe('LspServerLifecycleController', () => {
 
     beforeEach(() => {
         LoggerFactory.reset();
-        LoggerFactory.initialize(window.createOutputChannel('test'));
+        LoggerFactory.initialize(window.createOutputChannel('test', { log: true }));
 
         let resolveCount = 0;
         resolveServer = vi.fn(() => Promise.resolve(`server-${++resolveCount}`));

tst/lsp-server/installer/BaseLspInstaller.test.ts

@@ -267,7 +267,7 @@ describe('BaseLspInstaller', () => {
 
     beforeEach(() => {
         LoggerFactory.reset();
-        LoggerFactory.initialize(window.createOutputChannel('test'));
+        LoggerFactory.initialize(window.createOutputChannel('test', { log: true }));
         memFs = new MemoryFs();
         fetcher = new MockFetcher();
         installer = createInstaller(baseDir, memFs, fetcher);

tst/lsp-server/installer/CfnLspInstaller.test.ts

@@ -28,7 +28,7 @@ describe('CfnLspInstaller', () => {
 
     beforeEach(() => {
         LoggerFactory.reset();
-        LoggerFactory.initialize(window.createOutputChannel('test'));
+        LoggerFactory.initialize(window.createOutputChannel('test', { log: true }));
         testDir = nodeFs.mkdtempSync(join(tmpdir(), 'cfn-installer-test-'));
         installer = new CfnLspInstaller(testDir, 'prod', undefined, undefined, async () => {});
     });

tst/lsp-server/installer/platform.test.ts

@@ -13,7 +13,7 @@ vi.mock('vscode');
 describe('platform detection', () => {
     beforeEach(() => {
         LoggerFactory.reset();
-        LoggerFactory.initialize(window.createOutputChannel('test'));
+        LoggerFactory.initialize(window.createOutputChannel('test', { log: true }));
     });
 
     describe('detectPlatformTarget (default, no injection)', () => {

tst/lsp-server/installer/zipExtraction.test.ts

@@ -120,7 +120,7 @@ describe('BaseLspInstaller - zip extraction', () => {
 
     beforeEach(() => {
         LoggerFactory.reset();
-        LoggerFactory.initialize(window.createOutputChannel('test'));
+        LoggerFactory.initialize(window.createOutputChannel('test', { log: true }));
         testDir = nodeFs.mkdtempSync(join(tmpdir(), 'zip-test-'));
     });
 

tst/telemetry/ClientId.test.ts

@@ -14,7 +14,7 @@ describe('ClientId / telemetry', () => {
 
     beforeEach(() => {
         LoggerFactory.reset();
-        LoggerFactory.initialize(window.createOutputChannel('test'));
+        LoggerFactory.initialize(window.createOutputChannel('test', { log: true }));
         const data: Record<string, unknown> = {};
         mockState = {
             data,

tst/telemetry/TelemetryOptIn.test.ts

@@ -0,0 +1,68 @@
+import { beforeEach, describe, expect, it, vi } from 'vitest';
+import { window, workspace } from 'vscode';
+import { handleTelemetryOptIn } from '../../src/telemetry/TelemetryOptIn';
+import { LoggerFactory } from '../../src/utils/Logger';
+
+vi.mock('vscode');
+
+const settingKey = 'aws.iac.telemetry';
+
+function createContext(state: Record<string, unknown>) {
+    return {
+        globalState: {
+            get: (key: string) => state[key],
+            update: (key: string, value: unknown) => {
+                state[key] = value;
+                return Promise.resolve();
+            },
+        },
+    } as any;
+}
+
+describe('handleTelemetryOptIn', () => {
+    let settings: Record<string, Record<string, unknown>>;
+
+    beforeEach(() => {
+        LoggerFactory.reset();
+        LoggerFactory.initialize(window.createOutputChannel('test', { log: true }));
+
+        settings = {};
+        vi.mocked(workspace.getConfiguration).mockImplementation((section?: string) => {
+            const values = (settings[section ?? ''] ??= {});
+            return {
+                get: vi.fn((key: string, defaultValue?: unknown) => values[key] ?? defaultValue),
+                update: vi.fn((key: string, value: unknown) => {
+                    values[key] = value;
+                    return Promise.resolve();
+                }),
+                has: vi.fn(),
+                inspect: vi.fn(),
+            };
+        });
+    });
+
+    it('reads a previously recorded choice from aws.iac.telemetry.enabled', async () => {
+        settings[settingKey] = { enabled: true };
+        const context = createContext({ 'aws.cloudformation.telemetry.hasResponded': true });
+
+        await expect(handleTelemetryOptIn(context)).resolves.toBe(true);
+    });
+
+    it('ignores aws.cloudformation.telemetry.enabled contributed by other extensions', async () => {
+        settings['aws.cloudformation.telemetry'] = { enabled: true };
+        const context = createContext({ 'aws.cloudformation.telemetry.hasResponded': true });
+
+        await expect(handleTelemetryOptIn(context)).resolves.toBe(false);
+    });
+
+    it('persists the prompt answer under aws.iac.telemetry.enabled only', async () => {
+        vi.mocked(window.showInformationMessage).mockResolvedValue('Yes, Allow' as any);
+        const state: Record<string, unknown> = {};
+
+        await expect(handleTelemetryOptIn(createContext(state))).resolves.toBe(true);
+
+        expect(settings[settingKey]).toEqual({ enabled: true });
+        expect(settings['aws.cloudformation.telemetry']?.enabled).toBeUndefined();
+        expect(state['aws.cloudformation.telemetry.hasResponded']).toBe(true);
+    });
+});