node-bunyan icon indicating copy to clipboard operation
node-bunyan copied to clipboard

Making "v" (LOG_VERSION) optional and overridable

Open stephanebruckert opened this issue 8 years ago • 14 comments

Hi @trentm,

Some of our non-JS services are already using "v" as part of the JSON logs and for a different purpose. For consistency, we would like to use that "v" to indicate our JS service version instead.

Even though I can understand the usefulness of this field, I don't think it needs to be part of everyone's logs if we don't want it.

v: Required. Integer. Added by Bunyan. Cannot be overridden.

It appears that we couldn't currently disable or override this log version "v". Would you consider a PR that aims at making "v" overridable? https://github.com/trentm/node-bunyan/pull/463

Many thanks

stephanebruckert avatar Nov 17 '16 13:11 stephanebruckert

I'm struggling with this one. "v" is intended as the path for Bunyan to be able to change its format and have bunyan the CLI and other tools that process bunyan format logs adapt. Allowing that format to be looser puts a higher burden on bunyan log processing tools.

However, currently it isn't being used (it is 0 everywhere) and it isn't clear that it will be (i.e. perhaps the bunyan format is effectively be frozen -- at least for backward incompatible changes). Bunyan processing tools already need to be defensive (any of rec.time, rec.pid, etc. can be overridden).

The worst that could happen: A bunyan is released that allows rec.v to be overridden. The bunyan CLI is updated to validate that the rec.v field is an integer and ignores records where that isn't the case. There are users out there of bunyan that do have a 'v' field in some of their log records, they update to the newer bunyan and now the CLI no longer processes those log lines.

That isn't so bad. The up side is that people in your case here can still get some benefit from Bunyan. Note that if 'v' isn't a known value in a log record, then Bunyan processing is free to assume it is not a valid Bunyan log record and skip it. So a question for you @stephanebruckert: If bunyan (the CLI) changed to ignore log lines where 'v' was something other than 0, would that break your usage?

trentm avatar Dec 01 '16 15:12 trentm

If I understand correctly, people could make it overridable but that would make the CLI unusable for those people? I am personally not using the CLI so it wouldn't break my usage and I would be happy with the change.

However if I were using it, I would not want the log lines with an overriden "v" to be ignored in the CLI. To prevent this, Bunyan would need a variable that keeps the status of v: versionWasOverriden = [true | false], but you might not want such an addition?

stephanebruckert avatar Dec 08 '16 15:12 stephanebruckert

I think this opens a wider question. Could we make the logger creation configurable in a way that allows the user to get rid of any unwanted required field, or - more in general - rename required fields like time, name and msg?

Maybe the CLI should accept some options/flags defining how a record's version should be computed.

lbarasti avatar Feb 26 '17 13:02 lbarasti

@stephanebruckert: To clarify: when I said the bunyan CLI would "ignore" such log records (those with a v value other than an integer), I meant that the CLI would pass through the raw JSON line unrendered. The user of the CLI would still see that log record, but it would just be the raw JSON.

@lbarasti: About the more general question: It would help me a lot on motivating Bunyan changes if you could describe particular use cases you had for this (getting Bunyan to support excluding some of its core fields from records). Note also that a "raw" Bunyan stream could be used to emit log records with some fields excluded. A question then would be how the CLI would treat those records. The current design is that a Bunyan log processor (e.g. the bunyan CLI) is free to not render (just pass through) lines that don't meet the Bunyan log record "spec".

trentm avatar Apr 13 '17 07:04 trentm

@trentm the use case I'm thinking of is the one where the requirement is for the application to log to standard output JSON conforming to some schema. Imagine for example that the requirement is for every log line to have the message field - in place of msg. Furthermore, the requirement might be for the JSON to match the schema in a strict way, where log lines are expected not to have any extra field.

You say that

a "raw" Bunyan stream could be used to emit log records with some fields excluded

I'd be interested in seeing an example of that, as it sounds like that could partially solve the problem stated by @stephanebruckert as well.

lbarasti avatar Apr 28 '17 17:04 lbarasti

+1 for dropping/renaming the name/v/pid/level and msg fields.

my use case: importing jsons into google's bigquery. i'm forced to add these dummy fields to the schema to get load jobs to run.

adtecho avatar May 15 '17 20:05 adtecho

+1 for configurable unwanted required field

freshsun avatar May 23 '17 02:05 freshsun

I would love to be able to rename msg to message, for consistency with my logstash/kibana setup.

zwily avatar Jun 23 '17 12:06 zwily

Coming from golang (essentially using logrus and zap) I felt very bad about having these required fields 🤔

I get the fact that it's importante for the cli tools but for those that don't use it and adhere to others schemes having these requirements ends up really getting in the way.

I'd really like to make everything not required 👍

(I'm totally not trashing the library! I really like it 😃)

cirocosta avatar Jul 23 '17 04:07 cirocosta

To fix this I simply added this code after creating my logger:

let logger = bunyan.createLogger({name: 'mylogger'});
logger._emit = (rec, noemit) => {
  delete rec.v;
  bunyan.prototype._emit.call(logger, rec, noemit);
};

You could also do other transforms to it, like fixing field ordering if that mattered.

larsenjh avatar Oct 04 '17 18:10 larsenjh

@zwily you can rename msg to message like this

import bunyan from 'bunyan'

const log = bunyan.createLogger({
  name: "my Logger"
})

log._emit = (rec, noemit) => {
  rec['message'] = rec.msg
  delete rec.msg;
  bunyan.prototype._emit.call(log, rec, noemit);
}

maestr0 avatar Oct 26 '17 16:10 maestr0

Use case: I want to use logdna to store my logs. It expects a "message" field, not "msg". Thus I need to change the default field the message gets stored in. Used maestr0's & larsenjh's "hack". Thanks friends! Would appreciate more direct support for this though! I imagine there could be a simple config file read by both app and CLI that would allow for renaming of core fields.

EDIT: And thanks for making this tool, trentm! Enjoying using it! If you're accepting tips somewhere let me know! (Or if you have a Patreon.)

seeekr avatar Mar 24 '18 21:03 seeekr

Modifying logger._emit would work for all fields except level. As you can see from the implementation, it must check the level to decide whether to write to the stream.

In that case, you may have to be even more intrusive and copy / re-implement some of the logic from Logger.prototype._emit and remove the if (s.level <= level) line.

d4nyll avatar Apr 17 '18 16:04 d4nyll

I'm noticing that loggers created from an existing logger via logger.child do not retain the _emit workaround here, so you will have to override the child method and apply the same logic to the created logger there as well.

bijoythomas avatar Jun 24 '20 17:06 bijoythomas