1650423 - Extend FOG deletion-request ping integration test

Status: NEW

(Toolkit :: Telemetry, enhancement, P3)

Description

[Raphael Aurich [:raphael] UTC+02:00]

We currently test that FOG sends a deletion-request ping when a client opts out of data collection. This task is about extending this test by checking that FOG does not submit pending pings and recorded metrics.

https://mozilla.github.io/glean/book/user/pings/deletion_request.html

Comment 1

[Alessio Placitelli [:Dexter]]

(In reply to Raphael Pierzina [:raphael] UTC+02:00 from comment #0)

We currently test that FOG sends a deletion-request ping when a client opts out of data collection.

Does this need to be tested in FOG? This part is an implementation detail of the Glean SDK itself, I believe this should be tested as part of the Glean SDK.

Comment 2

[Chris H-C :chutten]

(In reply to Alessio Placitelli [:Dexter] from comment #1)

(In reply to Raphael Pierzina [:raphael] UTC+02:00 from comment #0)

We currently test that FOG sends a deletion-request ping when a client opts out of data collection.

Does this need to be tested in FOG? This part is an implementation detail of the Glean SDK itself, I believe this should be tested as part of the Glean SDK.

IMO Yes. The current test covers that nothing in the plumbing between Firefox's Preferences and the Glean SDK is broken (FOG Code) and that the "deletion-request" ping makes it to the network stack and is sent (FOG Code). It also unintentionally tests that setting a port for the localhost ping server endpoint works (which wouldn't exist if we didn't have integration tests, so it doesn't really count as coverage).

As for pending pings and recorded metrics, I'm on the fence. They're both behaviours that can be observed by an integration test so totally can be tested. But on the other hand, pending pings and recording suppression are Glean SDK-internal behaviours so the tests for those should be internal to the SDK so they run on SDK changes only. And I would expect these behaviours to be part of an end-to-end test specifically because they are core to the Glean SDK and so can be tested across a variety of clients.

Comment 3

[Raphael Aurich [:raphael] UTC+02:00]

When deciding whether a certain scenario needs to be covered by integration tests, I have found it helpful to identify the failure points and estimate the failure probability. Additionally I take into account if the scenario is covered by other test suites. I agree with chutten’s assessment and will add that as of right now FOG is only present on Nightly builds. The according build configuration is an additional failure point that we can only test on Firefox builds, which is what the current FOG test does.

I created this bug to start a conversation around whether we think it’s necessary to include checks for pending pings and recorded metrics. One potential failure point for the deletion-request ping that I can think of is that there are pending pings written to disk before a user opts out, then they opt out and the pref change triggers the deletion-request ping, the data platform processes the deletion-request ping, but then FOG submits the pending pings after the data platform deleted all data for this client ID and then the data platform stores the data from the pending pings. Now it is very possible that this is covered by checks on the data platform and/or in the Glean SDK, but I think it’s important we make sure we don’t have a blind spot around that.

Comment 4

[Alessio Placitelli [:Dexter]]

(In reply to Chris H-C :chutten from comment #2)

But on the other hand, pending pings and recording suppression are Glean SDK-internal behaviours so the tests for those should be internal to the SDK so they run on SDK changes only. And I would expect these behaviours to be part of an end-to-end test specifically because they are core to the Glean SDK and so can be tested across a variety of clients.

Yes, that's exactly what I was going for. Pending pings are an SDK implementation detail that's core for all the platform. FOG doesn't know and shouldn't care about pending pings, nor should FOG integration tests.

Glean end to end test should care.

(In reply to Raphael Pierzina [:raphael] UTC+02:00 from comment #3)

... One potential failure point for the deletion-request ping that I can think of is that there are pending pings written to disk before a user opts out, then they opt out and the pref change triggers the deletion-request ping, the data platform processes the deletion-request ping, but then FOG submits the pending pings after the data platform deleted all data for this client ID and then the data platform stores the data from the pending pings. Now it is very possible that this is covered by checks on the data platform and/or in the Glean SDK, but I think it’s important we make sure we don’t have a blind spot around that.

It is important to make a distinction here: FOG submits ping, but the SDK sends them. This is a behaviour that we want covered in the SDK, not in FOG, as FOG has no knowledge of pending pings.