719943 - new field in json metadata needs exposure in the UI

Status: VERIFIED FIXED

(Socorro :: General, task)

Description

[K Lars Lohn [:lars] [:klohn]]

the json metadata has a new field called "JsonStackTrace".  If present, it needs to be shown in the GUI when displaying an individual crash report.

Comment 1

[Laura Thomson :laura]

It's apparently called "JavaStackTrace".  I have a patch, waiting on sample data from bug 701002 to test.

Comment 2

[Laura Thomson :laura]

https://github.com/lauraxt/socorro/commit/d6b7dd9485824b50dbc8391c9af86ec03a57e7d8 is the patch.  When bug 701002 lands I'll put in a pull request.

Comment 3

[Laura Thomson :laura]

PR https://github.com/mozilla/socorro/pull/297

Comment 4

[[github robot]]

Commit pushed to https://github.com/mozilla/socorro

https://github.com/mozilla/socorro/commit/43baad1e347f1f640d205daa5ef75e4bff9ffaa4
Merge pull request #297 from lauraxt/719943

Fixes bug 719943

Comment 5

[Matt Brandt [:mbrandt]]

Attachment: qa - verified

thx :brandonsavage ... my mistake.

QA verfied on stage. Flash versions are displayed. The only caveat is that unspecified versions (browser crashes) display the flash version as "[blank]."

Comment 6

[Matt Brandt [:mbrandt]]

Please ignore comment 5. I mark the wrong bug verified.

Comment 7

[Robert Kaiser]

(In reply to Matt Brandt [:mbrandt] from comment #6)
> Please ignore comment 5. I mark the wrong bug verified.

And it seems you didn't actually get around to verifying this one, as it looks a lot like it doesn't work in production now.

Does that field actually arrive in the file this code looks at?
Here are examples of reports that should have it:
bp-076df6d0-b8f4-4da9-98af-579442120201
bp-e8820616-1462-49b6-9784-e99a32120201
bp-7b9186a6-997f-467b-ae40-cf5f52120201
bp-aadbcc9c-1dbd-475f-9ce2-b5cc02120201

Comment 8

[Matt Brandt [:mbrandt]]

:kairo - no I did not verify this bug prior to the push to production. 

Bumping to reopened

Comment 9

[Laura Thomson :laura]

(In reply to Robert Kaiser (:[e-mail]) from comment #7)
> (In reply to Matt Brandt [:mbrandt] from comment #6)
> > Please ignore comment 5. I mark the wrong bug verified.
> 
> And it seems you didn't actually get around to verifying this one, as it
> looks a lot like it doesn't work in production now.
> 
> Does that field actually arrive in the file this code looks at?
> Here are examples of reports that should have it:
> bp-076df6d0-b8f4-4da9-98af-579442120201
> bp-e8820616-1462-49b6-9784-e99a32120201
> bp-7b9186a6-997f-467b-ae40-cf5f52120201
> bp-aadbcc9c-1dbd-475f-9ce2-b5cc02120201

It was there in the .json on the first one I checked.  I should be able to figure this out and patch it tomorrow, or somebody else can do it tonight.

Comment 10

[Robert Kaiser]

(In reply to Laura Thomson :laura from comment #9)
> It was there in the .json on the first one I checked.  I should be able to
> figure this out and patch it tomorrow, or somebody else can do it tonight.

It's definitely there in the raw dump. Maybe the UI is looking at the processed dump and that's missing it? I'm just guessing...

Comment 11

[Laura Thomson :laura]

The UI looks at the processed dump, and this field is in the raw dump only.  This is my error: I had thought from my discussions with Lars that it would be passed through.  Since we have no end-to-end test environment, rhelmer and I agreed to test this in stage against the processed data...and then I forgot and QA did too.

I'll see if I can figure out a way to get at the raw dump from the UI: we need this for a related 2.4.2 bug anyway, bug 669108

Comment 12

[Laura Thomson :laura]

Did some digging.  In all other cases where we need data from the raw json, the processor stores it in PostgreSQL or writes it into the processed json.

I can rewrite the UI to also load the raw json.  It's a non trivial change and also incurs a performance penalty for the extra retrieval from HBase and extra parsing.

Pro: It will be really easy to add more things from the raw json to the UI in future

Con: It's inconsistent with what we currently have, and I'm slightly concerned about introducing a third way of doing things - loading data for an individual report already hits PG for most data and HBase for the processed JSON.  

Thoughts?  (lars? rhelmer?)

Comment 13

[Laura Thomson :laura]

PS this might end up in 2.4.1.1 but I suspect 2.4.2

Comment 15

[Chris Peterson [:cpeterson]]

laura, if there is any formatting or preprocessing I can do on the browser side to making json processing easier, just let me know.

Comment 16

[Laura Thomson :laura]

pull request: https://github.com/mozilla/socorro/pull/330

Comment 17

[[github robot]]

Commit pushed to master at https://github.com/mozilla/socorro

https://github.com/mozilla/socorro/commit/465abde536ed22c0094ffa8bbe848851c1bf9674
Merge pull request #330 from lauraxt/719943v2

Fixes bug 719943 properly, loads the JavaStackTrace from the raw JSON

Comment 18

[Robert Helmer [:rhelmer]]

To make testing easier, I took this crash from production:
https://crash-stats.mozilla.com/report/index/9a8799e3-2c94-40a1-af83-807422120206

Then submitted it to crash-reports-dev:
https://crash-stats-dev.allizom.org/report/index/95f1ed58-6ee8-4d28-8d5a-bbee22120208

Comment 19

[Matt Brandt [:mbrandt]]

Attachment: qa - verified

QA verified on dev. Thank you :rhelmer for making this simple to verify (comment 18).

Comment 20

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

Need this pushed to XUL as well (12 b2; 10.3 ESR)

Comment 21

[Alex Keybl [:akeybl]]

(In reply to Naoki Hirata :nhirata from comment #20)
> Need this pushed to XUL as well (12 b2; 10.3 ESR)

This bug is tracking server side changes - why should it be tracked for Firefox?

Comment 22

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

I understood it as a flag for firefox/fennec versioning, not just firefox desktop.
Perhaps it's my misunderstanding.

AFAIK, ESR10 will continue to be released for tablets until Native Fennec for tablets are ready.

Comment 23

[Robert Kaiser]

(In reply to Naoki Hirata :nhirata from comment #22)
> I understood it as a flag for firefox/fennec versioning, not just firefox
> desktop.
> Perhaps it's my misunderstanding.

No change that was done here is specific to any release train of Firefox or Fennec, and no client code was changed, this is all server-side in Socorro and doesn't care what product the reports are coming in for, as long as they have the data to be exposed.

> AFAIK, ESR10 will continue to be released for tablets until Native Fennec
> for tablets are ready.

Sure, but that doesn't influence this bug at all. If it sends Java Stack Trace fields, they will be shown, as long as it's not, there's nothing for Socorro to show.