Closed Bug 1883655 Opened 6 months ago Closed 5 months ago

Add disk size measurements for all extensions data

Categories

(Firefox :: Profile Backup, task, P1)

Product:
Firefox
Component:
Profile Backup
Type:
task

Tracking

()

Status:
RESOLVED FIXED
Milestone:
126 Branch
Tracking Flags:
Tracking Status
firefox126 --- fixed

People

(Reporter: mconley, Assigned: fchasen)

References

Details

(Whiteboard: [fidefe-device-migration])

Attachments

(2 files)

Bug 1883655 - Add disk size measurements for all addons and extensions data. data-review=jhirsch r=#backup-reviewers!
48 bytes, text/x-phabricator-request
Details | Review
Data collection request
3.65 KB, text/plain
jhirsch
: data-review+
Details

This is going to require communications with the extensions team to make sure we didn't forget anything, but these are the things I'm aware of that we'd back up in the profile directory related to extensions:

  1. extensions.json
  2. addonStartup.json.lz4
  3. extension-settings.json
  4. extension-store-permissions/data.safe.bin (if it exists, since it's only currently used on Nightly)
  5. extension-preferences.json (since bug 1646182 is still open)
  6. The sum size of everything under extensions/
  7. The contents of browser-extension-data
  8. storage-sync-v2.sqlite

There's some shared infrastructure here with bug 1883642 (for the metrics.yaml file, for setting up the private method in BackupService that does measurement, and for test infrastructure), so if this is being worked on in parallel by two people, they might want to coordinate a little bit so that they don't stomp on each other's toes a bit. What I'd recommend there is to have the person working on bug 1883642 sketch out the method and the test infrastructure first, and post that as a WIP, and then the person working on this bug pulls that bug down and creates a new commit on top of it with these measurements.

Bug 1883642 will need to land first, but if the initial WIP patch isn't too different from what ultimately gets landed, I suspect rebase conflicts won't be that bad.

Here's the list of things to do for this bug:

  1. Talk to someone from the extensions team about this. My short list of candidates is: rpl, willdurand, mixedpuppy or robwu. We need to make sure we're not missing any files that might be extension-related here.
  2. Create quantity probes for each of the items listed above
  3. Inside the private method from the WIP that does measurements, using IOUtils.stat to read the sizes of each of these things, and write the probes
  4. Add tests for those probes. This might be tricky - I'm not entirely sure how easy it is to install an XPI in an xpcshell test that gets puts into the extensions/ directory. That's something to work out with whoever you talk to in (1).

And of course, don't forget to request a data review for the probes. Maybe even do that after (2), so that the data review can occur in parallel to working on 3-4. Don't forget about ./mach data-review to help you out!

Blocks: 1883503
Depends on: 1883642
No longer depends on: 1883503
Assignee: nobody → fchasen
Status: NEW → ASSIGNED

I updated the list in comment 0 to include browser-extension-data.

I updated the list in comment 0 to include browser-preferences.json, which is used on non-Nightly channels instead of extension-store-permissions/data.safe.bin.

See Also: → 1646182

This is showing up in my triage queue; I assume P1 is accurate because this is assigned but please feel free to adjust.

Severity: -- → N/A
Priority: -- → P1

I updated the list in comment 0 to include storage-sync-v2.sqlite.

Depends on: 1884995
Attachment #9391318 - Attachment description: Bug 1883655 - Add disk size measurements for all extensions data. r=#backup-reviewers! → Bug 1883655 - Add disk size measurements for all addons and extensions data. data-review=jhirsch r=#backup-reviewers!
Attachment #9391576 - Flags: data-review?(jhirsch)

Comment on attachment 9391576 [details]
Data collection request

data-review+

  1. Is there or will there be documentation that describes the schema for the ultimate data set in a public, complete, and accurate way?

Yes, the Glean data docs.

  1. Is there a control mechanism that allows the user to turn the data collection on and off?

Yes, the usual Firefox data collection opt-out.

  1. If the request is for permanent data collection, is there someone who will monitor the data over time?

Yes, mconley will monitor.

  1. Using the category system of data types on the Mozilla wiki, what collection type of data do the requested measurements fall under?

Category 1, technical data.

  1. Is the data collection request for default-on or default-off?

Default-on.

  1. Does the instrumentation include the addition of any new identifiers (whether anonymous or otherwise; e.g., username, random IDs, etc. See the appendix for more details)?

No.

  1. Is the data collection covered by the existing Firefox privacy notice?

Yes.

  1. Does the data collection use a third-party collection tool?

No.

Attachment #9391576 - Flags: data-review?(jhirsch) → data-review+

From Rob Wu on Slack regarding the extensions directory:

The files of interest are xpi files. The directories "staged" and "trash" may also exist; these may also contain xpi files for add-ons that have an update available. Lastly, there may also be other files that contain the path to the actual xpi file (created by the user or other external software)

So sounds like we should be good just backing up the XPI files, though our current measurement will include those other files if there are present.

Lastly, there may also be other files that contain the path to the actual xpi file (created by the user or other external software)

Huh, that's interesting. How common is that? As it currently stands, we're doing nothing to scan for instances like that and to include those indirectly stored XPIs in the backup...

Blocks: 1887724

Comment on attachment 9391576 [details]
Data collection request

Added an additional browser_backup.extensions_storage_size measurement to get all of the IndexedDB database directories that include moz-extensions.

Measurement Name Measurement Description Data Collection Category Tracking Bug
browser_backup.extensions_storage_size The total size of all extensions storage directories, rounded to the nearest 10 kilobytes. technical https://bugzilla.mozilla.org/show_bug.cgi?id=1883655
Flags: needinfo?(jhirsch)
Duplicate of this bug: 1887724

data-review+ for the additional probe; everything from the earlier review applies.

Flags: needinfo?(jhirsch)

As mentioned in the phabricator review, when testing this it would be good to try out a profile that has storage from many origins to make sure that looping over every directory in "storage/default" does not have any performance impact.

Pushed by fchasen@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/adba8777ed32
Add disk size measurements for all addons and extensions data. data-review=jhirsch r=backup-reviewers,mconley,rpl

Backed out for causing xpcshell failures on test_measurements.js.

[task 2024-04-03T02:05:15.251Z] 02:05:15     INFO -  TEST-START | browser/components/backup/tests/xpcshell/test_measurements.js
[task 2024-04-03T02:05:15.786Z] 02:05:15  WARNING -  TEST-UNEXPECTED-FAIL | browser/components/backup/tests/xpcshell/test_measurements.js | xpcshell return code: 0
[task 2024-04-03T02:05:15.792Z] 02:05:15     INFO -  TEST-INFO took 535ms
[task 2024-04-03T02:05:15.792Z] 02:05:15     INFO -  >>>>>>>
[task 2024-04-03T02:05:15.793Z] 02:05:15     INFO -  (xpcshell/head.js) | test MAIN run_test pending (1)
[task 2024-04-03T02:05:15.793Z] 02:05:15     INFO -  (xpcshell/head.js) | test run_next_test 0 pending (2)
[task 2024-04-03T02:05:15.793Z] 02:05:15     INFO -  (xpcshell/head.js) | test MAIN run_test finished (2)
[task 2024-04-03T02:05:15.793Z] 02:05:15     INFO -  running event loop
[task 2024-04-03T02:05:15.793Z] 02:05:15     INFO -  browser/components/backup/tests/xpcshell/test_measurements.js | Starting setup
[task 2024-04-03T02:05:15.794Z] 02:05:15     INFO -  (xpcshell/head.js) | test pending (2)
[task 2024-04-03T02:05:15.794Z] 02:05:15     INFO -  (xpcshell/head.js) | test run_next_test 0 finished (2)
[task 2024-04-03T02:05:15.794Z] 02:05:15     INFO -  (xpcshell/head.js) | test run_next_test 1 pending (2)
[task 2024-04-03T02:05:15.794Z] 02:05:15     INFO -  (xpcshell/head.js) | test finished (2)
[task 2024-04-03T02:05:15.794Z] 02:05:15     INFO -  browser/components/backup/tests/xpcshell/test_measurements.js | Starting test_takeMeasurements
[task 2024-04-03T02:05:15.794Z] 02:05:15     INFO -  (xpcshell/head.js) | test test_takeMeasurements pending (2)
[task 2024-04-03T02:05:15.795Z] 02:05:15     INFO -  PID 9840 | console.debug: BackupService: "Instantiated"
[task 2024-04-03T02:05:15.795Z] 02:05:15     INFO -  PID 9840 | console.debug: BackupService: "Taking Telemetry measurements"
[task 2024-04-03T02:05:15.795Z] 02:05:15     INFO -  (xpcshell/head.js) | test run_next_test 1 finished (2)
[task 2024-04-03T02:05:15.795Z] 02:05:15     INFO -  PID 9840 | console.error: BackupService: "Failed to measure for resource: fake2" (new Error("Some failure to measure", "D:/task_171210582814172/build/tests/xpcshell/tests/browser/components/backup/tests/xpcshell/test_measurements.js", 52))
[task 2024-04-03T02:05:15.796Z] 02:05:15     INFO -  TEST-PASS | browser/components/backup/tests/xpcshell/test_measurements.js | test_takeMeasurements - [test_takeMeasurements : 58] Measure was called - true == true
[task 2024-04-03T02:05:15.796Z] 02:05:15     INFO -  TEST-PASS | browser/components/backup/tests/xpcshell/test_measurements.js | test_takeMeasurements - [test_takeMeasurements : 62] Measure was called with the profile directory argument - true == true
[task 2024-04-03T02:05:15.796Z] 02:05:15     INFO -  TEST-PASS | browser/components/backup/tests/xpcshell/test_measurements.js | test_takeMeasurements - [test_takeMeasurements : 58] Measure was called - true == true
[task 2024-04-03T02:05:15.797Z] 02:05:15     INFO -  TEST-PASS | browser/components/backup/tests/xpcshell/test_measurements.js | test_takeMeasurements - [test_takeMeasurements : 62] Measure was called with the profile directory argument - true == true
[task 2024-04-03T02:05:15.798Z] 02:05:15     INFO -  (xpcshell/head.js) | test run_next_test 2 pending (2)
[task 2024-04-03T02:05:15.798Z] 02:05:15     INFO -  (xpcshell/head.js) | test test_takeMeasurements finished (2)
[task 2024-04-03T02:05:15.798Z] 02:05:15     INFO -  browser/components/backup/tests/xpcshell/test_measurements.js | Starting test_profDDiskSpace
[task 2024-04-03T02:05:15.798Z] 02:05:15     INFO -  (xpcshell/head.js) | test test_profDDiskSpace pending (2)
[task 2024-04-03T02:05:15.798Z] 02:05:15     INFO -  PID 9840 | console.debug: BackupService: "Instantiated"
[task 2024-04-03T02:05:15.799Z] 02:05:15     INFO -  PID 9840 | console.debug: BackupService: "Taking Telemetry measurements"
[task 2024-04-03T02:05:15.799Z] 02:05:15     INFO -  (xpcshell/head.js) | test run_next_test 2 finished (2)
[task 2024-04-03T02:05:15.799Z] 02:05:15     INFO -  TEST-PASS | browser/components/backup/tests/xpcshell/test_measurements.js | test_profDDiskSpace - [test_profDDiskSpace : 18] 115100000 == 115100000
[task 2024-04-03T02:05:15.800Z] 02:05:15     INFO -  TEST-PASS | browser/components/backup/tests/xpcshell/test_measurements.js | test_profDDiskSpace - [test_profDDiskSpace : 84] Should have collected a measurement for the profile directory storage device - 115100000 > 0
[task 2024-04-03T02:05:15.800Z] 02:05:15     INFO -  (xpcshell/head.js) | test run_next_test 3 pending (2)
<...>
[task 2024-04-03T02:05:15.824Z] 02:05:15     INFO -  TEST-PASS | browser/components/backup/tests/xpcshell/test_measurements.js | test_AddonsBackupResource - [test_AddonsBackupResource : 18] Glean and telemetry measurements for extensions storage should be equal - 200 == 200
[task 2024-04-03T02:05:15.824Z] 02:05:15     INFO -  Unexpected exception OperationError: Could not remove `C:\\Users\\task_171210582814172\\AppData\\Local\\Temp\\xpc-profile-7im90out': the directory is not empty (NS_ERROR_FILE_DIR_NOT_EMPTY)
[task 2024-04-03T02:05:15.825Z] 02:05:15     INFO -  _do_main@D:\task_171210582814172\build\tests\xpcshell\head.js:245:6
[task 2024-04-03T02:05:15.825Z] 02:05:15     INFO -  _execute_test@D:\task_171210582814172\build\tests\xpcshell\head.js:596:5
[task 2024-04-03T02:05:15.825Z] 02:05:15     INFO -  @-e:1:1
[task 2024-04-03T02:05:15.825Z] 02:05:15     INFO -  exiting test
[task 2024-04-03T02:05:15.825Z] 02:05:15     INFO -  <<<<<<<
[task 2024-04-03T02:05:15.833Z] 02:05:15     INFO -  TEST-START | browser/components/backup/tests/xpcshell/test_measurements.js
Flags: needinfo?(fchasen)

Updated the tests to not fail if a file can't be removed in the temp folder.

Flags: needinfo?(fchasen)
Pushed by fchasen@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/21aa60d7cd79
Add disk size measurements for all addons and extensions data. data-review=jhirsch r=backup-reviewers,mconley,rpl

Backed out for causing xpcshell failures in test_measurements.js

Flags: needinfo?(fchasen)
Pushed by fchasen@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/7b6fd49403d4
Add disk size measurements for all addons and extensions data. data-review=jhirsch r=backup-reviewers,mconley,rpl

https://hg.mozilla.org/mozilla-central/rev/7b6fd49403d4

Status: ASSIGNED → RESOLVED
Closed: 5 months ago
status-firefox126: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → 126 Branch
Flags: needinfo?(fchasen)
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: