In addition to system data (num cpu's, memory, arch) it would be very useful to have basic gfxinfo included with telemetry submissions. At least the basic D2D/DirectWrite enabled/disabled along with details of the graphics driver. Proposal: add this to the getMetadata method within TelemetryPing http://mxr.mozilla.org/mozilla-central/source/toolkit/components/telemetry/TelemetryPing.js#203

Does this look like what we want? I'm mimicing the gfxinfo data in about:support, we may need more/less than this. Not quite sure how to test this but there appear to be unit tests for TelemetryPing.js.

Comment on attachment 578187 [details] [diff] [review] patch, add gfxinfo data to telemetry metadata This seems pretty reasonable to me.

Comment on attachment 578187 [details] [diff] [review] patch, add gfxinfo data to telemetry metadata Taras, any tips on how to put in a test for this code?

Is there a reason for the #ifdef XP_WIN fields? All the Windows-specific fields should trigger the catch handler below and be ignored, right? Also, this is going to need some privacy review love.

Comment on attachment 578187 [details] [diff] [review] patch, add gfxinfo data to telemetry metadata Are these fields available in xpcshell? If so we can modify test_TelemetryPing.js to check for these values

Comment on attachment 578187 [details] [diff] [review] patch, add gfxinfo data to telemetry metadata Do not need ifdef here. Still waiting on response on whether this is reachable in xpcshell.

To document what we're collecting in the interest of full transparency, could someone provide a sample of the data collected (I see the fields in the patch, but would like to see an example submission) and what we hope to learn from collecting this data?

The fields are those that are listed under "Graphics" on the about:support page. Including this info allows use to correlate better perf problems related to specific graphics cards, drivers or support library versions (e.g. DirectWrite version). I'll include a sample when I update the patch.

We need this to track d2d responsiveness problems(if any)

Adds gfxinfo metadata to telemetry metadata. On OSX this looks like: "adapterDescription":"","adapterVendorID":"0x1002","adapterDeviceID":"0x944a","adapterRAM":"","adapterDriver":"","adapterDriverVersion":"","adapterDriverDate":""

Additional metadata on Windows 7: "adapterDescription":"Intel(R) HD Graphics","adapterVendorID":"0x8086","adapterDeviceID":"0x0046","adapterRAM":"Unknown", "adapterDriver":"igdumd64 igd10umd64 igdumdx32 igd10umd32","adapterDriverVersion":"8.15.10.2202","adapterDriverDate":"8-25-2010", "adapterDescription2":"","adapterVendorID2":"","adapterDeviceID2":"","adapterRAM2":"","adapterDriver2":"","adapterDriverVersion2":"","adapterDriverDate2":"", "isGPU2Active":false,"DWriteEnabled":true,"DWriteVersion":"6.1.7601.17563", "cleartypeParameters":"DISPLAY1 [ Gamma: 2200 Pixel Structure: RGB ClearType Level: 100 Enhanced Contrast: 50 ] DISPLAY2 [ Gamma: 2200 Pixel Structure: RGB ClearType Level: 100 Enhanced Contrast: 50 ] "

(In reply to John Daggett (:jtd) from comment #11) > Additional metadata on Windows 7: > > "adapterDescription":"Intel(R) HD > Graphics","adapterVendorID":"0x8086","adapterDeviceID":"0x0046","adapterRAM": > "Unknown", > "adapterDriver":"igdumd64 igd10umd64 igdumdx32 > igd10umd32","adapterDriverVersion":"8.15.10.2202","adapterDriverDate":"8-25- > 2010", > "adapterDescription2":"","adapterVendorID2":"","adapterDeviceID2":"", > "adapterRAM2":"","adapterDriver2":"","adapterDriverVersion2":"", > "adapterDriverDate2":"", > "isGPU2Active":false,"DWriteEnabled":true,"DWriteVersion":"6.1.7601.17563", > "cleartypeParameters":"DISPLAY1 [ Gamma: 2200 Pixel Structure: RGB ClearType > Level: 100 Enhanced Contrast: 50 ] DISPLAY2 [ Gamma: 2200 Pixel Structure: > RGB ClearType Level: 100 Enhanced Contrast: 50 ] " I think we should omit empty fields

Can we prune the list of items collected to a subset of these (on top of omitting only empty fields), or are all the data points necessary?

(In reply to Sid Stamm [:geekboy] from comment #13) > Can we prune the list of items collected to a subset of these (on top of > omitting only empty fields), or are all the data points necessary? This is graphics card info so I think whether something is needed or not would depend on the nature of a specific problem. Your concern is that a combination of info items would id individual users?

I don't have a direct concern, but am driving at minimizing what we collect. If we need it all, that's fine, but I'm just asking: can we get by without some of this or do we really need it all? See principle 4 in this blog post: http://blog.mozilla.com/privacy/2011/01/12/mozillas-privacy-data-operating-principles/

(In reply to Sid Stamm [:geekboy] from comment #15) > I don't have a direct concern, but am driving at minimizing what we collect. > If we need it all, that's fine, but I'm just asking: can we get by without > some of this or do we really need it all? I'm not familiar with cleartype, but everything else is required to do gfx regressions.

John, how much do cleartype values vary, can we condense those?

Comment on attachment 589835 [details] [diff] [review] patch, add gfxinfo data to telemetry metadata w/ tests r- for sending empty fields.

(In reply to Taras Glek (:taras) from comment #17) > John, how much do cleartype values vary, can we condense those? It might be useful to correlate with specific perf problems but it's the least useful of those fields. I'll trim it.

Omit empty strings and cleartypeParameters.

Revised to only test on Win/OSX since some Linux platforms don't report adapter info.

Comment on attachment 590087 [details] [diff] [review] patch, add gfxinfo data to telemetry metadata w/ tests + if (gfxInfo) { + for each (let field in gfxfields) { + let value = ""; + try { + value = gfxInfo[field]; + } catch (e) { + continue + } + if (value != "") + ret[field] = value; + } + } value should be defined within try try { let value = ...; if (value != "") ret[field]=value }

Pushed to inbound https://hg.mozilla.org/integration/mozilla-inbound/rev/e29b78989aeb

removing privacy-review-needed keyword. Resolved in comment 16: needed for gfx regressions.