2074575 - Investigate erroneous ProductName annotation values

Status: RESOLVED FIXED

(Toolkit :: Crash Reporting, task)

Description

[Alex Franchuk [:afranchuk]]

In looking at ProductName values in crash pings (query here), I found some anomalies:

• Garbage values. This isn't particularly unusual on its own, but some look like partial offsets into other strings. This isn't high volume, but we should investigate what's going on here.
• null values are the most common.
• Fennec rather than Fenix.

Comment 1

[Alex Franchuk [:afranchuk]]

Many of the garbage values appear to be substrings of the Winsock_LSP annotation.

Comment 2

[Alex Franchuk [:afranchuk]]

Fennec comes from here. I recall talking with the Android team about this once, and it's a bit of legacy code that's stuck around, as Fennec hasn't been completely removed from the tree (see bug 1570409 and related bugs).

Comment 3

[Alex Franchuk [:afranchuk]]

I also found a case where the ProductName contained a substring from the TelemetryEnvironment annotation. I'm guessing we're hitting these annotations by chance because they are large strings.

Gabriele: I suspect this is due to reading annotations going awry (perhaps not a bug, just bad luck on unstable systems?). These are very low volume, but I'm not sure we can do anything about them (aside from adding more safeguards to the annotations in memory to avoid reading incorrectly).

Comment 4

[Alex Franchuk [:afranchuk]]

Note that a few of these cases have a string starting with "F", "Fi", "Fire", etc, so it may be that recording the annotation is the part to blame (it appears that ProductName was recorded and is being overwritten). Also, many of these pings have hardly any other annotations set.

Comment 5

[Alex Franchuk [:afranchuk]]

Bug 2074595 mentions the potential for data races when reading annotations. This may be the cause of the garbage values.

Comment 6

[Gabriele Svelto [:gsvelto]]

Yes, I think this is a case where we're reading partially written or corrupt annotations. I filed bug 2074790 as follow-up work for that and we might want to investigate further how to integrate reading annotations directly in the minidump-writer where we have much better machinery to stop a process. Now that I think about it this might also be the underlying cause for some of the oddities we've seen in paired minidumps. Ideally we'd need the annotation reader to do two things more robustly than it does now: read annotations from a crashed process while it's stopped, and read annotation from the main process while it's running - briefly stopping it to avoid corruption.

Comment 7

[Alex Franchuk [:afranchuk]]

WRT the null values: I've partitioned by OS and version, and this has revealed what I originally suspected: we don't set the ProductName annotation for Java crash pings (but we should!). Simply looking at all crash pings will still show a lot of nulls, because old clients didn't have the metric (so default to null). If we filter to more recent versions and then partition by OS, all of the nulls are from Android pings.

Comment 8

[Alex Franchuk [:afranchuk]]

The garbage values will be resolved/investigated further in bug 2074790.

The null values will be fixed in bug 2074880.

Fennec values are a build team concern (which I've brought up to them).