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

[NEW] Configuration option to output logs in JSON format or logfmt #12918

Open
enspritz opened this issue Jan 8, 2024 · 11 comments · May be fixed by #12934
Open

[NEW] Configuration option to output logs in JSON format or logfmt #12918

enspritz opened this issue Jan 8, 2024 · 11 comments · May be fixed by #12934

Comments

@enspritz
Copy link

enspritz commented Jan 8, 2024

The problem/use-case that the feature addresses
Running Redis 7.0.9 in a container. The logging format Redis currently employs presents friction to slurping its log output into monitoring.

Description of the feature
A configuration option that allows us to configure Redis to output its logs in at least one of JSON and logfmt.
Either of these formats are eminently grokkable by telemetry collection systems like fluentd, telegraf, promtail.
Having this option would make Redis more fluidly friendly with our systems, and allow us to more easily and reliably alert on errors and warnings.

Thanks!

@zuiderkwast
Copy link
Contributor

Everywhere throughout the Redis source code, logging is done with printf-like functions. It will be a very large change to change all those. I don't think it is acceptable to do it.

In a central place in the code is the formatting which adds the timestamp and a few other thing to each line. Here it is easy to add some conditional code to output it differently.

Here is an example of some log lines:

447467:M 03 Jan 2024 16:20:50.708 * Server initialized
447467:M 03 Jan 2024 16:20:50.708 * Ready to accept connections tcp
447467:M 03 Jan 2024 16:20:50.708 * Ready to accept connections unix
447467:M 03 Jan 2024 16:20:50.719 - Accepted 127.0.0.1:40327

First is the process id. Then is the role (M = master, S = replica, X = sentinel, C = RDB / AOF writing child). Then timestamp. Then a symbol (# = warning, * = notice, - = info, . = debug). After that is a log entry generated using print-like code like serverLog(LL_VERBOSE,"Accepted %s:%d", cip, cport); all over the source code.

What would be relatively each to achieve is change the formatting for the fixed fields, but keep the printf-like messages as they are, i.e. some formatting like this would be easy to achieve with a config:

pid=447467 role=M time=2024-01-03T16:20:50.719 level=info message="Accepted 127.0.0.1:40327"

Would that be good enough?

@sundb
Copy link
Collaborator

sundb commented Jan 8, 2024

Another alternative is to using like filebeat or logstash to format the logs and feed them to the monitor system.

@enspritz
Copy link
Author

enspritz commented Jan 8, 2024

@zuiderkwast I understand the refactoring load, and I don't presently have the latitude to take it on myself.
That last formatting example resemblant of logfmt you shared, at first glance it would indeed be good enough.

I'm no authority on logfmt, but as long as the telemetry collectors that grok logfmt will accept this style of output, then I imagine the process of integrating Redis into log collection/monitoring/alerting will merely be a matter of instructing the collector to parse loglines as logfmt and away we go!

@fede843
Copy link

fede843 commented Jan 9, 2024

I would really like to have this feature too.

@zuiderkwast
Copy link
Contributor

Feel free to implement it and open a PR. Then we can also see the complexity of it and take a decision. Personally I like the logfmt idea. We don't have JSON in Redis, so we can avoid adding the dependency. We probably need a simple function to do string escaping for the log message though.

@azuredream azuredream linked a pull request Jan 11, 2024 that will close this issue
@azuredream
Copy link

Hi, I opened a PR to print log in logfmt style.

pid=12073 role_char=M timestamp="10 Jan 2024 23:29:03.839" level=* message="Server initialized"
pid=12073 role_char=M timestamp="10 Jan 2024 23:29:03.839" level=* message="Ready to accept connections tcp"
^C12073:signal-handler (1704947359) Received SIGINT scheduling shutdown...
pid=12073 role_char=M timestamp="10 Jan 2024 23:29:19.433" level=* message="User requested shutdown..."
pid=12073 role_char=M timestamp="10 Jan 2024 23:29:19.433" level=* message="Saving the final RDB snapshot before exiting."
pid=12073 role_char=M timestamp="10 Jan 2024 23:29:19.436" level=* message="DB saved on disk"
pid=12073 role_char=M timestamp="10 Jan 2024 23:29:19.436" level=# message="Redis is now ready to exit, bye bye..."

I'll run more test tomorrow. I'm newcomer to this repo. I'd appreciate it if you could give me some advice on this. Thanks!

@azuredream
Copy link

Please review the sample output to see if that meets your requirements.
A question is that do you prefer role_char(M) or full role name (master). Same question for verbose level.
#12934 (comment)

@zuiderkwast zuiderkwast linked a pull request Jan 31, 2024 that will close this issue
@palmarisiva
Copy link

Is there any plan to include JSON format in the configuration option?
As per #12934, configuration changes using logfmt code changes are committed.

@zuiderkwast
Copy link
Contributor

@palmarisiva There is no plan, at least not yet. It seemed to me that either logfmt or JSON would be enough. I suggested logfmt because it seemed simple, but I guess JSON would be very similar to implement. Why you need JSON specifically?

@palmarisiva
Copy link

@palmarisiva There is no plan, at least not yet. It seemed to me that either logfmt or JSON would be enough. I suggested logfmt because it seemed simple, but I guess JSON would be very similar to implement. Why you need JSON specifically?

@zuiderkwast Our application uses logstash to process logs which supports JSON format. JSON format is used as a common logging mechanism by all the applications. Hence, expecting the JSON format in Redis too.

@ksemele
Copy link

ksemele commented Apr 10, 2024

@zuiderkwast we have a lot of apps in our production, and all of them can use JSON as the log format.
We use fluent bit for all our apps and third-party apps (such as Kafka, RabbitMQ, etc.). Why do we need to write custom parsers or use a different log collector only for Redis?
I think it's a great point for a roadmap.

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

Successfully merging a pull request may close this issue.

7 participants