Skip to content

Commit

Permalink
fix: dont write timing logs to file unless requested
Browse files Browse the repository at this point in the history
  • Loading branch information
lukekarrys committed Apr 24, 2024
1 parent 7e04417 commit 2538438
Show file tree
Hide file tree
Showing 3 changed files with 29 additions and 16 deletions.
1 change: 1 addition & 0 deletions lib/npm.js
Original file line number Diff line number Diff line change
Expand Up @@ -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({
Expand Down
42 changes: 26 additions & 16 deletions lib/utils/log-file.js
Original file line number Diff line number Diff line change
@@ -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')
Expand All @@ -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
Expand All @@ -29,6 +28,7 @@ class LogFiles {
#path = null
#logsMax = null
#files = []
#timing = false

constructor ({
maxLogsPerFile = 50_000,
Expand All @@ -40,7 +40,6 @@ class LogFiles {
}

on () {
this.#logStream = new Minipass()
process.on('log', this.#logHandler)
}

Expand All @@ -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) {
Expand All @@ -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
}
}

Expand All @@ -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
}
Expand All @@ -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
}

Expand All @@ -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)
Expand Down
2 changes: 2 additions & 0 deletions test/lib/utils/log-file.js
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down

0 comments on commit 2538438

Please sign in to comment.