TylerBrock / saw

Fast, multi-purpose tool for AWS CloudWatch Logs
MIT License
1.41k stars 79 forks source link

Display datetime and stream name in the output of get, and add flags #14

Closed will-ockmore closed 6 years ago

will-ockmore commented 6 years ago

Changes the way the formatting occurs for the log events, so that the format can be shared between the different commands. Also adds the flags available on the watch command to the get command for feature parity.

Closes #13

will-ockmore commented 6 years ago

Unsure what happened with the commit history, should only include the last one - able to squash if needed!

TylerBrock commented 6 years ago

Awesome, no need to squash but please rebase on master. Right now it's un-mergable due to conflicts.

will-ockmore commented 6 years ago

Yeah realised I'd not kept in sync with upstream locally. Fixed up the diff 👍

TylerBrock commented 6 years ago

Hey @will-ockmore, thanks for putting this up and for rebasing. I love it.

My only hesitation is that most of the time when using get I think people expect the non-color non-decorated output. Usually they just want to see the raw logs so that they can save/grep them. In my experience that is a large part of pitching saw to people who dislike dealing with cloudwatch logs directly, namely there are distinctly two modes: one for grabbing logs and one for watching logs.

If possible, do you think we could either:

I think I'd prefer the first but you could easily convince me the second option is the right way to go. It was my theory that the two modes thing was what people wanted but maybe i'm completely off base and people would want the colored pretty output but with the ability to turn it off... not sure.

To implement the first option (where the defaults between modes differ) the way I think we could do this is by having two default outputConfiguration structs (one pretty and one false). Let me know your thoughts on the defaults. Very open to suggestion here.

will-ockmore commented 6 years ago

I think two modes thing is definitely the way to go as well; and I think defaulting to the current behaviour is probably the best UX. Even in my own usage, I more often pipe the output of get to another program, and only sometimes want to display it in the terminal.

Sounds like a solid way to add the functionality also. Do you see this as a change for both watch and get (from your last paragraph)? If so I'll add the flags for both (--pretty for get and --raw for watch).

TylerBrock commented 6 years ago

Yeah I think that is the way to go. This will be great!

Lets add --pretty for get and --raw for watch.

will-ockmore commented 6 years ago

Hey @TylerBrock added the changes we discussed. Apologies for the long delay been out the country! Rather than create the two default structs (difficult choice between forgoing types, or adding more boilerplate functions) I just added both fields to the current OutputConfiguration struct. Think it's a bit cleaner but I might have misunderstood your suggestion!