fix: dont write timing logs to file unless requested

npm · Apr 24, 2024 · 2538438 · 2538438
1 parent 7e04417
commit 2538438
Show file tree

Hide file tree

Showing 3 changed files with 29 additions and 16 deletions.
diff --git a/lib/npm.js b/lib/npm.js
@@ -204,6 +204,7 @@ class Npm {
     this.#logFile.load({
       path: this.logPath,
       logsMax: this.config.get('logs-max'),
+      timing: this.config.get('timing'),
     })
 
     this.#timers.load({

diff --git a/lib/utils/log-file.js b/lib/utils/log-file.js
@@ -1,6 +1,5 @@
 const os = require('os')
 const { join, dirname, basename } = require('path')
-const { Minipass } = require('minipass')
 const fsMiniPass = require('fs-minipass')
 const fs = require('fs/promises')
 const { log } = require('proc-log')
@@ -9,9 +8,9 @@ const { formatWithOptions } = require('./format')
 const padZero = (n, length) => n.toString().padStart(length.toString().length, '0')
 
 class LogFiles {
-  // Default to a plain minipass stream so we can buffer
+  // Default to an array so we can buffer
   // initial writes before we know the cache location
-  #logStream = null
+  #logStream = []
 
   // We cap log files at a certain number of log events per file.
   // Note that each log event can write more than one line to the
@@ -29,6 +28,7 @@ class LogFiles {
   #path = null
   #logsMax = null
   #files = []
+  #timing = false
 
   constructor ({
     maxLogsPerFile = 50_000,
@@ -40,7 +40,6 @@ class LogFiles {
   }
 
   on () {
-    this.#logStream = new Minipass()
     process.on('log', this.#logHandler)
   }
 
@@ -49,11 +48,12 @@ class LogFiles {
     this.#endStream()
   }
 
-  load ({ path, logsMax = Infinity } = {}) {
+  load ({ path, logsMax = Infinity, timing } = {}) {
     // dir is user configurable and is required to exist so
     // this can error if the dir is missing or not configured correctly
     this.#path = path
     this.#logsMax = logsMax
+    this.#timing = timing
 
     // Log stream has already ended
     if (!this.#logStream) {
@@ -62,13 +62,19 @@ class LogFiles {
 
     log.verbose('logfile', `logs-max:${logsMax} dir:${this.#path}`)
 
-    // Pipe our initial stream to our new file stream and
+    // Write the contents of our array buffer to our new file stream and
     // set that as the new log logstream for future writes
     // if logs max is 0 then the user does not want a log file
     if (this.#logsMax > 0) {
       const initialFile = this.#openLogFile()
       if (initialFile) {
-        this.#logStream = this.#logStream.pipe(initialFile)
+        for (const item of this.#logStream) {
+          const formatted = this.#formatLogItem(...item)
+          if (formatted !== null) {
+            initialFile.write(formatted)
+          }
+        }
+        this.#logStream = initialFile
       }
     }
 
@@ -80,20 +86,16 @@ class LogFiles {
     return this.#cleanLogs()
   }
 
-  log (...args) {
-    this.#logHandler(...args)
-  }
-
   get files () {
     return this.#files
   }
 
   get #isBuffered () {
-    return this.#logStream instanceof Minipass
+    return Array.isArray(this.#logStream)
   }
 
   #endStream (output) {
-    if (this.#logStream) {
+    if (this.#logStream && !this.#isBuffered) {
       this.#logStream.end(output)
       this.#logStream = null
     }
@@ -111,12 +113,15 @@ class LogFiles {
       return
     }
 
-    const logOutput = this.#formatLogItem(level, ...args)
-
     if (this.#isBuffered) {
       // Cant do anything but buffer the output if we dont
       // have a file stream yet
-      this.#logStream.write(logOutput)
+      this.#logStream.push([level, ...args])
+      return
+    }
+
+    const logOutput = this.#formatLogItem(level, ...args)
+    if (logOutput === null) {
       return
     }
 
@@ -137,6 +142,11 @@ class LogFiles {
   }
 
   #formatLogItem (level, title, ...args) {
+    // Only right timing logs to logfile if explicitly requests
+    if (level === log.KEYS.timing && !this.#timing) {
+      return null
+    }
+
     this.#fileLogCount += 1
     const prefix = [this.#totalLogCount++, level, title || null]
     return formatWithOptions({ prefix, eol: os.EOL, colors: false }, ...args)

diff --git a/test/lib/utils/log-file.js b/test/lib/utils/log-file.js
@@ -46,6 +46,8 @@ const loadLogFile = async (t, { buffer = [], mocks, testdir = {}, ...options } =
   const MockLogFile = tmock(t, '{LIB}/utils/log-file.js', mocks)
   const logFile = new MockLogFile(Object.keys(options).length ? options : undefined)
 
+  // Create a fake public method since there is not one on logFile anymore
+  logFile.log = (...b) => process.emit('log', ...b)
   buffer.forEach((b) => logFile.log(...b))
 
   const id = getId()