log with Info only when verbosity enabled on logger #782
Conversation
Signed-off-by: Kanika Rana <[email protected]>
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #782 +/- ##
==========================================
- Coverage 54.26% 47.38% -6.88%
==========================================
Files 64 64
Lines 6164 6590 +426
==========================================
- Hits 3345 3123 -222
- Misses 2549 3210 +661
+ Partials 270 257 -13 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| "manager", manager, | ||
| "serverSideApply", serverSideApply, | ||
| "serverSideDiff", true).Info(fmt.Sprintf("Applying resource %s/%s in cluster: %s, namespace: %s", obj.GetKind(), obj.GetName(), k.config.Host, obj.GetNamespace())) | ||
| if logWithLevel.V(0).Enabled() { |
There was a problem hiding this comment.
why not avoiding the if and adding the V(0) in the log statement below?
There was a problem hiding this comment.
i thought it best to not disturb the verbosity level, since for For dry-run the verbosity level is set at V(1) and the if logWithLevel.V(0).Enabled() condition should pass for V>=0.
There was a problem hiding this comment.
I am not sure if this is going to make a difference if the intention is to suppress this log entry when Argo CD is running in error mode. Error logs are sent with log.Error() not with log.V().Info(). So.. My understanding is that no log entry sent with the Info() method would be visible when setting the log level with error.
This should respect the loglevel set on ArgoCD instance when logging applied resources too.