Add pkg/log package and cleanup logging around the codebase
#544
Conversation
| actionConfig := new(action.Configuration) | ||
|
|
||
| if err := actionConfig.Init(h.restClientGetter(namespace), namespace, "", log.Printf); err != nil { | ||
| log := log.FromContext(ctx).WithName("helm") |
| } | ||
|
|
||
| } else if node, err := nodeutil.GetControlPlaneNode(ctx, s, s.Name()); err != nil { | ||
| clusterRole = apiv1.ClusterRoleUnknown |
There was a problem hiding this comment.
Is the log here no longer necessary?
There was a problem hiding this comment.
yes, removed log lines that did not provide any useful context here and in some other places
There was a problem hiding this comment.
@bschimke95 is this useful to keep around?
There was a problem hiding this comment.
I would keep around the comment on when this might happen. Let's remove the logs though.
| @@ -58,6 +58,8 @@ func (c *UpdateNodeConfigurationController) retryNewK8sClient(ctx context.Contex | |||
| func (c *UpdateNodeConfigurationController) Run(ctx context.Context, getClusterConfig func(context.Context) (types.ClusterConfig, error)) { | |||
| c.waitReady() | |||
|
|
|||
| log := log.FromContext(ctx).WithValues("controller", "updatenodeconfiguration") | |||
There was a problem hiding this comment.
nit: snake case update_node_configuration would probably be easier to read.
| continue | ||
| case len(newEndpoints) == 0: | ||
| log.Println("warning: empty list of endpoints, skipping update") | ||
| log.Info("Warning: empty list of endpoints, skipping update") |
There was a problem hiding this comment.
should this be a warning level message instead of prefix this with "Warning"?
There was a problem hiding this comment.
Logs are either .Info() (with log levels) or .Error() (always printed)
| @@ -41,10 +42,15 @@ func startProxy(ctx context.Context, listenURL string, endpointURLs []string) er | |||
| MonitorInterval: time.Minute, | |||
| } | |||
|
|
|||
| log.Println("Starting proxy at", listenURL) | |||
| log := log.FromContext(ctx).WithValues( | |||
Summary
Add
pkg/logwith a standard logging interface and document how we should create logs across the codebase.Start from https://github.com/canonical/k8s-snap/compare/KU-1080/logging?expand=1#diff-3e6c5854477b8113c122f67474e7fba607496ef206b4a2a0761cdde6cc26b9b9
Replace usage of the standard library "log" with
pkg/logNotes
This is pre-requisite work for the CSRSigning controller for certificate refreshes.