Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Logging to stdout (instead of stderr) is a wrong default #2492

Open
dpc opened this issue Feb 27, 2023 · 3 comments
Open

Logging to stdout (instead of stderr) is a wrong default #2492

dpc opened this issue Feb 27, 2023 · 3 comments

Comments

@dpc
Copy link
Contributor

dpc commented Feb 27, 2023

Bug Report

I just lost decent amount of time debugging a problem that turned out to be tracing-subscriber logging to stdout which was overlooked during refactoring. This is a second, if not third time in the recent past where I've dealt with wrong defaults in tracing-subscriber creating issues.

I don't recall ever wanting to get my log output to stdout, and as far as I can tell logging to stderr is a well estabilished convention, to separate main program output from diagnostics.

At very least env_logger defaults to stderr , so does e.g. slog-term, so do most logging libraries I've used and can remember.

Is there any reason why tokio-subscriber would go against this convention?

I realize that at this point reverting the default might be a hassle, but just want to point it out and ask.

@mkpankov
Copy link

Also surprised with this.
This might be a performance consideration since logging a lot of data to unbuffered sink might be less performant. But I'd like the team to clarify this.

@dpc
Copy link
Contributor Author

dpc commented Sep 27, 2023

stdout will flush on newlines (uses std::io::LineWriter under the hood). Should be natural for any logging library to just write-out stuff in lines (or use LineWriter to gather output into newlines internally). In addition stdout is locked, which is making it possibly slower. So I doubt it is any perf optimization.

@hawkw
Copy link
Member

hawkw commented Sep 27, 2023

I agree that it would probably be more correct to make stderr the default. We should probably change it.

IMO, changing the default output stream is semver-breaking, so we would need to do this in a tracing-subscriber 0.4. If we do an 0.4 release soon, we probably want to reassess some other default behaviors as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants