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

*: Structured logging in JSON #1460

Open
adamroyjones opened this issue Sep 27, 2023 · 7 comments
Open

*: Structured logging in JSON #1460

adamroyjones opened this issue Sep 27, 2023 · 7 comments

Comments

@adamroyjones
Copy link

adamroyjones commented Sep 27, 2023

There are two existing, but stale, issues that bear on this, namely #853 and #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
Copy link
Member

jehiah commented Sep 28, 2023

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
Copy link
Author

adamroyjones commented Sep 28, 2023

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
Copy link
Member

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
Copy link
Author

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

@adamroyjones
Copy link
Author

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

@jclasley
Copy link

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

@adamroyjones
Copy link
Author

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

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

No branches or pull requests

4 participants