feat: add global.jsonLog Helm value for production logging mode#1040
feat: add global.jsonLog Helm value for production logging mode#1040geoah wants to merge 2 commits intokai-scheduler:mainfrom
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
The color coding is essential for humans to read those logs. As a person that reads them a lot it is much more convenient to do it from the pod than log aggregations as most of the time issues rise in test environments that don't have them and then the colors are important. I think that adding a different logging mode that will fit your needs is important, so maybe you can add a flag to switch between the two options with the default staying as it is today. |
| when running a service directly: | ||
|
|
||
| ```bash | ||
| ./binder --zap-devel=true |
There was a problem hiding this comment.
should be configurable from the operator / helm chart
| ## Logging | ||
|
|
||
| All KAI services use production-mode logging by default, producing JSON-formatted output | ||
| without ANSI color codes. This is optimized for log aggregation platforms where single-line |
There was a problem hiding this comment.
I don't want you to remove the colors, they are important for debugging. It is funny how LLMs have hard time to remove things so they will find ways to mention what they deleted to leave some trace of it in the world.
There was a problem hiding this comment.
Hey @enoodle, thank you for the review and comments, I've restored the original functionality and added a logJson config that disables zap-devel. Let me know if this is closer to how you think this should work. If not, some extra guidance would be appreciated! :) Thank you very much in advance!
Add opt-in JSON logging mode for log aggregation platforms. When global.jsonLog is enabled via Helm, the operator passes --zap-devel=false to controller-runtime services and --log-json to the scheduler, switching to JSON-encoded output.
geoah
changed the title
enoodle
left a comment
enoodle
left a comment
There was a problem hiding this comment.
Looks great, I have one small comment.
|
|
||
| common.AddK8sClientConfigToArgs(config.Service.K8sClientConfig, args) | ||
|
|
||
| if kaiConfig.Spec.Global.JSONLog != nil && *kaiConfig.Spec.Global.JSONLog { |
There was a problem hiding this comment.
Can you make a utility function for all those code clauses that repeat in the resources.go files?
Can be in pkg/operator/operands/common
2 participants
Description
All KAI services default to development-mode logging with colored, human-readable console output optimized for reading logs directly from pods. However, when deploying to clusters with log aggregation platforms (Datadog, Splunk, etc.), ANSI color codes and multi-line stack traces make logs difficult to parse and correlate.
This PR adds an opt-in production logging mode configurable via a single Helm value:
Related Issues
Fixes #1039
Checklist
Breaking Changes
Additional Notes