k9s
k9s copied to clipboard
derailed
[feat] Add toggle to allow log lines to be JSON colorized
I've got a tentative PR that adds colorized log lines when the line can be parsed as JSON (when it can't be parsed into JSON, the line is just returned as-is).
Colorizing can be applied permanently in the config, but by default is is toggled off, activated by pressing 'Ctrl-J':
I've marked this as "Draft" for the moment because I'd like to add some tests over this behaviour, and I've also got an outstanding PR open against colorjson (see https://github.com/TylerBrock/colorjson/pull/7) which would be nice to get merged rather than using my forked version, cc/ @TylerBrock.
I followed how the log timestamps were implemented, and added colorization to the model rather than the view. Initially, I had added it to the view and this didn't play nicely with filter or timestamps (this is how I discovered and came to fix #1776), whereas it works fine when located in the model.
Anyhow, hopefully this would be a useful feature on an already amazingly useful tool!
@rm-hull Very cool! This was a long awaited feature that many folks voiced. Thank you so much Richard for punching this thru and for the great explanation!! That said, I think we have to look in a bit closer about the cost of this implementation. Having to marshal/unmarshal each line is bound to run gc bunkers especially in this already less than optimal implementation. Let me noodle on this a bit more and take your branch out for a spin...
I didn't look too closely into the model code, but does it trigger the render for every log line or just the ones currently visible in the box view? If the former then, yes I imagine it could incur quite an overhead.
I did start off trying to write a naive parser that maintained a bit of state to determine the current color (whether in a string, in curly brackets, in square brackets, etc + managing that recursively) but it got complicated quickly.
Marshalling to JSON and the delegating to a lib to add color seemed like the right thing to do.
If we can restrict the view to just rendering the visible log lines this might still be ok?
@rm-hull Can you change the key to Shift-J for the json toggle? When viewing logs, I use the J key for scrolling down after scrolling up with the K key.
@rm-hull Thanks for this PR Richard. Looks like we have conflicts. Could you update?
@rm-hull Thanks for this PR Richard. Looks like we have conflicts. Could you update?
Conflicts fixed
Hi @derailed, @slimus - I rebased and fixed merge conflicts again, so the PR should be up-to-date w.r.t. the main branch .. would be good to get this in 🙏
LGTM! I checked out the PR and tested it with a few pod logs, works as expected.
@derailed I think this can be merged with the next release.
