feat: Send app data size in app context #2197
Conversation
|
| @@ -816,6 +827,7 @@ - (void)removeExtraDeviceContextFromEvent:(SentryEvent *)event | |||
| key:@"app" | |||
| block:^(NSMutableDictionary *app) { | |||
| [app removeObjectForKey:SentryDeviceContextAppMemoryKey]; | |||
| [app removeObjectForKey:@"data_storage"]; | |||
There was a problem hiding this comment.
m: How did you come up with that value? Is it the same as on some other platforms?
There was a problem hiding this comment.
As far as I can see, it doesn't exist for other platforms yet, it is undocumented. I said the same in the ticket.
| } | ||
|
|
||
| // Code from https://github.com/NikolaiRuhe/NRFoundation/blob/master/NRFoundation/NRFileManager.m | ||
| - (long long)readAppDataSize |
There was a problem hiding this comment.
m: While I have nothing against reusing open source code, how did you validate that it works properly? How big is the overhead of this for an iOS app? I think we should use a profiler to check its overhead. We should also try this for a big app with loads of files.
Firefox is using the code here https://github.com/mozilla-mobile/firefox-ios/blob/9c933bae94d324eee0ecaec1c63822c3522a3262/Shared/Extensions/NSFileManagerExtensions.swift, so that's already a good resource.
There was a problem hiding this comment.
I validated it by running the code and comparing the value to the actual folder that it checked. And this function was linked from the issue itself. Of course there is going to be overhead, that is why we do it in a background thread, once a minute.
As far as I can see from reading the docs, you can't get it much simpler than this: we get the contents of the folder (recursively), pre-fetch things like the file size, and fall back to getting the size manually if needed.
There was a problem hiding this comment.
If the overhead is significant, we should consider making this optional or not doing it.
There was a problem hiding this comment.
Even though it's in a background thread already? Overhead of course gets bigger the more files the app stores, but at that point that is probably exactly the info the developer wants to have.
We could add an option for it in SentryOptions, default it to true, but I honestly don't see the downside of doing this once a minute in a background thread.
| ^{ self.appDataSize = [self readAppDataSize]; }); | ||
| } | ||
|
|
||
| // Code from https://github.com/NikolaiRuhe/NRFoundation/blob/master/NRFoundation/NRFileManager.m |
There was a problem hiding this comment.
m: As the repo has no license, I think we should add a link to the issue explaining that it is licensed under MIP 2.0., see NikolaiRuhe/NRFoundation#2. @bruno-garcia is that fine?
|
We could use a file observer to monitor file changes. Here is a sample https://github.com/tblank555/iMonitorMyFiles/blob/master/iMonitorMyFiles/Classes/TABFileMonitor.m |
📜 Description
Calculate the size of the home directory (which is what "Documents & Data" is), in the background, every minute. And send it as part of the
appcontext.💡 Motivation and Context
Closes #1664
💚 How did you test it?
WIP
📝 Checklist
🔮 Next steps