[Kleopatra] Add support of timestamp to logging
ClosedPublic

Authored by andreylegayev on Apr 1 2020, 8:23 PM.

Details

Summary

Sometimes Kleopatra starts very slow on windows and get stuck sometimes.
Current version doesn't write timestamps to log file and it's hard to understand what exactly operation is slowing it down.
I've implented writing timestamp before message.
New feature can be enabled via setting environment variable KLEOPATRA_LOGOPTIONS=timestamp

Test Plan
  1. Enable logs by setting environment variable KLEOPATRA_LOGDIR=/your/log/path
  2. Enable debug logs output by setting QT_LOGGING_RULES="org.kde.pim.kleopatra.debug=true"
  3. Set environment variable QT_MESSAGE_PATTERN="%{time yyyyMMdd h:mm:ss.zzz} %{message}"
  4. Check logs - timestamp should appear before log messages

Diff Detail

Repository
R168 Kleopatra
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
andreylegayev created this revision.Apr 1 2020, 8:23 PM
Restricted Application added a project: KDE PIM. · View Herald TranscriptApr 1 2020, 8:23 PM
Restricted Application added a subscriber: kde-pim. · View Herald Transcript
andreylegayev requested review of this revision.Apr 1 2020, 8:23 PM
andreylegayev retitled this revision from Add support of timestamp to logging to [Kleopatra] Add support of timestamp to logging.Apr 1 2020, 8:41 PM
knauss added a subscriber: knauss.Apr 2 2020, 9:43 AM

Can't https://doc.qt.io/qt-5/qtglobal.html#qSetMessagePattern used? If Kleopatra using "normal" qtlogging you can easily test if you set the environmental variable QT_MESSAGE_PATTERN to print timestamps something like:
QT_MESSAGE_PATTERN="[%{time yyyyMMdd h:mm:ss.zzz t} - %{if-category}%{category}: %{endif} - %{file}:%{line} - %{message}

knauss - thanks for hint.
I've tried to set system environment variable in Win10 - as I see it did not affect log message format.
I'll try to debug it.

andreylegayev updated this revision to Diff 79247.EditedApr 3 2020, 7:36 PM
andreylegayev edited the test plan for this revision. (Show Details)

The variable QT_MESSAGE_PATTERN didn't impact output format, because of custom message handler.
I've rolled back my changes and added a call to qFormatLogMessage() to add timestamp and all other information to log messages.
Please review when you have time.

I've tested this change on KDE Neon Developer Unstable and in Windows 10 (test build of gpg4win) - works fine.

andreylegayev edited the test plan for this revision. (Show Details)Apr 3 2020, 8:03 PM
dfaure accepted this revision.Apr 3 2020, 10:57 PM
This revision is now accepted and ready to land.Apr 3 2020, 10:57 PM
knauss added a comment.Apr 4 2020, 8:44 AM

@andreylegayev - as this seems your first patch to KDE via Phabrictor and this got accepted, the next step is to push your change to the git repostory.
Do you have a developer account to push your changes to our git repository yourself? If others need to do this, we need your name and email address for the git commit, as Phabrictor often does not give us these information.

@knauss - I don't have KDE dev account yet. I'll try to request it via page "Apply for a Developer Account".
My name/email: Andrey Legayev <legaev.andrey@gmail.com>

This revision was automatically updated to reflect the committed changes.
knauss added a comment.Apr 4 2020, 3:00 PM

I'll try to request it via page "Apply for a Developer Account".

We normally give users an developer account only after they have submitted some patches and upvote the person. A "Developer account" give you the permission to modify everything within the KDE repos, that's why we want some kind of trust...

Btw. kdepim has currently a online sprint in IRC #kontact (freenode) also availalable via matrix (https://webchat.kde.org/#/room/#kontact:kde.org). Feel free to join, if you want to meet us and/or be interested in the discussions within the team.

Quite dangerous permissions, I agree, it requires trust.
Thanks for explanation.

Sprint and Webchat - thanks for invite! :)
So far I have some time for open source because of quarantine... At least some benefit from it.