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
Convert to logxi #1755
Convert to logxi #1755
Conversation
fa89965
to
803696c
Compare
@@ -281,18 +291,20 @@ func (b *backend) getLdapGroups(cfg *ConfigEntry, c *ldap.Conn, userDN string, u | |||
ldapMap := make(map[string]bool) | |||
|
|||
if cfg.GroupFilter == "" { | |||
b.Logger().Printf("[WARN] auth/ldap: GroupFilter is empty, will not query server") | |||
b.Logger().Warn("auth/ldap: GroupFilter is empty, will not query server") |
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.
IsWarn check is missing here and in the next log message as well.
As a general comment, it is a real pain to check IsDebug or IsWarn before printing anything.
Is it not possible that the Logger().Debug() will only print if Debug is enabled. So we can conveniently skip the check.
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.
ba934f6
to
dedbf27
Compare
c.Ui.Output(fmt.Sprintf("Unknown log level %s", logLevel)) | ||
return 1 | ||
} | ||
switch os.Getenv("LOGXI_FORMAT") { |
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.
Should we rename this to VAULT_LOGXI_FORMAT?
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.
ed5480d
to
821c71f
Compare
writer.Write([]byte(" [ALL] ")) | ||
} | ||
|
||
writer.Write([]byte(msg + " ")) |
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.
If we remove the " " from here, we can remove the if i > 0 {
check below?
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! Very good job! |
No description provided.