nsqio / nsq

A realtime distributed messaging platform
https://nsq.io
MIT License
24.95k stars 2.9k forks source link

*: Structured logging in JSON #1460

Open adamroyjones opened 1 year ago

adamroyjones commented 1 year ago

There are two existing, but stale, issues that bear on this, namely https://github.com/nsqio/nsq/issues/853 and https://github.com/nsqio/nsq/issues/1218, but I thought it'd be worth creating something anew with a clear set of proposals.


I help to run NSQ in Google Kubernetes Engine (GKE). NSQ writes its log messages to standard error (stderr) and these log messages are ingested through Google Operations Suite (formerly Stackdriver) for viewing and querying in Google Cloud Logging. The documentation for Google Operations Suite says that

Severities: By default, logs written to the standard output are on the INFO level and logs written to the standard error are on the ERROR level. Structured logs can include a severity field, which defines the log's severity.

This maps to what I see: INFO-level logs emitted by NSQ are miscategorised as ERROR-level logs in Google Cloud Logging.

This could be solved if NSQ were capable of optionally emitting structured logs in JSON. This would allow us to parse, filter, and forward the logs as needed.

I think there are two natural approaches.

  1. NSQ could lift its minimum version of Go from 1.17 to 1.21; log/slog.JSONHandler could be used to optionally construct the logs.
  2. NSQ could continue to pin its minimum version of Go at 1.17; a structured logger (e.g., zerolog) could be used to optionally construct the logs.

In both cases, the option could be conditionally enabled by a new configuration flag, -log-json bool, for each component of NSQ.

I've no view on which implementation would be preferable, assuming either is desirable. I'd be happy to work towards either.

jehiah commented 1 year ago

NSQ could lift its minimum version of Go from 1.17 to 1.21; log/slog.JSONHandler could be used to optionally construct the logs.

I haven't tried, but we should be able to add an implementation that uses slog guarded by build tags to 1.21+ and backwards compatible with 1.20. That would allow for an flag to enable json logging.

We want to maintain compatibility with the last two go versions so that means 1.20+ for now, so that does constrain us somewhat but i'd love to see a PR for slog support if you are up for it.

adamroyjones commented 1 year ago

I'd be up for trying to introduce the feature; it may take a little while to find the time to finish it.

Before I embark, which do you think matches the goal of NSQ closest, given that we can't just lift the minimum version of Go to 1.21? I'm inclined towards 4; it seems the least error-prone.

  1. Lift the minimum version of Go to 1.20 and essentially vendorise log/slog into the project until the version can be lifted to 1.21. (I don't know how viable this is. I know mvdan/gofumpt vendorises Go code in this kind of way but I don't know how fraught it is.)
  2. Guard the entire JSON logging feature behind //go:build go1.21 and use log/slog.
  3. Provide two implementations of the feature: something like zerolog behind //go:build !go1.21 and log/slog behind //go:build go1.21.
  4. Use zerolog, but put it behind a log/slog-style interface that should make a future substitution to log/slog.JSONHandler easy.
mreiferson commented 1 year ago

When we publish new official binaries, we do so with the latest stable release of Go. The only situation where any of this backwards compat matters is if someone is building their own binaries (my intuition is this is rare) or using nsq as a Go package in their own application (my intuition is this is extremely rare).

As such, my recommendation would be (2), the "forward looking" approach with the least overhead that will benefit the most users.

adamroyjones commented 1 year ago

Lovely—thanks for the clear guidance. I'll try to have a solid stab at (2) soon.

adamroyjones commented 8 months ago

(Apologies for bumping, but I wanted to say that I've not forgotten about this at all—I've just been very busy.)

jclasley commented 4 months ago

@adamroyjones are you still working on this? If not, I'm happy to pick this one up

adamroyjones commented 4 months ago

@jclasley: I've not even started! I'm stretched, but not as I'd like. Please feel free to run with things.