Skip to content

Do not override the default log output in the server #160

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

Merged
merged 2 commits into from
Nov 6, 2020

Conversation

jbardin
Copy link
Member

@jbardin jbardin commented Oct 19, 2020

In order to transition large existing codebases to use end-to-end structured logging, we want to be able to make use of the hclog Logger.StandardWriter. The entails setting the default std library log output to a wrapped instance of an hclog.Logger while running the server.

The plugin.Server function however overrides the log output when initializing the server. This means the implementation currently needs to restore its logger again after the server has started (there is no callback, so this must be checked on the first method call). Failing to do so does not cause an error, however it does cause the log output to lose its structure.

Removing the call to log.SetOutput makes it easier to setup a std library logger for all implementations, at the minor expense of requiring all implementations to ensure they write to os.Stderr, which is almost always the default case.

Fixes #155

The go-plugin server should not override the std library log output. In
order to migrate to structured logging with hclog, the server may want
to set the default std logger to an hclog instance. This cannot be done
if the server overrides the default log output after calling Serve.
Copy link
Contributor

@mitchellh mitchellh left a comment

Choose a reason for hiding this comment

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

Yeah this seems good.

@jbardin jbardin merged commit c2aca9f into master Nov 6, 2020
@jbardin jbardin deleted the jbardin/allow-modified-logger branch November 6, 2020 20:51
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.

Standard log.Logger output overwritten
2 participants