Annotate GPU Process in App Notes field

RESOLVED FIXED

Status

RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: ashughes, Assigned: dvander)

Tracking

(Blocks: 1 bug)

unspecified

Firefox Tracking Flags

(firefox55 fixed)

Details

Attachments

(1 attachment)

(Reporter)

Description

2 years ago
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)
(Reporter)

Comment 4

2 years ago
(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)
(Reporter)

Comment 7

2 years ago
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)
(Reporter)

Comment 8

2 years ago
(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
Created attachment 8849829 [details] [diff] [review]
bug1343650.patch

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?
(Reporter)

Comment 11

2 years ago
(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+

Comment 12

2 years ago
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)

Comment 13

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/328c81f2402c
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
status-firefox55: --- → fixed
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.