Skip to content

Add sloglint and fix issues #37851

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 3 commits into from
Feb 27, 2025
Merged

Add sloglint and fix issues #37851

merged 3 commits into from
Feb 27, 2025

Conversation

aanm
Copy link
Member

@aanm aanm commented Feb 25, 2025 •
edited
Loading

See per commit basis

@aanm aanm added release-note/misc This PR makes changes that have no direct user impact. feature/slog labels Feb 25, 2025
@aanm
Copy link
Member Author

aanm commented Feb 25, 2025

/test

@github-actions github-actions bot added sig/policy Impacts whether traffic is allowed or denied based on user-defined policies. hubble-cli PRs or GitHub issues related with hubble-cli cilium-cli This PR contains changes related with cilium-cli labels Feb 25, 2025
@aanm
Copy link
Member Author

aanm commented Feb 26, 2025

/test

@aanm aanm marked this pull request as ready for review February 26, 2025 10:14
@aanm aanm requested review from a team as code owners February 26, 2025 10:14
@aanm aanm requested review from Artyop, sayboras and bimmlerd February 26, 2025 10:14
@aanm aanm requested a review from rolinh February 26, 2025 10:14
giorio94
giorio94 approved these changes Feb 26, 2025
View reviewed changes
Copy link
Member

@giorio94 giorio94 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! Feel free to disregard the suggestions if out of scope.

joamaki
joamaki approved these changes Feb 26, 2025
View reviewed changes
aanm
aanm commented Feb 26, 2025
View reviewed changes
rolinh
rolinh reviewed Feb 26, 2025
View reviewed changes
harsimran-pabla
harsimran-pabla reviewed Feb 26, 2025
View reviewed changes
@aanm aanm enabled auto-merge February 26, 2025 16:01
@aanm
Copy link
Member Author

aanm commented Feb 26, 2025

/test

rolinh
rolinh approved these changes Feb 26, 2025
View reviewed changes
Copy link
Member

@rolinh rolinh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hubble changes lgtm

derailed
derailed approved these changes Feb 26, 2025
View reviewed changes
Copy link
Contributor

@derailed derailed left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@aanm Great addition. Thank you Andre!

aanm added 2 commits February 27, 2025 07:25
@aanm
.golangci: add sloglint
288d188 
Add sloglint with some rules so that we have consistency across the
code base for the slog usage.

Signed-off-by: André Martins <andre@cilium.io>
@aanm
test: check for slog's BADKEY
ed497e9 
The presence of slog's BADKEY typically means that the provided key
for a log attribute is invalid, thus we should detect this string in the
logs during our CI.

Signed-off-by: André Martins <andre@cilium.io>
@aanm
Copy link
Member Author

aanm commented Feb 27, 2025

/test

@aanm
fix all issues found by sloglint
783670f 
Fix all of the issues found by sloglint in our code base.

Signed-off-by: André Martins <andre@cilium.io>
@aanm
Copy link
Member Author

aanm commented Feb 27, 2025

/test

sayboras
sayboras approved these changes Feb 27, 2025
View reviewed changes
Copy link
Member

@sayboras sayboras left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM for service mesh related files 👍

bimmlerd
bimmlerd approved these changes Feb 27, 2025
View reviewed changes
Copy link
Member

@bimmlerd bimmlerd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM - I guess we could bikeshed forever on what options to enable/disable 😅

@maintainer-s-little-helper maintainer-s-little-helper bot added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Feb 27, 2025
@aanm aanm added this pull request to the merge queue Feb 27, 2025
Merged via the queue into main with commit 912c18d Feb 27, 2025
279 of 281 checks passed
@aanm aanm deleted the pr/add-sloglint branch February 27, 2025 09:06
@github-actions github-actions bot mentioned this pull request Mar 10, 2025
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bimmlerd bimmlerd bimmlerd approved these changes

@giorio94 giorio94 giorio94 approved these changes

@derailed derailed derailed approved these changes

@joamaki joamaki joamaki approved these changes

@rolinh rolinh rolinh approved these changes

@aspsk aspsk aspsk approved these changes

@ldelossa ldelossa ldelossa approved these changes

@sayboras sayboras sayboras approved these changes

@Artyop Artyop Artyop approved these changes

@harsimran-pabla harsimran-pabla harsimran-pabla approved these changes

Assignees
No one assigned
Labels
cilium-cli This PR contains changes related with cilium-cli feature/slog hubble-cli PRs or GitHub issues related with hubble-cli ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-note/misc This PR makes changes that have no direct user impact. sig/policy Impacts whether traffic is allowed or denied based on user-defined policies.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.