-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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
How to separate Stdout and Stderr output? #403
Comments
This is how I accomplished this (info/debug to stdout, warn/error/fatal/panic to stderr). It's simple but kind of ugly: So far seems to work for me. Not sure if I'd want to do this in a large application or a production environment. Basically: type stderrHook struct{}
func (h *stderrHook) Levels() []logrus.Level {
return []logrus.Level{logrus.PanicLevel, logrus.FatalLevel, logrus.ErrorLevel, logrus.WarnLevel}
}
func (h *stderrHook) Fire(entry *logrus.Entry) error {
// logrus.Entry.log() is a non-pointer receiver function so it's goroutine safe to re-define *entry.Logger. The
// only race condition is between hooks since there is no locking. However .log() calls all hooks in series, not
// parallel. Therefore it should be ok to "duplicate" Logger and only change the Out field.
loggerCopy := reflect.ValueOf(*entry.Logger).Interface().(logrus.Logger)
entry.Logger = &loggerCopy
entry.Logger.Out = os.Stderr
return nil
}
// logrus.SetOutput(os.Stdout) // Default is stdout for info/debug which are emitted most often.
// logrus.AddHook(&stderrHook{}) |
Even better after a coworker pointed this out (still a bad idea though): type stderrHook struct {
logger *logrus.Logger
}
func (h *stderrHook) Levels() []logrus.Level {
return []logrus.Level{logrus.PanicLevel, logrus.FatalLevel, logrus.ErrorLevel, logrus.WarnLevel}
}
func (h *stderrHook) Fire(entry *logrus.Entry) error {
entry.Logger = h.logger
return nil
}
//logrus.SetOutput(os.Stdout) // Default is stdout for info/debug which are emitted most often.
//loggerCopy := reflect.ValueOf(*logrus.StandardLogger()).Interface().(logrus.Logger)
//hook := stderrHook{logger: &loggerCopy}
//hook.logger.Out = os.Stderr
//logrus.AddHook(&hook) |
Yeah, we won't support this directly in core since the use-case is too narrow. A custom hook or formatter is recommended. |
Please be aware that the suggestion here does not work now with this commit 89742ae I get 'fatal error: sync: unlock of unlocked mutex' now. |
@sirupsen any chance you could consider re-opening this? I think there are log forwarders or aggregators that will assume messages received on A co-worker recently ran into this with Cloud Foundry:
Given the prevalence of various container orchestrators and PaaSes and the wide array of log forwarders and aggregators they use (fluentd and logspout come to mind), this issue is likely to affect more and more people. |
A simple solution is type OutputSplitter struct{}
func (splitter *OutputSplitter) Write(p []byte) (n int, err error) {
if bytes.Contains(p, []byte("level=error")) {
return os.Stderr.Write(p)
}
return os.Stdout.Write(p)
} and then logrus.SetOutput(&OutputSplitter{}) But this introduces an extra step for each log write that shouldn't be necessary. Please add this feature! It's the only was we can get Stackdriver to display logs correctly when running as a Kubernetes app in Google's cloud. |
not supporting both stdout and stderr streams for logging IS NOT a narrow use case. it is THE way of separating output and is depended upon by at least 40years of convention. |
I opened #671 to address this nearly a year ago and it hasn't even been reviewed. This is a real problem with a solution already having been offered... it's frankly ridiculous that this hasn't been taken seriously. |
It sucks @sirupsen doesn't even want to have a conversation about this. this is so dumb |
I have to create two instances of loggers that one of them is set to stdout and the other is set to stderr. Although it is ugly using way, but it is simple solution and not necessary to care the unmarshal the serialized data or overriding the write method. var (
Logger *logrus.Entry
ErrLogger *logrus.Entry
)
func init() {
initStdoutLogger()
initStderrLogger()
}
func initStdoutLogger() {
stdoutLogger := logrus.New()
stdoutLogger.SetOutput(os.Stdout)
stdoutLogger.SetLevel(logrus.TraceLevel)
Logger = stdoutLogger.WithFields(logrus.Fields{
"type": "Console",
})
}
func initStderrLogger() {
stderrLogger := logrus.New()
stderrLogger.SetOutput(os.Stderr)
stderrLogger.SetLevel(logrus.WarnLevel)
ErrLogger = stderrLogger.WithFields(logrus.Fields{
"type": "Console",
})
} |
@fearblackcat, presumably you're going to use one logger only for writing messages less sever than a warning (to stdout) and the other for those as severe or moreso than a warning (to stderr)? If you're putting direct knowledge of what messages go where directly in your logic, doesn't that almost entirely defeat the purpose of using a runtime configurable logger? |
@krancour In my way, I also encapsulate the logger interface for user. e.g.: type Log struct {
context.Context
}
func (log *Log) Trace(format string, v ...interface{}) {
common.Logger.WithContext(log.Context).Tracef(format, v...)
}
func (log *Log) Debug(format string, v ...interface{}) {
common.Logger.WithContext(log.Context).Debugf(format, v...)
}
func (log *Log) Info(format string, v ...interface{}) {
common.Logger.WithContext(log.Context).Infof(format, v...)
}
func (log *Log) Warn(format string, v ...interface{}) {
common.ErrLogger.WithContext(log.Context).Warnf(format, v...)
}
func (log *Log) Error(format string, v ...interface{}) {
common.ErrLogger.WithContext(log.Context).Errorf(format, v...)
} If you want to configure in runtime, it will always do what u want with one handler ( the Log instance). |
@fearblackcat, clever. I like it. |
This is implemented and well documented here: |
@iam-veeramalla many folks have considered and rejected the solution you linked to for performance reasons. Every log message will be formatted twice and in one of those two cases, only to subsequently be discarded. |
@krancour let's reopen this issue, because discussion here will happen below the radar. We can see what can be done on this topic. |
We have two options here either:
|
I just realised that. Thanks for correcting @dgsb |
Why not add a |
@edoger did you implement a |
@sanderdekoning I am happy to add implementations to this solution, but it still requires the author's views on this solution. If most people agree with this scheme, it is not difficult to implement it. |
This issue is stale because it has been open for 30 days with no activity. |
@bunyk your committed example does not appear to work:
Just FYI. |
@alexec I'm not sure which example, the one here? Because that one is committed but has no log variable. |
Ah - you're right - my bad. I realise that I changed it for my use case (I'm using |
related to this question maybe? should I apply for this issue? |
This issue is stale because it has been open for 30 days with no activity. |
This issue was closed because it has been inactive for 14 days since being marked as stale. |
* Added std type to logging level. Added demux-ing of logging to stdout/err working around sirupsen/logrus#403 Turned off the default logging in favour of using hooks always. Added fallback, if no stdout/err logger was configured, add one automatically. This prevents unexpected lack of logging after an upgrade in case the user's configuration file is not updated. Fixes: #2054 Signed-off-by: Martin Ashby <[email protected]> * Fix build on Windows - revert function rename SetupHookLogging Fixes https://github.com/matrix-org/dendrite/pull/2060/files#r761692681 * Revert logging formatter changes to log.go, base.go. The same formatting should be applied on both windows and unix, so it makes sense to keep it in files which are shared between both platforms. Fixes #2060 (comment) * Remove unnecessary startup logging about info level logger. Fixes #2060 (comment) Co-authored-by: Neil Alexander <[email protected]>
This is something that needs to be fixed at the library level. Why are INFO logs being logged to stderr? That is not what stderr is for and every cloud provider will classify logs to stderr as errors (as they should, makes sense completely). I would like to propose that the change come from the library level. Expecting people to pass around receiver functions to fix what should be the correct way of logging standard output is not a long-term and correct solution. Especially using reflect, that's just asking for panics. Need to rip this library out of our services now and find a replacement for the time being. |
@sanderdekoning Considering the current state of this library, I built a logging library that is consistent with the logrus style, to solve this problem, you can try it if you want: zkits-logger |
Hi!
How can I out levels debug, info, warn to Stdout and levels error, fatal, panic to Stderr in one logger instance?
The text was updated successfully, but these errors were encountered: