Skip to content

fix: call to logger with odd number of key/value params#4478

Merged
erikgb merged 1 commit intoweaveworks:mainfrom
erikgb:fix-log-panic
Jan 9, 2025
Merged

fix: call to logger with odd number of key/value params#4478
erikgb merged 1 commit intoweaveworks:mainfrom
erikgb:fix-log-panic

Conversation

@erikgb
Copy link
Copy Markdown
Contributor

@erikgb erikgb commented Jan 9, 2025

Closes #4193

What changed?

Fix bug with odd number of key/value pairs sent to logger. There is already an open PR to fix this, but we got no response from the author: #4193.

I am also adding a linter to avoid this from happening again: https://github.com/timonwong/loggercheck

Without fixing the bug, but enabling the new linter, it fails:

$ make lint
/home/erikbo/projects/github/weave-gitops/bin/golangci-lint run
core/server/inventory.go:290:74: odd number of arguments passed as key-value pairs for logging (loggercheck)
                                                logger.Error(err, "failed to determine if %s is namespace scoped", obj.GetObjectKind().GroupVersionKind().Kind)
                                                                                                                   ^
make: *** [Makefile:113: lint] Error 1

Why was this change made?

Bugfix to avoid panic if attempting to log from here..

How was this change implemented?

How did you validate the change?

Release notes

Documentation Changes

@erikgb erikgb requested review from casibbald and gusevda January 9, 2025 21:07
@erikgb erikgb changed the title fix: call to logger with odd key/value params fix: call to logger with odd number of key/value params Jan 9, 2025
@erikgb erikgb enabled auto-merge January 9, 2025 21:10
@erikgb erikgb disabled auto-merge January 9, 2025 21:11
@erikgb erikgb enabled auto-merge (rebase) January 9, 2025 21:12
@erikgb erikgb merged commit 9a1b73b into weaveworks:main Jan 9, 2025
This was referenced Jan 15, 2025
This was referenced Feb 4, 2025
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.

2 participants