Skip to content
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

NVDA logging: add originating thread to log entry #10259

Merged
merged 5 commits into from Sep 23, 2019

Conversation

leonardder
Copy link
Collaborator

@leonardder leonardder commented Sep 21, 2019

Link to issue number:

None. Hopefully helps debugging #10247

Summary of the issue:

As NVDA is getting a larger application, there are multiple threads involved in its operation. Logging can also be initiated from multiple threads. However, in the current situation,, we do not know from what thread a log entry is coming. Knowing the originating thread helps in following the flow in a log file.

Description of how this pull request fixes the issue:

  1. Add the thread name and thread number to the log entry. It is added after the timestamp, so people who aren't interested in the thread won't be bothered by it
  2. Give every thread that is created within NVDA a name, so it can be identified in the log
  3. Update the log formatter to use new functionality in python 3. In particular, there is now a clearer method to format the timestamp. We are also using Python string formatting syntax {msg} instead of %(msg)s. THe latter is more difficult to read.

Testing performed:

Tested that NVDA still runs and appropriate thread info is logged. Paid extra attention to watchdog.CancellableCallThread to set the name of the thread to the function it is being executed with.

Known issues with pull request:

None

Change log entry:

  • Changes
    • For every entry in the NVDA log, information about the originating thread is now logged.

@leonardder leonardder requested a review from michaelDCurran Sep 21, 2019
@JulienCochuyt
Copy link
Collaborator

@JulienCochuyt JulienCochuyt commented Sep 21, 2019

@leonardder, thanks a lot for this PR!
I strongly up-vote the need for this feature.

@JulienCochuyt
Copy link
Collaborator

@JulienCochuyt JulienCochuyt commented Sep 21, 2019

@leonardder, please find in BabbageCom#10 my suggestion to generalize the use of fully qualified names.
I started by trying to use the review comments facility for this, but it makes quite a lot of individual patches and I couldn't review multiple lines at once or extend the diff context. Hope you don't mind.

@leonardder
Copy link
Collaborator Author

@leonardder leonardder commented Sep 21, 2019

…cess#10259) (#10)

* Thread name logging: Generalize logging of full qualified names (nvaccess#10259)

* Review action #10 (comment)

Co-Authored-By: Leonard de Ruijter <leonardder@users.noreply.github.com>
@leonardder
Copy link
Collaborator Author

@leonardder leonardder commented Sep 23, 2019

@michaelDCurran: @JulienCochuyt filed a pr against this pr, which has been merged into it. I think this is ready for review. Would be pretty helpful to debug #10247, probably.

@michaelDCurran michaelDCurran merged commit 0a4a182 into nvaccess:master Sep 23, 2019
1 check passed
@nvaccessAuto nvaccessAuto added this to the 2019.3 milestone Sep 23, 2019
JulienCochuyt added a commit to accessolutions/nvda that referenced this issue Sep 23, 2019
…s#10259)

`__module__` is `None` for functions defined in the Python console.
@leonardder leonardder deleted the threadLog branch Sep 24, 2019
@leonardder leonardder added the BabbageWork label Oct 11, 2019
JulienCochuyt added a commit to accessolutions/nvda that referenced this issue Oct 30, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
BabbageWork
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants