lrlna / pino-colada

cute ndjson formatter for pino 🌲🍹
MIT License
245 stars 46 forks source link

Expose formatter #16

Closed marcbachmann closed 7 years ago

marcbachmann commented 7 years ago

That way you can use it as pino formatter:

pino({
  prettyPrint: {
    formatter: require('pino-colada/formatter')()
  }
})

With the current module I had to do a workaround:

  const stream = require('pino-colada')()
  stream.pipe(process.stdout)
  return pino(opts, stream)
mcollina commented 7 years ago

Are you sure about this change? Formatter is only in pino.pretty(): https://github.com/pinojs/pino/blob/539d37cabe0627497c1ba04fcab4fd74ec230df3/docs/API.md#prettyoptions.

marcbachmann commented 7 years ago

True, I'm using

 pino({
    prettyPrint: {
      formatter: require('pino-colada/formatter')()
    }
  })
marcbachmann commented 7 years ago

I've updated the description and will force push to update the commit message.

lrlna commented 7 years ago

hey @marcbachmann, I am not a fan of this. pretty is a stream, and imo should be passed in as such rather than getting used as a formatter. We do it like this over in bankai, which results in basically what you've written down above. To me that's a way to go about it:

var pinoColada = require('pino-colada')()
var pino = require('pino')

var pretty = pinoColada()
pretty.pipe(process.stdout)
// when dealing with argv obj
argv.log = pino({ name: 'bankai', level: 'info' }, pretty)

alternatively in my work projects, i set up a pretty.js file that looks like this:

#!/usr/bin/env node
const pinoColada = require('pino-colada')()

// provide human readable logger in development
process.stdin
  .pipe(pinoColada)
  .pipe(process.stdout)

and then in package.json's scripts portion:

    "start": "node ./ | ./pretty.js"
marcbachmann commented 7 years ago

I'm good with that. 👍 There would be one disadvantage of exposing the formatter like that. It's a contradiction to the concept of modular line delimited json streams. You basically loose all the benefits of pino which are the compatibilty of formatters & collectors. But if you're doing workarounds to use it anyways, we could change the pino prettyPrint option to support transform streams. Or is this already supported?

mcollina commented 7 years ago

I'm happy in adding more functionality to pino to simplify this use case. Would you mind firing a PR @marcbachmann?