-
Notifications
You must be signed in to change notification settings - Fork 677
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
switch to go-kit logger #1192
switch to go-kit logger #1192
Conversation
f8b2afd
to
17f3d1d
Compare
@breed808 can you do take a look here? |
2e18796
to
8bd9cb8
Compare
@breed808 I would appreciate a review here. |
dd8ec18
to
45f9149
Compare
rebased |
@breed808 I would appreciate a review here, please let me know, if I can assist here. |
Signed-off-by: Jan-Otto Kröpke <mail@jkroepke.de>
Signed-off-by: Jan-Otto Kröpke <mail@jkroepke.de>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Apologies for my absence in reviewing this. Thanks very much for the time and effort you've put into this change, it's another really good PR 😄
I've left a few minor comments, but aside from those I have no issues with change proposed by this PR.
Signed-off-by: Jan-Otto Kröpke <mail@jkroepke.de>
Signed-off-by: Jan-Otto Kröpke <mail@jkroepke.de>
I addressed all your changes. I hope thats fine now. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, thanks for integrating those changes 👍
Fixes #979
Replaces #987
See
for similar changes
depends on
This is a breaking change for the
log.format
CLI flags.--log.format logger:stderr
->--log.file stderr --log.format logfmt
--log.format logger:stdout?json=true
->--log.file stdout --log.format json
--log.format logger:eventlog?name=windows_exporter
->--log.file eventlog
The breaking change happens on node_exporter before version 1, too. They dont add any backwards compability.
windows_exporter is still < version 1. In terms of semver, a breaking change is fine here.