pinojs / pino

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

custom console/stream in browser version #207

Closed thomheymann closed 7 years ago

thomheymann commented 7 years ago

Would you guys be open to adding the ability to override the console/stream in the browser version of pino?

We've got a use case were we'd like to send log lines to keen.io or Google Analytics instead of printing to the console (for front-end monitoring).

The server side version supports the ability to pass in a custom stream using pino(options, stream). It would be great to be able to do something similar in the browser:

const console = { info: trackEvent, error: trackError };
const logger = pino(options, console);

Not a massive fan or diverging the interface between server and client (stream has different API than console) but would be great to support custom transports in the browser.

Thoughts?

mcollina commented 7 years ago

@cyberthom I would say send a PR!

davidmarkclements commented 7 years ago

wow. .. interesting (also 👋 hi Thomas).

I don't think we need to diverge interfaces, instead we could do

const logger = pino({
  browser: {
   map: {
     error: trackError,
     info: trackEvent
   }
  }
})

Then relevant mappings simply override, this gives granularity and implies fallback in the API

We might want to support numbers as well {50: trackError, 30: trackEvent}

(@mcollina @jsumners just realised we have actually diverged, browser doesn't support custom levels)

This would be ignored server side (in case of isouniversamorphical code), in fact it might be best to pass browser options like pino({browser: require('./lib/pino-browser-opts')}) so that can an empty object server side, but the relevant opts in browser build

jsumners commented 7 years ago

@davidmarkclements re: browser and custom levels -- :(

I write so little frontend code that the browser doesn't even occur to me. Indeed, I have never bundled Pino for the browser.

thomheymann commented 7 years ago

Awesome - Will put together a pull request.

Another issue we noticed is that the client side logger doesn't support serializers. I guess we will try and duplicate that logic in browser.js for now to align the two.

temsa commented 7 years ago

@davidmarkclements , @mcollina , I'm needing the stream API on the browser side (to send back logs to the server through the beacon API ), but the browser version only maps ton console.*, I guess @cyberthom PR will solve that, but, meanwhile, is there any good way to tell webpack to use the node version instead of the browser one ? I'm considering using an alias, but I guess there is a better API for that :)

davidmarkclements commented 7 years ago

hmmm currently @cyberthom PR would partially solve that in that you could, say

const format = require('quick-format-unescaped')
const logger = pino({
  browser: {
   map: {
     info: (...args) => beaconStream.write(format(...args))
   }
  }
})

For now, I'd actually say aliasing pino to pino/pino.js is fine - let us know how it goes

temsa commented 7 years ago

So, I could alias pino/pino.js without problem with the 4.1.2 release. I only needed to also alias fs to an empty file :)

I am now waiting for #218 :+1: to land in a release to try switching to it instead of this "hack"

davidmarkclements commented 7 years ago

done :bowtie: v4.2.0

github-actions[bot] commented 2 years 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.