Skip to content
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

Merged
merged 4 commits into from
Jan 13, 2025

Conversation

crenshaw-dev
Copy link
Member

@crenshaw-dev crenshaw-dev commented Jan 10, 2025

Fixes #5715

Modeled on https://github.com/argoproj/argo-rollouts/pull/2701/files

Took the opportunity to upgrade the adapter package.

Test:

> dist/argocd version
ERRO[0000] test                                          error="<nil>"
argocd: v2.14.0+2d0f48b.dirty
  BuildDate: 2025-01-10T21:56:48Z
  GitCommit: 2d0f48b67638f25f147f9e13ba6239fc047a40e4
  GitTreeState: dirty
  GoVersion: go1.22.5
  Compiler: gc
  Platform: darwin/arm64
FATA[0001] oauth2: "invalid_request" "Refresh token is invalid or has already been claimed by another client." 
> ARGOCD_LOG_FORMAT=json dist/argocd version
{"error":null,"level":"error","msg":"test","time":"2025-01-10T16:57:20-05:00"}
argocd: v2.14.0+2d0f48b.dirty
  BuildDate: 2025-01-10T21:56:48Z
  GitCommit: 2d0f48b67638f25f147f9e13ba6239fc047a40e4
  GitTreeState: dirty
  GoVersion: go1.22.5
  Compiler: gc
  Platform: darwin/arm64
FATA[0001] oauth2: "invalid_request" "Refresh token is invalid or has already been claimed by another client." 

@crenshaw-dev crenshaw-dev requested a review from a team as a code owner January 10, 2025 22:03
Copy link

bunnyshell bot commented Jan 10, 2025

❌ Preview Environment deleted from Bunnyshell

Available commands (reply to this comment):

  • 🚀 /bns:deploy to deploy the environment

Signed-off-by: Michael Crenshaw <[email protected]>
Signed-off-by: Michael Crenshaw <[email protected]>
Copy link

codecov bot commented Jan 10, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 55.25%. Comparing base (4d98359) to head (1addb68).
Report is 4 commits behind head on master.

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.
📢 Have feedback on the report? Share it here.

Copy link
Collaborator

@leoluz leoluz left a 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()))
Copy link
Collaborator

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.

@crenshaw-dev crenshaw-dev merged commit dbdc1e7 into argoproj:master Jan 13, 2025
28 checks passed
@crenshaw-dev crenshaw-dev deleted the klog-format branch January 13, 2025 15:50
dudo pushed a commit to dudo/argo-cd that referenced this pull request Jan 18, 2025
* 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]>
revitalbarletz pushed a commit to revitalbarletz/argo-cd that referenced this pull request Jan 20, 2025
* 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]>
flbla pushed a commit to flbla/argo-cd that referenced this pull request Jan 20, 2025
* 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]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

inconsistent log output format
2 participants