Collect diagnostic informations #1300

buhtz · 2022-09-17T23:00:50Z

Related to #1295

Introducing the module backintime.diagnostics in common/diagnostics.py.
Collect information' useful for debuging about Bit, the operating system, used packages and "external" comandline tools. The result is a nested dict.

This is an example output

{
    "backintime": {
        "name": "Back In Time",
        "version": "1.3.2",
        "config-version": 6,
        "distribution-package": "/home/user/ownCloud/my.work/bit/backintime",
        "git-branch": "diag",
        "git-hash": "527da39d887e1029b4aec57444ea45861e4514db"
    },
    "paths": {
        "common": "/home/user/ownCloud/my.work/bit/backintime/common"
    },
    "running_as_root": false,
    "python": "3.9.2 default Feb 28 2021 17:03:44 CPython GCC 10.2.1 20210110",
    "platform": "Linux-5.10.0-18-amd64-x86_64-with-glibc2.31",
    "system": "Linux #1 SMP Debian 5.10.140-1 (2022-09-02)",
    "os-release": "Debian GNU/Linux 11 (bullseye)",
    "display-system": "x11",
    "locale": "de_DE, UTF-8",
    "dependencies": {
        "qt": "PyQt 5.15.2 / Qt 5.15.2"
    },
    "external": {
        "rsync": "3.2.3",
        "ssh": "OpenSSH_8.4p1 Debian-5+deb11u1, OpenSSL 1.1.1n  15 Mar 2022",
        "sshfs": "3.7.1",
        "encfs": "1.9.5",
        "shell": "/bin/bash",
        "shell-version": "GNU bash, Version 5.1.4(1)-release (x86_64-pc-linux-gnu)"
    }
}

Limitations and notes

Path for config file and user-callback scripts are missing. This is because there is no elegant way to access the existing backintime.Config instance. I will later add a PR implementing that class as singleton.

I assume that the information's about Qt are collected on a "usual" system. But I had problems doing this on TravisCI. I will work on that "problem".

The function backintime.diagnostics.get_git_repository_info() is quit redundant to backintime.tools.gitRevisionAndHash(). I won't touch it yet. Of course IMHO my code is cleaner, more modern (using pathlib instead of os.path) and does handle the edge case (happens on TravisCI) having "detached head".

Personal notes

This took me longer than I thought. TravisCI was driving me nuts! The code was IMHO quit elegant and tests on my local machine all green. But on Travis not. Depending on the OS and environment of Travis a lot of problems occurred and I needed to modify the code. More on that on bit-dev list.

It was a good idea to first create a PR in my own repo against buhtzz/backintime:master to test how Travis reacts.

aryoda · 2022-09-18T09:05:44Z

🙃 This wealth of output is a dream 🙃

A few "first impressions" from my side:

To be sure that the output is really printed each possible exception must be caught per element
(and maybe defaulted as "unknown"). Otherwise nothing is printed because of a single exception.
I think (I have not deeply checked this) some possible exceptions within exception handlers may be unhandled.
I would call the first element "App" or "App-info" instead of "backintime" otherwise it is kinda "recursive" (via the "name").
The "paths" tag should be renamed to be more explicit (which path?), eg. "Executable (location)". It points to the "executable" file or (nearly the) installation path of BiT.
I would also put this path into the first "backintime"/"app" element
The elements in the middle part could be grouped into a "System" or "Platform" element so that all leaf elements of the JSON tree end at the same tree depth level
The recognition of the X11/wayland could be improved to be more reliable, see:

https://stackoverflow.com/questions/45536141/how-i-can-find-out-if-a-linux-system-uses-wayland-or-x11/45537237#45537237
https://stackoverflow.com/questions/2035657/what-is-my-current-desktop-environment
For privacy reasons it would be great to replace all "/home/xxxxx/" path parts with a generic "/home/<current user>/" to avoid disclosure of the user name
What is the idea behind removing the pub/private ssh keys from Travis CI?
https://github.com/bit-team/backintime/pull/1300/files#diff-6ac3f79fc25d95cd1e3d51da53a4b21b939437392578a35ae8cd6d5366ca5485R48

Next step: We should test the diagnostics output on different OSes (-> vagrant VMs?) to see how it works "in the wild"

PS: Please bear with my many renaming proposals, that's a typical "linguistics disease" 😉

buhtz · 2022-09-18T11:56:11Z

Thanks for your feedback.

To be sure that the output is really printed each possible exception must be caught per element
(and maybe defaulted as "unknown"). Otherwise nothing is printed because of a single exception.
I think (I have not deeply checked this) some possible exceptions within exception handlers may be unhandled.

Totally agree. I also suspect that there are some edge cases. I found a lot while wrangeling with Travis. ;)
Feel free to upgrade the unittests if you see something. If not we can add this later when it comes up.

I would call the first element "App" or "App-info" instead of "backintime" otherwise it is kinda "recursive" (via the "name").

"App" is a markting buzz-word. ;) I don't see a recursion or redundancy. There is everything collected related directly to Back In Time that is why I used the key backintime. The field name is the name used in the gui and outputs.

The "paths" tag should be renamed to be more explicit (which path?),

Sorry, this is misleading. The paths will become a collection of multiple paths e.g. user-callback location, config file etc. This isn't not implemented yet because of a limitation I mentioned in my first post. The common entry is simply the path where the common folder is located. This is sometime somewhere in /usr/bin, sometimes in /home or in a virtual environment or or or
I really would like to know where the py-files are located when a user runs them.

The elements in the middle part could be grouped into a "System" or "Platform" element so that all leaf elements of the JSON tree end at the same tree depth level

That elements still have a system and platform entry. I didn't choose that names by myself. That is how pythons platform package does use them. I also can't find a better name. And I don't see much advantage to group that keys. Keep in mind that this data is not primary intended to be human readable.

The recognition of the X11/wayland could be improved to be more reliable, see:
https://stackoverflow.com/questions/45536141/how-i-can-find-out-if-a-linux-system-uses-wayland-or-x11/45537237#45537237
https://stackoverflow.com/questions/2035657/what-is-my-current-desktop-environment

Yes, that is quit tricky and for a first shoot I used the simplest approach. I also know that discussions on SO. I would recommend to keep it now as it is and improve this later. You could open an Issue for that.

For privacy reasons it would be great to replace all "/home/xxxxx/" path parts with a generic "/home//" to avoid disclosure of the user name

Done.

What is the idea behind removing the pub/private ssh keys from Travis CI?
https://github.com/bit-team/backintime/pull/1300/files#diff-6ac3f79fc25d95cd1e3d51da53a4b21b939437392578a35ae8cd6d5366ca5485R48

I don't see what you mean. Maybe you misinterpreted the - sign as a "git diff" sign? But it is just the beginning of a list entry.

Next step: We should test the diagnostics output on different OSes (-> vagrant VMs?) to see how it works "in the wild"

I still tested some of them.

PS: Please bear with my many renaming proposals, that's a typical "linguistics disease" wink

I like that. Everything is fine.

This is the new output:

{
    "backintime": {
        "name": "Back In Time",
        "version": "1.3.2",
        "config-version": 6,
        "distribution-package": "/home/UsernameReplaced/ownCloud/my.work/bit/backintime",
        "git-branch": "diag",
        "git-hash": "38027679f8de3e7270923f1925df655d7b6b4e7e"
    },
    "paths": {
        "common": "/home/UsernameReplaced/ownCloud/my.work/bit/backintime/common"
    },
    "running_as_root": false,
    "python": "3.9.2 default Feb 28 2021 17:03:44 CPython GCC 10.2.1 20210110",
    "platform": "Linux-5.10.0-18-amd64-x86_64-with-glibc2.31",
    "system": "Linux #1 SMP Debian 5.10.140-1 (2022-09-02)",
    "os-release": "Debian GNU/Linux 11 (bullseye)",
    "display-system": "x11",
    "locale": "de_DE, UTF-8",
    "dependencies": {
        "qt": "PyQt 5.15.2 / Qt 5.15.2"
    },
    "external": {
        "rsync": "3.2.3",
        "ssh": "OpenSSH_8.4p1 Debian-5+deb11u1, OpenSSL 1.1.1n  15 Mar 2022",
        "sshfs": "3.7.1",
        "encfs": "1.9.5",
        "shell": "/bin/bash",
        "shell-version": "GNU bash, Version 5.1.4(1)-release (x86_64-pc-linux-gnu)"
    }
}

aryoda · 2022-09-18T19:55:35Z

I would call the first element "App" or "App-info" instead of "backintime" otherwise it is kinda "recursive" (via the "name").

... The field name is the name used in the gui and outputs.

ACK

The "paths" tag should be renamed to be more explicit (which path?),

Sorry, this is misleading. The paths will become a collection of multiple paths e.g. user-callback location, config file etc.
This isn't not implemented yet because of a limitation I mentioned in my first post.
The common entry is simply the path where the common folder is located.

I see your point. IMHO I would drop the paths node and move common
either the to backintime node (and further paths to the parent node they belong to).
I understand the motivation for "common". I think something like started-from is more intuitive to
understand for everyone and the "common" folder may change in the future after an refactoring
(eg. into "cli" [command line interface] which fits better to "qt" then).

The elements in the middle part could be grouped into a "System" or "Platform" element so that all leaf elements of the JSON tree end at the same tree depth level

That elements already have a system and platform entry. I didn't choose that names by myself. That is how pythons platform package does use them.

Correct, I didn't realize this :-)

And I don't see much advantage to group that keys.
Keep in mind that this data is not primary intended to be human readable.

At least we as developers (and hopefully new developers) will read this so I think literate names and clear sub groups
can improve readability, but right, it is not strictly required to be understandable.

The recognition of the X11/wayland could be improved to be more reliable, see:

...I would recommend to keep it now as it is and improve this later. You could open an Issue for that.

Absolutely OK (minimal viable product for now), no urgent need to improve this

For privacy reasons it would be great to replace all "/home/xxxxx/" path parts with a generic "/home//" to avoid disclosure of the user name

Done.

👍

What is the idea behind removing the pub/private ssh keys from Travis CI?

I don't see what you mean. Maybe you misinterpreted the - sign as a "git diff" sign?

Gotcha, I was blinded ;-)

Next step: We should test the diagnostics output on different OSes (-> vagrant VMs?) to see how it works "in the wild"

I still tested some of them.

I will also do some more tests when I return from vacation.

aryoda · 2022-09-18T20:17:51Z

And finally I want to show an example of my proposed changes to make it comparable (eg. regarding readability):

{
    "backintime": {
        "name": "Back In Time",
        "version": "1.3.2",
        "config-version": 6,
        "distribution-package": "/home/UsernameReplaced/dev/backintime",
        "git-branch": "playground/1100_with_ext_diagnostics",
        "git-hash": "9984dd7c408e2e23a2c16399861924b7f2eeed45"
        "started-from": "/home/UsernameReplaced/dev/backintime/common"        # was: paths > common
        "running_as_root": false,                                             # moved
    },
    "host-setup": {                                                           # new element for grouping
        "os-release": "Ubuntu 20.04.5 LTS",
        "platform": "Linux-5.15.0-46-generic-x86_64-with-glibc2.29",
        "system": "Linux #49~20.04.1-Ubuntu SMP Thu Aug 4 19:15:44 UTC 2022",
        "display-system": "x11",
        "locale": "en_US, UTF-8",
        # here eg. you could also find the search path env var:
        # "path-env-var": "/usr/local/sbin:/usr/local/bin:/usr/sbin:..."
    },
    "python-setup": {                                                         # new element for grouping
        "python": "3.8.10 default Jun 22 2022 20:18:18 CPython GCC 9.4.0",
        "qt": "PyQt 5.14.1 / Qt 5.12.8"
    },
    "external-programs": {                                                    # renamed
        "rsync": "3.1.3",
        "ssh": "OpenSSH_8.2p1 Ubuntu-4ubuntu0.5, OpenSSL 1.1.1f  31 Mar 2020",
        "sshfs": "2.10.0",
        "encfs": "(no encfs)",
        "shell": "/bin/bash",
        "shell-version": "GNU bash, version 5.0.17(1)-release (x86_64-pc-linux-gnu)"
    }
}

PS: I have written so much above now but don't want to forget to say thank you for your great work for this PR!

buhtz · 2022-09-20T07:40:08Z

Very well and nice ideas.

{
    "backintime": {
        "name": "Back In Time",
        "version": "1.3.2",
        "config-version": 6,
        "distribution-package": "/home/UsernameReplaced/ownCloud/my.work/bit/backintime",
        "started-from": "/home/UsernameReplaced/ownCloud/my.work/bit/backintime/common",
        "running_as_root": false,
        "git-branch": "diag",
        "git-hash": "9506ad7035f8680d79eb75bca4107b1a34582f8d"
    },
    "host-setup": {
        "platform": "Linux-5.10.0-18-arm64-aarch64-with-glibc2.31",
        "system": "Linux #1 SMP Debian 5.10.140-1 (2022-09-02)",
        "os-release": "Debian GNU/Linux 11 (bullseye)",
        "display-system": "tty",
        "locale": "de_DE, UTF-8",
        "PATH": "/home/UsernameReplaced/.local/bin:/usr/local/bin:/usr/bin:/bin:/usr/local/games:/usr/games"
    },
    "python-setup": {
        "python": "3.9.2 default Feb 28 2021 17:03:44 CPython GCC 10.2.1 20210110",
        "sys.path": [
            "/home/UsernameReplaced/ownCloud/my.work/bit/backintime/qt/plugins",
            "/home/UsernameReplaced/ownCloud/my.work/bit/backintime/common/plugins",
            "/home/UsernameReplaced/ownCloud/my.work/bit/backintime/plugins",
            "/home/UsernameReplaced/ownCloud/my.work/bit/backintime/common",
            "",
            "/usr/lib/python39.zip",
            "/usr/lib/python3.9",
            "/usr/lib/python3.9/lib-dynload",
            "/home/UsernameReplaced/.local/lib/python3.9/site-packages",
            "/home/UsernameReplaced/ownCloud/my.work/orgparse",
            "/home/UsernameReplaced/ownCloud/my.work/hyperorg/src",
            "/home/UsernameReplaced/ownCloud/my.work/buhtzology/src",
            "/usr/local/lib/python3.9/dist-packages",
            "/usr/lib/python3/dist-packages",
            "/usr/lib/python3.9/dist-packages"
        ],
        "qt": "PyQt 5.15.2 / Qt 5.15.2"
    },
    "external-programs": {
        "rsync": "3.2.3",
        "ssh": "OpenSSH_8.4p1 Debian-5+deb11u1, OpenSSL 1.1.1n  15 Mar 2022",
        "sshfs": "3.7.1",
        "encfs": "1.9.5",
        "shell": "/bin/bash",
        "shell-version": "GNU bash, Version 5.1.4(1)-release (aarch64-unknown-linux-gnu)"
    }
}

emtiu · 2022-09-20T16:18:44Z

Thanks a lot! I've just committed #1295, so you could now integrate this with the new --diagnostics switch, right?

However, if I understand correctly, this would introduce the "regression" that the user-callback path is no longer shown, which @aryoda implemented with --diagnostics ;)

Also, please add a line to CHANGES. Thanks!

buhtz · 2022-09-20T19:32:37Z

Thanks a lot! I've just committed #1295, so you could now integrate this with the new --diagnostics switch, right?

Right. We want to improve that in a next step.

However, if I understand correctly, this would introduce the "regression" that the user-callback path is no longer shown, which @aryoda implemented with --diagnostics ;)

Nearly correct. But even #1295 doesn't show the users user-callback path. An instance of Config class is created and then asked for that variable. That is not the instance that the user does use. There are several reasons (e.g. --config switch) that the users called bit with a different config file. But you never know that when you create your own (second) instance of Config.

I will improve that in a next step via implementing the Config class as a Singleton. That is very easy but I would like to do that in a separate PR. Then we can get the current Config instance from everywhere via config.Config.get_instance(). And the class take care itself that there is only one instance of it.

Also, please add a line to CHANGES. Thanks!

Done.

buhtz · 2022-09-21T07:40:49Z

FYI: I'm planing to implement the Config singleton tonight.

buhtz · 2022-09-22T07:42:12Z

FYI: Of course it will take longer. ;) As I had already feared there are multiple instances of config.Config in memory while BIT or its unittests runs. Exactly because of that we need a singleton approach. The current progress can be monitored here.

@aryoda

* Integrate extended diagnostics (#1300 with #1295) * Add user-callback path to collect_diagnostics() * Remove TravisCI test failure workaround (was fixed with #1305). * Correct some typos in comments * Diagnostics: Add local and global config file info. Rename "config-version" to "latest-config-version" by @aryoda and @buhtzz

buhtz added 2 commits September 18, 2022 00:52

Collect diagnostic informations

527da39

removed debug print()

3802767

Username replaced

9506ad7

pathes and reorga

166c9aa

CHANGES

de20ff4

emtiu merged commit be3c256 into bit-team:master Sep 20, 2022

buhtz deleted the diag branch September 20, 2022 20:00

aryoda mentioned this pull request Sep 25, 2022

Integrate extended diagnostics (#1300 with #1295) #1310

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Collect diagnostic informations #1300

Collect diagnostic informations #1300

buhtz commented Sep 17, 2022 •

edited

Loading

aryoda commented Sep 18, 2022

buhtz commented Sep 18, 2022

aryoda commented Sep 18, 2022 •

edited

Loading

aryoda commented Sep 18, 2022

buhtz commented Sep 20, 2022

emtiu commented Sep 20, 2022

buhtz commented Sep 20, 2022

buhtz commented Sep 21, 2022

buhtz commented Sep 22, 2022

Collect diagnostic informations #1300

Collect diagnostic informations #1300

Conversation

buhtz commented Sep 17, 2022 • edited Loading

Limitations and notes

Personal notes

aryoda commented Sep 18, 2022

buhtz commented Sep 18, 2022

aryoda commented Sep 18, 2022 • edited Loading

aryoda commented Sep 18, 2022

buhtz commented Sep 20, 2022

emtiu commented Sep 20, 2022

buhtz commented Sep 20, 2022

buhtz commented Sep 21, 2022

buhtz commented Sep 22, 2022

buhtz commented Sep 17, 2022 •

edited

Loading

aryoda commented Sep 18, 2022 •

edited

Loading