Change standard output destination depending on log level

[ureuzy]

What this PR does / why we need it:

Since all log levels are output to stderr, logging and its filters are tainted by Loki's logs. To avoid this, we think it is a good idea to change the output destination depending on the level

Which issue(s) this PR fixes:
Fixes #5331

Special notes for your reviewer:

./loki -config.file=./cmd/loki/loki-local-config.yaml 2> /dev/null
level=info ts=2022-06-22T09:32:49.577972Z caller=main.go:103 msg="Starting Loki" version="(version=, branch=, revision=)"
level=info ts=2022-06-22T09:32:49.578361Z caller=server.go:260 http=[::]:3100 grpc=[::]:9096 msg="server listening on addresses"
level=info ts=2022-06-22T09:32:49.581057Z caller=table_manager.go:134 msg="uploading tables"

over level warning

./loki -config.file=./cmd/loki/loki-local-config.yaml 1> /dev/null
level=warn ts=2022-06-22T09:33:21.758191Z caller=experimental.go:20 msg="experimental feature in use" feature="In-memory (FIFO) cache - chunksfifocache"
level=error ts=2022-06-22T09:33:21.762942Z caller=mapper.go:50 msg="unable to read rules directory" path=/tmp/loki/rules-temp err="open /tmp/loki/rules-temp: no such file or directory"

Checklist

• Documentation added
• Tests updated
• Is this an important fix or new feature? Add an entry in the CHANGELOG.md.
• Changes that require user attention or interaction to upgrade are documented in docs/sources/upgrading/_index.md

[CLAassistant]

All committers have signed the CLA.

[dannykopping]

I appreciate the effort here, and the solution is certainly nice, however I have trouble with this PR for two reasons:

1. the logs can easily be filtered using the level field
2. this is a major backwards-incompatibility for all folks who are currently relying on logs being written to stderr

It would've been nice if we adopted this strategy from day 0, but here we are and it's tricky to change something so fundamental.

On that basis, I'm inclined to not accept this PR, but I'm interested in your response to the above and the opinions of my fellow maintainers.

[MichelHollands]

Yes, I don't think this is good idea either. It will be too easy to inadvertently lose logs if you forget to read either stderr or stdout. Additionally it's not easy to change. I can imagine a scenario where you want levels Info, Warning and Error to go to stdout and Debug level output to stderr. Better to do this in your logging tool or when you capture these logs.

[ureuzy]

@dannykopping @MichelHollands

I appreciate both of your opinions. I understood that those changes are not easy and can be solved with logging tools and that this modification is not appropriate

[muffl0n]

I'm a bit sad that this will not be fixed. Writing errors to stdout seems like a major flaw and should be fixed, maybe in a major release.
My old CS teacher would have let me fail for something like this 😆

[kemics]

@dannykopping Google Cloud counts logs written to stderr as error logs and it makes sense why. This creates false positive alerts in case of Loki and makes it more challenging to monitor. It would be helpful to have this to be configurable

Here is how other tool made similar change without breaking backwards compatibility GoogleCloudPlatform/cloud-sql-proxy#150

[muffl0n]

Having a flag like described in the linked PR sounds awesome as it maintains the mentioned backward compatibility.
Would be awesome to have something similar for loki!
Thanks for linking this @kemics!