Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor logging, switch to zerolog, log structured objects, enable caller logging in debug #106

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

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
cert/storage/command.go Outdated Show resolved Hide resolved
cert/transport.go Show resolved Hide resolved
certmgr/mgr/file.go Outdated Show resolved Hide resolved
certmgr/metrics/metrics.go Show resolved Hide resolved
certmgr/mgr/file.go Outdated Show resolved Hide resolved
cert/storage/disk.go Outdated Show resolved Hide resolved
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