#135 - child logger names

[osher]

This does the trick.

I also thought about fixing the name in the chindings if it's found there - just because in this case we may be able to prevent this duplication from resulting strings.

But - I could not find a robust and high performing way that that in case there are nested objects with name property - will replace the right name attribute ... 😛

solves #135

[jsumners]

In my opinion this is a conflict. The name property being adjusted is actually an option for the initial instance of the logger. Child loggers are meant to inherit the name because the name is tied to the overall logger instance.

To properly solve #135, if overriding is desired, then it should only happen to the chindings. And the check should not be specific to a single property; it should be a generic check for any properties to override.

[davidmarkclements]

yah I had this same discussion with @mcollina a while back - I wanted to overwrite (like @osher) but eventually we agreed on the current behaviour because it's equivalent to bunyan

[mcollina]

The current approach is to not set a global name, and then add it to all the children. Would that work for you?

Possibly it's fine to support the override

[osher]

EDITED x 2

well. My use-case is around managing warn levels.

root-logger created with name: "<the application name>", application: "<the application name>"
By name, it takes the default warn-levels. fun.
This logger is used for all initiation and shutdown tasks, and/or background efforts not related to requests.

Then, the web server creates a child-logger per request, adds req_id (the infra does that, great).
Or the queue consumer creates a child-logger per message, and adds msg_id.
So far - Fun.

Now in our infra-level user code we create more specific child-loggers with more binding (through some infra API), that provides a more specific name - based on the web-operation invoked (see openapi spec).
So I try to find what's the log-level for this more specific name, so I can set child.level accordingly, but the child.name returns me the name of the parent. WTF??!

[osher]

I think that if the user choses to override name - it's his choice and it should be respected.

I did spent time thinking how to prevent the repetition in the chindings (there will be a the default name, and a later cascading passed name).

I could not find an elegant way to do it.
A simple replace might miss the name attribute, because there could be a name attribute of some nested object, and there's no guarantee that the logger name comes first of last because Pino does not ensist on being provided with a name upon creation (unlike bunyan that throws if you don't provide a name)

So I left it...

[osher]

@mcollina - thanks for asking.
looks like the current approach is a workaround the problem. It's counter intuitive...

[mcollina]

I think we should change this. How about we do parent:child or parent-child, so we can maintain the hierarchy?

[mcollina]

definitely not needed, we use this without a name to set serializers and other things.

[mcollina]

We will need a unit test to merge this.

[jsumners]

Let me clarify something. We already deal with duplicate keys on child bindings. It is in our documentation -- https://github.com/pinojs/pino#duplicate-keys

var pino = require('pino')
var log = pino()
var child = log.child({foo: 'bar'})
var grandchild = child.child({foo: 'baz'})

child.info('child')
grandchild.info('grandchild')

// {"pid":9907,"hostname":"dalekprime.local","level":30,"time":1480600215813,"msg":"child","foo":"bar","v":1}
// {"pid":9907,"hostname":"dalekprime.local","level":30,"time":1480600215815,"msg":"grandchild","foo":"bar","foo":"baz","v":1}

There is confusion because @osher wants to override the name of the logger. As I have been saying, this is an option of the logger and really doesn't have anything to do with the child bindings.

So let's look at what Bunyan does with children and names:

var bunyan = require('bunyan')
var log = bunyan({name: 'blogger'})
var child = log.child({name: 'child'})
var grandchild = child.child({name: 'grandchild'})

child.info('child')
grandchild.info('grandchild')

// /private/tmp/01/node_modules/bunyan/lib/bunyan.js:378
//             throw new TypeError(
//             ^
//
// TypeError: invalid options.name: child cannot set logger name
//    at new Logger (/private/tmp/01/node_modules/bunyan/lib/bunyan.js:378:19)
//    at Logger.child (/private/tmp/01/node_modules/bunyan/lib/bunyan.js:689:12)
//    at Object.<anonymous> (/private/tmp/01/btest.js:3:17)
//    at Module._compile (module.js:570:32)
//    at Object.Module._extensions..js (module.js:579:10)
//    at Module.load (module.js:487:32)
//    at tryModuleLoad (module.js:446:12)
//    at Function.Module._load (module.js:438:3)
//    at Module.runMain (module.js:604:10)
//    at run (bootstrap_node.js:394:7)

So I stand by my assertion that the name option shouldn't be overridable via child bindings.

[mcollina]

@jsumners on second thought, I agree.

[davidmarkclements]

yes I see this now - the logger is a child - a sub-logger - so the name should be the same

[osher]

@jsumners - good answer. thanks 😺
I'll find another way for this logic

[github-actions]

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