pinojs / pino

🌲 super fast, all natural json logger
http://getpino.io
MIT License
14.21k stars 875 forks source link

Preloading pino with -r flag causes memory leaks #1591

Closed MrEmanuel closed 1 year ago

MrEmanuel commented 1 year ago

As I guess many people do, I'm trying to override the standard console methods. It works great when I import or require my wrapper, but when I try to preload it, my app runs wild on CPU resources.

I've narrowed it down to have something to do with config.transport. I don't understand how workers function, and I don't understand much about preloading node modules and node's lifecycle, but it makes sense that something's wrong then trying to preload a module that start a worker. Maybe someone here can help me figure out how to get it working, if it's even possible?

What seems to happen is that my package, where I import and set up pino , gets called repeatedly. If I print "test" in my index file, where I setup pino I get a wall of "test" messages filling my terminal.

Here's my test example. Run npm start to see the issue. When I comment out pinoConfig.transport the error goes away.

package.json

{
  "name": "my-test",
  "version": "1.0.0",
  "description": "",
  "private": true,
  "scripts": {
    "start": "node -r pino-logger/overrideLogger index.js"
  },
  "author": "",
  "dependencies": {
    "pino-logger": "file:./pino-logger"
  }
}

index.js

// Sample code to keep the app running.
const readline = require('readline').createInterface({
  input: process.stdin,
  output: process.stdout,
})

readline.question('Who are you?', (name) => {
  console.log(`Hey there ${name}!`)
  readline.close()
})

/pino-logger/index.js

const pino = require('pino')
require('pino-pretty')
const pinoConfig = {
  name: process.env.npm_package_name, //appName, // Get the name defined in the parent application's package.json file.
  timestamp: false,
  base: undefined,
  transport: {
    target: 'pino-pretty',
    options: {
      colorize: true,
      singleLine: false,
    },
  },
}

console.log('test')

const pinoLogger = {
  log: pino(pinoConfig),
  override: () => {
    console.log = (...args) => pinoLogger.log.info(...args)
    console.info = (...args) => pinoLogger.log.info(...args)
    console.warn = (...args) => pinoLogger.log.warn(...args)
    console.error = (...args) => pinoLogger.log.error(...args)
    console.debug = (...args) => pinoLogger.log.debug(...args)
  },
}

module.exports = pinoLogger

/pino-logger/overrideLogger.js

const { override } = require('./pinoLogger')

override()

/pino-logger/package.json

{
  "name": "pino-logger",
  "version": "1.0.0",
  "private": true,
  "license": "MIT",
  "description": "Will override the default console.log and other logging methods, substituting them with pino logger that's better suited for searchable production logs",
  "main": "pinoLogger.js",
  "dependencies": {
    "pino": "^8.7.0",
    "pino-pretty": "^9.1.1"
  }
}

Help is much appreciated. Thanks! :)

MrEmanuel commented 1 year ago

A possible solution for me is to only use pino in production, thus eliminating my need to config.transport to get the pretty print functionality.

I just remove config.transport and add the following to my pino-logger/overrideLogger.js

 if (process.env.NODE_ENV === "production") {
     override() 
}

But it would be nice to use it in development too.

mcollina commented 1 year ago

This look like a very interesting situation to investigate. However I would hardly have time for this :(. I would be happy to accept a PR.

MrEmanuel commented 1 year ago

I would be happy to investigate, but I don't know where to begin unfortunately. If you could give me some tips, I could look in to it. Otherwise it's totally fine to close the issue. :)

My current workaround is to only use pino in production because then I don't need the transport property.

mcollina commented 1 year ago

Closing then.

github-actions[bot] commented 1 year ago

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.