Combine child metadata with additional metadata

[sebakerckhof]

I'm not sure this is a bug or feature request. I'll use the bug template since it also contains the behavior I'd expect.

Please tell us about your environment:

• winston version?
  • winston@2
  • winston@3
• node -v outputs: v12.14.0
• Operating System? (Windows, macOS, or Linux) Linux
• Language? (all | TypeScript X.X | ES6/7 | ES5 | Dart) ES6/7

What is the problem?

There seems to be no way to combine child metadata with additional properties.
In fact, I find the multiple api signatures in which one can log a message to be quite conflicting/confusing.
For example, consider this code:

const winston = require('winston');

const logger = winston.createLogger({
  level: 'info',
  format: winston.format.printf(({ message, prefix}) => `[${prefix}]: ${message}`),
  defaultMeta: { prefix: 'root' },
  transports: [new winston.transports.Console()],
});

logger.info('foo 1', { foo: 'bar' });
logger.log('info','foo 2');
logger.log('info','foo 3', {});
logger.log({ level: 'info', message: 'foo 4' });

const childLogger = logger.child({ prefix: 'child' });
childLogger.info('foo 5', { foo: 'bar' });
childLogger.log('info','foo 6');
childLogger.log('info','foo 7', {});
childLogger.log({ level: 'info', message: 'foo 8' });

All of these are valid, but produce different/unexpected results:

[root]: foo 1
[undefined]: foo 2
[root]: foo 3
[root]: foo 4
[root]: foo 5
[child]: foo 6
[root]: foo 7
[root]: foo 8

What do you expect to happen instead?

I can't really find an api reference, so it's hard to know what is to be expected and what not.
But I would expect that when I create a child logger with metadata, the metadata acts in the way the default metadata on the logger works. Because now, there is no way to add in additional properties in a log call, without losing the metadata passed in to the child logger.

So I would expect that I can still call childLogger.log('info', 'message', { additionalProp: 'foo' }) and still have the prefix=child property in the metadata. And in fact, I would expect this to be the case for any of the example calls to childLogger.

[aressler38]

I just ran into this problem too, and I can see what is causing it. In the logger.js file, the child's write method override gets an infoClone with defaultMeta from the parent. I think we need to switch the order of the Object.assign call so that the child overrides the parent as described by the documentation.

Object.assign(
            {},
            defaultRequestMetadata,
            info
          );

should be

Object.assign(
            {},
            info,
            defaultRequestMetadata
          );

The readme clearly says, "You can create child loggers from existing loggers to pass metadata overrides," so I'd expect my object to override the defaultMeta defined in the parent logger. https://github.com/winstonjs/winston#creating-child-loggers

[aressler38]

I made a PR and ran the unit tests. It wasn't as straightforward as rearranging the Object.assign() calls because there are use cases when you call logger.info('message', { stuff }), and "stuff" can contain defaultMeta overrides. That all gets mixed into the info object. I fixed this by looking at the parent's defaultMeta and checking to see if the values in the info object matched the values in the parent's defaultMeta. If they match, then I assume you didn't override it (why would you override it with the parent's value?), so I take the child's defaultMeta. I am not sure if this is the best approach. I did modify a unit test to produce the issue. My change does fix the behavior.

#1875

[traceon]

@aressler38 I took a more radical and seemingly consistent approach in #1879. Reused your test though.

[maverick1872]

Pretty sure the maintainers have abandoned the project. I've had a PR in the works for this same issue for a very long time. #1598

I've thought about publishing my fork of the project and moving it forward but I don't personally have the time to commit to the project as it currently stands.

[maverick1872]

Closing as this issue is being consolidated into #2029