Expensify
HIGH: [Debugability] [$250] Save client-side logs and profile traces to the Downloads folder
Problem:
We have a great system in place to enable client-side logging and profile tracing with a four-finger tap. This allows for really fantastic debugging on production devices out in the real world. However, the files are saved in a private folder that -- so far as I can tell -- is impossible for a user on a non-rooted device to access. So after getting the logs and profile traces, if they don't share them at that exact moment, there's no way to get them in the future.
Solution:
Save client side logging and profile trace files to the /Downloads folder on Android (and whatever similarly accessible folder is generally accessible on iOS.
Upwork Automation - Do Not Edit
- Upwork Job URL: https://www.upwork.com/jobs/~0143be8cb9f6347710
- Upwork Job ID: 1779286276762456064
- Last Price Increase: 2024-04-13
- Automatic offers:
- ishpaul777 | Reviewer | 0
- ShridharGoel | Contributor | 0
Job added to Upwork: https://www.upwork.com/jobs/~0143be8cb9f6347710
![melvin-bot[bot] avatar](https://avatars.githubusercontent.com/in/179547?v=4 "Published by a pub.dev verified publisher"){kind=small}
Triggered auto assignment to Contributor-plus team member for initial proposal review - @ishpaul777 (External)
Triggered auto assignment to @stephanieelliott (NewFeature), see https://stackoverflowteams.com/c/expensify/questions/14418#:~:text=BugZero%20process%20steps%20for%20feature%20requests for more details. Please add this Feature request to a GH project, as outlined in the SO.
Proposal
Please re-state the problem that we are trying to solve in this issue.
We need to save logs in downloads folder on android
What is the root cause of that problem?
Feature requests
What changes do you think we should make in order to solve the problem?
We need to update the native files below:
https://github.com/Expensify/App/blob/db9e01cc302df7f2b3dd53a99c1082404676aedb/src/libs/localFileDownload/index.android.ts#L11
In here, we can use the function RNFetchBlob.fs.dirs.DownloadDir to first get the path of the download directory on android and then later use RNFetchBlob.fs.cp(path, destinationPath) to download the file on android device, the updated code would look like:
RNFetchBlob.fs.exists(RNFetchBlob.fs.dirs.DownloadDir)
.then(() => {
i
const destinationPath = `${RNFetchBlob.fs.dirs.DownloadDir}/${newFileName}`;
RNFetchBlob.fs.cp(path, destinationPath)
.then(() => {
What alternative solutions did you explore? (Optional)
Proposal
Please re-state the problem that we are trying to solve in this issue.
Show meaningful path of saved logs.
What is the root cause of that problem?
New change.
What changes do you think we should make in order to solve the problem?
Since files are already there in public folders, we can just show a meaningful path.
Instead of using file.path, just show /Downloads/${file.newFileName} for Android and /New Expensify Dev/${file.newFileName} for iOS.
https://github.com/Expensify/App/blob/68e69623a8bca0dce5f384fa351d5cbb2214aeb4/src/components/ClientSideLoggingToolMenu/BaseClientSideLoggingToolMenu.tsx#L73
Same changes are needed for profiling path.
https://github.com/Expensify/App/blob/68e69623a8bca0dce5f384fa351d5cbb2214aeb4/src/components/ProfilingToolMenu/index.native.tsx#L156
Also, change the newFilePath to use DownloadDirectoryPath for Android. For iOS, we can use the existing DocumentDirectoryPath.
Proposal
Please re-state the problem that we are trying to solve in this issue.
We want to save client side logging and profile trace files to the /Downloads folder on Android (and whatever similarly accessible folder is generally accessible on iOS.
What is the root cause of that problem?
- For Android, we're already saving to
Downloadsfolder (can be seen here), but the file path that we're showing is of thelocalFile, which might seem misleading to users because they might think the file hasn't been saved toDownloads. - For iOS, we're not saving the file to
Downloads, but only have a local file, can be seen here
What changes do you think we should make in order to solve the problem?
- We should show the Downloads file path instead of the
localFilepath (the one that's in private folder as discouraged in the OP).
In here, set the path returned after saving the file to Downloads to a mediaPath field:
.then(path => {
setFile({
...localFile,
mediaPath: path,
});
});
Then in here, use the file.mediaPath to show in path: , if file.mediaPath does not exist then fallback to file.path
(Since the device-download path looks cryptic, we maybe should instead show to the users the readable file name instead so they know how to access the file, something like path: /Downloads/logs-2024-04-14 10_58_29.432.txt, this can easily be done by using the newFileName)
- We should start to save the file to
Downloads(or just toFiles), similar to what we did with Android, and also use the samemediaPathapproach too.
What alternative solutions did you explore? (Optional)
NA
Proposal
Please re-state the problem that we are trying to solve in this issue.
New feature to save app logs to Downloads instead of default app private folder.
What is the root cause of that problem?
This is a new feature.
What changes do you think we should make in order to solve the problem?
Before suggesting my solution, @quinthar / @ishpaul777 I would like to bring 2 points to think about and consider before proceeding with the solution.
- Even if we save the logs to private folder or to Downloads, the logs are still local to the device and we are at the mercy of the end user to upload or share them with us for Expensify team to support in any debugging.
- Local logs in silos are still a huge amount of valuable data that is scattered in individual devices. They do not add any value to us until it reaches us.
I propose to have some or all of these logs centralized and collected to one of the cloud service provider data stores location so that the same can be used for big data analytics and even for supporting in troubleshooting purposes. This must give a great insight into how the user is using the application and will also give insight into how we can improve our application and its user interface to provide better service to the user. Of course, this essentially means that we must get user consent (maybe as an optional service) at the time of installation and have these uploaded to data stores at regular intervals. Points to consider:
- Chat data must be end-to-end encrypted, if these are also in the logs. Or these logs must be omitted from uploading.
- Privacy is at most priority. Any sensitive and personally identifiable information must be masked or anonymized .
- Automatic uploading must be done only for analytical and app usage data.
- In case when a user raises a support ticket, they can be provided an option to share logs for troubleshooting purposes and then we will already have all the data already available to the support person from the cloud server. Any logs that are not opted in for automatic upload must be uploaded automatically from device private folder itself to the cloud server.
Having a log available in Downloads folder means that the same can be accessed or read by others or by other applications. Having the logs saved into the app private folder structure is the best solution according to me.
@quinthar / @ishpaul777 : If my above thoughts make sense, I would like to work on this from backend as well as front end (from a full stack capacity) to architect, design and implement the solution.
What alternative solutions did you explore? (Optional)
N.A.
@brkdavis - Good questions! We already centrally aggregate these logs. They are just inconvenient to access, and only by a limited set of people (given all the concerns you raised). The point of local log capture is to make someone's own logs much easier for them to access. By capturing local logs into the Download folder, we make someone's own logs easier for them to access for whatever purpose they want. So, good questions, but I still would like to go ahead with this issue as stated.
@brkdavis - Good questions! We already centrally aggregate these logs. They are just inconvenient to access, and only by a limited set of people (given all the concerns you raised). The point of local log capture is to make someone's own logs much easier for them to access. By capturing local logs into the Download folder, we make someone's own logs easier for them to access for whatever purpose they want. So, good questions, but I still would like to go ahead with this issue as stated.
@quinthar : thanks a lot for the clarifications. Couple of questions in direction of logs in Downloads folder.
- Do we have a log viewer within the app?
- Should the logs automatically get logged to
Downloadsfolder ss and when the log entries and generated? Or is it only upon 4 finger tap we must export the logs from private folder toDownloadsso that user can share from there at any point in time? I believe it is the latter. Kindly confirm.
Do we have a log viewer within the app?
Yes, in About > Troubleshooting > View console logs
Should the logs automatically get logged to Downloads folder ss and when the log entries and generated? Or is it only upon 4 finger tap we must export the logs from private folder to Downloads so that user can share from there at any point in time? I believe it is the latter. Kindly confirm.
This issue is just about changing where it's logged, not when. So it's currently logged when you enable via 4-finger tap, or via About > Troubleshooting. So let's keep it working as is, just change where it saves the logs to.
What are the next steps here; are we waiting on @ishpaul777 to pick a proposal? What's the ETA?
Hey @ishpaul777 we have a couple proposals here -- can you please review ASAP?
Started android build to test existing proposals, i'll test and update in a while
Thanks for your proposals everyone!
For Android, we're already saving to Downloads folder (can be seen here), but the file path that we're showing is of the localFile, which might seem misleading to users because they might think the file hasn't been saved to Downloads. -> from @dominictb Proposal
This seems correct we are just not showing the correct path but we do save logs in /downloads folder but not the profile traces as we can see here
https://github.com/Expensify/App/blob/51bebd5c1e5a438ac1299424917d5a6311cbdfe1/src/components/ProfilingToolMenu/index.native.tsx#L52
so first we need to fix it to save in downloads then we need to fix path shown for profile trace to downloads path
I'd like to see a proposal that fixes this
For iOS, we're not saving the file to Downloads, but only have a local file, can be seen here
i dont this we need fix the destination for ios, i think we already save it in a accessible folder i.e On my iphone -> New expensify, @quinthar Do you think we need to change this one?
Also as part of proposal please include how you plan to clean the path shown to user as most time its cryptic
As of now, on Android file is getting saved to internal directory and then being copied to Downloads. My proposal directly saves it to Downloads, because of which the path also shows correctly.
Contributor details Your Expensify account email: [email protected] Upwork Profile Link: https://www.upwork.com/freelancers/~01063aad97ce0e22c6
As of now, on Android file is getting saved to internal directory and then being copied to Downloads. My proposal directly saves it to Downloads, because of which the path also shows correctly.
I beleive the correct approach here is to just show the correct path instead of changing the whole approach
But isn't it redundant to first download it in one folder and then copy to another? We can improve performance by directly downloading to Downloads folder. Also, logs can become huge, so wastage of space to have it in two places.
But isn't it redundant to first download it in one folder and then copy to another? We can improve performance by directly downloading to Downloads folder. Also, logs can become huge, so wastage of space to have it in two places.
For Android, we might have to check for permission before saving (because of scoped storage). We can use the hasAndroidPermission method from fileDownload/index.android.ts.
i feel this is the reason we use this approach in first place so dont have to ask for extra permission, this was done quite intentionally https://github.com/Expensify/App/pull/38803/files#r1537727645, let me confirm from PR author asked here https://github.com/Expensify/App/pull/38803/files#r1571237179
@ShridharGoel Do you have branch for your solution i'll test it again meanwhile
@ishpaul777 https://github.com/ShridharGoel/ExpensifyApp/tree/test-download
Why do we need a seprate function we can just use existing one with a param "saveToDownloads" and when true we save to downloads folder also why LegacyDownloadDir ?
Yes, same function can be used with an extra parameter.
LegacyDownloadsDirpoints to the common Downloads folder.
Okay so after a bit of research i found this issue https://github.com/RonRadtke/react-native-blob-util/issues/294, so it looks like LegacyDownloadsDir uses depriciated apis under the hood and its not a future proof solution to use it, so i prefer to stick with current approach
@ShridharGoel and @dominictb For profile trace tool in android, file is not saved in downloads folder so proposals are incomplete, Please include changes required to save it in downloads folder, Thanks!
@ishpaul777 I had updated the infoFilePath while testing, missed it in the proposal. Updated now.