Skip to content

Conversation

@jmunson
Copy link
Contributor

@jmunson jmunson commented Jul 13, 2020

This is decent sized refactor of all logging code. The "file" backend could still use more work, as it doesn't retain the calling spec.

We still respect the configuration options to configure log level and json logging, though debug logging now enables logging where in the source code the log was emitted.

All log messages have been lowercased for consistency, and log objects prefer snake_case for object names.

Some redundant logging(where we log, then return an error to a function that logs it) has been removed.

@jmunson jmunson requested a review from szechuen July 13, 2020 21:54
@szechuen
Copy link

CI is failing since it uses vendoring and the added dependency hasn't been vendored. Do we still want to vendor dependencies?

@jmunson
Copy link
Contributor Author

jmunson commented Jul 17, 2020

I updated the PR to update the vendored dependencies. I'll save the decision on moving away from vendored deps until later

willbuckner
willbuckner previously approved these changes Jul 18, 2020
szechuen
szechuen previously approved these changes Jul 20, 2020
Copy link

@szechuen szechuen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Second @willbuckner's suggestions, but otherwise lgtm

@jmunson jmunson changed the title Refactor logging, switch to echolog, log structured objects, enable caller logging in debug Refactor logging, switch to zerolog, log structured objects, enable caller logging in debug Jul 21, 2020
@jmunson jmunson dismissed stale reviews from szechuen and willbuckner via bd9004b July 21, 2020 21:28
@jmunson
Copy link
Contributor Author

jmunson commented Jul 21, 2020

Pushed a rewritten commit to address Wills feedback

…aller logging in debug

This is decent sized refactor of all logging code. The "file" backend could still use more work, as it doesn't retain the calling spec.

We still respect the configuration options to configure log level and json logging, though debug logging now enables logging where in the source code the log was emitted.

All log messages have been lowercased for consistency, and log objects prefer snake_case for object names.

Some redundant logging(where we log, then return an error to a function that logs it) has been removed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants