1533861 - Add hard drive type (i.e. SSD vs HDD) to Telemetry Environment

Status: RESOLVED FIXED

(Core :: Performance, enhancement)

Description

[Andrew Creskey [:acreskey]]

Attachment: RequestForm.txt

It would be useful to capture the type of storage that the application is running on, specifically if it's a solid state drive or a spinning hard drive.

The performance characteristics are quite different and this could be use to optimize application heuristics (e.g. the http disk cache)

One example use-case: Bug 1524609

I propose adding the hard drive type to the Telemetry Environment.
i.e.

hdd: {
      profile: { // hdd where the profile folder is located
          ...
          type: <string>, // ssd or hdd
      },
      binary:  { // hdd where the application binary is located
          ...
          type: <string>, // ssd or hdd
      },
      system:  { // hdd where the system files are located
          ...
          type: <string>, // ssd or hdd
      },

Comment 1

[Andrew Creskey [:acreskey]]

I think we'll need to extract this here:
https://searchfox.org/mozilla-central/source/xpcom/base/nsSystemInfo.cpp#103

and expose it here:
https://searchfox.org/mozilla-central/source/toolkit/components/telemetry/app/TelemetryEnvironment.jsm#1727

And it should be brought to a Telemetry Peer (https://wiki.mozilla.org/Modules/All#Toolkit)

Comment 2

[Sean Feng [:sefeng]]

Attachment: Bug 1533861 - Add a telemetry to detect disk type r=chutten,froydnj,bdekoz

Comment 3

[Alex Thayer [:alexical] (she/her)]

While I think this could be handy, I'm not sure it's strictly necessary since you should be able to just have a lookup table based on the model string of the drive which we already expose. That's what I've done in the past and I didn't run into any special difficulties.

Comment 4

[Sean Feng [:sefeng]]

dthayer: Interesting point. I tried to find one table example online, but I couldn't. Do you have the table off-hand? I also wonder if we need to update this table frequently to align with new types of SSD/HDD? Thanks!

Comment 5

[Alex Thayer [:alexical] (she/her)]

Well, I say lookup table but I guess that's actually more sophisticated than what I've used. I went through the top 30 (by number of pings) or so model strings by hand, looking each one up, and noticed that every SSD I ran across had "SSD" in the model. So I just used that as my indicator. It's probably not perfect but I think the top 30 models cover most of the userbase. You may want to double-check me on that though.

Comment 6

[Sean Feng [:sefeng]]

I see, thanks for the update. From what I've tested, the model of the SSD that I had is "PM951 NVMe SAMSUNG 1024GB", so I don't think checking the model has "SSD" is reliable.

Comment 7

[Alex Thayer [:alexical] (she/her)]

Ah, good call. However I imagine nearly all NVMe drives are SSDs, so you could try checking for either substring, and the same logic would apply that vendors tend to place this information in the model. In general though the approach of sorting the models by frequency descending and looking up the top N% by volume and making a table if the strings don't work should still be sufficient.

(However, I'm not saying we shouldn't have this field - having an explicit field for this is certainly more future-proof and discoverable for engineers who are newly trying to answer questions that depend on this.)

Comment 8

[Sean Feng [:sefeng]]

Ah I see. Thanks for putting these up together!

Comment 9

[Benjamin De Kosnik [:bdekoz]]

Comment on attachment 9049611 [details]
RequestForm.txt

Seems like the right starting point for collecting this type of system info

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c2f648fbcbf1
Add a telemetry to detect disk type r=chutten,froydnj,bdekoz

Comment 11

[Bogdan Tara[:bogdan_tara | bogdant]]

Backed out changeset c2f648fbcbf1 (Bug 1533861) for test_TelemetryEnvironment.js failures

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&fromchange=0ef6e3e9552df27c04a2e839d686aaa45ef094e2&searchStr=windows%2C7%2Cdebug%2Cxpcshell%2Ctests%2Ctest-windows7-32%2Fdebug-xpcshell-e10s%2Cx%28x%29&tochange=c49960f2ec7b13be989b173d5a76493f6b821fed&selectedJob=244377699

Backout link: https://hg.mozilla.org/integration/autoland/rev/c49960f2ec7b13be989b173d5a76493f6b821fed

Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=244377699&repo=autoland&lineNumber=13349

02:17:48 INFO - TEST-START | toolkit/components/reputationservice/test/unit/test_app_rep.js
02:17:48 INFO - TEST-PASS | toolkit/components/reputationservice/test/unit/test_app_rep.js | took 529ms
02:17:48 INFO - TEST-START | toolkit/components/telemetry/tests/unit/test_TelemetryEnvironment.js
02:17:50 WARNING - TEST-UNEXPECTED-FAIL | toolkit/components/telemetry/tests/unit/test_TelemetryEnvironment.js | xpcshell return code: 0
02:17:50 INFO - TEST-INFO took 1566ms
02:17:50 INFO - >>>>>>>
02:17:50 INFO - PID 6880 | [6880, Main Thread] WARNING: Failed to get directory to cache.: file z:/build/build/src/security/sandbox/win/src/sandboxbroker/sandboxBroker.cpp, line 82
02:17:50 INFO - PID 6880 | [6880, Main Thread] WARNING: Failed to get directory to cache.: file z:/build/build/src/security/sandbox/win/src/sandboxbroker/sandboxBroker.cpp, line 82
02:17:50 INFO - PID 6880 | [6880, Main Thread] WARNING: Failed to get directory to cache.: file z:/build/build/src/security/sandbox/win/src/sandboxbroker/sandboxBroker.cpp, line 82
02:17:50 INFO - PID 6880 | [6880, Main Thread] WARNING: Failed to get directory to cache.: file z:/build/build/src/security/sandbox/win/src/sandboxbroker/sandboxBroker.cpp, line 82
02:17:50 INFO - PID 6880 | [6880, Main Thread] WARNING: Failed to get directory to cache.: file z:/build/build/src/security/sandbox/win/src/sandboxbroker/sandboxBroker.cpp, line 82
02:17:50 INFO - PID 6880 | [6880, Main Thread] WARNING: Couldn't get the user appdata directory. Crash events may not be produced.: file z:/build/build/src/toolkit/crashreporter/nsExceptionHandler.cpp, line 2528
02:17:50 INFO - (xpcshell/head.js) | test MAIN run_test pending (1)
02:17:50 INFO - (xpcshell/head.js) | test run_next_test 0 pending (2)
02:17:50 INFO - (xpcshell/head.js) | test MAIN run_test finished (2)
02:17:50 INFO - running event loop
...
02:17:50 INFO - TEST-PASS | toolkit/components/telemetry/tests/unit/test_TelemetryEnvironment.js | test_checkEnvironment - [test_checkEnvironment : 582] true == true
02:17:50 INFO - TEST-PASS | toolkit/components/telemetry/tests/unit/test_TelemetryEnvironment.js | test_checkEnvironment - [test_checkEnvironment : 583] true == true
02:17:50 INFO - TEST-PASS | toolkit/components/telemetry/tests/unit/test_TelemetryEnvironment.js | test_checkEnvironment - [test_checkEnvironment : 587] ServicePackMajor must be a number. - true == true
02:17:50 INFO - TEST-PASS | toolkit/components/telemetry/tests/unit/test_TelemetryEnvironment.js | test_checkEnvironment - [test_checkEnvironment : 589] ServicePackMinor must be a number. - true == true
02:17:50 INFO - TEST-PASS | toolkit/components/telemetry/tests/unit/test_TelemetryEnvironment.js | test_checkEnvironment - [test_checkEnvironment : 593] windowsBuildNumber must be a number. - true == true
02:17:50 WARNING - TEST-UNEXPECTED-FAIL | toolkit/components/telemetry/tests/unit/test_TelemetryEnvironment.js | test_checkEnvironment - [test_checkEnvironment : 607] false == true
02:17:50 INFO - Z:/task_1556845676/build/tests/xpcshell/tests/toolkit/components/telemetry/tests/unit/test_TelemetryEnvironment.js:checkSystemSection:607
02:17:50 INFO - Z:/task_1556845676/build/tests/xpcshell/tests/toolkit/components/telemetry/tests/unit/test_TelemetryEnvironment.js:checkEnvironmentData:863
02:17:50 INFO - Z:/task_1556845676/build/tests/xpcshell/tests/toolkit/components/telemetry/tests/unit/test_TelemetryEnvironment.js:test_checkEnvironment:935
02:17:50 INFO - exiting test
02:17:50 INFO - Unexpected exception NS_ERROR_ABORT:
02:17:50 INFO - _abort_failed_test@Z:\task_1556845676\build\tests\xpcshell\head.js:742:20
02:17:50 INFO - do_report_result@Z:\task_1556845676\build\tests\xpcshell\head.js:849:5
02:17:50 INFO - Assert<@Z:\task_1556845676\build\tests\xpcshell\head.js:57:5
02:17:50 INFO - proto.report@resource://testing-common/Assert.jsm:213:10
02:17:50 INFO - proto.ok@resource://testing-common/Assert.jsm:233:10
02:17:50 INFO - checkSystemSection@Z:/task_1556845676/build/tests/xpcshell/tests/toolkit/components/telemetry/tests/unit/test_TelemetryEnvironment.js:607:12
02:17:50 INFO - checkEnvironmentData@Z:/task_1556845676/build/tests/xpcshell/tests/toolkit/components/telemetry/tests/unit/test_TelemetryEnvironment.js:863:3
02:17:50 INFO - test_checkEnvironment@Z:/task_1556845676/build/tests/xpcshell/tests/toolkit/components/telemetry/tests/unit/test_TelemetryEnvironment.js:935:3
02:17:50 INFO - async*run_next_test/_run_next_test/<@Z:\task_1556845676\build\tests\xpcshell\head.js:1437:22
02:17:50 INFO - _run_next_test@Z:\task_1556845676\build\tests\xpcshell\head.js:1437:38
02:17:50 INFO - run@Z:\task_1556845676\build\tests\xpcshell\head.js:688:9
02:17:50 INFO - _do_main@Z:\task_1556845676\build\tests\xpcshell\head.js:227:6
02:17:50 INFO - _execute_test@Z:\task_1556845676\build\tests\xpcshell\head.js:529:5
02:17:50 INFO - @-e:1:1
02:17:50 INFO - exiting test
02:17:50 INFO - "CONSOLE_MESSAGE: (info) 1556849869830 addons.xpi-utils DEBUG Successfully read XPI database"
02:17:50 INFO - "CONSOLE_MESSAGE: (info) 1556849869830 Toolkit.Telemetry TRACE TelemetryEnvironment::_updateAddons"
02:17:50 INFO - "CONSOLE_MESSAGE: (info) 1556849869840 Toolkit.Telemetry TRACE TelemetryEnvironment::_updateAddons: addons differ"
02:17:50 INFO - "CONSOLE_MESSAGE: (info) 1556849869841 Toolkit.Telemetry TRACE TelemetryEnvironment::_startWatchingPrefs - [object Map]"

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/035696395109
Add a telemetry to detect disk type r=chutten,froydnj,bdekoz

Comment 13

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/035696395109

Comment 14

[Sean Feng [:sefeng]]

The following bug to verify the data Bug 1550738.

Comment 15

[Tim Smith (inactive) 👨‍🔬 [:tdsmith]]

Hello! Please flag one of the data stewards listed at https://wiki.mozilla.org/Firefox/Data_Collection for data collection reviews (data-review?).

Comment 16

[Tim Smith (inactive) 👨‍🔬 [:tdsmith]]

Comment on attachment 9049611 [details]
RequestForm.txt

data-review+

--

1) Is there or will there be **documentation** that describes the schema for the ultimate data set available publicly, complete and accurate?

Yes, the collection is described in the source tree docs.

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

Yes, the [telemetry opt-out](https://support.mozilla.org/en-US/kb/share-data-mozilla-help-improve-firefox).

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

Yes; Andrew Creskey will monitor the data.

4) Using the [category system of data types](https://wiki.mozilla.org/Firefox/Data_Collection) on the Mozilla wiki, what collection type of data do the requested measurements fall under?

Type 1, technical data.

5) Is the data collection request for default-on or default-off?

Default-on.

6) Does the instrumentation include the addition of **any *new* identifiers**?

No.

7) Is the data collection covered by the existing Firefox privacy notice?

Yes.

8) Does there need to be a check-in in the future to determine whether to renew the data?

No, permanent collection.

9) Does the data collection use a third-party collection tool? **If yes, escalate to legal.**

No.