Zhou Zheng Sheng has posted comments on this change.

Change subject: Logging: config logging in new process of remoteFileHandler
......................................................................


Patch Set 1: (1 inline comment)

....................................................
File vdsm/storage/remoteFileHandler.py
Line 36: 
Line 37: if __name__ == "__main__":
Line 38:     from logging import config as lconfig
Line 39:     loggerConfFile = constants.P_VDSM_CONF + 'logger.conf'
Line 40:     lconfig.fileConfig(loggerConfFile)
When you import a Python module, it does not just get the function and class 
definitions, but also can execute code, call other functions, and so on. So 
there are always chances to indirectly call a function that writes logs.

1. An example of calling a function during the import is at image.py line 41.
  rmanager = rm.ResourceManager.getInstance()
That means we could run function during module import. You may say these 
functions do not write logs, so comes the case 2.

2. An example of indirectly calling a function that writes logs is at lvm.py 
line 618.
  _lvminfo = LVMCache()
And LVMCache() calls _setupLVM() and it calls log.warning()

3. An example of initializing a logger object during the class definition is at 
threadPool.py line 27
class ThreadPool:
    log = logging.getLogger('Misc.ThreadPool')

These pieces of code will be executed when we import their modules. These are 
just the cases I found, probably more cases like these exist in VDSM.

For case 1 and 2, the problem is trivial because there will be not many logs 
during the import. For case 3, the logger object is constructed during the 
module import and will be not updated when later we call 
logging.config.configFile(), so we will lost all the logs of "ThreadPool", and 
Case 3 could be very popular in VDSM.

Currently remoteFileHandler just imports misc and fileUtils, but misc and 
fileUtils can indirectly import other modules and after several level of 
indirection we could import some modules that will trigger case 3.

For now I will focus on fixing case 3, and ignore case 2. Thank you guys for 
reviewing the code, I will submit a new version according to your suggestions.
Line 41: 
Line 42: import misc
Line 43: import fileUtils
Line 44: 


--
To view, visit http://gerrit.ovirt.org/9182
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I1b3b9e2837d632c7532fcd8f7306ed50b0865b5c
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Zhou Zheng Sheng <[email protected]>
Gerrit-Reviewer: Federico Simoncelli <[email protected]>
Gerrit-Reviewer: Mark Wu <[email protected]>
Gerrit-Reviewer: Saggi Mizrahi <[email protected]>
Gerrit-Reviewer: ShaoHe Feng <[email protected]>
Gerrit-Reviewer: Shu Ming <[email protected]>
Gerrit-Reviewer: Zhou Zheng Sheng <[email protected]>
_______________________________________________
vdsm-patches mailing list
[email protected]
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Reply via email to