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.

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

update logging #227

Merged
merged 3 commits into from
Jun 6, 2024
Merged

update logging #227

merged 3 commits into from
Jun 6, 2024

Conversation

aobort
Copy link
Collaborator

@aobort aobort commented May 28, 2024 •
edited
Loading

Changes:

  • explicit flags to set logging level, logging level to attach stacktrace, development mode
  • explicit flag to disable webhooks
  • logging wrapper with zap & slog (might be reasonable to replace it with custom slog handler and remove zap in the future)
  • replace logr.V(N).* calls with more readable and clear wrapped log.Info(...), log.Debug(...) and log.Error(...)

Fixes: #208

@aobort aobort marked this pull request as draft May 28, 2024 12:45
@aobort aobort marked this pull request as ready for review May 28, 2024 15:09
@aobort
update logging
ff275f6 
Signed-off-by: Artem Bortnikov <artem.bortnikov@telekom.com>
@aobort
fix panic caused by nil logger in tests
a8c04d2 
Signed-off-by: Artem Bortnikov <artem.bortnikov@telekom.com>
kvaps
kvaps approved these changes Jun 5, 2024
View reviewed changes
Copy link
Member

@kvaps kvaps left a comment

Choose a reason for hiding this comment

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

LGTM

@Kirill-Garbar
Copy link
Collaborator

Also could you please add comments to functions and methods? For now it is not clear from the first glance what are the functions for.

@aobort
update according to code review notes
0782ef6 
Signed-off-by: Artem Bortnikov <artem.bortnikov@telekom.com>
@aobort aobort merged commit 3d90173 into aenix-io:main Jun 6, 2024
9 checks passed
@aobort aobort deleted the chore/208-logging branch June 6, 2024 10:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Kirill-Garbar Kirill-Garbar Kirill-Garbar approved these changes

@kvaps kvaps kvaps approved these changes

@sircthulhu sircthulhu Awaiting requested review from sircthulhu sircthulhu is a code owner

@sergeyshevch sergeyshevch Awaiting requested review from sergeyshevch sergeyshevch is a code owner

Assignees
No one assigned
Labels
chore controllers
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Disable development logger
3 participants
@aobort @Kirill-Garbar @kvaps