*: switch to glog

[andyxning]

Currently, iiuc, nsq does not support log rotation. This is not user friendly for some users.

How about we move log package to glog which is used by Kubernetes organization and supports log rotation when log file size exceed 1.8GB by default.

/cc @mreiferson @jehiah

[andyxning]

@mreiferson Any thoughts about this?

[ploxiln]

glog isn't strictly required in order for logs to be rotated.

There are a few different systems that collect stdout/stderr from processes/services and manage the writing of log files. This could be considered a better separation of concerns than if every service had its own logic for log files/rotation/relaying.

Docker, which Kubernetes typically uses, can (now) rotate its own "json-file" logs for each container. It can also forward logs to other logging systems like logstash or fluentd which can rotate logs. I currently have it forwarding to rsyslog, which actually can't rotate logs on its own, it depends on the old standard "logrotate" to help it do that (and so does syslog-ng by the way). I don't know anything about Kubernetes though.

A more feature-full logging package could be nice... but most core users seem content with the current logging, and a replacement will need some work and consensus.

[andyxning]

@ploxiln

There are a few different systems that collect stdout/stderr from processes/services and manage the writing of log files. This could be considered a better separation of concerns than if every service had its own logic for log files/rotation/relaying.

Agree. Just like what is recommended in 12factor#log. glog can meets this, too. By default, glog will send all logs to stderr which is consistent with log usage in nsq. At the same time, glog supports to log to a file with file size rotation. glog even supports logging to stderr and file at the same time. By enabling glog, we add more option for users and do not import any inconsistence. WDYT.

[ploxiln]

I do not object to a few more logging features, or to glog in particular.

[judwhite]

Coming from the wonderful world of Windows, when executables are started by Service Control Manager (running as a Windows Service, which any semi-serious production app does) you don't get access to stdin, stdout, or stderr. For real. Of all the things that annoy me about platform differences this is probably the top. Handling the streams is the right way to do it IMO, but when you're under SCM you're cut off and your only option is to wrap it or write log files yourself.

That said, Windows users are used to this and are happy if they can even specify where the logs and written. Rolling, max size, and max age would be downright magical. I use a combination of Logrus for structured, leveled logs and Lumberjack for rolling logs. This combo is common enough in my own work I rolled them into one, Logrjack (don't judge, naming is hard :D).

As a client library, go-nsq is a different story and already does the right thing by letting the user specify their log implementation. If you wanted to support writing structured logs from go-nsq a few tweaks wouldn't be so hard and could be done by expanding the API without breaking backwards compat.

It would be nice to add structure to the log output of nsqd/nsqlookupd for easier parsing. For example, maybe logfmt (my preference, easy for both machines and people to read) or JSON for nsqd/nsqlookupd, and go-nsq could provide the ability to write structured logs, but still be agnostic about how that happens.

I'd be happy to contribute to this if we come up with goals and non-goals and which packages to use (based solely on prior experience, Logrus and Lumberjack have worked well for me). If Linux users want to use direct-to-file logging I don't see why not, but I'd much rather be in their position and use stdout/stderr. Thoughts on next steps? Maybe a WIP PR to move the conversation toward an implementation?

EDIT: Related, if we do this we can close #817.

[ploxiln]

For windows support, I'd personally suggest ripping out the windows service hooks that are in nsqd/nsqlookupd, and tell people to use winsw to wrap them for windows. It looks like it runs any normal long-lived process, and also handles stdout/stderr logging to files: https://github.com/kohsuke/winsw/blob/master/doc/loggingAndErrorReporting.md

[judwhite]

@ploxiln NSSM is the de facto standard, but I'm not in favor of this approach. It would add another thing that can go wrong, can have compatibility issues, and has to be deployed. It's more work for the user and a barrier to entry. It would also change how people are deploying NSQ on Windows today. I added this support in a fork of 0.3.2 before it was merged in, we've been running like this for 2 years and it's been working great as-is.

I know it's related, but "ripping out windows service hooks" seems outside the scope of this discussion. Can we move this conversation back toward logging? If you want to discuss Windows Services can we please do that in another issue or on IRC/Slack?

[ploxiln]

Not arguing, just bringing up the datapoint to be considered. To be clear, it seems that #817 is in fact caused by the windows service hooks in nsqd and nsqlookupd, so users can't run processes as services with their preferred manager (nssm or winsw), and thus can't log to file and rotate logs.

[ploxiln]

Also, honest question, how do you manage nsqd / nsqlookupd logs on windows?

[judwhite]

@ploxiln Honest answer, we don't log nsqd/nsqlookupd. We monitor the clients. If people prefer to use NSSM there are ways to support that without removing native support.

[andyxning]

@judwhite @ploxiln @judwhite

I previously mean that whether we should replace log package with glog. IMO, this has nothing to do with the nsq deployment platform. We should make a decision about this movement in a general way instead of adjusting for any particular OS. :)

If you want to discuss this further, please move to #817.

For the discussion about this topic please take a look at #853 comment.

[judwhite]

@andyxning Sorry we got on a tangent 😃 If you have previous experience with glog and it meets the needs then great, let's try that if the core team is open to it. It does overlap a bit if we're seriously considering Logrus (overall one of the most popular golang packages, used in Prometheus and other high profile projects) which also handles the "leveling" aspect, but not the rotating part.

[andyxning]

@judwhite
glog also supports log leveles. :) For more info about it, please consult https://github.com/golang/glog @mreiferson .

glog has been used in almost all kubernetes projects. It has been tested already. :)

One of the biggest benefit is that log destination can be customized through command line.

[andyxning]

Finally, i googled that cronolog is a awesome tool for redirect and rotate stderr/stdout. 👏

[mreiferson]

I agree with @ploxiln — the handling of logs is the responsibility of the environment, not the process.

@judwhite I'm interested in the answer to #853 (comment)

[judwhite]

@mreiferson We don't log nsqd/nsqlookupd.

I also agree that handling logs is the responsibility of the environment. In practice, on Windows, it doesn't work that way. I wish it did.

[andyxning]

@mreiferson IIUC, you mean that we will not consider replacing log with glog, right?

[ploxiln]

If nsqd/nsqlookupd/... were to use a more sophisticated logging library, like glog or logrus, it would need to be made to integrate nicely with nsqd/nsqlookupd/... command-line flags and config files, and it would need to default to logging to stderr. If a pull request were opened that accomplished those goals in an elegant way, I don't know if it would be accepted, but it would have the best chance, IMHO.

[mreiferson]

If someone did all that work I think there's a 99% chance we would accept that PR. There's a 0% chance that I'm going to do that work.

[andyxning]

I would like to give this a try. :)

[judwhite]

@andyxning Would you like to pair on this? You can find me on the Gopher Slack.

[andyxning]

Yes, of cause. My Gopher Slack name is andyxning.

[lihan]

Is there an update for this? Having the same issue here, would like to check to avoid double up work. @andyxning @judwhite

[andyxning]

@lihan No work has been done. Hard work :(.
If you have time for adding this feature, I may review it. :)

[mreiferson]

ping #892 for an initial set of changes that relate to this