Advanced logging

[twip-os]

We at twip would love to have the ability to log everything to the same file. From cpp code, from python and also if we are only using the addInManager without the itom IDE. Also log rotation would be nice to log permanently without infinitely growing files.

I created a proposal of how this could be done here:

• The logging is now initialized and handled in an extra class "Logger" (common/itomLog.h). In main.c only an instance of Logger has to be created then and the files are also copied to the SDK directory to enable using the same Logger when not even using the itom IDE.
• The new class "pythonLogger" (Qitom/python/pythonLogger.h) redirects the python errors to the logger. The handling of the stream was mainly copied from the consoleWidget. The pythonLogger gets the Logger instance from AppManagement where it is set by main.c. The pythonLogger is initialized in mainApplication.c after the error stream was initialized.
• A new itom function "log" was added in pythonItom.cpp which can be used to write custom messages to the log from python. For the moment this throws a warning if itom was started without the "log" argument, maybe it would be better to just ignore it when no log is required?

I tried to match the structure and style of the existing code but was not exactly sure where to put everything. Also I left out the copyright and definitions in the headers regarding the visual leak detector and qt mocs which I found in other headers because I was not sure how it would be right.

Please tell me if you also think that this would be a nice feature and if so, if any adaptions are needed.

Cheers Thomas

[photoniker]

@twip-os definitely a good feature. Here are some remarks.

• In general, we should add a chapter to the documentation about itom logging and a maybe also a demo
• @twip-os @magro11 Should we name the Python function more itom-specific than log?
• Should the itomlog.txt file be located in the root folder instead in Qitom folder?

[magro11]

Thank you for this pull request.

I have to review a little bit more in detail, however I have similar questions than Johann and would like to briefly discuss them here:

1. Yes, it would be great to extend the documentation a little bit about this. The Python itom.log Method will be automatically documented in the script reference. The startup arguments are documented here: https://itom-project.github.io/latest/docs/03_gettingStarted/startup-arguments.html. It would be great to add some more details about the logging.
2. I think that the Python method itom.log is ok, since it is contained in the itom module. However, wouldn't it be great if one could optionally add a logging category (info, warning, error among others)? This could either be done by a new enumeration or maybe by an optional string value. Check the method PyArg_ParseTupleAndKeywords for a simpler parsing of mandatory and optional parameters, including a proper error message, if failed. Python cerr failures would then be added as category error.
3. I also think that the root directory of itom itself would not be the best place to put logfiles. Up to now, the log method was just a very small, and usually unused function. However, I would say that it will be better if one or multiple logfiles (especially if rotating) are located for instance in the user directory. Maybe QStandardPaths::AppLocalDataLocation would be an option for this (maybe check how other applications are doing this). An alternative would be to add another startup argument with the logging path and filename or extend log by an alternative log=C:/mylogdir/logfile.txt.

[twip-os]

Thank you for the answers.

2. I don't now if a logging category in the itom.lock method is required, since python errors and warnings are already captured and can be used for this purpose but if you think it is useful I can add it of course. The other question would be about categories in the log file. At the moment there are the qt categories (qDebug, qWarning...) and I added only a "Python" category since I thought that the format of the python errors sticks out very good already. But separate categories like e.g. "PyError", "PyWarning" and "PyInfo" could of course be added and would simplify searching for the category.

3. I checked out some other programs and they are logging automatically to the user directory (AppData/Local or AppData/Roaming on windows). This would be sufficient for our needs. My proposal would be:

   • Removing the "log" argument entirely and log automatically to AppData/Local/itom/logs. Automatic logging would have the advantage that the itom.log funktion would always work.
   • Using log rotation to avoid unlimited file growing. Maybe 2 backups with 5 MB (would be about 30.000 lines) each?
   • (Maybe add a "Copy Logs" funktion to the itom toolbar under the "Help" section to copy these logs to e.g. an USB stick. This would be easier for customers or service engineers)

[magro11]

2. As far as I know, Python exceptions and warnings are both sent to cerr, such that a differentiation on the side of the logger will not be possible. Therefore, a generic category [Python] will be ok for this. I only thought, that one might use the itom.log(...) for different categories, like info, warning, error. In any case, it might not be possible to redirect a real Python warning to the warning category, since this is not possible. This will be a reason to not have different categories here. Any further opinions here?
3. log rotation: ok, copy logs method in the help section (if logs are available): ok. I am only not sure, if I would always enable logging into the user directory. For instance we are using a special version of itom (minimal version) on our machines. On these machines, I have to check if such a user directory is available. In general, it will not be the preferred directory for loggings. I see two possibilities: a) logging enabled to user-directory as default, an optional startup argument can disable logging (e.g. nolog) or set an alternative directory / filepath: e.g. log="C:...") b) logging disabled per default and the startup argument log will just enable it to the user directory and can also be log="C:..." to set an alternative directory. Please give your opinion on this.

Just another question / remark:

4. Why is it necessary to put itomLog.h / itomLog.cpp into the commonQt project? commonQt is part of the itom SDK and can therefore also be used by plugins. Currently, I don't see why plugins need to access the logger directly, since they can use qDebug() ,qWarning() etc.
5. Do you think, that itom.log is to close to the log-function as a logarithm, especially if itom is globally imported, like in the comand line / console? We had a similar conflict with the (old) itom.filter function (now itom.algorithms.xyz), that was in conflict with the built-in method filter. However, log is not built-in, but only part of numpy, among others. An alternative could be itom.addLog. Just a question?
6. Any file access should be secured, such that if the log file could not be opened..., no error or exception occurs. The logger is of minor importance, therefore this should not badly influence the program.

[photoniker]

itom.log is fine for me. I cust wanted to discuss it... As it is not a build-in method we can keep it like this. You may overwrite it by import from e.g. numpy, but that's same issue for all build-in methods which can bei overwritten...

[twip-os]

3. The advantage of automated logging for us would be that we would not have to create an extra launcher script that sets the "log" argument but could use the qitom.exe directly with our customers. So for us the solution a) would be slightly more comfortable.
4. One of our use cases is a Cpp Application without the itom IDE which uses only the addInManager to use some plugins. We would like to use the logging in this scenario too and thus have to create the Logger instance ourselves. I thought that the easyest way would be to put the itomLog.h/itomLog.cpp in the SDK to make it available to our application.
5. As it is now, all the QT file functions do not throw any error if an operation can not be performed. They only return if the operation was successful which is yet ignored. So at the moment e.g. a write protected or inexistent log file will produce no errors or warnings.

[twip-os]

I added the Copy Log method in the help section. I was not sure if it was necessary to use invoke to call the copyLog method but did it like it was done on methods in UiOrganizer. At the moment the copy is not performed if a file with the same name already exists in the target directory. Then a error message is displayed for the first file that could not be copied. Please tell me if this behaviour is OK or if you'd like to e.g. copy and replace without asking.

[magro11]

For me, the copy log behaviour is ok. Since there is almost no user interaction here, it would maybe make sense to show a simple message bar after a successful copy operation, that tells the user that all available log files of itom are properly copied to directory XY.

Furthermore, I checked our Windows system on our machines and it would be ok that log files are saved per default to something like C:\Users\<UserName>\AppData\Local\qitom. Therefore, I would propose the following scheme:

1. Per default, log files are enabled and saved to this local directory
2. It will be possible to indicate another directory as argument (e.g. log="C:/mylogfiles")
3. It will be possible to disable logging with another argument (e.g. nolog)

What do you think about this?

[twip-os]

By message bar you mean an information dialog with only an OK button or some message bar down on the window?

For me your proposed logging behavior would be good. I will implement it if no one complains.

[twip-os]

OK, I implemented the command line arguments and logging to the user directory. I also updated the documentation for the command line arguments and the main window for the copy log function. If I got you right, you would like some more detailed documentation. Where should this be located?

[photoniker]

I added the information of default logging in the getting started chapter. I think we can find some better or additional chapters to add such information and refering to the startup chapter?

Should we add a new chapter about logging describing new funtionalities?

[magro11]

besides the two spelling errors in the rst-anchor (see hint above), everything is super now. I just would say, that adding a small sub-section concerning the logging, especially the rotating files and the quota, could be added to the startup-and-arguments section. It is not the perfect place, however it is too small for a dedicated page. Therefore let it put there. Once, this is done, it would say that we can merge it. Thx again.

[twip-os]

I finally found the time to write some lines about the logging process.