From af04c73d31ebc84ff872d083e1ae7daa2002b3f6 Mon Sep 17 00:00:00 2001 From: Tom Zu Date: Fri, 13 Sep 2024 10:22:15 -0400 Subject: [PATCH 1/2] implemented topic logger --- packages/core/src/shared/logger/logger.ts | 135 ++++++++++++++++++---- 1 file changed, 114 insertions(+), 21 deletions(-) diff --git a/packages/core/src/shared/logger/logger.ts b/packages/core/src/shared/logger/logger.ts index 0bf0e9dbaa6..8542b4a08cb 100644 --- a/packages/core/src/shared/logger/logger.ts +++ b/packages/core/src/shared/logger/logger.ts @@ -5,25 +5,20 @@ import * as vscode from 'vscode' +/** define log topics */ +type LogTopic = 'Unknown' | 'Test' + 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 setLogLevel(logLevel: LogLevel): void /** Returns true if the given log level is being logged. */ logLevelEnabled(logLevel: LogLevel): boolean @@ -32,6 +27,85 @@ export interface Logger { enableDebugConsole(): void } +/** + * Base Logger class + * Used as a wrapper around the logger interface for appending messages + * Also more compatible with future change + */ +abstract class baseLogger implements Logger { + abstract coreLogger: 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) + } + setLogLevel(logLevel: LogLevel): void { + this.coreLogger.setLogLevel(logLevel) + } + logLevelEnabled(logLevel: LogLevel): boolean { + return this.coreLogger.logLevelEnabled(logLevel) + } + getLogById(logID: number, file: vscode.Uri): string | undefined { + return this.coreLogger.getLogById(logID, file) + } + /** HACK: Enables logging to vscode Debug Console. */ + enableDebugConsole(): void { + this.coreLogger.enableDebugConsole() + } + abstract sendToLog( + type: 'debug' | 'verbose' | 'info' | 'warn' | 'error', + message: string | Error, + ...meta: any[] + ): number +} +/** + * Logger with topic headers + * + * @param topic identifies the message topic, appended to the front of message followed by a colen. + * @param coreLogger the actual logger it wraps around, in this case the 'main' logger + */ +export class TopicLogger extends baseLogger { + private topic: LogTopic + override coreLogger: Logger + /** Default topic is 'Unknown' */ + constructor(logger: Logger, topic: LogTopic = 'Unknown') { + super() + this.coreLogger = logger + this.topic = topic + } + /** Format the message with topic header */ + private addTopicToMessage(message: string | Error): string | Error { + const topicPrefix = `${this.topic}: ` + if (typeof message === 'string') { + return topicPrefix + message + } else if (message instanceof Error) { + /** Create a new Error object to avoid modifying the original */ + const topicError = new Error(topicPrefix + message.message) + topicError.name = message.name + topicError.stack = message.stack + return topicError + } + return message + } + override sendToLog( + type: 'debug' | 'verbose' | 'info' | 'warn' | 'error', + message: string | Error, + ...meta: any[] + ): number { + return this.coreLogger[type](this.addTopicToMessage(message), meta) + } +} + /** * Log levels ordered for comparison. * @@ -85,18 +159,37 @@ 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 identifies the message topic, appended to the front of message followed by a colen. */ -export function getLogger(): Logger { - const logger = toolkitLoggers['main'] - if (!logger) { +export function getLogger(topic?: LogTopic): Logger { + const coreLogger = toolkitLoggers['main'] + if (!coreLogger) { return new ConsoleLogger() } - - return logger + /** + * Temperpory check so we don't get a million Unknown logs + * TODO: remove once the majority of log calls are migrated + */ + if (!topic) { + return coreLogger + } + return new TopicLogger(coreLogger, topic) } -export function getDebugConsoleLogger(): Logger { - return toolkitLoggers['debugConsole'] ?? new ConsoleLogger() +/** + * Gets the logger if it has been initialized + * the logger is of `'debugConsole', logs to IDE debug console channel + * @param topic identifies the message topic, appended to the front of message followed by a colen. + */ +export function getDebugConsoleLogger(topic?: LogTopic): Logger { + const baseLogger = toolkitLoggers['debugConsole'] + if (!baseLogger) { + return new ConsoleLogger() + } + if (!topic) { + return baseLogger + } + return new TopicLogger(baseLogger, topic) } export class NullLogger implements Logger { From cd136e7d668aebcfd4f5cb2b7a1e998ae599519f Mon Sep 17 00:00:00 2001 From: Tom Zu Date: Fri, 13 Sep 2024 10:22:48 -0400 Subject: [PATCH 2/2] Add test to topic logger --- .../test/shared/logger/topicLogger.test.ts | 71 +++++++++++++++++++ 1 file changed, 71 insertions(+) create mode 100644 packages/core/src/test/shared/logger/topicLogger.test.ts diff --git a/packages/core/src/test/shared/logger/topicLogger.test.ts b/packages/core/src/test/shared/logger/topicLogger.test.ts new file mode 100644 index 00000000000..2e10b42995f --- /dev/null +++ b/packages/core/src/test/shared/logger/topicLogger.test.ts @@ -0,0 +1,71 @@ +/*! + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * SPDX-License-Identifier: Apache-2.0 + */ + +import assert from 'assert' +import { stub, SinonStubbedInstance } from 'sinon' +import { TopicLogger, Logger, LogLevel } from '../../../shared/logger/logger' + +describe('TopicLogger', () => { + let mockCoreLogger: SinonStubbedInstance + let topicLogger: TopicLogger + + beforeEach(() => { + mockCoreLogger = { + debug: stub(), + verbose: stub(), + info: stub(), + warn: stub(), + error: stub(), + setLogLevel: stub(), + logLevelEnabled: stub(), + getLogById: stub(), + enableDebugConsole: stub(), + } as SinonStubbedInstance + topicLogger = new TopicLogger(mockCoreLogger as Logger, 'Test') + }) + + it('adds topic prefix to string type messages', () => { + topicLogger.info('Test message') + assert(mockCoreLogger.info.calledWith('Test: Test message')) + }) + + it('adds topic prefix to Error type messages', () => { + const testError = new Error('Test error') + topicLogger.error(testError) + assert(mockCoreLogger.error.calledOnce) + const calledArg = mockCoreLogger.error.firstCall.args[0] + assert(calledArg instanceof Error) + assert.strictEqual(calledArg.message, 'Test: Test error') + assert.strictEqual(calledArg.name, testError.name) + assert.strictEqual(calledArg.stack, testError.stack) + }) + + it('topic is "Unknown" when not specified', () => { + const defaultTopicLogger = new TopicLogger(mockCoreLogger as Logger) + defaultTopicLogger.debug('Verbose message') + assert(mockCoreLogger.debug.calledWith('Unknown: Verbose message')) + }) + + it('delegates setLogLevel to core logger', () => { + topicLogger.setLogLevel('debug' as LogLevel) + assert(mockCoreLogger.setLogLevel.calledWith('debug')) + }) + + it('delegates logLevelEnabled to core logger', () => { + topicLogger.logLevelEnabled('info' as LogLevel) + assert(mockCoreLogger.logLevelEnabled.calledWith('info')) + }) + + it('delegates getLogById to core logger', () => { + const mockUri = {} as any + topicLogger.getLogById(123, mockUri) + assert(mockCoreLogger.getLogById.calledWith(123, mockUri)) + }) + + it('delegates enableDebugConsole to core logger', () => { + topicLogger.enableDebugConsole() + assert(mockCoreLogger.enableDebugConsole.called) + }) +})