pinojs / pino

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

Add a global way to append a property #304

Closed gajus closed 7 years ago

gajus commented 7 years ago

This would be similar to .child(), e.g.

const pino = require('pino')();

pino.set({
  taskId: 1
});

pino.info('test');

// => {taskId: 1, msg: 'test'}

This is handy when logs need to be associated with a certain parameter for a duration of the program execution (e.g. while executing a specific task) and there is no way to pass an instance of a logger to the dependent components.

e.g.

import {
  foo
} from './utilities';

const taskIds = [1, 2, 3];

for (const taskId of taskIds) {
  // `foo` utility utilises pino logger.
  // I need a way to append parameters to those logs in order
  // to get a comprehensive log of events associated with a particular task.
  foo(taskId);
}
jsumners commented 7 years ago

So why doesn't foo(taskId) have a line like log.info('(task: %s) some information', taskId)?

gajus commented 7 years ago

That would require injecting logger instance to every utility function/ dependency in the library.

This sort of tight coupling is asking for a problem.

gajus commented 7 years ago

Anyway, I am closing this as I have decided to work on a fork/ logger of my own.

Pino is great. But I have strong opinions about how logger should be structured and these do not align with Pino at the moment.

jsumners commented 7 years ago

I was just trying to get an understanding of the problem.

marcbachmann commented 7 years ago

I'd advise against such an api cause it will result in a mutable instance. Then modules could modify your log parameters and mess up your log instance.

gajus commented 7 years ago

I'd advise against such an api cause it will result in a mutable instance. Then modules could modify your log parameters and mess up your log instance.

Yup, I agree. It is not something I'd recommend 99% of the time.

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.