From 716453ce78f77c47d0f1e041c42b456d29f28ecd Mon Sep 17 00:00:00 2001 From: "Justin M. Keyes" Date: Sat, 27 Apr 2024 12:02:48 -0700 Subject: [PATCH 1/2] refactor: openSettings --- packages/core/src/codewhisperer/activation.ts | 3 ++- packages/core/src/shared/settings.ts | 7 ++++++- packages/core/src/shared/telemetry/activation.ts | 4 ++-- 3 files changed, 10 insertions(+), 4 deletions(-) diff --git a/packages/core/src/codewhisperer/activation.ts b/packages/core/src/codewhisperer/activation.ts index f4c0fb2b1df..c2f7b0690e4 100644 --- a/packages/core/src/codewhisperer/activation.ts +++ b/packages/core/src/codewhisperer/activation.ts @@ -69,6 +69,7 @@ import { debounceStartSecurityScan } from './commands/startSecurityScan' import { securityScanLanguageContext } from './util/securityScanLanguageContext' import { registerWebviewErrorHandler } from '../webviews/server' import { logAndShowWebviewError } from '../shared/utilities/logAndShowUtils' +import { openSettings } from '../shared/settings' let localize: nls.LocalizeFunc @@ -190,7 +191,7 @@ export async function activate(context: ExtContext): Promise { `@id:amazonQ.showInlineCodeSuggestionsWithCodeReferences` ) } else { - await vscode.commands.executeCommand('workbench.action.openSettings', `amazonQ`) + await openSettings('amazonQ') } }), Commands.register('aws.amazonq.refreshAnnotation', async (forceProceed: boolean = false) => { diff --git a/packages/core/src/shared/settings.ts b/packages/core/src/shared/settings.ts index 263992f96ef..97e53a122b2 100644 --- a/packages/core/src/shared/settings.ts +++ b/packages/core/src/shared/settings.ts @@ -894,11 +894,16 @@ export async function migrateSetting( await migrateForScope(vscode.ConfigurationTarget.Global) } +/** Opens the settings UI filtered by the given prefix. */ +export async function openSettings(prefix: string): Promise { + await vscode.commands.executeCommand('workbench.action.openSettings', prefix) +} + /** * Opens the settings UI at the specified key. * * This only works for keys that are considered "top-level", e.g. keys of {@link settingsProps}. */ -export async function openSettings(key: K): Promise { +export async function openSettingsId(key: K): Promise { await vscode.commands.executeCommand('workbench.action.openSettings', `@id:${key}`) } diff --git a/packages/core/src/shared/telemetry/activation.ts b/packages/core/src/shared/telemetry/activation.ts index 8ea220d035f..07e719fb21b 100644 --- a/packages/core/src/shared/telemetry/activation.ts +++ b/packages/core/src/shared/telemetry/activation.ts @@ -13,7 +13,7 @@ import { AwsContext } from '../awsContext' import { DefaultTelemetryService } from './telemetryService' import { getLogger } from '../logger' import { getComputeRegion, isAmazonQ, isCloud9, productName } from '../extensionUtilities' -import { openSettings, Settings } from '../settings' +import { openSettingsId, Settings } from '../settings' import { TelemetryConfig, setupTelemetryId } from './util' import { isAutomation, isReleaseVersion } from '../vscode/env' import { AWSProduct } from './clienttelemetry' @@ -133,7 +133,7 @@ export async function handleTelemetryNoticeResponse( // noticeResponseOk is a no-op if (response === noticeResponseViewSettings) { - await openSettings(isAmazonQ() ? 'amazonQ.telemetry' : 'aws.telemetry') + await openSettingsId(isAmazonQ() ? 'amazonQ.telemetry' : 'aws.telemetry') } } catch (err) { getLogger().error('Error while handling response from telemetry notice: %O', err as Error) From a7418e4d20f471cb603d47d384a1e6249e8755c3 Mon Sep 17 00:00:00 2001 From: "Justin M. Keyes" Date: Sat, 27 Apr 2024 12:03:31 -0700 Subject: [PATCH 2/2] 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. - For very old vscode (older than 1.74), default to "debug" log-level. https://github.com/microsoft/vscode/commit/2cd8ea24f2e227bba112343246f3bee7dc28c719 --- CONTRIBUTING.md | 12 ++- packages/amazonq/package.json | 24 ----- packages/core/package.json | 24 ----- packages/core/package.nls.json | 2 - .../commands/startSecurityScan.ts | 1 + packages/core/src/shared/logger/activation.ts | 93 +++++++------------ packages/core/src/shared/logger/logger.ts | 29 +++++- .../shared/logger/outputChannelTransport.ts | 8 +- .../src/test/shared/logger/activation.test.ts | 2 +- packages/core/src/test/techdebt.test.ts | 20 ---- 10 files changed, 69 insertions(+), 146 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 51eba829681..2f4a54277d9 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -315,9 +315,14 @@ The `aws.dev.forceDevMode` setting enables or disables Toolkit "dev mode". Witho ### Logging -- Use the `aws.dev.logfile` setting to set the logfile path to a fixed location, so you can easily - follow and filter the logfile using shell tools like `tail` and `grep`. For example in - settings.json, +- Use `getLogger()` to log debugging messages, warnings, etc. + - Example: `getLogger().error('topic: widget failed: %O', { foo: 'bar', baz: 42 })` +- Log messages are written to the extension Output channel, which you can view in vscode by visiting the "Output" panel and selecting `AWS Toolkit Logs` or `Amazon Q Logs`. +- While viewing the Output channel (`AWS Toolkit Logs` or `Amazon Q Logs`) in vscode: + - Click the "gear" icon to [select a log level](https://github.com/aws/aws-toolkit-vscode/pull/4859) ("Debug", "Info", "Error", …). + - Click the "..." icon to open the log file. +- Use the `aws.dev.logfile` setting to set the logfile path to a fixed location, so you can follow + and filter logs using shell tools like `tail` and `grep`. For example in settings.json, ``` "aws.dev.logfile": "~/awstoolkit.log", ``` @@ -328,7 +333,6 @@ 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". - Enter text in the Debug Console filter box to show only log messages with that text.
VSCode Debug Console diff --git a/packages/amazonq/package.json b/packages/amazonq/package.json index 664b650eb7d..c27bd97657e 100644 --- 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 index 3ec2ccff671..314b258108d 100644 --- 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/package.nls.json b/packages/core/package.nls.json index 1d7ece8f0f2..46a3af48e3e 100644 --- a/packages/core/package.nls.json +++ b/packages/core/package.nls.json @@ -9,8 +9,6 @@ "AWS.configuration.profileDescription": "The name of the credential profile to obtain credentials from.", "AWS.configuration.description.lambda.recentlyUploaded": "Recently selected Lambda upload targets.", "AWS.configuration.description.ecs.openTerminalCommand": "The command to run when starting a new interactive terminal session.", - "AWS.configuration.description.logLevel": "AWS IDE Extensions log level (changes reflected on restart)", - "AWS.configuration.description.logLevel.cn": "The Amazon Toolkit's log level (changes reflected on restart)", "AWS.configuration.description.iot.maxItemsPerPage": "Controls how many IoT Things, Certificates, or Policies are listed before showing a node to `Load More...`.", "AWS.configuration.description.s3.maxItemsPerPage": "Controls how many S3 items are listed before showing a node to `Load More...`.\nThis corresponds to the `MaxKeys` requested in a single call to S3. [Learn More](https://docs.aws.amazon.com/AmazonS3/latest/API/API_ListObjectsV2.html#AmazonS3-ListObjectsV2-response-MaxKeys)", "AWS.configuration.description.samcli.lambdaTimeout": "Maximum time (in milliseconds) to wait for SAM output while starting a Local Lambda session", diff --git a/packages/core/src/codewhisperer/commands/startSecurityScan.ts b/packages/core/src/codewhisperer/commands/startSecurityScan.ts index 2997c80fd54..cb01a1d97ab 100644 --- 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 index 59f7c53943a..9c7926ce955 100644 --- 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 index 551b0db997b..91fd5c732da 100644 --- 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,27 @@ const logLevels = new Map([ export type LogLevel = 'error' | 'warn' | 'info' | 'verbose' | 'debug' +export function fromVscodeLogLevel(logLevel: vscode.LogLevel): LogLevel { + if (!vscode.LogLevel) { + // vscode version <= 1.73 + return 'info' + } + + switch (logLevel) { + case vscode.LogLevel.Trace: + case vscode.LogLevel.Debug: + return 'debug' + case vscode.LogLevel.Info: + return 'info' + case vscode.LogLevel.Warning: + return 'warn' + case vscode.LogLevel.Error: + case vscode.LogLevel.Off: + default: + return 'error' + } +} + /** * Compares log levels. * @@ -96,7 +117,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 +152,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 index 6196dd87ffc..bffca0d8bf0 100644 --- 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 index 56c8d07f550..3cd072573b0 100644 --- 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 index 72986edc9e4..a6d18dbcb58 100644 --- a/packages/core/src/test/techdebt.test.ts +++ b/packages/core/src/test/techdebt.test.ts @@ -6,8 +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 // when possible. @@ -35,22 +33,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)' - ) - }) })