logger: make consistent flexible levels and contextual logging #1268
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/hold |
openshift-ci
bot
requested review from
bartoszmajsak,
lburgazzoli and
zdtsw
ykaliuta
force-pushed
the
slogger
branch
4 times, most recently
from
a306541
to
c18cd7e
Compare
|
/retest-required |
ykaliuta
force-pushed
the
slogger
branch
2 times, most recently
from
089d0e0
to
e83817e
Compare
|
/cc @aslakknutsen |
5 tasks
ykaliuta
force-pushed
the
slogger
branch
5 times, most recently
from
a1d88b7
to
2af9fab
Compare
ykaliuta
changed the title
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## incubation #1268 +/- ##
=============================================
Coverage ? 18.22%
=============================================
Files ? 30
Lines ? 2688
Branches ? 0
=============================================
Hits ? 490
Misses ? 2135
Partials ? 63 ☔ View full report in Codecov by Sentry. |
Switch ReconcileComponent from passing logger explicitly to wrapping it into context[1][2] Makes one parameter less to pass and will allow called utilities to report component context where they are called from. No user or logging format impact until utilities takes contextual logging in use. [1] https://www.kubernetes.dev/blog/2022/05/25/contextual-logging/ [2] https://github.com/kubernetes/enhancements/blob/master/keps/sig-instrumentation/3077-contextual-logging/README.md Credits-To: Bartosz Majsak [email protected] Signed-off-by: Yauheni Kaliuta <[email protected]>
Remove increasing logging level for controllers (it was also passed
to components if not overridden from DSCI) since:
- it made logging inconsistent. The base contoller runtime logger is
set up with INFO level for all log modes, so when controllers are
configured for Error level, user sees INFO messages from both
controller-runtime and other parts of operator which use
controller-runtime's logger directly;
- since the base logger is configured for INFO, there is no
difference in levels between "default" and "devel". Having levels 1
and 2 there is misleading.
Update documentation.
This patch changes default logging, former filtered Info messages
are displayed now.
There is no _big_ difference in practice since currently the log is
anyway full of info messages from parts which do not use
reconciler's logger, like:
{"level":"info","ts":"2024-10-09T13:23:11Z","msg":"waiting for 1 deployment to be ready for dashboard"}
Signed-off-by: Yauheni Kaliuta <[email protected]>
To avoid patch polluting with the next changes where uber's zap is imported as `zap`. Signed-off-by: Yauheni Kaliuta <[email protected]>
Allow to override global zap log level from DSCI devFlags. Accepts the same values as to `--zap-log-level` command line switch. Signed-off-by: Yauheni Kaliuta <[email protected]>
Since the log level is overridable with its own field of devFlags, do not use logmode anymore. It was used to create own logger with own zap backend in case if devFlags exist. Just add name and value to the existing logger instead. Signed-off-by: Yauheni Kaliuta <[email protected]>
Remove own logger from controllers' reconcilers and switch to k8s contextual logger instead [1]. Use contextual logger for SecretGeneratorReconciler and CertConfigmapGeneratorReconciler setups as well. [1] https://www.kubernetes.dev/blog/2022/05/25/contextual-logging/ Signed-off-by: Yauheni Kaliuta <[email protected]>
All the users should have proper context. Webhook is a separate server so makes own logger based on the controller-runtime's one. The log level changes will affect it as well. Signed-off-by: Yauheni Kaliuta <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Jira: https://issues.redhat.com/browse/RHOAIENG-13892
The patchset's intention is to address following issues:
Deprecate
logmodeand use logLevel instead to tweak zap log level. Use standard zap command line switches to tune it from command line.The patchset takes in use k8s contextual logger and it also propagates it down the chain to components and utility functions.
See the individual patches' messages.
Parts of this work:
#1253
#1271#1272#1280#1289
#1295Description
How Has This Been Tested?
Screenshot or short clip
Merge criteria