-
Notifications
You must be signed in to change notification settings - Fork 452
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
Change log message to V(2), be sure to pass strings so it doesn't panic #1069
Change log message to V(2), be sure to pass strings so it doesn't panic #1069
Conversation
Signed-off-by: Kevin Earls <[email protected]>
@kevinearls could you please explain why the logger is changed to V(2)? |
internal/config/main.go
Outdated
@@ -140,7 +140,7 @@ func (c *Config) AutoDetect() error { | |||
return err | |||
} | |||
c.autoscalingVersion = hpaVersion | |||
c.logger.Info("In Autodetect, Set HPA version to [", c.autoscalingVersion, "] from [", hpaVersion, "]") | |||
c.logger.V(2).Info("In Autodetect, Set HPA version to [", c.autoscalingVersion.String(), "] from [", hpaVersion.String(), "]") |
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.
Aren't autoscalingVersion
and hpaVersion
the same value?
Also - the first string is a message but the remaining in the list are key value pairs, leaving the values as ] from [
and ]
- is this what we want?
I tried to fix this when I was working on an operator change and came up with:
c.logger.Info("In Autodetect, Set HPA version", "from", c.autoscalingVersion, "to", hpaVersion)
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.
+1, after the log message follows a list of key-value pairs. so it should be something like info("message line", "param1-name", param1-value)
Regarding the v(2)
. It makes sense to use the lowest value, however if the codebase already uses v(2) as a debug level then 2 is fine.
@pavolloffay I changed this as it previously had been an Info message and was appearing every 30 seconds or 1 minute in the log. It is not needed that often. @kristinapathak I will update the message. |
Signed-off-by: Kevin Earls <[email protected]>
…ic (open-telemetry#1069) * Change log message to V(2), be sure to pass strings so it doesn't panic Signed-off-by: Kevin Earls <[email protected]> * Changed log message to use V(1), updated parameters Signed-off-by: Kevin Earls <[email protected]> Signed-off-by: Kevin Earls <[email protected]>
Signed-off-by: Kevin Earls [email protected]