Format timestamp and meta independently of log function and allow timestamp to be passed into to log function

[JiCiT]

I had a need to be able to format both timestamp and meta without making a call to common.log, so I separated those into separate functions.

The bigger issue is that I'm doing some logging directly within a database and also feeding the logging info back to node for console, etc. logging. I want the timestamp to be consistent across both the database and other logging, so I want to be able to feed the timestamp directly into the call to log.

I made an effort to ensure that if the timestamp is not passed into the call to log, then existing functionality remains the same.

[indexzero]

We can't change the method signature to log. Too many third party transports have agreed to that contract.

[indexzero]

That being said your changes to common.log are very welcome 👍. That method is a beast right now.

[JiCiT]

That makes sense about so many down-streams having agreed to the contract. I just added a new commit to check the transport.log definition and either include timestamp in the call or not based on transport's defintion.

[sajacy]

A different perspective here: I think instead of going down the path of changing method signatures, it might be better to keep this timestamp coordination functionality out of the logging internals, and instead, use the options.timestamp function to achieve the desired behavior. We can make it essentially a data provider:

If the timestamp function implementation closes over the logger, and subscribes to events for logging, it can be aware of when different transports are logging, and provide them with the same timestamp (i.e. keep a queue, and shift the head off after the desired loggers are done logging). Sketched out what it might look like:

var logger = // winston stuff

// LTC: "locally coordinated time"
var LTC = {
  "ts": [],
  "getTimestamp": function getTimestamp() { return LTC.ts[0]; },
  "log": function log() { 
    LTC.ts.push(new Date()); // this is the timestamp to log
    logger.prototype.log.apply(logger, arguments);  // log stuff...
  }
};

logger.on("logging", function() { LTC.ts.shift(); }); // discard the timestamp

// configure transports:
logger.add(SomeTransport, { "timestamp": LTC.getTimestamp, ... });

[indexzero]

We cannot change this method signature.

[indexzero]

Just following up here. We really can't change method signatures. If you'd like to update your implementation please let us know.

Your internal changes to common.log are welcome and will likely land in [email protected]

[JiCiT]

I changed the implementation so the function signature hasn't changed. Optional timestamp would still be passed as first parameter.

[indexzero]

This is too much of a performance hit. I've been working on a number of benchmarks for comparing [email protected] to [email protected] and this will be a big hit.

[pose]

@JiCiT Could you compare new Date(level).toString() to doing isNaN(Date.parse('asd'))? I think the second option could be cheaper.

[indexzero]

Howdy! Thank you for your contribution. This PR is against the [email protected] release line. Since [email protected] is now out we are only accepting major bug fixes.

This is no longer an issue in winston@3 since the info Objects are no longer decycled or cloned. Please checkout [email protected] today!

npm install winston@next