Closed Bug 1343650 Opened 3 years ago Closed 3 years ago

Annotate GPU Process in App Notes field

Categories

(Socorro :: General, task)

task
Not set

Tracking

(firefox55 fixed)

RESOLVED FIXED
Tracking Status
firefox55 --- fixed

People

(Reporter: ashughes, Assigned: dvander)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

The GFX team would like to have an annotation in the App Notes field to indicate if the user reporting a crash has GPU Process enabled or not. Nomenclature-wise this could be something as simple as "GPUProcess+" for enabled and "GPUProcess-" for disabled. Having these annotations will help us understand the impact of GPU Process on crashes coming through some other Firefox process.

CCing Milan and David to provide further details as needed.
David mentioned that we should already have enough information in the telemetry environment, it's just a question of whether we can get to it with supersearch and see if in the crash report.  David, how do we check this?
Flags: needinfo?(dvander)
When logged in to crash-stats, the "TelemetryEnvironment" metadata key has a big JSON string. That string should contain:

  "gpuProcess":{"status":"available"}

If the GPU process is enabled. If that's not enough to super-search by, I can add a new annotation or app note.
Flags: needinfo?(dvander)
(In reply to David Anderson [:dvander] from comment #2)
> When logged in to crash-stats, the "TelemetryEnvironment" metadata key has a
> big JSON string. That string should contain:
> 
>   "gpuProcess":{"status":"available"}
> 
> If the GPU process is enabled. If that's not enough to super-search by, I
> can add a new annotation or app note.

Anthony, is this enough?

David, what will that look like if the GPU process goes away?  Will it be updated to a different status (Broken?)
Flags: needinfo?(anthony.s.hughes)
(In reply to Milan Sreckovic [:milan] (not likely to respond before 3/27) from comment #3)
> (In reply to David Anderson [:dvander] from comment #2)
> > When logged in to crash-stats, the "TelemetryEnvironment" metadata key has a
> > big JSON string. That string should contain:
> > 
> >   "gpuProcess":{"status":"available"}
> > 
> > If the GPU process is enabled. If that's not enough to super-search by, I
> > can add a new annotation or app note.
> 
> Anthony, is this enough?

In theory, but as far as I can tell TelemetryEnvironment is not searchable via Supersearch whether signed in or not. I suppose that would need a different bug report.
Flags: needinfo?(anthony.s.hughes)
(In reply to Milan Sreckovic [:milan] (not likely to respond before 3/27) from comment #3)
> (In reply to David Anderson [:dvander] from comment #2)
> > When logged in to crash-stats, the "TelemetryEnvironment" metadata key has a
> > big JSON string. That string should contain:
> > 
> >   "gpuProcess":{"status":"available"}
> 

David, what will that look like if the GPU process goes away?  Will it be
updated to a different status (Broken?)
Flags: needinfo?(dvander)
Let's just add an annotation, it's easy.
Assignee: nobody → dvander
Status: NEW → ASSIGNED
Flags: needinfo?(dvander)
I was thinking about this again in the context of stability metrics for PR around the release of GPU Process in Firefox 53. I'm guessing that adding a new annotation would only apply to newly submitted crash reports and is something that would need to be uplifted into Beta.

Perhaps a better approach would be to make it so that TelemetryExperiment was searchable, even if that was only available to authenticated Socorro users. 

David, what do you think?
Flags: needinfo?(dvander)
(In reply to Anthony Hughes (:ashughes) [GFX][QA][Mentor] from comment #7) 
> Perhaps a better approach would be to make it so that TelemetryExperiment
> was searchable, even if that was only available to authenticated Socorro
> users.

s/TelemetryExperiment/TelemetryEnvironment
Attached patch bug1343650.patchSplinter Review
Indeed just using the existing data we have would be ideal.

We should add the annotation anyway I guess. If the super search for some reason doesn't work out this is easy to uplift.
Flags: needinfo?(dvander)
Attachment #8849829 - Flags: review?(wmccloskey)
Adding a new field to Super Search is a trivial matter, but it seems that this one contains sensitive data. Could you please provide more details about what it contains?

Also, not sure if I understand everything about this bug. Is the data already available in crash reports?
(In reply to Adrian Gaudebert [:adrian] from comment #10)
> Adding a new field to Super Search is a trivial matter, but it seems that
> this one contains sensitive data. Could you please provide more details
> about what it contains?

Hi Adrian,

I don't know for sure if there is any sensitive data but this is why I proposed above restricting it to Socorro users with privileged access. Furthermore, the TelemetryEnvironment field is not currently public and can only be viewed when signed in with LDAP. There are other fields such as email addresses and URLs which fall into this same category that are currently searchable via Supersearch. 

> Also, not sure if I understand everything about this bug. Is the data
> already available in crash reports?

Yes, you can see a sample crash report here when authenticated:
https://crash-stats.mozilla.com/report/index/4778b2cf-1c1a-44d5-83f4-f312a2170322#tab-metadata
Attachment #8849829 - Flags: review?(wmccloskey) → review+
Pushed by danderson@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/328c81f2402c
Add crash reporter annotations for the GPU process status. (bug 1343650, r=billm)
https://hg.mozilla.org/mozilla-central/rev/328c81f2402c
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.