added log filters to filter the log output #209
Conversation
|
I did an upgrade on all dependencies, mainly for cobracli. The mutually exclusive feature of the new cobraCli is great as it lets us restrict which flags can be used together. It seems there was a change in the aws sdk which caused an issue, a small fix solved that. |
|
Thank you very much for this PR. Would it be possible to add another filter which only displays the logs after the newest CloudFormation entry |
internal/cmd/logs/logs.go
Outdated
| @@ -59,4 +61,7 @@ func init() { | |||
| Cmd.Flags().BoolVarP(&allLogs, "all", "a", false, "include uninteresting logs") | |||
| Cmd.Flags().BoolVarP(&chart, "chart", "c", false, "Output a gantt chart of the most recent action as an html file") | |||
| Cmd.Flags().BoolVar(&config.Debug, "debug", false, "Output debugging information") | |||
| Cmd.Flags().IntVarP(&logsLength, "length", "l", 0, "Number of logs to display") | |||
| Cmd.Flags().IntVarP(&logsDays, "days", "d", 0, "Age of the logs to display in days") | |||
| Cmd.MarkFlagsMutuallyExclusive("days", "length") | |||
There was a problem hiding this comment.
Why are these exclusive? What if a single day has a large number of log entries?
There was a problem hiding this comment.
just did not test those things together, will add tests and remove that exclusive option, they should work like you said a test till solidify it.
internal/cmd/logs/util.go
Outdated
| @@ -34,7 +34,38 @@ func (e events) Less(i, j int) bool { | |||
| return ptr.ToTime(e[i].Timestamp).Unix() < ptr.ToTime(e[j].Timestamp).Unix() | |||
| } | |||
|
|
|||
| func printLogs(logs events) { | |||
| func reduceLogsByLength(logsRange int, logs *events) *events { | |||
There was a problem hiding this comment.
I think reduceLogsToLength would be a better name. "By length" to me implies that you will subtract that much from it.
There was a problem hiding this comment.
agreed, will change it to that
| duration := time.Duration(time.Hour * time.Duration(logsDays*-24)) | ||
| logs = *reduceLogsByDuration(duration, time.Now(), &logs) | ||
| } | ||
| if logsRange > 0 { |
There was a problem hiding this comment.
I think this will just work as expected if you remove the mutually exclusive flag.
There was a problem hiding this comment.
yep, I will also add tests to make sure the combination works
internal/cmd/logs/util.go
Outdated
| func printLogs(logsRange int, logsDays int, logs events) { | ||
| if logsDays > 0 { | ||
| duration := time.Duration(time.Hour * time.Duration(logsDays*-24)) | ||
| logs = *reduceLogsByDuration(duration, time.Now(), &logs) |
There was a problem hiding this comment.
Why time.Now() here? Why does this need to be a param?
There was a problem hiding this comment.
I did it for the tests, but on second thought I could have done it better, I'll redo the way i handle time and testing and remove that param
|
added a commit addressing all the above issues mentioned above |
Nice work! |
*Issue #192 *
Description of changes:
Added two flags to rain log command
--lengthor-lfor limiting the length of logs and--daysor-dfor limiting logs output by number of daysBy submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.