Switch to use json logging with slog#97
Conversation
| handler := slog.NewJSONHandler(os.Stdout, &slog.HandlerOptions{ | ||
| Level: logLevel, | ||
| }) | ||
| ctrl.SetLogger(logr.FromSlogHandler(handler)) |
There was a problem hiding this comment.
blocking: I don't want to cut a major version for this, but I also don't think it's appropriate to change the default log output format without doing so, as end users may be relying on that for their log ingestion/processing. Can we maintain the same behavior by default, and introduce a flag that controls the log format?
There was a problem hiding this comment.
Sure, as long as we don't want it exposed through the helm chart, which will make it rather annoying to actually use. That'd depend on us being able to parameterize the helm chart, will will require a major version bump anyways.
There was a problem hiding this comment.
can we just turn it on with an env var which wouldn't require any changes to the helm chart?
There was a problem hiding this comment.
Sorry I didn't actually push the change with my last comment but yeah, we'll add the env var LOG_FORMAT=json. We just can't easily expose that through the chart right now.
|
fixes #71 (I think) |
emily-curry
left a comment
emily-curry
left a comment
There was a problem hiding this comment.
Can you please add documentation here before merging, and coordinate with support on getting our main docs page updated?
|
any chance this can be merged and released? Pretty-please ❤️ |
5 participants
No description provided.