-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Add 'log-format' CLI flag, along with associated config flag, for 'vault server' command. #6840
Conversation
…a 'log_level' flag
…flag. Also, get rid of 'log_json' flag.
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.
It looks like ParseEnvLogFormat
is untested, do you think it's necessary to test it? I only see it used (in these files at least) when called from NewVaultLoggerWithWriter
(which is also untested, but as a simple New*
function I'm not worried about it)
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.
This is looking really good, just a couple of minor questions.
@catsby It was a good idea to add a test for |
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.
This looks great! I tested adding log_format = json
to my config file, and I also tried the env var and CLI parameter. I also tried configuring various empty strings, and they defaulted to standard, and I tried doing it without any designation. Everything worked as expected. Good work!
Add a
log-format
CLI flag that can specify either "standard" or "json" for the log format for thevault server
command. Also add alog_format
config file flag that does the same thing. If the format is not specified, then standard log format is used.We already had the ability to specify this via an environment variable. Note that unlike some of the other environment variable flags, the definition of how json format is enabled is rather permissive, since it allows two different names for the variable, and a few different names for json format. This has been preserved for backwards compatibility. See https://github.com/hashicorp/vault/blob/issue-6796/sdk/helper/logging/logging.go#L67-L81
The precedence of the flags for this setting is CLI, then env var, then config file.
This pull request address issue #6796