iron / logger

Morgan-inspired request logger middleware for Iron
MIT License
25 stars 37 forks source link

Stop supporting terminals #82

Closed Fiedzia closed 7 years ago

Fiedzia commented 8 years ago

I believe that supporting output to terminal does not belong in standard logger. It consist half of logging codebase, breaks docker, provides no benefits whatsoever in most production scenarios, its very easy to cause hard to debug issues, and the work has already been done: https://github.com/alexander-irbis/logger/commit/fe6dab0f81e77224e553fbcb1179f3003f86f007

so it would be nice if someone would just merge it back into master.

reem commented 8 years ago

Looking back on this, I think this is a good idea. I would like to keep the terminal support available in a crate somewhere, so users can use it for development logging, but I agree that the base logger should more concern itself with emitting useful and parseable information.

If you or anyone else wants to make a pull request with the proposed changes, I'd be happy to review and merge them.

Hoverbear commented 7 years ago

I'm personally a fan of the colored output, and journalctl supports colorized logs at least partially.

At the same time code complexity would be lovely to reduce.

Fiedzia commented 7 years ago

I'm personally a fan of the colored output

For development this may be ok. But it shouldn't go to production, and Iron needs reliable logging solution. Those two should not be mixed.

and journalctl supports colorized logs at least partially.

Big no. Logs are grepped, parsed and data-mined, adding platform-dependent color codes makes it harder. You can get improved readability (and more) by using structured logs.

Hoverbear commented 7 years ago

Right, journalctl requires an explicit flag to enable colored output. :)

Anyways, a PR would be welcome and I'd review/merge it if we got one.

Fiedzia commented 7 years ago

PR: https://github.com/iron/logger/pull/91 (removes terminal support) For those who like colors - I think the best way would be to have separate iron-fancy-dev-logger that could provide that, but it should not be the default behaviour.

Hoverbear commented 7 years ago

Thanks! Will review tonight. :)