Logger activation is more consistent with other activation code, no longer tailored for use by tests (#784)

Author: awschristou

src/extension.ts

@@ -21,7 +21,7 @@ import { EnvironmentVariables } from './shared/environmentVariables'
 import { ext } from './shared/extensionGlobals'
 import { safeGet, showQuickStartWebview, toastNewUser } from './shared/extensionUtilities'
 import { getLogger } from './shared/logger'
-import { initialize as activateLogger } from './shared/logger/activation'
+import { activate as activateLogger } from './shared/logger/activation'
 import { DefaultRegionProvider } from './shared/regions/defaultRegionProvider'
 import { activate as activateServerless } from './shared/sam/activation'
 import { DefaultSettingsConfiguration } from './shared/settingsConfiguration'

src/shared/logger/activation.ts

@@ -8,7 +8,7 @@ import * as os from 'os'
 import * as path from 'path'
 import * as vscode from 'vscode'
 import * as nls from 'vscode-nls'
-import { getLogger, Logger, LogLevel } from '.'
+import { Logger, LogLevel } from '.'
 import { extensionSettingsPrefix } from '../constants'
 import { mkdir } from '../filesystem'
 import { fileExists } from '../filesystemUtilities'
@@ -19,113 +19,54 @@ import { WinstonToolkitLogger } from './winstonToolkitLogger'
 
 const localize = nls.loadMessageBundle()
 
-const LOG_RELATIVE_PATH: string = path.join('Code', 'logs', 'aws_toolkit')
+const LOG_PATH = path.join(getLogBasePath(), 'Code', 'logs', 'aws_toolkit', makeLogFilename())
 const DEFAULT_LOG_LEVEL: LogLevel = 'info'
-const DEFAULT_LOG_NAME: string = makeDefaultLogName()
-const DEFAULT_OUTPUT_CHANNEL: vscode.OutputChannel = vscode.window.createOutputChannel('AWS Toolkit Logs')
+const LOG_OUTPUT_CHANNEL: vscode.OutputChannel = vscode.window.createOutputChannel('AWS Toolkit Logs')
 
 /**
- * logPath is not required (as Winston will work without a file path defined) but will output errors to stderr.
+ * Activate Logger functionality for the extension.
  */
-export interface LoggerParams {
-    outputChannel?: vscode.OutputChannel
-    logPath?: string
-    logLevel?: LogLevel
-}
+export async function activate(): Promise<void> {
+    const outputChannel = LOG_OUTPUT_CHANNEL
+    const logPath = LOG_PATH
+    const logLevel = getLogLevel()
 
-/**
- * @param params: LoggerParams
- * Creates the "default logger" (returnable here and through getLogger) using specified parameters or default values.
- * Initializing again will create a new default logger
- * --however, existing logger objects using the old default logger will be unaffected.
- */
-export async function initialize(params?: LoggerParams): Promise<Logger> {
-    let outputChannel: vscode.OutputChannel | undefined
-    let logPath: string | undefined
-
-    if (!params) {
-        outputChannel = DEFAULT_OUTPUT_CHANNEL
-        logPath = getDefaultLogPath()
-
-        const logFolder = path.dirname(logPath)
-        if (!(await fileExists(logFolder))) {
-            await mkdir(logFolder, { recursive: true })
-        }
-
-        // TODO : Determine log level here, then createLogger calls in this method can converge
-        setLogger(
-            createLogger({
-                outputChannel,
-                logPath
-            })
-        )
-        // only the default logger (with default params) gets a registered command
-        // check list of registered commands to see if aws.viewLogs has already been registered.
-        // if so, don't register again--this will cause an error visible to the user.
-        for (const command of await vscode.commands.getCommands(true)) {
-            if (command === 'aws.viewLogs') {
-                return getLogger()
-            }
-        }
-        registerCommand({
-            command: 'aws.viewLogs',
-            callback: async () => await vscode.window.showTextDocument(vscode.Uri.file(path.normalize(logPath!)))
-        })
-    } else {
-        outputChannel = params.outputChannel
-        logPath = params.logPath
+    await ensureLogFolderExists(path.dirname(logPath))
 
-        setLogger(createLogger(params))
-    }
+    setLogger(makeLogger(logLevel, logPath, outputChannel))
 
-    if (outputChannel && logPath) {
-        outputChannel.appendLine(
-            localize('AWS.log.fileLocation', 'Error logs for this session are permanently stored in {0}', logPath)
-        )
-    }
+    await registerLoggerCommands()
 
-    return getLogger()
+    outputChannel.appendLine(
+        localize('AWS.log.fileLocation', 'Error logs for this session are permanently stored in {0}', logPath)
+    )
 }
 
-/**
- * @param params: LoggerParams--nothing is required, but a LogPath is highly recommended so Winston doesn't throw errors
- *
- * Outputs a logger object that isn't stored anywhere--it's up to the caller to keep track of this.
- */
-export function createLogger(params: LoggerParams): Logger {
-    // TODO : log level should be a concern of the caller
-    let level: LogLevel
-    if (params.logLevel) {
-        level = params.logLevel
-    } else {
-        const configuration: SettingsConfiguration = new DefaultSettingsConfiguration(extensionSettingsPrefix)
-        const setLevel = configuration.readSetting<string>('logLevel')
-        level = setLevel ? (setLevel as LogLevel) : DEFAULT_LOG_LEVEL
-    }
+export function makeLogger(logLevel: LogLevel, logPath: string, outputChannel: vscode.OutputChannel): Logger {
+    const logger = new WinstonToolkitLogger(logLevel)
+    logger.logToFile(logPath)
+    logger.logToOutputChannel(outputChannel)
 
-    const logger = new WinstonToolkitLogger(level)
-    if (params.logPath) {
-        logger.logToFile(params.logPath)
-    }
+    return logger
+}
 
-    if (params.outputChannel) {
-        logger.logToOutputChannel(params.outputChannel)
-    }
+function getLogLevel(): LogLevel {
+    const configuration: SettingsConfiguration = new DefaultSettingsConfiguration(extensionSettingsPrefix)
 
-    return logger
+    return configuration.readSetting<LogLevel>('logLevel', DEFAULT_LOG_LEVEL)
 }
 
-function getDefaultLogPath(): string {
+function getLogBasePath(): string {
     if (os.platform() === 'win32') {
-        return path.join(os.homedir(), 'AppData', 'Roaming', LOG_RELATIVE_PATH, DEFAULT_LOG_NAME)
+        return path.join(os.homedir(), 'AppData', 'Roaming')
     } else if (os.platform() === 'darwin') {
-        return path.join(os.homedir(), 'Library', 'Application Support', LOG_RELATIVE_PATH, DEFAULT_LOG_NAME)
+        return path.join(os.homedir(), 'Library', 'Application Support')
     } else {
-        return path.join(os.homedir(), '.config', LOG_RELATIVE_PATH, DEFAULT_LOG_NAME)
+        return path.join(os.homedir(), '.config')
     }
 }
 
-function makeDefaultLogName(): string {
+function makeLogFilename(): string {
     const m = moment()
     const date = m.format('YYYYMMDD')
     const time = m.format('HHmmss')
@@ -134,3 +75,16 @@ function makeDefaultLogName(): string {
 
     return `aws_toolkit_${datetime}.log`
 }
+
+async function ensureLogFolderExists(logFolder: string): Promise<void> {
+    if (!(await fileExists(logFolder))) {
+        await mkdir(logFolder, { recursive: true })
+    }
+}
+
+async function registerLoggerCommands(): Promise<void> {
+    registerCommand({
+        command: 'aws.viewLogs',
+        callback: async () => await vscode.window.showTextDocument(vscode.Uri.file(path.normalize(LOG_PATH)))
+    })
+}

src/test/shared/logger/activation.test.ts

@@ -4,21 +4,30 @@
  */
 
 import * as assert from 'assert'
+import * as del from 'del'
+import { join } from 'path'
+import { makeTemporaryToolkitFolder } from '../../../shared/filesystemUtilities'
 import { Logger } from '../../../shared/logger'
-import { createLogger } from '../../../shared/logger/activation'
+import { makeLogger } from '../../../shared/logger/activation'
 import { WinstonToolkitLogger } from '../../../shared/logger/winstonToolkitLogger'
+import { MockOutputChannel } from '../../mockOutputChannel'
 
-describe('createLogger', () => {
+describe('makeLogger', () => {
+    let tempFolder: string
     let testLogger: Logger | undefined
 
     before(async () => {
-        testLogger = createLogger({
-            logLevel: 'debug'
-        })
+        tempFolder = await makeTemporaryToolkitFolder()
+        testLogger = makeLogger('debug', join(tempFolder, 'log.txt'), new MockOutputChannel())
     })
 
     after(async () => {
+        if (testLogger && testLogger instanceof WinstonToolkitLogger) {
+            testLogger.dispose()
+        }
+
         testLogger = undefined
+        await del([tempFolder], { force: true })
     })
 
     it('creates a logger object', () => {