-
Notifications
You must be signed in to change notification settings - Fork 2k
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
wrap log
messages with hclog
#11291
wrap log
messages with hclog
#11291
Conversation
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.
LGTM. Is it worth updating the changelog entry to detail logging output, otherwise I find it's a little vague.
Good call. I reworded the CHANGELOG entry to be more descriptive. |
Might be worth filing an issue with hclog about InferLevels not handling https://github.com/hashicorp/mdns/blob/master/client.go#L120 right since these are all HashiCorp libraries. |
@schmichael it should based on this, so there seems to be something fishy going on. |
@josegonzalez Good catch! That mdns package has not needed much attention, so it might be doing something strange by modern Go standards. |
Ahh thanks @josegonzalez .
I thought |
I tried setting |
I'm going to lock this pull request because it has been closed for 120 days ⏳. This helps our maintainers find and focus on the active contributions. |
hclog
provides aWriter
that can be used to configure Go'slog
package to write logs through the application logger, but the agent configuration was not actually using thehclog
writer.This PR calls
log.SetOutput
with the correct input. ThelogOutput
value that was being used before is already being set as thelogger.Output
value.InferLevels
doesn't always work, but at least now all messages are proper JSON strings.Closes #11232
Sample agent config
Before
After