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

Ordering in format.combine matters #104

Open
patrickwall57 opened this issue Mar 17, 2020 · 3 comments
Open

Ordering in format.combine matters #104

patrickwall57 opened this issue Mar 17, 2020 · 3 comments

Comments

@patrickwall57
Copy link

patrickwall57 commented Mar 17, 2020

This probably just needs a documentation enhancement but want to call this out. After several hours of fighting with the formatter I realized that ordering matters when defining a format.

// The following code prints a json formatted error message with a timestamp
export const logger = createLogger({
  format: format.combine(
    format.timestamp(),
    format.json()
  ),
  transports: [
    new transports.Console()
  ]
});

logger.info("timestamp is")

// {"message":"timestamp is","level":"info","timestamp":"2020-03-17T15:38:01.527Z"}

The following code prints a json formatted message but excludes the timestamp. 
export const logger = createLogger({
  format: format.combine(
    format.json(),
    format.timestamp()
  ),
  transports: [
    new transports.Console()
  ]
});

logger.info("timestamp is")

//{"message":"timestamp is","level":"info"}

If this is already covered in the documentation and I am missing it then we can close this otherwise id like to propose an addendum to the docs to make this more clear. The examples are correct and it took me finally copy/pasting an example into my project to see the timestamp working to realize what was happening.

@patrickwall57 patrickwall57 changed the title Ordering in format matters Ordering in format.combine matters Mar 17, 2020
@csakai
Copy link

csakai commented May 19, 2020

I agree, the README does a very poor job of conveying this and should be changed. There is a concept of "finalizing formatters" (that exact phrase only appears once in the whole README), which must be placed last in the format.combine chain. In the section that lists all the formats, if the format has the word "finalize" in the description, it's a finalizing formatter. Additionally, a list of the formatters that are considered "finalizing formatters" is in the "info objects" section, where it describes what is in the Symbol.for('message') property.

I've only messed around with a few of the formatters, I don't know how they behave if you have multiple "finalizing formatters" in the combine chain, or if the library works without one at the end (I believe it's required).

@gnitsenko93
Copy link

+1 to that point. The README does not explain it.

@c-vetter
Copy link

While I find it intuitive that order matters, I don't see the point of even having “finalizing” and “non-finalizing” formatters. To my mind, that just hamstrings composability.

I just came across this when I tried to combine label with cli in such a way that the label would be before the level. For that, I now need a custom label formatter that works on the “finalized” message instead of the normal one. I can do that, but this kind of thing is quite annoying and makes the decision to use winston for logging a dubious one…

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

No branches or pull requests

4 participants