-
Notifications
You must be signed in to change notification settings - Fork 107
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
Remove verbose=True from getMSLogger calls #10996
base: master
Are you sure you want to change the base?
Conversation
The changes to gitlab |
Jenkins results:
|
Here are some additional places within WMCore with explicit DEBUG level:
@amaltaro please advise which parts should be changed. |
@vkuznet thanks for providing these options, but the last bullet in the expected outcome - from the GH ticket description - states:
and this is the hard part. We need to review every single log record from all the microservices, and decide which ones should be logged all the time (INFO) and which ones are really meant to be debug mode only. I am afraid to that that the best way to deal with those would be to have Todor and myself looking into those microservices and swapping logs, e.g.::
to
when relevant. |
Alan, I understand, and I only wanted to help with what I can. In fact, I want to suggest a better approach to logging rather then simply swap INFO/DEBUG. In DBS GO server I use verbosity level, e.g. 0,1,2,3 such that I can change it in configuration and at run time and get more verbose information. Here it is a similar use-case. You actually need flexibility, i.e. increase the logging level during debugging phase and reduce it in normal operation. As such for MS services (since they are web services) you may add an extra end-point where you can send verbosity level (via POST request) and it will dynamically change web server behavior. Think about it. Meanwhile, I provided necessary PRs and relevant info which you can use. |
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.
Just to "silence" this review request.
As already discussed in this PR, we need to review all the log records throughout the microservices and decide which ones should be defined as logger.debug()
and logger.info()
. Such that when we move log level to INFO, we do not miss important information currently set as logger.debug.
I suggest to either Todor or myself to look into it, while looking into the production logs in parallel and assess what is the minimum log that we need to have in the service logs (INFO level). We might want to come up with an extra PR to complement this one.
test this please |
Jenkins results:
|
test this please |
Jenkins results:
|
test this please |
Jenkins results:
|
test this please |
Jenkins results:
|
test this please |
Jenkins results:
|
Can one of the admins verify this patch? |
Fixes #10317
Status
need confirmation from @amaltaro
Description
remove
verbose=True
in getMSLogger, the level should be applied by passing proper logger instance.Is it backward compatible (if not, which system it affects?)
MAYBE
Related PRs
dmwm/deployment#1124
External dependencies / deployment changes