Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Bug]: Nested objects in defaultMeta won't merge with nested objects in the log meta #2105

Closed
MitchellCash opened this issue Apr 7, 2022 · 5 comments

Comments

@MitchellCash
Copy link

🔎 Search Terms

defaultMeta, nested, object

The problem

When defaultMeta sets a nested object it won't be merged with another nested object in the log meta using the same key. Instead, the nested object in the log meta overrides the defaultMeta nested object, instead of merging them.

This might be by design due to any performance or other concerns of iterating through nested objects, but thought I would raise it nonetheless.

What version of Winston presents the issue?

v3.7.2

What version of Node are you using?

v16.14.1

If this worked in a previous version of Winston, which was it?

No response

Minimum Working Example

import { hostname } from 'node:os';
import * as winston from 'winston';

const logger = winston.createLogger({
  level: 'info',
  format: winston.format.combine(winston.format.json()),
  transports: [new winston.transports.File({ filename: 'logs/application.log' })],
  defaultMeta: {
    syslog: {
      hostname: hostname()
    }
  }
});

logger.info(undefined, { env: "dev" };

// RESULT (✅):
// {"env":"dev","syslog":{"hostname":"app-server-01"}}

logger.info(undefined, { env: "dev", syslog: { timestamp: new Date() }};

// RESULT (❌):
// {"env":"dev","syslog":{"timestamp":"2022-04-07T09:15:41.636Z"}}

// EXPECTATION (✅):
// {"env":"dev","syslog":{"hostname":"app-server-01","timestamp":"2022-04-07T09:15:41.636Z"}}

Additional information

No response

@MitchellCash
Copy link
Author

Apologies, I didn't even look at the pinned issue #2029 which links to #1884, both of which cover my described issue! Will close this and look forward to future updates!

@wbt
Copy link
Contributor

wbt commented Apr 7, 2022

@MitchellCash Can you try temporarily using 3.7.1 and see if that solves your issue?
It accidentally broke some code (#2103) without a major version bump, so we quickly rolled back the changes in a 3.7.2, but anticipate re-rolling out those changes once #2103 is fixed. It would be nice if you can test and verify whether that's going to fix your issue when the rollout happens, and running 3.7.1 is a reasonable strategy to test for that.

@MitchellCash
Copy link
Author

@wbt I pinned to version 3.7.1 and unfortunately it didn't fix the above issue. I performed the same tests outlined in the top post and had the same results.

@wbt
Copy link
Contributor

wbt commented Apr 7, 2022

Hmm, that's unfortunate. It might be a good test case for @maverick1872 to have a look at.

@maverick1872
Copy link
Member

maverick1872 commented Apr 7, 2022

This is by design currently. As it stands the meta implementation doesn't support a deep merge strategy as it's a departure from the current merge strategy.

Id like to get add a configuration that allows one to define a merge strategy on a given logger instance but fixing the current issues is higher priority. Unfortunately I'm pretty slammed with other responsibilities right now so I've been unable to diagnose the issues with the latest release let alone investigate new features.

I welcome any contributions though! 😁

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants