Add logging configuration to the main configuration file #9056
Conversation
ghost
assigned 
jsternberg
force-pushed
the
js-configure-logging
branch
4 times, most recently
from
1aafd6c to
88e72a9
Compare
jsternberg
force-pushed
the
js-configure-logging
branch
6 times, most recently
from
8adc0c4 to
4402b51
Compare
|
Would this change also suppress the extra timestamp printed currently? (Hoping that it does.) Or would that become configurable. (Didn't see any clear options for choosing parameters.) |
|
@Artanicus can you clarify what you mean? Thanks. |
jsternberg
force-pushed
the
js-configure-logging
branch
from
4402b51 to
67bb28b
Compare
Make logfmt the default log output format with json and a more user friendly console into possible options.
For any systems that want to read the log file in the specific format, the logo being printed on restart may not be good for those parsers since the log parser would have to be aware of the logos existance or capable of just ignoring lines it couldn't parse. This gives an option to disable the printed logo if required. Like other logging options, this will fail if the configuration file itself is invalid.
jsternberg
force-pushed
the
js-configure-logging
branch
from
67bb28b to
13951a8
Compare
jsternberg
force-pushed
the
js-configure-logging
branch
2 times, most recently
from
d32a7ed to
ae1afde
Compare
There was a problem hiding this comment.
Looks good to me.
To clarify, it appears we can expect to receive logfmt from 3rd parties, if they use default settings (auto).
My only nit is that we could receive any of the three formats (JSON, logfmt or console) from a 3rd party and therefore maintaining tools to parse all three will be painful. Given the console format is a little harder to parse (since elements of the data are in different formats), I'd suggest we only allow the console format when it rendering to a tty, limiting what we receive as log files to JSON or logfmt, which already have plenty of tooling.
jsternberg
force-pushed
the
js-configure-logging
branch
2 times, most recently
from
f1a1ad3 to
4f0a6fc
Compare
This restricts the logging format options so console cannot be chosen. This way, logs written to a non-TTY will never be the console format.
jsternberg
force-pushed
the
js-configure-logging
branch
from
4f0a6fc to
2bb5b68
Compare
| func (c *Config) New(defaultOutput io.Writer) (*zap.Logger, error) { | ||
| w := defaultOutput | ||
| format := c.Format | ||
| if format == "console" { |
There was a problem hiding this comment.
this will work, but you could also switch this to a white list
if format != "json" && format != "logfmt" && format != "auto" {
return nil, fmt.Errorf("unknown logging format: %s", format)
}There was a problem hiding this comment.
The error message for most log formats is handled by newEncoder. The reason why this is handled specially is because I didn't want to modify newEncoder to accept auto. Since newEncoder can't make the decision since it doesn't have access to the io.Writer, I wanted to make it explicit that console was specified. Since that means console is a valid input, I had to make it so a user-specified console would be intercepted and would pretend to be an invalid format.
…from multiple goroutines
Make logfmt the default log output format with json and a more user
friendly console into possible options.
This also adds the ability to suppress the logo output in the configuration
file. If the configuration file fails to be parsed in any way, these options
also fail and the defaults are used.
This change makes it so InfluxDB has a more consistent user output and follows
along with Kapacitor for the default output. That should make it easier to
consolidate Kapacitor and InfluxDB to use the same logging framework and
patterns.
The console logging output can only be used by specifying
autoand writing to a TTY. It is not an available log format for logging to a file.etc/config.sample.toml), serverNewDemoConfigmethod, andDiagnosticsmethods reporting config settings, if necessary