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

Make it easier for user to provide debug information #2514

Merged
merged 7 commits into from
Oct 7, 2020

Conversation

er-vin
Copy link
Member

@er-vin er-vin commented Oct 5, 2020

EDIT: Adding a screenshot for a first impression ;)

screenshot

Copy link
Member

@misch7 misch7 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a few nit-picks.

Cool feature, this will make (not only) our lives easier :)

Works well, I just noticed under Linux that the dialog doesn't auto-append the selected filter format to the filename, but this is a Qt issue ( https://bugreports.qt.io/browse/QTBUG-27186 ).

src/gui/generalsettings.cpp Show resolved Hide resolved
src/gui/generalsettings.cpp Show resolved Hide resolved
src/gui/generalsettings.cpp Outdated Show resolved Hide resolved
src/gui/updater/ocupdater.cpp Outdated Show resolved Hide resolved
@DominiqueFuchs
Copy link
Contributor

Neat! Only added some comments.

@DominiqueFuchs
Copy link
Contributor

Also: B/c it would be so easy and handy, I'd love to see an optional automated removal of at least all domain names from the archive content as this is an recurring concern and annoyance within issues in need for more log data. However, this may be questionable from a

  • data integrity POV
  • responsibility POV (we would in no way want to claim complete pseudonymization with that nor produce false feeling of it)

But I still think it would be nice, so I'll just drop the idea.

@er-vin
Copy link
Member Author

er-vin commented Oct 6, 2020

Works well, I just noticed under Linux that the dialog doesn't auto-append the selected filter format to the filename, but this is a Qt issue ( https://bugreports.qt.io/browse/QTBUG-27186 ).

Interestingly I tested under Linux and didn't have that problem. I guess it might depend on the QPA plugin being picked.

@er-vin
Copy link
Member Author

er-vin commented Oct 6, 2020

Neat! Only added some comments.

Hm... somehow can't find them. You sure they got published?

@er-vin
Copy link
Member Author

er-vin commented Oct 6, 2020

Also: B/c it would be so easy and handy, I'd love to see an optional automated removal of at least all domain names from the archive content as this is an recurring concern and annoyance within issues in need for more log data. However, this may be questionable from a

* data integrity POV

* responsibility POV (we would in no way want to claim complete pseudonymization with that nor produce false feeling of it)

But I still think it would be nice, so I'll just drop the idea.

Yes, it's kind of on my radar but it's not that easy... I think it's start with better hygiene in our log messages and that'd take time to review those. Once they're a bit in a better shape then there could be a postprocess phase for cleanups but a) it's not that easy and b) my experience so far with such processed logs is that they're not very useful in the end. Indeed, once you removed domain names and file paths (because those can leak information as well)... it's hard to figure out what's wrong and where.

That's why I've been experimenting a bit with the file drop solution on our Nextcloud instance. Means the user has to trust us with their data obviously but that seems to be an interesting middle ground.

@er-vin er-vin force-pushed the make_it_easier_for_user_to_provide_debug_information branch 2 times, most recently from 1ee3b2d to 86c3c0b Compare October 6, 2020 06:14
@er-vin er-vin requested a review from misch7 October 6, 2020 12:05
src/gui/application.cpp Show resolved Hide resolved
src/gui/application.cpp Outdated Show resolved Hide resolved
src/gui/generalsettings.cpp Show resolved Hide resolved
@DominiqueFuchs
Copy link
Contributor

Hm... somehow can't find them. You sure they got published?

Sry, just learned that the mobile app defaults to draft until you submit the overall review type (same as website in the end, but not as obvious).. Submitted now.

That's why I've been experimenting a bit with the file drop solution on our Nextcloud instance. Means the user has to trust us with their data obviously but that seems to be an interesting middle ground.

👍

@er-vin er-vin force-pushed the make_it_easier_for_user_to_provide_debug_information branch 2 times, most recently from a5cdfdd to 25fc801 Compare October 6, 2020 12:42
@misch7
Copy link
Member

misch7 commented Oct 7, 2020

/rebase

Kevin Ottens added 7 commits October 7, 2020 13:33
Turn on the logging by default for everyone. Let's use a log dir within
the config directory of the application and have debug logs expiring
after a day.

This obviously means we'll generate quite some logs but with the
automated compression it shouldn't be too horrible. Obviously that
scales with the amount of files and syncs occurring. In our tests with a
large setup we're around 100 MB for a day worth of logs, this shouldn't
be too much of an issue on today's average desktop/laptop.

Signed-off-by: Kevin Ottens <[email protected]>
Since we changed the default in the config file and since log dir had
precedence on log file, the --logfile command line option wasn't doing
anything anymore.

We make sure it has an effect again overriding --logdir or the logDir
config entry.

Signed-off-by: Kevin Ottens <[email protected]>
It is better to rely on the Logger state to know exactly where we're
logging. Indeed due to the the various ways to impact its state the
config alone might not now where we're logging.

Signed-off-by: Kevin Ottens <[email protected]>
I'm not a huge fan of using private APIs but QZip is really the API with
the least hassles for our debug archive need. No external dependency and
we know it is generally available and stable despite the lack of
stability promise.

Signed-off-by: Kevin Ottens <[email protected]>
This will harvest everything we might need for debugging purposes:
 * config file
 * sync journal dbs
 * log files

Signed-off-by: Kevin Ottens <[email protected]>
@github-actions github-actions bot force-pushed the make_it_easier_for_user_to_provide_debug_information branch from 25fc801 to 3fca307 Compare October 7, 2020 13:33
@nextcloud-desktop-bot
Copy link

AppImage file: Nextcloud-PR-2514-3fca307fbb3a183504e0abb2b988bf26c892bea8-x86_64.AppImage

To test this change/fix you can simply download above AppImage file and test it.

Please make sure to quit your existing Nextcloud app and backup your data.

@misch7 misch7 merged commit 211dbad into master Oct 7, 2020
@misch7 misch7 deleted the make_it_easier_for_user_to_provide_debug_information branch October 7, 2020 15:12
@misch7 misch7 added this to the Desktop 3.0 milestone Oct 7, 2020
@misch7

This comment has been minimized.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants