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 system to use structured logging #7060

Closed
wants to merge 1 commit into from

Conversation

jsternberg
Copy link
Contributor

Add a custom log formatter to match our output conventions and colorize
the terminal output when run interactively. Includes colors, spacing,
and output of actual timestamps rather than seconds since the program
started.

Fixes #7036.

@jsternberg jsternberg force-pushed the js-7036-structured-logging branch from 66e58d9 to 8a3e042 Compare July 25, 2016 21:25
select {
case <-signalCh:
m.Logger.Println("second signal received, initializing hard shutdown")
log.Warn("second signal received, initializing hard shutdown")
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think either of these are warn or error. You 100% want all of them to show up in your log, regardless of log level. I think they should all be info.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I know we don't particularly want to use warn very much, but I do think this is a reasonable place to use it. A second signal could end up getting received by an automated system and it would be important information to know that, say, systemd needed to use a hard shutdown.

Add a custom log formatter to match our output conventions and colorize
the terminal output when run interactively. Includes colors, spacing,
and output of actual timestamps rather than seconds since the program
started.
@jsternberg jsternberg force-pushed the js-7036-structured-logging branch from 8a3e042 to 894436a Compare July 26, 2016 17:46
@jsternberg
Copy link
Contributor Author

We'll be revisiting this in the future.

@jsternberg jsternberg closed this Nov 29, 2016
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.

2 participants