-
Notifications
You must be signed in to change notification settings - Fork 2.8k
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
[v1.13] Make logging.InitializeDefaultLogger private #29700
Conversation
[ upstream commit: b175548 ] The function InitializeDefaultLogger creates a new logger with the default settings and returns it. This commit removes a call to this function that doesn't save the return value, essentially calling it for no reason. Signed-off-by: Ryan Drew <ryan.drew@isovalent.com> Signed-off-by: Anton Ippolitov <anton.ippolitov@datadoghq.com>
/test-backport-1.13 |
[ upstream commit 00ab252 ] [ Backporter's notes: ignoring changes from cilium#26327 and cilium#23971 ] The DefaultLogger variable in the logging module serves as a parent logger which all other loggers can be derived from. It is initialized using the InitializeDefaultLogger function and then adjusted on startup based on user configuration. Users should not call InitializeDefaultLogger to create a parent logger for their logger, since the logger returned by InitializeDefaultLogger will always use the hardcoded defaults. For example, the logger returned will always be of level INFO, even if a user has enabled debug logging. To make this clear, this commit renames InitializeDefaultLogger to initializeDefaultLogger to signal that it should not be used outside of the logging module. Fixes: cilium#29215 Signed-off-by: Ryan Drew <ryan.drew@isovalent.com> Signed-off-by: Anton Ippolitov <anton.ippolitov@datadoghq.com>
ffb16ef
to
d05348f
Compare
I missed a small issue in the tests, going to re-run the suite again :( |
/test-backport-1.13 |
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.
I've requested a review from @learnitall to double check. I'll unblock the @cilium/tophat review.
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.
Looks good, thank you!
/test-backport-1.13 |
The failing "Cilium IPsec upgrade" tests seem to be completely unrelated, looks like it's a known issue described here: #29351 (comment) |
1.13 backport for #29495
As mentioned in #29495 (comment), the original PR contained code which relied on changes made in #26327 and #23971. I tried to workaround this as much as possible to make the backport as minimal and self-contained as possible:
atomic.StorePointer
+unsafe.Pointer
inpkg/endpoint/log.go
and didn't touch other files changed in that PRpkg/endpoint/log.go
and didn't touch other files changed in that PROnce this PR is merged, a GitHub action will update the labels of these PRs: