2053888 - Explicitly return Crash Reports only in Crash Stats APIs and views

Status: ASSIGNED

(Socorro :: General, task, P1)

Description

[Bianca Danforth [:bdanforth]]

Once Milestone 1 - 3 are complete for this project (meta bug: Bug 2049499), Crash Pings will exist in Socorro's storage alongside Crash Reports. Without an explicit filter, SuperSearch queries and views that use them (/topcrashers, /signature, /search, etc.) would return both submission types.

This ticket adds a mechanism to apply a default submission_type filter to SuperSearch queries. This filter is applied at the model layer in SuperSearch.get() and SuperSearchUnredacted.get() (in webapp/crashstats/supersearch/models.py). The default is configurable via a feature flag, so that the eventual cutover (Milestone 6) can be executed without code changes.

Initially, and for this ticket, the SuperSearch API default will be "report". The cutover later on will change this default to "ping". Callers can override the default by explicitly including the submission_type filter in their query. This will allow us to test Crash Ping support prior to the cutover.

Importantly, this only affects the default for the SuperSearch API. The SuperSearchUnredacted API will always default to "report", since Crash Pings do not contain the protected fields this endpoint exists to surface.

Notably, reports processed prior to Bug 2053871 will not have a submission_type field. While we could backfill this data or reprocess these reports to add submission_type="report", it would be operationally easier to create a compound Elasticsearch query, such that documents are matched where submission_type="report" OR the field is absent.

Acceptance criteria:

• Feature flag (e.g. SUPERSEARCH_DEFAULT_SUBMISSION_TYPE) added with default value "report".
• SuperSearch.get() applies the setting as the default submission_type filter when the parameter is absent from the incoming request; explicit values in the request override the default.
• SuperSearchUnredacted.get() always applies submission_type=report regardless of the setting.
• This filter propagates correctly to all views that use SuperSearch: /topcrashers, /signature Summary/Aggregations/Reports/Graphs, /search, /search/results, and any others.
• submission_type=report (whether default or explicit) matches new crashes with submission_type=report in the processed crash AND legacy crashes where the field is absent from the indexed document (crashes processed before Bug 2053871).
• submission_type=ping (explicit) matches only crashes with submission_type=ping in the processed crash.
• submission_type=report&submission_type=ping (multi-value, explicit) matches new crashes with submission_type=report or submission_type=ping, and legacy documents where the field is absent.
• Unit tests cover: default filter behavior for SuperSearch and SuperSearchUnredacted, explicit override behavior, legacy document handling (missing field), and filter propagation to the affected views.

Comment 1

[Sven Marnach :sven]

:bdanforth I tried implementing this as specified, but I'm running into a problem. A lot of views (e.g. top crashers, signature overview page) are based on SuperSearchUnredacted. If I use the setting SUPERSEARCH_DEFAULT_SUBMISSION_TYPE only for SuperSearch and not for SuperSearchUnredacted, I have to treat all these pages separately. For this reason, I propose to use the same deafult for both SuperSearch and SuperSearchUnredacted, at least for now. This makes this change trivial (essentially just a few lines of code). Given that we eventually plan to unify SuperSearch and SuperSearchUnredacted, I also have some doubts about giving them different defaults, since this will make unifying them harder.

What do you think?

Comment 2

[Bianca Danforth [:bdanforth]]

Sven and I discussed this sync, and I agree that we should make both APIs have the same default (crash reports in the short term and crash pings in the long term).

AFAICT, the only reason I said we'd keep SuperSearchUnredacted defaulting to crash reports is because crash pings were presumed to contain only public fields and therefore have nothing to redact[1]. Users will still be able to obtain crash reports by using submission_type=report with both APIs after the default cutover. Sven noted that per the logs, BugBot is the only user of the SuperSearchUnredacted API, and IIRC, the only reason they were using SuperSearchUnredacted in the first place was because of Bug 2050638 which is now fixed (:suhaib, can you confirm?). We also have long term plans to get rid of SuperSearchUnredacted (Bug 1545475), so we would like to keep the APIs' behavior as similar as possible to that end.

The downstream consequences of this are that BugBot could use the SuperSearch API instead of SuperSearchUnredacted and set submission_type=report in the cases where they are relying on fields only in crash reports. The API change isn't strictly needed, since both APIs can return protected data now for authorized users, but that would negate the need for any further changes should we eventually remove the SuperSearchUnredacted API completely. We have already been talking with our critical path consumers (including Suhaib on BugBot), and they are expecting that some changes will be needed, so this does not represent a significant deviation from our original plan. I will update the migration plan accordingly, and I will also add a note to the project page about this change relative to the RFC.

[1]: In hindsight, there are some protected fields, including glean_client_id (Bug 2052944).

Comment 3

[Suhaib Mujahid [:suhaib]]

Bug 2050638 which is now fixed (:suhaib, can you confirm?)

IIRC, that was the reason, and if now SuperSearch returns the same things, then we do not really need SuperSearchUnredacted. Is it safe to migrate now to use SuperSearch? Is it just a simpler switch or do we need other changes?

Comment 4

[Sven Marnach :sven]

:suhaib I think you can simply switch to the SuperSearch endpoint without any further changes. The main difference between the two endpoints now is their behaviour if you don't have permission to access PII: SuperSearch will redact the PII, while SuperSearchUnredacted will return an error.

The SuperSearchUnredacted endpoint is used in both libmozdata and bugbot, so you'd need to change it in both codebases.

Comment 5

[Sven Marnach :sven]

Attachment: Don't return crash pings unless explicitly requested.