refactor(app-shell,app-shell-odd): update winston and improve logging

app-shell-odd/package.json

@@ -24,9 +24,7 @@
   },
   "homepage": "https://github.com/Opentrons/opentrons",
   "workspaces": {
-    "nohoist": [
-      "**"
-    ]
+    "nohoist": ["**"]
   },
   "devDependencies": {
     "@opentrons/app": "link:../app"
@@ -58,7 +56,7 @@
     "semver": "5.7.2",
     "tempy": "1.0.1",
     "uuid": "3.2.1",
-    "winston": "3.1.0",
+    "winston": "3.15.0",
     "yargs-parser": "13.1.2"
   }
 }

app-shell-odd/src/log.ts

@@ -1,20 +1,19 @@
 // create logger function
 import { inspect } from 'util'
-import fse from 'fs-extra'
 import path from 'path'
 import dateFormat from 'dateformat'
 import winston from 'winston'
 
 import { getConfig } from './config'
 
-import type Transport from 'winston-transport'
+import Transport from 'winston-transport'
 import type { Config } from './config'
 
 const ODD_DIR = '/data/ODD'
 const LOG_DIR = path.join(ODD_DIR, 'logs')
 const ERROR_LOG = path.join(LOG_DIR, 'error.log')
 const COMBINED_LOG = path.join(LOG_DIR, 'combined.log')
 const FILE_OPTIONS = {
   // JSON logs
   format: winston.format.json(),
   // 1 MB max log file size (to ensure emailablity)
@@ -25,66 +24,61 @@
   tailable: true,
 }
 
-let config: Config['log']
-let transports: Transport[]
-let log: winston.Logger
+// Use our own logger type because winston (a) doesn't allow these by default
+// but (b) does it by binding something other than a function to these props.
+export type OTLogger = Omit<
+  winston.Logger,
+  'emerg' | 'alert' | 'crit' | 'warning' | 'notice'
+>
 
-export function createLogger(filename: string): winston.Logger {
-  // eslint-disable-next-line @typescript-eslint/strict-boolean-expressions
-  if (!config) config = getConfig('log')
-  // eslint-disable-next-line @typescript-eslint/strict-boolean-expressions
-  if (!transports) initializeTransports()
-
-  return createWinstonLogger(filename)
+export function createLogger(label: string): OTLogger {
+  const rootLogger = ensureRootLogger()
+  return rootLogger.child({ label })
 }
 
-function initializeTransports(): void {
-  let error = null
+let _rootLog: OTLogger | null = null
 
-  // sync is ok here because this only happens once
-  try {
-    fse.ensureDirSync(LOG_DIR)
-  } catch (e: unknown) {
-    error = e
+function ensureRootLogger(): OTLogger {
+  if (_rootLog == null) {
+    return buildRootLogger()
+  } else {
+    return _rootLog
   }
+}
 
-  transports = createTransports()
-  log = createWinstonLogger('log')
+function buildRootLogger(): OTLogger {
+  const config = getConfig('log')
 
-  // eslint-disable-next-line @typescript-eslint/strict-boolean-expressions
-  if (error) log.error('Could not create log directory', { error })
-  log.info(`Level "error" and higher logging to ${ERROR_LOG}`)
-  log.info(`Level "${config.level.file}" and higher logging to ${COMBINED_LOG}`)
-  log.info(`Level "${config.level.console}" and higher logging to console`)
+  const transports = createTransports(config)
+
+  const formats = [
+    winston.format.timestamp(),
+    winston.format.metadata({
+      key: 'meta',
+      fillExcept: ['level', 'message', 'timestamp', 'label'],
+    }),
+  ]
+
+  _rootLog = winston.createLogger({
+    transports,
+    format: winston.format.combine(...formats),
+  })
+  const loggingLog = _rootLog.child({ label: 'logging' })
+  loggingLog.info(`Level "error" and higher logging to ${ERROR_LOG}`)
+  loggingLog.info(
+    `Level "${config.level.file}" and higher logging to ${COMBINED_LOG}`
+  )
+  loggingLog.info(
+    `Level "${config.level.console}" and higher logging to console`
+  )
+  return _rootLog
 }
 
-function createTransports(): Transport[] {
+function createTransports(config: Config['log']): Transport[] {
   const timeFromStamp = (ts: string): string =>
     dateFormat(new Date(ts), 'HH:MM:ss.l')
 
   return [
-    // error file log
-    new winston.transports.File(
-      Object.assign(
-        {
-          level: 'error',
-          filename: ERROR_LOG,
-        },
-        FILE_OPTIONS
-      )
-    ),
-
-    // regular combined file log
-    new winston.transports.File(
-      Object.assign(
-        {
-          level: config.level.file,
-          filename: COMBINED_LOG,
-        },
-        FILE_OPTIONS
-      )
-    ),
-
     // console log
     new winston.transports.Console({
       level: config.level.console,
@@ -105,22 +99,3 @@
     }),
   ]
 }
-
-function createWinstonLogger(label: string): winston.Logger {
-  // eslint-disable-next-line @typescript-eslint/strict-boolean-expressions, @typescript-eslint/prefer-optional-chain
-  log && log.debug(`Creating logger for ${label}`)
-
-  const formats = [
-    winston.format.label({ label }),
-    winston.format.timestamp(),
-    winston.format.metadata({
-      key: 'meta',
-      fillExcept: ['level', 'message', 'timestamp', 'label'],
-    }),
-  ]
-
-  return winston.createLogger({
-    transports,
-    format: winston.format.combine(...formats),
-  })
-}

app-shell/package.json

@@ -24,9 +24,7 @@
   },
   "homepage": "https://github.com/Opentrons/opentrons",
   "workspaces": {
-    "nohoist": [
-      "**"
-    ]
+    "nohoist": ["**"]
   },
   "devDependencies": {
     "electron-notarize": "^1.2.1",
@@ -68,7 +66,7 @@
     "tempy": "1.0.1",
     "usb": "^2.11.0",
     "uuid": "3.2.1",
-    "winston": "3.1.0",
+    "winston": "3.15.0",
     "yargs-parser": "13.1.2"
   }
 }