Structured logging #179
Structured logging #179
Conversation
* add log level option and logging for non-matching requests * format * replace all log imports for logrus * structured logs for watcher.go * add log fields helper and log for route_matchers * add exits on fatal errors, log level flag/config, logging field methods for requests and imposters * replace printf for leveled logs * add logs for handlers * logging on configurations * add os.Exit(1) after fatal errors * replace all calls to log.Fatal to log.Error+os.Exit * write body for missing requests * undo makefile changes
|
@joanlopez thanks for the review, i addressed the comments, would you mind giving it another round? |
| } | ||
| } | ||
|
|
||
| // not used? |
There was a problem hiding this comment.
also, @joanlopez , is this method ever used? since we configure s.router.NotFoundHandler i would assume that already acts as a catch all
|
@joanlopez just came back to this one to resolve a conflict, could you please take a look so future changes in main don't require this PR to be updated too? |
|
Hey @tomp21! Sorry, we have prioritized #139 because it's critical. Killgrave's codebase is increasing, as well as the contributors base, and we have many cool features on the pull requests pipeline, that's risky to introduce without a decent amount of testing (see the example of #173, where we had to fix an issue introduced in a previous changeset). The good news is that we're very very close to merging it. I'll ping you once ready, so you can pull latest changes, and I'll have a final look to your PR, and see if there's any additional feedback I'd like to leave. So, you don't experience any additional conflict, and we can get this merged asap. Meanwhile, thanks for your patience! 🙌🏻 |
Closes #39
This is a first iteration, some testing is pending still but i wanted to raise the PR to have some early feedback, specially on the creation of
loghelper.goand thefunc (i Imposter) LogFields()method, it just didn't feel right to me to repeat the fields everywhere and this way it looks more maintainable to me, but perhaps there's something i'm overseeing