444106 - collect and display textual data from "notes" field of crash reports

Status: RESOLVED FIXED

(Socorro :: General, task)

Description

[Josh Aas]

See bug 444103 first, about adding textual notes to crash reports. We would need to collect and display those notes on Socorro.

Comment 1

[(not currently active) Ted Mielczarek]

Attachment: collect and display notes and distribution/distribution_version fields

This should fix this (as well as bug 437487, while I'm at it). I have no idea how to run the processor on khan, so I haven't tested it at all. lars: could you help me test this on khan? Ping me on irc if you get a chance.

Comment 2

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

ted: on khan, there is a user called 'processor'.  From that account, you can start the processor.  Unfortunately, I've not kept that account up to date with the latest monitor/processor code, as I have found it much simpler to work in my own directory.

I will update it and then write some quick instructions on how to start it.  I'm at OSCON and having a tough time keeping a network connection for more than a few minutes.  I will get this done as I have the opportunity.

Comment 3

[(not currently active) Ted Mielczarek]

Ok, no big rush. Maybe you can show me at the summit. :)

Comment 4

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

Did this become a casualty of the escape from Whistler? ;)

Comment 5

[(not currently active) Ted Mielczarek]

I think it got lost in the shuffle of the Socorro rework. Lars: I'm sure this patch has completely bitrotted, but could you take a look at it? It's pretty trivial, just grabbing one extra field out of the submitted data, and adding one extra field to the reports table to hold it. The reporter patch will need to be rewritten, but that's probably trivial as well.

Comment 6

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

yeah, that's got bit rot.  Interestingly, I can find no example where a json file has got a "notes" field in it.  Is this something new?  Is it related to the field called "comments"?  I also have no examples of a json file actually having that field either.

I also notice in your patch two other new fields: distributor and distributor_version.  Can I assume you want these added, too.

In a couple weeks there is going to be a major schema upgrade to implement saner partitioning.  I will roll these changes in at that time.

Comment 7

[(not currently active) Ted Mielczarek]

Yep, it was added in bug 444103 (less than 3 months ago). It's just an optional plain-text field. I'd like to add those other fields to fix bug 437487 in the hopes that we might one day collect crash reports from Linux distributions' builds of Firefox. If you could add those fields to the db I would be grateful. If you can unbitrot this patch, I'll owe you a beer.

Comment 8

[(not currently active) Ted Mielczarek]

Also, "Notes" and "Comments" are similar, except that "Comments" is a text field provided to the user in the crash reporter client, whereas "Notes" is a place for the application to stick freeform textual data that may be useful for debugging crashes, without us having to refactor the database every time. It was added specifically to collect more info about ObjC exceptions on OS X, because currently they all get lumped under the same signature in the topcrash list.

Comment 9

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

After adding this column, the reports table will have three similar ambiguously named columns:

comments - notes from the user
notes - messages from the application
message - comments and errors from the processor

Would there be objections if I disambiguate these with new names in the database: user_notes, app_notes, processor_notes?

Comment 10

[(not currently active) Ted Mielczarek]

I'd make the first "user_comments", because we refer to it as "comments" in a few other places, but otherwise that's fine with me.

Comment 11

[(not currently active) Ted Mielczarek]

Reassigning, hope you don't mind!

Comment 12

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

Currently adding the following new columns to the reports table: 

'user_comments':  from the json field 'comments' or NULL - varchar(???)
'app_notes': from the json field 'notes' or NULL - varchar(???)
'distributor': from the json field distributor or NULL - varchar(20)
'distributor_version': from json field distributor_version or NULL - varchar(20)

What should the size be for the ??? above?

Changes to be deployed at the same time as the new database partitioning scheme beginning next week.

Comment 13

[(not currently active) Ted Mielczarek]

user_comments is limited by the crashreporter app to 500 bytes in the UTF-8 encoding, currently.
app_notes should probably be 500-1k chars. I have no idea how much our one user of that is currently sticking in there, so if we can give it 1k to have some headroom, might be good. (most crash reports shouldn't have this field anyway)

Comment 14

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

point of clarification regarding Comment #12:

I've just noticed that the json document keys begin with uppercase characters.  With that in mind, I will use the following mapping:

'user_comments':  from the json field 'Comments' or NULL - varchar(500)
'app_notes': from the json field 'Notes' or NULL - varchar(1000)
'distributor': from the json field 'Distributor' or NULL - varchar(20)
'distributor_version': from json field 'Distributor_version' or NULL - varchar(20)

Comment 15

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

also for the record to assist aking in implementing the display side of this issue, these columns are all in the 'reports' table:

'user_comments' is the existing 'comments' column renamed
'app_notes' is a new column
'distributor' is a new column (see Bug 437487)
'distributor_version' is a new column (see Bug 437487)
'processor_notes' is the existing 'message' column renamed

Comment 16

[Samuel Sidler (old account; do not CC)]

We're now sending this data (as of Firefox 3.0.5) and it'd be very helpful in debugging some of our top crashers. Where are we with getting this deployed on the Socorro side?

Comment 17

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

it's part of the database schema changes in Bug 432450 which was to be pushed by  Bug 468465 which is blocked by Bug 469215 which is slowed by these pesky holidays.  I've been tapping my foot impatiently since Dec 3...

Comment 18

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

The server end of this bug is done and is deployed into production.  The app_notes field is being saved in the production database

Now passing this bug on to aking to add it to the UI

Comment 19

[Austin King [:ozten]]

Attachment: Adds App and Processor Note fields. Adds Distributor and Distributor versions

Comment 20

[Austin King [:ozten]]

Attachment: patch1 had some code from a different bug, clean patch attached

Comment 21

[Michael Morgan [:morgamic]]

Comment on attachment 361788 [details] [diff] [review]
patch1 had some code from a different bug, clean patch attached

WFM

Comment 22

[Michael Morgan [:morgamic]]

Comment on attachment 331090 [details] [diff] [review]
collect and display notes and distribution/distribution_version fields

I think this is obsolete?

Comment 23

[Austin King [:ozten]]

Fixed in r890
Should ship tonight.