Add a flag for including only certain fields #15
Conversation
|
@jalvz Were you intending to start PRs for the three issues? Or did you want to go through them one at a time to minimize merge conflicts? Or if you don't have time to work through and finish PRs, that's totally cool tool. |
cmd/ecslog/main_test.go
Outdated
| "ecslog --include-fields ...all the title fields...", | ||
| []string{"ecslog", "-i", "foo", "./testdata/exclude-fields.log"}, | ||
| 0, | ||
| regexp.MustCompile(`^\[2021-01-19T22:51:12.142Z\] INFO: hi\n foo: "bar"\n spam: \n$`), |
There was a problem hiding this comment.
Shouldn't the spam: ... line be complete gone here?
I'm slightly hesitant to have "-i" fields not apply to the "title" line fields, mainly because the title line is underspecified. What is included in the title line by default may change in future versions and could also differ between formats. However, I think I agree with you that it'll be useful as you have it. (Aside: It also makes me want to have a "oneline" format that puts all the extra (non-title line) fields on the same line. Something like: This is a case where I think Anyway, that is a long way to say that I think your call to have |
internal/ecslog/ecslog_test.go
Outdated
| "only log", | ||
| "no", "", "default", "", "", []string{"log"}, | ||
| `{"log.level":"info","@timestamp":"2021-01-19T22:51:12.142Z","ecs":{"version":"1.5.0"},"message":"hi","log.origin":{"foo":"bar",file.name":"main.go","file.line":"42"}}`, | ||
| "{\"log.level\":\"info\",\"@timestamp\":\"2021-01-19T22:51:12.142Z\",\"ecs\":{\"version\":\"1.5.0\"},\"message\":\"hi\",\"log.origin\":{\"foo\":\"bar\",file.name\":\"main.go\",\"file.line\":\"42\"}}\n", |
There was a problem hiding this comment.
it should, was missing a ', fixed now
|
My intention was to do one at a time as time permits, yes. Thanks for the review, I'll have a closer look shortly! |
NOTE: this does not affect fields that are part of the title (timestamp, message, etc); I don't think it would be convenient for users to have to explicitly type those.