-
Notifications
You must be signed in to change notification settings - Fork 5.7k
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
feat: use log format config for klog (#5715) #21458
Conversation
Signed-off-by: Michael Crenshaw <[email protected]>
Signed-off-by: Michael Crenshaw <[email protected]>
❌ Preview Environment deleted from BunnyshellAvailable commands (reply to this comment):
|
Signed-off-by: Michael Crenshaw <[email protected]>
Signed-off-by: Michael Crenshaw <[email protected]>
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #21458 +/- ##
==========================================
+ Coverage 55.22% 55.25% +0.03%
==========================================
Files 337 337
Lines 56919 56922 +3
==========================================
+ Hits 31431 31450 +19
+ Misses 22808 22799 -9
+ Partials 2680 2673 -7 ☔ View full report in Codecov by Sentry. |
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. Added a comment as a reference to a future improvement that we should maybe consider in the future.
) | ||
|
||
const ( | ||
binaryNameEnv = "ARGOCD_BINARY_NAME" | ||
) | ||
|
||
func init() { | ||
// Make sure klog uses the configured log level and format. | ||
klog.SetLogger(log.NewLogrusLogger(log.NewWithCurrentConfig())) |
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.
The problem with static loggers is that anybody can change this configs in a different package which would reflect in the entire application. Unfortunately, at this point there is no much we can do to improve this situation other than refactoring the log usage in the entire codebase.
* feat: use log format config for klog (argoproj#5715) Signed-off-by: Michael Crenshaw <[email protected]> * use init, remove test line Signed-off-by: Michael Crenshaw <[email protected]> * sort imports Signed-off-by: Michael Crenshaw <[email protected]> * fix comment Signed-off-by: Michael Crenshaw <[email protected]> --------- Signed-off-by: Michael Crenshaw <[email protected]> Signed-off-by: Brett C. Dudo <[email protected]>
* feat: use log format config for klog (argoproj#5715) Signed-off-by: Michael Crenshaw <[email protected]> * use init, remove test line Signed-off-by: Michael Crenshaw <[email protected]> * sort imports Signed-off-by: Michael Crenshaw <[email protected]> * fix comment Signed-off-by: Michael Crenshaw <[email protected]> --------- Signed-off-by: Michael Crenshaw <[email protected]>
* feat: use log format config for klog (argoproj#5715) Signed-off-by: Michael Crenshaw <[email protected]> * use init, remove test line Signed-off-by: Michael Crenshaw <[email protected]> * sort imports Signed-off-by: Michael Crenshaw <[email protected]> * fix comment Signed-off-by: Michael Crenshaw <[email protected]> --------- Signed-off-by: Michael Crenshaw <[email protected]> Signed-off-by: flbla <[email protected]>
Fixes #5715
Modeled on https://github.com/argoproj/argo-rollouts/pull/2701/files
Took the opportunity to upgrade the adapter package.
Test: