feat(logging): topic header for logs (#5610)

## Problem
Log message can appear in output channels(such as Amazon Q Logs as
shown), IDE Debug console, or write to pre-defined log file
destinations. They come from all parts of our services and can amount to
large and lengthy chunks quickly.

We don’t have a reliable way to identify log messages from various
services and modules of our product. Some messages might have headers
while others don’t, and the existing headers are hard-coded on a message
by message basis, with no checks for consistency.

1. It’s difficult to locate and trace unidentified log messages when the
size of logs grow quickly.
2. Without regulation, the hard-coded custom headers can lead to legacy
issues down-the-road and inconsistencies within the codebase.

## Solution
1. Make message topic identifiers an input variable of the getLogger()
function.
2. Append the topic at the front of each message, for example:
`getLogger('S3').error('Something went wrong')`
Would print:
`S3: Something went wrong`
3. Define a set of topics to be used in message headers, covering all
services and modules.


## Implementation notes:
1. The `WinstonToolkitLogger` is renamed to `ToolkitLogger` to avoid
confusion, since its our own logger not winston's. The log topic feature
is added here.
2. A `baseLogger` abstract class is created to make the existing
`NullLogger`, `ConsoleLogger`, and `ToolkitLogger` cleaner and their
responsibilities more obvious and compact.
3. Currently the topics are optional, and the values are just `Test` and
`Unknown`, this will be expanded during migration of existing logger
calls.

---

<!--- REMINDER: Ensure that your PR meets the guidelines in
CONTRIBUTING.md -->

License: I confirm that my contribution is made under the terms of the
Apache 2.0 license.

---------

Co-authored-by: Nikolas Komonen <[email protected]>

Author: tomcat323

packages/core/src/shared/errors.ts

@@ -140,23 +140,28 @@ export class ToolkitError extends Error implements ErrorInformation {
      * sensitive information and should be limited in technical detail.
      */
     public override readonly message: string
-    public readonly code = this.info.code
-    public readonly details = this.info.details
+    public readonly code: string | undefined
+    public readonly details: Record<string, unknown> | undefined
 
     /**
      * We guard against mutation to stop a developer from creating a circular chain of errors.
      * The alternative is to truncate errors to an arbitrary depth though that doesn't address
      * why the error chain is deep.
      */
-    readonly #cause = this.info.cause
-    readonly #name = this.info.name ?? super.name
+    readonly #cause: Error | undefined
+    readonly #name: string
+    readonly #documentationUri: any
+    readonly #cancelled: boolean | undefined
 
-    public constructor(
-        message: string,
-        protected readonly info: ErrorInformation = {}
-    ) {
+    public constructor(message: string, info: ErrorInformation = {}) {
         super(message)
         this.message = message
+        this.code = info.code
+        this.details = info.details
+        this.#cause = info.cause
+        this.#name = info.name ?? super.name
+        this.#cancelled = info.cancelled
+        this.#documentationUri = info.documentationUri
     }
 
     /**
@@ -180,14 +185,14 @@ export class ToolkitError extends Error implements ErrorInformation {
      * assignment on construction or by finding a 'cancelled' error within its causal chain.
      */
     public get cancelled(): boolean {
-        return this.info.cancelled ?? isUserCancelledError(this.cause)
+        return this.#cancelled ?? isUserCancelledError(this.cause)
     }
 
     /**
      * The associated documentation, if it exists. Otherwise undefined.
      */
     public get documentationUri(): vscode.Uri | undefined {
-        return this.info.documentationUri
+        return this.#documentationUri
     }
 
     /**

packages/core/src/shared/logger/activation.ts

@@ -6,7 +6,7 @@
 import * as vscode from 'vscode'
 import { Logger, LogLevel, getLogger } from '.'
 import { fromVscodeLogLevel, setLogger } from './logger'
-import { WinstonToolkitLogger } from './winstonToolkitLogger'
+import { ToolkitLogger } from './toolkitLogger'
 import { Settings } from '../settings'
 import { Logging } from './commands'
 import { resolvePath } from '../utilities/pathUtils'
@@ -79,7 +79,7 @@ export function makeLogger(opts: {
     outputChannels?: vscode.OutputChannel[]
     useConsoleLog?: boolean
 }): Logger {
-    const logger = new WinstonToolkitLogger(opts.logLevel)
+    const logger = new ToolkitLogger(opts.logLevel)
     // debug console can show ANSI colors, output channels can not
     for (const logPath of opts.logPaths ?? []) {
         logger.logToFile(logPath)

packages/core/src/shared/logger/logger.ts

@@ -4,26 +4,20 @@
  */
 
 import * as vscode from 'vscode'
+import { LogTopic, type ToolkitLogger } from './toolkitLogger'
 
 const toolkitLoggers: {
     main: Logger | undefined
-    channel: Logger | undefined
     debugConsole: Logger | undefined
-} = { main: undefined, channel: undefined, debugConsole: undefined }
+} = { main: undefined, debugConsole: undefined }
 
 export interface Logger {
-    debug(message: string, ...meta: any[]): number
-    debug(error: Error, ...meta: any[]): number
-    verbose(message: string, ...meta: any[]): number
-    verbose(error: Error, ...meta: any[]): number
-    info(message: string, ...meta: any[]): number
-    info(error: Error, ...meta: any[]): number
-    warn(message: string, ...meta: any[]): number
-    warn(error: Error, ...meta: any[]): number
-    error(message: string, ...meta: any[]): number
-    error(error: Error, ...meta: any[]): number
-    log(logLevel: LogLevel, message: string, ...meta: any[]): number
-    log(logLevel: LogLevel, error: Error, ...meta: any[]): number
+    debug(message: string | Error, ...meta: any[]): number
+    verbose(message: string | Error, ...meta: any[]): number
+    info(message: string | Error, ...meta: any[]): number
+    warn(message: string | Error, ...meta: any[]): number
+    error(message: string | Error, ...meta: any[]): number
+    log(logLevel: LogLevel, message: string | Error, ...meta: any[]): number
     setLogLevel(logLevel: LogLevel): void
     /** Returns true if the given log level is being logged.  */
     logLevelEnabled(logLevel: LogLevel): boolean
@@ -32,6 +26,38 @@ export interface Logger {
     enableDebugConsole(): void
 }
 
+export abstract class BaseLogger implements Logger {
+    debug(message: string | Error, ...meta: any[]): number {
+        return this.sendToLog('debug', message, meta)
+    }
+    verbose(message: string | Error, ...meta: any[]): number {
+        return this.sendToLog('verbose', message, meta)
+    }
+    info(message: string | Error, ...meta: any[]): number {
+        return this.sendToLog('info', message, meta)
+    }
+    warn(message: string | Error, ...meta: any[]): number {
+        return this.sendToLog('warn', message, meta)
+    }
+    error(message: string | Error, ...meta: any[]): number {
+        return this.sendToLog('error', message, meta)
+    }
+    log(logLevel: LogLevel, message: string | Error, ...meta: any[]): number {
+        return this.sendToLog(logLevel, message, ...meta)
+    }
+    abstract setLogLevel(logLevel: LogLevel): void
+    abstract logLevelEnabled(logLevel: LogLevel): boolean
+    abstract getLogById(logID: number, file: vscode.Uri): string | undefined
+    /** HACK: Enables logging to vscode Debug Console. */
+    abstract enableDebugConsole(): void
+
+    abstract sendToLog(
+        logLevel: 'debug' | 'verbose' | 'info' | 'warn' | 'error',
+        message: string | Error,
+        ...meta: any[]
+    ): number
+}
+
 /**
  * Log levels ordered for comparison.
  *
@@ -85,107 +111,91 @@ export function compareLogLevel(l1: LogLevel, l2: LogLevel): number {
 /**
  * Gets the logger if it has been initialized
  * the logger is of `'main'` or `undefined`: Main logger; default impl: logs to log file and log output channel
+ * @param topic: topic to be appended in front of the message.
  */
-export function getLogger(): Logger {
+export function getLogger(topic?: LogTopic): Logger {
     const logger = toolkitLoggers['main']
     if (!logger) {
         return new ConsoleLogger()
     }
-
+    /**
+     * need this check so that setTopic can be recognized
+     * using instanceof would lead to dependency loop error
+     */
+    if (isToolkitLogger(logger)) {
+        logger.setTopic(topic)
+    }
     return logger
 }
 
-export function getDebugConsoleLogger(): Logger {
-    return toolkitLoggers['debugConsole'] ?? new ConsoleLogger()
+/**
+ * check if the logger is of type `ToolkitLogger`. This avoids Circular Dependencies, but `instanceof ToolkitLogger` is preferred.
+ * @param logger
+ * @returns bool, true if is `ToolkitLogger`
+ */
+function isToolkitLogger(logger: Logger): logger is ToolkitLogger {
+    return 'setTopic' in logger && typeof logger.setTopic === 'function'
+}
+
+export function getDebugConsoleLogger(topic?: LogTopic): Logger {
+    const logger = toolkitLoggers['debugConsole']
+    if (!logger) {
+        return new ConsoleLogger()
+    }
+    if (isToolkitLogger(logger)) {
+        logger.setTopic(topic)
+    }
+    return logger
 }
 
-export class NullLogger implements Logger {
+export class NullLogger extends BaseLogger {
     public setLogLevel(logLevel: LogLevel) {}
     public logLevelEnabled(logLevel: LogLevel): boolean {
         return false
     }
-    public log(logLevel: LogLevel, message: string | Error, ...meta: any[]): number {
-        return 0
-    }
-    public debug(message: string | Error, ...meta: any[]): number {
-        return 0
-    }
-    public verbose(message: string | Error, ...meta: any[]): number {
-        return 0
-    }
-    public info(message: string | Error, ...meta: any[]): number {
-        return 0
-    }
-    public warn(message: string | Error, ...meta: any[]): number {
-        return 0
-    }
-    public error(message: string | Error, ...meta: any[]): number {
-        return 0
-    }
     public getLogById(logID: number, file: vscode.Uri): string | undefined {
         return undefined
     }
     public enableDebugConsole(): void {}
+    override sendToLog(
+        logLevel: 'error' | 'warn' | 'info' | 'verbose' | 'debug',
+        message: string | Error,
+        ...meta: any[]
+    ): number {
+        return 0
+    }
 }
 
 /**
  * Fallback used if {@link getLogger()} is requested before logging is fully initialized.
  */
-export class ConsoleLogger implements Logger {
+export class ConsoleLogger extends BaseLogger {
     public setLogLevel(logLevel: LogLevel) {}
     public logLevelEnabled(logLevel: LogLevel): boolean {
         return false
     }
-    public log(logLevel: LogLevel, message: string | Error, ...meta: any[]): number {
-        switch (logLevel) {
-            case 'error':
-                this.error(message, ...meta)
-                return 0
-            case 'warn':
-                this.warn(message, ...meta)
-                return 0
-            case 'verbose':
-                this.verbose(message, ...meta)
-                return 0
-            case 'debug':
-                this.debug(message, ...meta)
-                return 0
-            case 'info':
-            default:
-                this.info(message, ...meta)
-                return 0
-        }
-    }
-    public debug(message: string | Error, ...meta: any[]): number {
-        // eslint-disable-next-line aws-toolkits/no-console-log
-        console.debug(message, ...meta)
-        return 0
-    }
-    public verbose(message: string | Error, ...meta: any[]): number {
-        // eslint-disable-next-line aws-toolkits/no-console-log
-        console.debug(message, ...meta)
-        return 0
-    }
-    public info(message: string | Error, ...meta: any[]): number {
-        // eslint-disable-next-line aws-toolkits/no-console-log
-        console.info(message, ...meta)
-        return 0
-    }
-    public warn(message: string | Error, ...meta: any[]): number {
-        // eslint-disable-next-line aws-toolkits/no-console-log
-        console.warn(message, ...meta)
-        return 0
-    }
-    /** Note: In nodejs this prints to `stderr` (see {@link Console.error}). */
-    public error(message: string | Error, ...meta: any[]): number {
-        // eslint-disable-next-line aws-toolkits/no-console-log
-        console.error(message, ...meta)
-        return 0
-    }
     public getLogById(logID: number, file: vscode.Uri): string | undefined {
         return undefined
     }
     public enableDebugConsole(): void {}
+    override sendToLog(
+        logLevel: 'error' | 'warn' | 'info' | 'verbose' | 'debug',
+        message: string | Error,
+        ...meta: any[]
+    ): number {
+        /**
+         * This is here because we pipe verbose to debug currentlly
+         * TODO: remove in the next stage
+         */
+        if (logLevel === 'verbose') {
+            // eslint-disable-next-line aws-toolkits/no-console-log
+            console.debug(message, ...meta)
+        } else {
+            // eslint-disable-next-line aws-toolkits/no-console-log
+            console[logLevel](message, ...meta)
+        }
+        return 0
+    }
 }
 
 export function getNullLogger(type?: 'debugConsole' | 'main'): Logger {

packages/core/src/shared/logger/sharedFileTransport.ts

@@ -8,12 +8,12 @@ import fs from '../fs/fs'
 import * as vscode from 'vscode'
 import globals from '../extensionGlobals'
 import { MESSAGE } from './consoleLogTransport'
-import { WinstonToolkitLogger } from './winstonToolkitLogger'
+import { ToolkitLogger } from './toolkitLogger'
 
 interface LogEntry {
     level: string
     message: string
-    /** This is the formatted message from {@link WinstonToolkitLogger} in the winston.createLogger() call */
+    /** This is the formatted message from {@link ToolkitLogger} in the winston.createLogger() call */
     [MESSAGE]: string
 }