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

…#16516)

Update [winston](https://www.npmjs.com/package/winston/v/3.15.0) which
is what we use for logging on the node side to the most recent version
(3.15) so that we can use `logger.child()`, which lets you override
logger metadata without having to create another logger instance.

The reason to do this is that each full winston logger instance hangs
event listeners off its shared transports
(winstonjs/winston#1334) which results in
annoying node warning messages about memory leaks that aren't real and
are just based on "did you add more than a magic number of listeners to
this event". With the child logs, nothing adds the events, and we don't
get the warnings.

Also, get rid of the file logging to `/app/ODD/logs/error.log` and
`/app/ODD/logs/combined.log`, because we're already sending everything
to journald via the console and [using that to provide the logs via
http](https://github.com/Opentrons/opentrons/blob/edge/robot-server/robot_server/service/legacy/routers/logs.py#L16)
so it's just extra storage wear and space usage that we don't actually
need.

## testing
- [x] do the logs still go

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,6 +1,5 @@
 // create logger function
 import { inspect } from 'util'
-import fse from 'fs-extra'
 import path from 'path'
 import dateFormat from 'dateformat'
 import winston from 'winston'
@@ -14,77 +13,62 @@ 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)
-  maxsize: 1024 * 1024,
-  // keep 10 backups at most
-  maxFiles: 10,
-  // roll filenames in accending order (larger the number, older the log)
-  tailable: true,
-}
-
-let config: Config['log']
-let transports: Transport[]
-let log: winston.Logger
 
-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()
+// 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'
+>
 
-  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
   }
+}
+
+function buildRootLogger(): OTLogger {
+  const config = getConfig('log')
 
-  transports = createTransports()
-  log = createWinstonLogger('log')
+  const transports = createTransports(config)
+
+  const formats = [
+    winston.format.timestamp(),
+    winston.format.metadata({
+      key: 'meta',
+      fillExcept: ['level', 'message', 'timestamp', 'label'],
+    }),
+  ]
 
-  // 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`)
+  _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 +89,3 @@ function createTransports(): Transport[] {
     }),
   ]
 }
-
-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"
   }
 }