feat(logging): use OutputChannel log level

Problem: Q extension may show error message: [amazonwebservices.amazon-q-vscode]: Cannot register 'aws.logLevel'. This property is already registered. Solution: Remove the `aws.logLevel` setting from both Toolkit and Q. Let the OutputChannel drive the loglevel.
aws · Apr 27, 2024 · 8f7e5e8 · 8f7e5e8
1 parent 5e59ee4
commit 8f7e5e8
Show file tree

Hide file tree

Showing 9 changed files with 57 additions and 140 deletions.
diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md
@@ -328,7 +328,7 @@ The `aws.dev.forceDevMode` setting enables or disables Toolkit "dev mode". Witho
 -   Use the `AWS (Developer): Watch Logs` command to watch and filter Toolkit logs (including
     telemetry) in VSCode.
     -   Only available if you enabled "dev mode" (`aws.dev.forceDevMode` setting, see above).
-    -   Sets `aws.logLevel` to "debug".
+    -   Sets the "AWS Toolkit Logs" Output Channel **log-level** to "debug".
     -   Enter text in the Debug Console filter box to show only log messages with that text. <br/>
         <img src="./docs/images/debug-console-filter.png" alt="VSCode Debug Console" width="320"/>
 

diff --git a/packages/amazonq/package.json b/packages/amazonq/package.json
@@ -60,30 +60,6 @@
                 }
             },
             "properties": {
-                "aws.logLevel": {
-                    "type": "string",
-                    "default": "debug",
-                    "enum": [
-                        "error",
-                        "warn",
-                        "info",
-                        "verbose",
-                        "debug"
-                    ],
-                    "enumDescriptions": [
-                        "Errors Only",
-                        "Errors and Warnings",
-                        "Errors, Warnings, and Info",
-                        "Errors, Warnings, Info, and Verbose",
-                        "Errors, Warnings, Info, Verbose, and Debug"
-                    ],
-                    "markdownDescription": "%AWS.configuration.description.logLevel%",
-                    "cloud9": {
-                        "cn": {
-                            "markdownDescription": "%AWS.configuration.description.logLevel.cn%"
-                        }
-                    }
-                },
                 "amazonQ.telemetry": {
                     "type": "boolean",
                     "default": true,

diff --git a/packages/core/package.json b/packages/core/package.json
@@ -90,30 +90,6 @@
                     "default": false,
                     "markdownDescription": "%AWS.configuration.description.samcli.legacyDeploy%"
                 },
-                "aws.logLevel": {
-                    "type": "string",
-                    "default": "debug",
-                    "enum": [
-                        "error",
-                        "warn",
-                        "info",
-                        "verbose",
-                        "debug"
-                    ],
-                    "enumDescriptions": [
-                        "Errors Only",
-                        "Errors and Warnings",
-                        "Errors, Warnings, and Info",
-                        "Errors, Warnings, Info, and Verbose",
-                        "Errors, Warnings, Info, Verbose, and Debug"
-                    ],
-                    "markdownDescription": "%AWS.configuration.description.logLevel%",
-                    "cloud9": {
-                        "cn": {
-                            "markdownDescription": "%AWS.configuration.description.logLevel.cn%"
-                        }
-                    }
-                },
                 "aws.telemetry": {
                     "type": "boolean",
                     "default": true,

diff --git a/packages/core/src/codewhisperer/commands/startSecurityScan.ts b/packages/core/src/codewhisperer/commands/startSecurityScan.ts
@@ -49,6 +49,7 @@ export const stopScanButton = localize('aws.codewhisperer.stopscan', 'Stop Scan'
 const getLogOutputChan = once(() => {
     const codeScanOutpuChan = vscode.window.createOutputChannel('Amazon Q Security Scan Logs')
     const codeScanLogger = makeLogger({
+        logLevel: 'info',
         outputChannels: [codeScanOutpuChan],
     })
     return [codeScanLogger, codeScanOutpuChan] as const

diff --git a/packages/core/src/shared/logger/activation.ts b/packages/core/src/shared/logger/activation.ts
@@ -5,15 +5,13 @@
 
 import * as vscode from 'vscode'
 import { Logger, LogLevel, getLogger } from '.'
-import { setLogger } from './logger'
+import { fromVscodeLogLevel, setLogger } from './logger'
 import { WinstonToolkitLogger } from './winstonToolkitLogger'
 import { Settings } from '../settings'
 import { Logging } from './commands'
 import { resolvePath } from '../utilities/pathUtils'
 import { fsCommon } from '../../srcShared/fs'
-import globals, { isWeb } from '../extensionGlobals'
-
-export const defaultLogLevel: LogLevel = 'debug'
+import { isWeb } from '../extensionGlobals'
 
 /**
  * Activate Logger functionality for the extension.
@@ -27,43 +25,41 @@ export async function activate(
     const settings = Settings.instance.getSection('aws')
     const devLogfile = settings.get('dev.logfile', '')
     const logUri = devLogfile ? vscode.Uri.file(resolvePath(devLogfile)) : undefined
+    const chanLogLevel = fromVscodeLogLevel(logChannel.logLevel)
 
     await fsCommon.mkdir(extensionContext.logUri)
 
-    const mainLogger = makeLogger(
-        {
-            logPaths: logUri ? [logUri] : undefined,
-            outputChannels: [logChannel],
-            useConsoleLog: isWeb(),
-        },
-        extensionContext.subscriptions
-    )
+    const mainLogger = makeLogger({
+        logLevel: chanLogLevel,
+        logPaths: logUri ? [logUri] : undefined,
+        outputChannels: [logChannel],
+        useConsoleLog: isWeb(),
+    })
+    logChannel.onDidChangeLogLevel(logLevel => {
+        const newLogLevel = fromVscodeLogLevel(logLevel)
+        mainLogger.setLogLevel(newLogLevel) // Also logs a message.
+    })
 
     setLogger(mainLogger)
-    getLogger().info(`log level: ${getLogLevel()}`)
+    getLogger().info(`log level: ${chanLogLevel}`)
 
     // Logs to "AWS Toolkit" output channel.
     setLogger(
-        makeLogger(
-            {
-                logPaths: logUri ? [logUri] : undefined,
-                outputChannels: [outputChannel, logChannel],
-            },
-            extensionContext.subscriptions
-        ),
+        makeLogger({
+            logLevel: chanLogLevel,
+            logPaths: logUri ? [logUri] : undefined,
+            outputChannels: [outputChannel, logChannel],
+        }),
         'channel'
     )
 
     // Logs to vscode Debug Console.
     setLogger(
-        makeLogger(
-            {
-                staticLogLevel: 'debug',
-                outputChannels: [outputChannel, logChannel],
-                useDebugConsole: true,
-            },
-            extensionContext.subscriptions
-        ),
+        makeLogger({
+            logLevel: chanLogLevel,
+            outputChannels: [outputChannel, logChannel],
+            useDebugConsole: true,
+        }),
         'debugConsole'
     )
 
@@ -75,25 +71,20 @@ export async function activate(
 
 /**
  * Creates a logger off of specified params
- * @param opts Specified parameters, all optional:
- * @param opts.staticLogLevel Static log level, overriding config value. Will persist overridden config value even if the config value changes.
+ * @param opts.logLevel Log messages at or above this level
  * @param opts.logPaths Array of paths to output log entries to
  * @param opts.outputChannels Array of output channels to log entries to
  * @param opts.useDebugConsole If true, outputs log entries to `vscode.debug.activeDebugConsole`
  * @param opts.useConsoleLog If true, outputs log entries to the nodejs or browser devtools console.
- * @param disposables Array of disposables to add a subscription to
  */
-export function makeLogger(
-    opts: {
-        staticLogLevel?: LogLevel
-        logPaths?: vscode.Uri[]
-        outputChannels?: vscode.OutputChannel[]
-        useDebugConsole?: boolean
-        useConsoleLog?: boolean
-    },
-    disposables?: vscode.Disposable[]
-): Logger {
-    const logger = new WinstonToolkitLogger(opts.staticLogLevel ?? getLogLevel())
+export function makeLogger(opts: {
+    logLevel: LogLevel
+    logPaths?: vscode.Uri[]
+    outputChannels?: vscode.OutputChannel[]
+    useDebugConsole?: boolean
+    useConsoleLog?: boolean
+}): Logger {
+    const logger = new WinstonToolkitLogger(opts.logLevel)
     // debug console can show ANSI colors, output channels can not
     const stripAnsi = opts.useDebugConsole ?? false
     for (const logPath of opts.logPaths ?? []) {
@@ -109,23 +100,5 @@ export function makeLogger(
         logger.logToConsole()
     }
 
-    if (!opts.staticLogLevel) {
-        vscode.workspace.onDidChangeConfiguration(
-            configurationChangeEvent => {
-                if (configurationChangeEvent.affectsConfiguration('aws.logLevel')) {
-                    const newLogLevel = getLogLevel()
-                    logger.setLogLevel(newLogLevel)
-                }
-            },
-            undefined,
-            disposables
-        )
-    }
-
     return logger
 }
-
-function getLogLevel(): LogLevel {
-    const configuration = Settings.instance.getSection('aws')
-    return configuration.get(`${globals.contextPrefix}logLevel`, defaultLogLevel)
-}
diff --git a/packages/core/src/shared/logger/logger.ts b/packages/core/src/shared/logger/logger.ts
@@ -3,7 +3,7 @@
  * SPDX-License-Identifier: Apache-2.0
  */
 
-import { Uri } from 'vscode'
+import * as vscode from 'vscode'
 import globals from '../extensionGlobals'
 
 const toolkitLoggers: {
@@ -26,7 +26,7 @@ export interface Logger {
     setLogLevel(logLevel: LogLevel): void
     /** Returns true if the given log level is being logged.  */
     logLevelEnabled(logLevel: LogLevel): boolean
-    getLogById(logID: number, file: Uri): string | undefined
+    getLogById(logID: number, file: vscode.Uri): string | undefined
     /** HACK: Enables logging to vscode Debug Console. */
     enableDebugConsole(): void
 }
@@ -48,6 +48,22 @@ const logLevels = new Map<LogLevel, number>([
 
 export type LogLevel = 'error' | 'warn' | 'info' | 'verbose' | 'debug'
 
+export function fromVscodeLogLevel(logLevel: vscode.LogLevel): LogLevel {
+    switch (logLevel) {
+        case vscode.LogLevel.Off:
+        case vscode.LogLevel.Error:
+            return 'error'
+        case vscode.LogLevel.Warning:
+            return 'warn'
+        case vscode.LogLevel.Info:
+            return 'info'
+        case vscode.LogLevel.Debug:
+            return 'debug'
+        case vscode.LogLevel.Trace:
+            return 'debug'
+    }
+}
+
 /**
  * Compares log levels.
  *
@@ -96,7 +112,7 @@ export class NullLogger implements Logger {
     public error(message: string | Error, ...meta: any[]): number {
         return 0
     }
-    public getLogById(logID: number, file: Uri): string | undefined {
+    public getLogById(logID: number, file: vscode.Uri): string | undefined {
         return undefined
     }
     public enableDebugConsole(): void {}
@@ -131,7 +147,7 @@ export class ConsoleLogger implements Logger {
         console.error(message, ...meta)
         return 0
     }
-    public getLogById(logID: number, file: Uri): string | undefined {
+    public getLogById(logID: number, file: vscode.Uri): string | undefined {
         return undefined
     }
     public enableDebugConsole(): void {}

diff --git a/packages/core/src/shared/logger/outputChannelTransport.ts b/packages/core/src/shared/logger/outputChannelTransport.ts
@@ -64,13 +64,7 @@ export class OutputChannelTransport extends Transport {
                 } else if (loglevel === 'warn') {
                     c.warn(msg)
                 } else if (loglevel === 'debug' || loglevel === 'verbose') {
-                    // XXX: `vscode.LogOutputChannel` loglevel is currently readonly:
-                    //      https://github.com/microsoft/vscode/issues/170450
-                    //      https://github.com/PowerShell/vscode-powershell/issues/4441
-                    // So debug() will just drop messages unless the user configures vscode (via
-                    // `code --log …` or `.vscode/argv.json` https://stackoverflow.com/a/77257398/152142).
-                    // Use info() until vscode adds a way to set the loglevel.
-                    c.info(msg)
+                    c.debug(msg)
                 } else {
                     c.info(msg)
                 }

diff --git a/packages/core/src/test/shared/logger/activation.test.ts b/packages/core/src/test/shared/logger/activation.test.ts
@@ -18,7 +18,7 @@ describe('makeLogger', function () {
     before(async function () {
         tempFolder = await makeTemporaryToolkitFolder()
         const logPath = vscode.Uri.joinPath(vscode.Uri.file(tempFolder), 'log.txt')
-        testLogger = makeLogger({ staticLogLevel: 'debug', logPaths: [logPath] })
+        testLogger = makeLogger({ logLevel: 'debug', logPaths: [logPath] })
     })
 
     after(async function () {

diff --git a/packages/core/src/test/techdebt.test.ts b/packages/core/src/test/techdebt.test.ts
@@ -6,7 +6,6 @@
 import assert from 'assert'
 import * as semver from 'semver'
 import * as env from '../shared/vscode/env'
-import { defaultLogLevel } from '../shared/logger/activation'
 import packageJson from '../../package.json'
 
 // Checks project config and dependencies, to remind us to remove old things
@@ -35,22 +34,4 @@ describe('tech debt', function () {
             'with node16+, we can now use AbortController to cancel Node things (child processes, HTTP requests, etc.)'
         )
     })
-
-    it('feature/standalone branch temporary debug log level for testing', async function () {
-        if (!(process.env.GITHUB_BASE_REF ?? '').includes('master')) {
-            this.skip()
-        }
-
-        assert.strictEqual(
-            defaultLogLevel,
-            'info',
-            'set loglevel defaults back to info for src/shared/logger/activation.ts. (revert this commit)'
-        )
-
-        assert.strictEqual(
-            packageJson.contributes.configuration.properties['aws.logLevel'].default,
-            'info',
-            'set loglevel defaults back to info for packages/amazonq/package.json, packages/toolkit/package.json. (revert this commit)'
-        )
-    })
 })