"SubmittedFromInfobar" value is true and not "true" for recent crash reports
Categories
(Socorro :: General, defect, P2)
Tracking
(Not tracked)
People
(Reporter: willkg, Assigned: willkg)
References
Details
Attachments
(2 files)
Crash reports submitted using the new JSON-encoded field have a "SubmittedFromInfobar" annotation that is true and false in the JSON and not "1" and "0".
This is breaking schema validation which fetches the data from the RawCrash API so it's the raw version of what we collected. I think I can fix it in the validation script.
This might also be sullying the crash report data we're sending to Telemetry which is expecting types string or null. If it is, we'll need to write a processor rule to fix it.
Also, it's entirely possible that Antenna doesn't understand true and is looking for "1".
This bug covers fixing all that.
|
Assignee | |
Comment 1•6 years ago
|
||
Making this a P1 because of the consequences on collection and exporting data to Telemetry.
|
|
Assignee | |
Comment 2•6 years ago
|
||
Gabriele: ^^^ This is a similar issue to the one we have where BuildID is null and ModulesForSignature is a JSON object. Want me to write up a corresponding bug in the crash reporter?
|
|
Assignee | |
Comment 3•6 years ago
|
||
I thought the original value was "1", but it's not--it's "true". So crash reports from recent versions send true while older versions send "true".
We have one throttler rule in the collector that relates to SubmittedFromInfobar, but it only looks at Firefox versions 52 through 59 so it doesn't make a difference and the collector is fine.
I know the schema doesn't validate with SubmittedFromInfobar as true. The TelemetryBotoS3CrashStorage will save the crash report--it doesn't seem to do any validation (it probably should). I looked at a couple of crashes from 2020-03-25 since that's long enough ago that those crash reports should be in telemetry.socorro_crash data set. They're there and the data is right. So maybe telemetry ingestion handles true just like "true" and it doesn't matter that the types are "string" and "null".
I adjusted the summary and dropped this down to a P2 since it doesn't seem to affect collection and exporting to Telemetry.
|
||
Comment 4•6 years ago
|
||
It's a regression from bug 1420363. I didn't notice it at the time but that field is being set as true in an object here. Before bug 1420363 this would be serialized as the string "true", now it's using the JSON value true directly. What would you prefer here, having it as "1" or "0" like other booleans in crash reports or go back to the previous behavior?
|
|
Assignee | |
Comment 5•6 years ago
|
||
I know I use this field in the collector to weed out crash reports submitted because of a bug a while back, but since older versions of Firefox aren't changing, that code is fine regardless of what the new value is. I have no idea if anyone else uses it nor do I know how to find out.
I like consistency because it means fewer things to remember, so I'd prefer to change it to "1". But we've got years of precedence, so maybe we should switch it back to "true". I went to look at the docs and noticed it's not listed:
https://hg.mozilla.org/mozilla-central/file/tip/toolkit/crashreporter/CrashAnnotations.yaml
I think if it's not listed, we're not on the hook for anything, so we should make it like annotations and the value should be "1".
Regardless of what the value ends up as, we should add the field to the docs and if we go with "true", we should note that.
|
|
||
Comment 6•6 years ago
|
||
(In reply to Will Kahn-Greene [:willkg] ET needinfo? me from comment #5)
I know I use this field in the collector to weed out crash reports submitted because of a bug a while back, but since older versions of Firefox aren't changing, that code is fine regardless of what the new value is. I have no idea if anyone else uses it nor do I know how to find out.
I like consistency because it means fewer things to remember, so I'd prefer to change it to
"1". But we've got years of precedence, so maybe we should switch it back to"true".
Alright, I'll change it to "1" for consistency since it changed anyway and this way it will be consistent with all the other boolean annotations.
I went to look at the docs and noticed it's not listed:
https://hg.mozilla.org/mozilla-central/file/tip/toolkit/crashreporter/CrashAnnotations.yaml
I think if it's not listed, we're not on the hook for anything, so we should make it like annotations and the value should be
"1".
I agree, the reason why it's not listed is the same as with bug 1564483: they're not added via the crash reporter. The submission code used to "stick" that annotation in the HTTP POST command directly when sending to Socorro. I will add it to CrashAnnotations.yaml so that it's documented.
One sad thing about all of this is just how poorly crash reports are abstracted in Gecko, which is not at all. Ideally one would have just one nice object, holding the minidump and all possible annotations behind a nice interface usable from C++ and JS. Then we'd just "add" that annotatino to that object and it would all be cleanly serialized into the JSON required for the POST. Instead we have two hundred different ad-hoc solutions for code that needs to deal with it and when they're not synchronized this stuff happens. sigh
Maybe one day I'll fix it. In the meantime I'm fixing this.
|
|
Assignee | |
Comment 7•6 years ago
|
||
Thank you!
|
|
Assignee | |
Comment 8•6 years ago
|
||
I'll write a processor rule for changing "true" and true to "1". I won't reprocess because there are a bazillion of them and it's probably the case that no one is looking at this field.
However, I can reprocess if that turns out not to be true.
|
|
Assignee | |
Comment 9•6 years ago
|
||
|
|
Assignee | |
Comment 10•6 years ago
|
||
|
|
Assignee | |
Comment 11•6 years ago
|
||
I feel incredibly stupid. I wrote the rule and it works nicely, but I didn't commit the change to add it to the pipeline, so it's not doing anything. I need to do another fix to enable it.
|
|
Assignee | |
Comment 12•6 years ago
|
||
|
|
Assignee | |
Comment 13•6 years ago
|
||
|
|
Assignee | |
Comment 14•5 years ago
|
||
Deployed today in bug #1635485.
I reprocessed a crash and verified that it's fixed by looking at how it shows up in Supersearch results. The value went from True to 1.
Description
•