pinojs / pino

🌲 super fast, all natural json logger
http://getpino.io
MIT License
14.21k stars 875 forks source link

An option to have level be a string instead of a number #279

Closed robinjoseph08 closed 6 years ago

robinjoseph08 commented 7 years ago

Is it at all possible to have pino output the level string (info, warn, error, etc) instead of the number? The log viewer we're using can autodetect "level":"error" and highlight it red for us, but it doesn't do it for "level":50. Could there possibly be a option to toggle between integer and string?

Thanks!

mcollina commented 7 years ago

No, write a transport if you need to convert these. Using numbers provide a very easy and straightforward way to do filtering. Which logviewer are you using?

robinjoseph08 commented 7 years ago

Is there a way we can hook up a transport that's within the same node process then? In our setup, we are aggregating logs through a docker log driver and then forwarding them to LogDNA (our log viewer). Because it's just pulling from docker's stdout logs, there's not a great way to pipe those through the transport.

mcollina commented 7 years ago

Check out the metadata protocol and pino-multi-stream.

You might be able to solve your issues. I would pass in a custom object, and in its write I would do: line.replace(/"level":30/, "level":"info"). With the metadata protocol you will know which numbered level to replace.

jsumners commented 6 years ago

Closing because I think the issue has been addressed with appropriate suggestions. Please re-open if you have further questions.

pastjean commented 6 years ago

@robinjoseph08 Here https://www.npmjs.com/package/pino-text-level-transport (I may be super late though) but I had the same problem with logdna.

thanpolas commented 6 years ago

A natural evolution in logging is to normalize all logging formatting across your entire organization, regardless of platform or stack. To that end, an organization-wide Logging Schema is needed and from what research I did I came up with the Log Event JSON Schema which provides a solid schema to work on.

In that schema, log levels are strings, for good reasons as it makes filtering and processing log messages easier. What works for Node.js might not be the case for a Java application, so the schema needs to be, and actually is, platform agnostic.

A custom transport is a workaround, rather than a solution to this problem.

I'd ask the maintainers to reconsider their choices and allow Pino to be fully configurable to allow for string format in level propagation.

davidmarkclements commented 6 years ago

I really like the idea of a universal cross-platform JSON logging schema – if such a schema was a standard spec I would absolutely start making changes to pino core with the emphasis being on the most performant implementation according to the spec.

Since this schema is more of an independent effort, it reduces the strength of the argument for level strings in core vs the arguments against.

Here's the problem for us, if we change the level key from number to string, we've broken backward compatibility of the logging format. Even if we add it as an opt-in in core, the v key of the logging format should actually be changed - this could in itself end up being a footgun for systemic inconsistency.

thanpolas commented 6 years ago

Here's the problem for us, if we change the level key from number to string, we've broken backward compatibility of the logging format.

I understand how this can impact existing installations. I assume when you refer to "us", that is what you mean, that you have an existing installation where you would break BC of your logging format. Yet I'm missing how an "opt-in" configuration to switch the level type would impact an existing codebase.

If that's not the case, please elaborate more cause the next sensible explanation to BC would be against a "logging-schema-contract" that exists somewhere I'm not aware of.

...

As a sidenote, why not have Pino lead or help the effort to make that, or any, a "standard spec"?

From my point of view, where I am making decisions on a project that has not yet been implemented. I don't have anything in place to break (BC wise) and will invest in the most widely accepted solution for a cross-platform logging JSON schema. I cannot wait for an RFC on that topic to be authored to make up my mind.

thanpolas commented 6 years ago

following up on this, if you find yourselves in this desperate situation, there is a hack you can do to force pino to print level however you want, on the new pino instance you need to overwrite the _lscache property like this:

  Object.defineProperty(pinoLogger, '_lscache', {
    value: {
      10: '{"level": "trace"',
      20: '{"level": "debug"',
      30: '{"level": "info"',
      40: '{"level": "warn"',
      50: '{"level": "error"',
      60: '{"level": "fatal"',
    },
  });
willmendesneto commented 6 years ago

If you want to write with level as a string, one thing you can do to have that result is to use a formatter as well.

const pino = require('pino');

const formatter = (chunk) => JSON.stringify(
    Object.assign(
      {},
      chunk,
      { level: pino.levels.labels[chunk.level] || chunk.level },
    ),
);

const pretty = pino.pretty({ formatter });
pretty.pipe(process.stdout);

const logger = pino({
  name: 'your-logger-name',
  safe: true,
  timestamp: pino.stdTimeFunctions.slowTime,
  serializers: {
    req: pino.stdSerializers.req,
    res: pino.stdSerializers.res,
  },
}, pretty);

logger.info('Should render level as a string'); // {..., "level": "info"} 

If not, the best option is to write a transform, as @mcollina suggested.

davidmarkclements commented 6 years ago

@willmendesneto that approach will significantly affect logging overhead

willmendesneto commented 6 years ago

I see. So the best way to do that it's via transport? Can we add that in the docs or add a feature (like a flag) to add level as a text (and share that this will affect the performance as well)?

More than keen in apply this change, if is that the case

mcollina commented 6 years ago

I see. So the best way to do that it's via transport?

Yes

Can we add that in the docs or add a feature (like a flag) to add level as a text (and share that this will affect the performance as well)?

I'm +1 in adding it to the docs. I suspect adding a flag will affect throughput even when disabled.

jsumners commented 6 years ago

I believe we would consider a PR to pino-pretty (against the next-major branch) for such a feature.

gcampes commented 4 years ago

Just wanna point others to the right direction if they land here: You can use useLevelLabels to make level a string https://github.com/pinojs/pino/blob/master/docs/api.md#uselevellabels-boolean


pino({
    useLevelLabels: true,
    // ...
});
tbrophy commented 4 years ago

@Gcampes snippet works for me on pino@5.13.4.

thetumper commented 4 years ago

Now seeing warnings with "useLevelLabels":

[PINODEP001] Warning: useLevelLabels is deprecated, use the formatters.level option instead

If this option is being taken away, docs need to be greatly improved on how to achieve comparable behavior, without having to write formatter functions. I don't want to override formatters when the only thing I want changed is a level name to be output instead of a number. Is this still possible?

Note: I just upgraded to pino 6.0.0.

jsumners commented 4 years ago

Note: I just upgraded to pino 6.0.0.

Note that v6 is a semver major that has deprecated some features to be removed in v7 and we have very kindly issued warnings for those deprecations so that you can deal with them.

If this option is being taken away, docs need to be greatly improved on how to achieve comparable behavior, without having to write formatter functions.

This sort of antagonistic language will not be tolerated. You are welcome to submit PRs to improve the documentation if you find it lacking.