Closed Bug 444106 Opened 12 years ago Closed 11 years ago

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


(Socorro :: General, task)

Not set


(Not tracked)



(Reporter: jaas, Assigned: ozten)




(1 file, 2 obsolete files)

See bug 444103 first, about adding textual notes to crash reports. We would need to collect and display those notes on Socorro.
Depends on: 444103
OS: Mac OS X → All
QA Contact: bonsai → socorro
Hardware: PC → All
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.
Attachment #331090 - Flags: review?(morgamic)
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.
Ok, no big rush. Maybe you can show me at the summit. :)
Did this become a casualty of the escape from Whistler? ;)
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.
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.
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.
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.
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?
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.
Reassigning, hope you don't mind!
Assignee: ted.mielczarek → lars
Depends on: 432450
No longer depends on: 444103
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.
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)
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)
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
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?
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...
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
Assignee: lars → aking
Attachment #361686 - Attachment is obsolete: true
Attachment #361788 - Flags: review?(morgamic)
Attachment #361686 - Flags: review?(morgamic)
Comment on attachment 361788 [details] [diff] [review]
patch1 had some code from a different bug, clean patch attached

Attachment #361788 - Flags: review?(morgamic) → review+
Attachment #331090 - Attachment is obsolete: true
Attachment #331090 - Flags: review?(morgamic)
Comment on attachment 331090 [details] [diff] [review]
collect and display notes and distribution/distribution_version fields

I think this is obsolete?
Fixed in r890
Should ship tonight.
Closed: 11 years ago
Resolution: --- → FIXED
Component: Socorro → General
Product: Webtools → Socorro
You need to log in before you can comment on or make changes to this bug.