bug-1746636: breakdown by process type is missing process types

[Haseeb702]

Because:

• This is a fix for bug-1746636

This PR:

• Ensures the PROCESS_TYPES only need to be updated in mozilla_settings.py and the changes are dynamically reflected on the topcrashers page and in the supersearch
• Adds missing process types rdd, socket, utility. And adds other as a fallback option if there is an unknown process type.
• Adds additional test cases to test the functionality of the other option

How I tested it:

• Ran just test for our unit tests
• Manually uploaded crash data for our missing types and verified that they are queried properly in topcrashers and supersearch

[biancadanforth]

Partial review; still looking at a few things. But good job!

[biancadanforth]

I reviewed more of the PR. I'm still testing this locally. I think some additional tests for supersearch for this new "other" bucket should also be added though in addition to these comments (i.e. that you get the expected results when searching for process_type of other in Supersearch -- in the view and via the API). I am out of time for today, but I can chat with you next week on what that may look like if you have questions!

[biancadanforth]

I noticed there's an = here before other in the value compared to no = before parent for the process_type parameter.

Per the Supersearch operator docs: = is "is exactly", whereas no operator means "matches".

Why does other only work when we search using the "is exactly" operator (i.e. with an =) and not parent? example URL Its process_type is not EXACTLY other, so that doesn't make semantic sense. Why does parent only work when we search using the "matches" operator (i.e. no operator character)? This could be due to changes in case or something.

Interestingly, searching for process_type "is exactly" any or all does not return any results (neither in local in this branch nor in prod), while using the pseudo process type of other does: example URL

It's not obvious to me, so I doubt it would be obvious to users that they should use the "is exactly" operator when searching for "other" process types, but "matches" when searching for specific process types in SuperSearch. Which operator makes the most semantic sense? How can we adjust the behavior so that it's intuitive to search for these pseudo process types like "any"/"all" and "other"? Why doesn't "any"/"all" work in the same way as "other"? This seems like a bug.

Right now all I have are a bunch of questions, and I think we need to look more carefully at how these filters/operators are being applied and potentially adjust things for "other" and file one or more follow-ups for the other pseudo process types. I'll try to dig in a bit more tomorrow.

[smarnach]

@Haseeb702 and me just discussed this synchroneously. The existing any and all special values exist only for the benefit of the topcrashers page. They are useless in Supersearch. If you want to allow all process types in search results, you simply don't filter by that field. You can select any and all in Supersearch, but they don't work – these searches would return any crashes with the literal process type any or all, and of course there are none. I think this is mostly how it should be, except that we shouldn't even be showing any and all in Supersearch at all, since they don't do anything useful.

The new pseudo process type other should behave the same way. It should work on the topcrashers page, but we don't need it in Supersearch. It's not something that's needed frequently; in general, we simply won't have any crashes with some other process type. And if in special cases someone needs that in Supersearch, they can use the is not exactly filter and select all process types in the drop down to achieve the same effect.

In summary, we came to the conclusion that the any, all and other process types can be supported in the view code for the topcrashers code, but we don't need to add any special casing for that to the low-level Supersearch code. We can then also remove these special values from the PROCESS_TYPES list, since they are excluded at every place where that setting is used, except on the topcrashers page, where we can instead add them. We can probably also remove all, which seems redundant and unused.

@biancadanforth Does this make sense? I think this will also resolve the concerns in this comment, since we won't be touching the Supersearch code at all.

[biancadanforth]

Thanks Sven for taking a look and sharing your thoughts! You're completely right that there's no reason to filter in Supersearch for process type of "any"/"all" -- you wouldn't use a filter at all in that case. Since those pseudo process types have never been supported in SuperSearch, I agree we don't need to start now to support the "other" pseudo process type. It would be simpler to remove "any", "all" and "other" from the SuperSearch dropdown.

And if in special cases someone needs that in Supersearch, they can use the is not exactly filter and select all process types in the drop down to achieve the same effect.

This would be the does not match filter in the current implementation -- I assume because of some case difference between the filter value and what's in the processed crash report document -- but yes, there are other ways they can get to that information in SuperSearch.

@Haseeb702 : Can you update your PR to:

• Remove "any", "all" and "other" from PROCESS_TYPES
• Only use these pseudo process types in the topcrashers view code
• Update tests accordingly
• Most of my comments from this latest review are for the topcrashers code and test, so it'd be good if you can go through those still relevant suggestions as well.

[biancadanforth]

I meant to add an overall comment to my review -- basically I'm hung up on the test case for "other" process types in the SuperSearch view test case. See #7165 (comment), where I have a lot of open questions about the current behavior, and how it's supposed to behave. I think we need to resolve this before we can move forward with this PR. I'll have to look into that more tomorrow.

[biancadanforth]

R+WC (with changes)

Just a couple of leftover diffs from an earlier revision to remove.

Good job!

I know rob-bugson isn't working to make it easy to add GitHub PRs as attachments and merge comments to Bugzilla bugs, but can you ensure this PR is linked in the ticket (attachment example and merge comment example)? Then you'll want to verify it in stage once the deploy completes shortly. Then we can deploy it to prod, and once it's verified there, you can mark this ticket as resolved (example), and we can move forward with reprocessing the main process type crash reports in bug-2021204.